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: payments section of Orders 2.0 #5279

Merged
merged 25 commits into from
Jul 9, 2019

Conversation

kieckhafer
Copy link
Member

@kieckhafer kieckhafer commented Jul 4, 2019

Fixes the Payments portion of #5157
Impact: major
Type: refactor

Issue

Refactoring the Orders Operator UI to deal with multiple fulfillment groups, graphql, etc, to make it 2.0 ready. This PR splits off the Payments portion of the overall work to make it easier to test in smaller chunks.

Portions of the new UI are already available, but this in particular will update strictly the payments section, and the mutations associated with it: approvePayments, and capturePayments.

Testing

  • Create multiple orders to perform various actions on. Some should have multiple payment methods (this can be achieved by using the IOU payment method, and putting in a number less than the overall total into the amount field). When viewing the orders table, click on an order, and then once on the single order page, change the URL from /orders/ to /orders-2.0/ to see the new UI. The payments section, which is all that is being tested here, should look like this:

Order_Details_for_order_reference__NLuZFydBS6oHLS4Zb

Test the following workflows:

  • Create a user that has admin permission, but not reaction-order permission, and see that the user can see a "read-only" version of the payments, i.e. no Capture buttons show, but all other data does. If you don't want to create a new user, you can manually go into the OrderCardPayments and OrderCardPayment files, and change all hasPermission consts to be false., instead of doing an actual permissions check.

  • On an order with multiple payment methods, use the Capture all payments button to capture all payments. Do the same on an order with a single payment method. See that the Capture button disappears when a payment has already been captured, and statuses have been updated accordingly.

  • On an order with multiple payment methods, use the Capture payment button on each payment to individually capture each payment. Do the same on an order with a single payment method. See that the Capture button disappears when a payment has already been captured.

  • On an order with 4+ payment methods, in the database, manually change one or more payments to have riskLevel: highest. This will cause the Capture button to open a dialog and confirm you want to capture with a second click. Use the Capture all payments button and make sure the dialog opens. On another order, do the same thing with the riskLevel, and capture a single non-risky payment. See that the confirm doesn't pop up, since that particular payment is not risky. Then, us the single capture to capture the risky payment. See that the dialog still comes up. Then, capture the rest of the the non-risky payments using the Capture all payments button, and see the dialog does not come up, since the risky payment has been captured.

  • On an order, either force an error when capturing the payment (I'm not sure how to do this), or manually add a string field called captureErrorMessage to the payments object (the data can be any string, like "there was an error oh no". See that the error shows inside the Payment.

Order_Details_for_order_reference__NLuZFydBS6oHLS4Zb 2

@kieckhafer
Copy link
Member Author

kieckhafer commented Jul 4, 2019

@mikemurray the main new code here is in the components OrderCardPayment, and OrderCardPayments. Most of the other file changes are just moving code around for better organization, fixing some styling issues due to the new designs, etc.

There are a couple files which seem like duplicates, but I've left a comment on the top DEPRECATED: DELETE WHEN METEOR IS REMOVED. This is because orders-2.0 is still not the default, and the untouched meteor version of the page is still what everyone sees if they don't go to the special URL.

@kieckhafer
Copy link
Member Author

kieckhafer commented Jul 6, 2019

@aldeed @spencern @mikemurray I've run into the following lint warning in a few files that I've converted from Class to functional components: react/no-multi-comp

In my two files here that threw that warning, I put a disable line for that rule at the top of the file.

I've done some reading into it, it seems like this is a frequent issue, so much so that they added a flag to only use that eslint rule on Class components. What do you all think about flipping the ignoreStateless bool to true for this particular rule?

https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/no-multi-comp.md`

Signed-off-by: Erik Kieckhafer <[email protected]>
Signed-off-by: Erik Kieckhafer <[email protected]>
Signed-off-by: Erik Kieckhafer <[email protected]>
Signed-off-by: Erik Kieckhafer <[email protected]>
Signed-off-by: Erik Kieckhafer <[email protected]>
Signed-off-by: Erik Kieckhafer <[email protected]>
Signed-off-by: Erik Kieckhafer <[email protected]>
Signed-off-by: Erik Kieckhafer <[email protected]>
Signed-off-by: Erik Kieckhafer <[email protected]>
Copy link
Member

@mikemurray mikemurray left a comment

Choose a reason for hiding this comment

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

👍 Looks good when the snyk issue is resolved in this PR. #5299

@kieckhafer kieckhafer merged commit 185afb3 into develop Jul 9, 2019
@kieckhafer kieckhafer deleted the ref-kieckhafer-orderCardPayments branch July 9, 2019 23:12
@kieckhafer kieckhafer mentioned this pull request Aug 2, 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