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

[Consignments] Reenable Convection Stitching #2221

Merged
merged 5 commits into from
Mar 6, 2020

Conversation

damassi
Copy link
Member

@damassi damassi commented Mar 5, 2020

Related: artsy/convection#504

This unwires unstitched mutations related to creating / updating a consignments submission, in favor of stitching.

Now that convection PR has been merged, if one pulls down this PR and points Eigen at it, can create stitched submission at consignments staging. See https://convection-staging.artsy.net/admin/submissions/31121 for an example.

The schema conflicts below relate to old consignments convection schema, before it was modified to match MP, and can thus be ignored.

This is a phased roll-out:

  • Merge convection PR up above
  • Merge this PR
  • Enable ENABLE_CONSIGNMENTS_STITCHING on staging
  • Test submissions in iOS app thoroughly
  • Deploy to prod

There are details in the PR up above, but the main change required was to match the types / schema in Convection to what was manually built using GraphQL loaders and the REST API. This was required so as to avoid breaking changes to our iOS app and enable a code-free transition on the Eigen side of things.

If everything goes smoothly we can follow-up and delete a bunch of consignments code from MP as it will be entirely replaced by stitching.

This flow has been tested by:

  1. Setting CONVECTION_API_BASE=http://localhost:5000/api
  2. Setting ENABLE_CONSIGNMENTS_STITCHING=true`
  3. Booting Convection
  4. Running yarn sync-remote-schema to sync Convection's localhost schema
  5. Booting MP
  6. Booting Eigen and running the "shake device" action to show settings screen where MP endpoint was then pointed at MP localhost
  7. Creating a new Consignments submission, including image upload.

History

Previously, we ran into issues stitching in Convection. I believe this was before we had a pattern for transforming / aliasing type names across services. The problems observed in this issue no longer seem to apply.

@artsy-peril
Copy link
Contributor

artsy-peril bot commented Mar 6, 2020

Warnings
⚠️

The V2 schema in this PR has breaking changes with Force. Remember to update Reaction if necessary.

Type 'Asset' was removed
Field 'ConsignmentSubmissionConnection.edges' changed type from '[ConsignmentSubmissionEdge]' to '[SubmissionEdge]'
Field 'artistID' was removed from object type 'ConsignmentSubmission'
Field 'authenticityCertificate' was removed from object type 'ConsignmentSubmission'
Field 'dimensionsMetric' was removed from object type 'ConsignmentSubmission'
Field 'editionNumber' was removed from object type 'ConsignmentSubmission'
Field 'editionSize' was removed from object type 'ConsignmentSubmission'
Field 'locationCity' was removed from object type 'ConsignmentSubmission'
Field 'locationCountry' was removed from object type 'ConsignmentSubmission'
Field 'locationState' was removed from object type 'ConsignmentSubmission'
Field 'userID' was removed from object type 'ConsignmentSubmission'
Field 'ConsignmentSubmission.category' changed type from 'SubmissionCategoryAggregation' to 'ConsignmentSubmissionCategoryAggregation'
Field 'ConsignmentSubmission.edition' changed type from 'Boolean' to 'String'
Field 'ConsignmentSubmission.state' changed type from 'SubmissionStateAggregation' to 'ConsignmentSubmissionStateAggregation'
Input field 'CreateSubmissionMutationInput.category' changed type from 'SubmissionCategoryAggregation' to 'ConsignmentSubmissionCategoryAggregation'
Input field 'CreateSubmissionMutationInput.dimensionsMetric' changed type from 'SubmissionDimensionAggregation' to 'String'
Input field 'CreateSubmissionMutationInput.state' changed type from 'SubmissionStateAggregation' to 'ConsignmentSubmissionStateAggregation'
Input field 'userID' was removed from input object type 'CreateSubmissionMutationInput'
Input field 'UpdateSubmissionMutationInput.id' changed type from 'String!' to 'ID!'
Input field 'UpdateSubmissionMutationInput.category' changed type from 'SubmissionCategoryAggregation' to 'ConsignmentSubmissionCategoryAggregation'
Input field 'UpdateSubmissionMutationInput.dimensionsMetric' changed type from 'SubmissionDimensionAggregation' to 'String'
Input field 'UpdateSubmissionMutationInput.state' changed type from 'SubmissionStateAggregation' to 'ConsignmentSubmissionStateAggregation'
Input field 'userID' was removed from input object type 'UpdateSubmissionMutationInput'
Input field 'AddAssetToConsignmentSubmissionInput.submissionID' changed type from 'String!' to 'ID!'
Field 'AddAssetToConsignmentSubmissionPayload.asset' changed type from 'Asset' to 'ConsignmentSubmissionCategoryAsset'

Generated by 🚫 dangerJS against 2e242d6

@damassi damassi force-pushed the reenable-convection-stitching branch from 88cf857 to c162a0d Compare March 6, 2020 02:24
@damassi
Copy link
Member Author

damassi commented Mar 6, 2020

@mzikherman / @jonallured - this is ready to go.

@damassi damassi force-pushed the reenable-convection-stitching branch from c162a0d to 0a13845 Compare March 6, 2020 20:03
@damassi damassi assigned ashfurrow and unassigned mzikherman Mar 6, 2020
Copy link
Contributor

@ashfurrow ashfurrow left a comment

Choose a reason for hiding this comment

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

Very cool! Makes sense to me, thank you for annotating soon-to-be-deleted tests. Very very cool how much code we get to delete 🤩 The schema changes themselves appear to be backwards-compatible, but I agree that thorough testing on staging would be prudent.

To make sure we have adequate staging testing before promoting to prod, I'm going to assign this back to @damassi to merge at his discretion.

@ashfurrow ashfurrow assigned damassi and unassigned ashfurrow Mar 6, 2020
@damassi damassi merged commit a5d6b3d into artsy:master Mar 6, 2020
@artsy-peril artsy-peril bot mentioned this pull request Mar 6, 2020
@damassi damassi deleted the reenable-convection-stitching branch March 6, 2020 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants