Feature: add bulk mutations to manage products and tags

[willopez]

Resolves #5384
Impact: minor
Type: feature

Summary

This PR adds mutations to allow store operators to manage products and tags in bulk, a list of implemented mutations follows:

addTagsToProducts(input: ProductTagsOperationInput):ProductTagsOperationPayload

removeTagsFromProducts(input: ProductTagsOperationInput):ProductTagsOperationPayload

Breaking changes

None

Testing

Test info for addTagsToProducts

Mutation

mutation addTagsToProducts($input: ProductTagsOperationInput!) {
  addTagsToProducts(input: $input) {
    foundCount
    notFoundCount
    updatedCount
    writeErrors {
      documentId
      errorMsg
    }
  }
}

Sample variables
NOTE Actual values for productIds and tagIds will vary depending on your Reaction instance, replace values as necessary

{
  "input": {
    "productIds": ["BCTMZ6HTxFSppJESk"],
    "tagIds": ["cseCBSSrJ3t8HQSNP"]
  }
}

Test info for removTagsFromProducts

Mutation

mutation removeTagsFromProducts($input: ProductTagsOperationInput!) {
  removeTagsFromProducts(input: $input) {
    foundCount
    notFoundCount
    updatedCount
    writeErrors {
      documentId
      errorMsg
    }
  }
}

Sample variables
NOTE Actual values for productIds and tagIds will vary depending on your Reaction instance, replace values as necessary

{
  "input": {
    "productIds": ["BCTMZ6HTxFSppJESk"],
    "tagIds": ["cseCBSSrJ3t8HQSNP"]
  }
}

[aldeed]

I think it's better if we have a single GraphQL mutation, maybe updateProducts, which accepts a list of actions to perform with associated data.

By actions, I mean an enum with values such as "addTags", "removeTags", "setVisibility".

I did initially think that more specific mutations would be best, but I've changed my mind for a couple reasons:

• If a client needs to do several things in a row with the same product set, it will be much more efficient to do all three in a single bulk operation. For example, I want to "add a tag, remove a different tag, make visible, then publish". The UI can allow you to define this entire proposed update, and then execute it with a single GraphQL call. If we did it by specifying multiple mutations in the same request, the server would be doing extra work for permission checks, lookups, and multiple bulkWrites.
• A secondary benefit is that it requires less code and effort to add additional supported update actions.

@focusaurus @willopez Thoughts?

[willopez]

@aldeed I do like the idea applying multiple actions to a set of products. However, the design team has gone through a couple of iterations, and has gotten feedback from a client(SDI) for executing one action at a time. See image for design details. I believe that for the first implementation, executing one action(a single mutation) at a time will suffice. Further, the design would need to be updated to support multiple actions + getting feedback from clients, which would delay this feature from being released.

In short, I believe multiple action support can be added in the future, in the mean time this will be a good start.

[focusaurus]

The proposed bulk update mechanism seems like a bespoke/parochial thing outside of graphql norms so I would lean against it, especially if we can already do a series of mutations in the same graphql request. The more complexity and time a request needs to run the more issues we'll hit in terms of timeouts, partial applications, transactional errors, client retries, etc. If we're not cheaply queuing up work for background processing, I think we need to keep overall request workload within fairly small bounds.

Looking at your specific example, can we not already set the tags and visibility in a single mutation? Or is tag management always add/remove one tag per mutation?

[aldeed]

@focusaurus We could support setting tags along with visibility, but we currently don't. However, the designs call for needing to add or remove tags without necessarily having fetched the existing tag list, so we probably need all three: add, remove, set.

@willopez I don't have a strong opinion and I believe we may someday need both varieties, so if we want to go with the simpler, more Graphql-like individual mutations for now, that works. After getting burned by performance issues on bulk updates recently, I just thought it needed some discussion with that mindset.

[aldeed]

Other than my inline comments, I would remove "bulk" prefix from all the functions / mutations. Having plural "products" seems like enough since the docs and the parameter descriptions make it clear.

[willopez]

@aldeed I have addressed all your comments, ready for another review. FYI, the lint errors are not generated by the files changed on this PR.

Transferred review to Mike, Eric is on vacation

[mikemurray]

Got some eslint issues to fixes in changed files:
https://circleci.com/gh/reactioncommerce/reaction/52555?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-checks-link

[mikemurray]

Just merged from develop to see if there are changes to the changed files check.

Updating from develop resolved the issue

[mikemurray]

👍