Standardize action verbs

[jasonrhodes]

Since this round of 2.0 PRs is probably our last chance for breaking changes in a while, it would be nice if we could standardize all of the action verbs we're using across resources. Right now, after the PRs:

// templates, relay webhooks, recipient lists, and sending domains

{resource}.all()              // get collection
{resource}.find()             // get one by id
{resource}.create()           // create one
{resource}.update()           // update one by id
{resource}.delete()           // delete one by id


// webhooks

webhooks.all()                // get collection
webhooks.describe()           // get one by id
webhooks.create()             // create one
webhooks.update()             // update one by id
webhooks.delete()             // delete one by id


// suppression

suppressionList.search()      // get collection (can filter by criteria)
suppressionList.getEntry()    // get one by id
suppressionList.upsert()      // create, or update one by id
suppressionList.deleteEntry() // delete one by id

I think all and maybe find come from ActiveRecord, but even AR has find_by and you can find by any column criteria you want, assuming a relational db backend. I don't think describe fits at all, and looking at everything as a whole now, I don't think suppression needs to be different.

The main difference in suppression's search is that it can filter by criteria, but at some point we will most likely be adding pagination to all of our API endpoints, which means you'll be specifying criteria in all no matter what, so the word "all" makes a little less sense.

Here's what I propose across the board. Happy to hear reasons why this won't work, needs improvement, etc.

{resource}.list()           // get collection (can filter by criteria where applicable)

{resource}.get(id)          // get one by id

{resource}.create()         // create one
{resource}.update(id)       // update one by id
{resource}.upsert([id])     // replaces both create and update, where applicable

{resource}.delete(id)       // delete one by id

{resource}.{custom}()       // resource-specific action, e.g. getBatchStatus

UPDATE: Per comment discussion, removed bc aliases, added required params. Optional params like options and callback not specified here.

Side note: If we go this way I am of course happy to help with the conversion, examples, docs, tests, etc.

[aydrian]

I'm not sure we need to deprecate since there are many breaking changes in this release.

[nancyhabecker]

i like all your suggestions. :D

i was a little confused with how we are using the method names get and find. it seemed like in some places, we were using find and requiring the id. here are my impressions on what get vs find imply:

• get seems to apply to things with ids, like webhooks or domains. so i expect the signature would be like get(id, opts) (e.g. sendingDomains.get('sparkpost.com') or webhooks.get('some-uuid', {timezone:'UTC'}) or something like that.
• find seems to accept search criteria, and we would return the first match. (like webhooks.find({target:'something'}) would be an example if we supported that). i don't think we have any cases like this with our api, though, right? so probably don't need to use find.

what do you think?

[aydrian]

I believe once upon a time I had get setup as get(id, opts[, callback]) which I liked but we ended up switching to get(opts[, callback]). I think we were just failing to correctly implement it. I'm not against going back to that as it seems to make sense.

1. We can make breaking changes, so let's break whatever we need to give the best experience.
2. Let's be forward thinking enough to not have to break anything for quite a while. I don't want to have to do a 3.0 release for a loooong time.

[jasonrhodes]

I'm on board for not aliasing, punting find, and separating out required params from the opts array like get(id, opts) ... let's let this sit over the weekend in case others have thoughts.

If we go this route should we PR against the existing PRs to swap verbs and signatures, or should we review these as-is, merge em, then do another round?

[aydrian]

It would be great if we could keep the existing PRs because it makes the changelog cleaner.

[jasonrhodes]

OK so PR against them to swap verbs and signatures, before we merge the open ones. Works for me.

[aydrian]

Do we want to update your proposal above with parameters? i.e. {resource}.get(id[, options, callback])?

[jasonrhodes]

Updated to include required params only, and pulled out the bc aliases.

[aydrian]

Also, for transmissions are we going to keep it as send instead of create?

[jasonrhodes]

I think send is perfect as-is, imo. No need to get totally purist.

[aydrian]

This should be complete. I'm just going to check over all the files and make sure we didn't miss any.

[aydrian]

Libraries left to refactor:

• Message Events (convert to wrapper)
• Transmissions
• Webhooks
• Subaccounts (examples)