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: move shipping and surcharges from server/no-meteor to node-app #5632

Merged
merged 20 commits into from Oct 2, 2019

Conversation

kieckhafer
Copy link
Member

@kieckhafer kieckhafer commented Sep 30, 2019

Resolves #5620 & #5618
Impact: minor
Type: refactor

As part of our effort to separate our Reaction Admin from our Reaction Node API, we are moving our GraphQL services from their respective /imports/plugins/*/server/no-meteor/ folders into their new /node-app/{core-services|plugins}/* folder.

Updates also include future-proofing some existing code, including adding filetype extensions (and index.js to imports)

This PR moves both the shipping and surcharges packages, as surcharges relies on shipping.

Breaking changes

Any custom plugins that import files directly from the address package will need to be updated.

Testing

  1. Start the application and see that it works as it should.
  2. Test any GraphQL mutation that has been moved in this process, such as:
query {
  surchargeById(surchargeId: "SURCHARGE_ID", shopId: "SHOP_ID") {
    _id
    amount
  }
}

Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
@kieckhafer kieckhafer changed the title Refactor kieckhafer move package to node app surcharges refactor: move shipping and surcharges from server/no-meteor to node-app Sep 30, 2019
Signed-off-by: Erik Kieckhafer <ek@ato.la>
@kieckhafer kieckhafer changed the title refactor: move shipping and surcharges from server/no-meteor to node-app [BLOCKED] refactor: move shipping and surcharges from server/no-meteor to node-app Sep 30, 2019
@kieckhafer
Copy link
Member Author

kieckhafer commented Sep 30, 2019

@aldeed there is a new query created here, so this is a bit more in depth than just moving files between packages, please double check the new getCommonOrderForCartGroup query, which replaces xformCartForCommonOrder.

@kieckhafer kieckhafer changed the title [BLOCKED] refactor: move shipping and surcharges from server/no-meteor to node-app refactor: move shipping and surcharges from server/no-meteor to node-app Sep 30, 2019
@kieckhafer kieckhafer added this to In progress in Demetorization Pod: Moving packages via automation Sep 30, 2019
@kieckhafer kieckhafer moved this from In progress to In Review in Demetorization Pod: Moving packages Sep 30, 2019
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
…group

Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
cartId: decodeCartOpaqueId(cartId),
fulfillmentGroupId: decodeFulfillmentGroupOpaqueId(fulfillmentGroupId)
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for not being clear. I don't think it's a good idea to have this query in GraphQL. It should be for internal use only. Maybe this would change at some point but for now CommonOrder is more of an internal concept among plugins that doesn't need to be part of the GraphQL schema.

Signed-off-by: Erik Kieckhafer <ek@ato.la>
Signed-off-by: Erik Kieckhafer <ek@ato.la>
@kieckhafer
Copy link
Member Author

@aldeed comments addressed

Demetorization Pod: Moving packages automation moved this from In Review to Approved Oct 2, 2019
@aldeed aldeed merged commit 38eefaf into develop Oct 2, 2019
Demetorization Pod: Moving packages automation moved this from Approved to Done Oct 2, 2019
@aldeed aldeed deleted the refactor-kieckhafer-movePackageToNodeApp-surcharges branch October 2, 2019 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants