Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: remove catalog/publish/products meteor method, use publishProductsToCatalog GQL Mutation instead #5541

Merged

Conversation

kieckhafer
Copy link
Member

@kieckhafer kieckhafer commented Sep 11, 2019

This is a replacement of #5528, which I messed up with some rebasing while trying to get DCO to pass.

Resolves parts of #5529
Impact: minor
Type: refactor

Issue

publishContainer and productGridContainer still use the catalog/publish/products meteor method to publish products, which in turn uses getGraphQLContextInMeteorMethod.

Solution

Re-write anywhere that calls catalog/publish/products to use the publishProuctsToCatalog mutation instead, remove the catalog/publish/products meteor method, which will remove getGraphQLContextInMeteorMethod in this instance.

Breaking changes

Any custom code that uses the catalog/publish/products Meteor method will need to be re-written to use the publishProductsToCatalog graphql mutation

Testing

  1. See the catalog/publish/products meteor method has been removed
  2. Attempt to publish changes to a product to the catalog.
  3. See you are successful

@kieckhafer
Copy link
Member Author

@mikemurray @aldeed I messed something up big time while trying to get DCO to pass on #5528, so I'm closing that PR in favor of this one. It should be the same as the last one, with your comments all addressed.

Copy link
Contributor

@aldeed aldeed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kieckhafer Code looks good, but I did find one issue when testing. See inline comment.

@aldeed
Copy link
Contributor

aldeed commented Sep 11, 2019

I'm not sure what's up with the integration tests failing on CI. Hopefully just a fluke.

@kieckhafer
Copy link
Member Author

@aldeed updated Mutation to use callbacks.

Tests seems to be passing again.

@aldeed aldeed merged commit e1c5c1d into develop Sep 12, 2019
@aldeed aldeed deleted the refactor-kieckhafer-removeContextInGraphQL-catalogPublishTake2 branch September 12, 2019 21:21
@aldeed aldeed mentioned this pull request Sep 23, 2019
45 tasks
@kieckhafer kieckhafer mentioned this pull request Sep 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants