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

[Marketplace] Orders line items refund #2590

Merged
merged 174 commits into from
Sep 6, 2017

Conversation

joykare
Copy link
Contributor

@joykare joykare commented Jul 24, 2017

Resolves #2377

Notes

  • Include shipping option doesn't work as of now coz there isn't a way to have separate shipping costs for individual line items

What to test

How to test

  • Clone branch
  • Log in as an admin
  • Make an order with more than 2 items (of the same variant) on your cart
  • Navigate to orders dashboard

Part 1:

  • Make sure invoice card works as it did before
  • Approve, Capture and apply refund to make sure it still works

Part 2:

  • Select a line item
  • Note popover opens
  • Edit line items to return some of the items ordered
  • Go over designs to make sure workflow matches
  • Check order on mongo billing.paymentMethod/status should be partialRefund
  • Select all items for refund and refund.
  • Check order on mongo billing.paymentMethod/status should be refunded

@brent-hoover
Copy link
Collaborator

@joykare Did you deal with the issue of not bubbling up errors from the API? I think that is the only blocker I had left.

@joykare
Copy link
Contributor Author

joykare commented Sep 6, 2017

@zenweasel the bubbling issue is fixed now.

Notes

  • At the moment changing the email sent
  • For now renaming all the variables named return... to refunds

[Update] Since the email issue involves creating a new template for emails sent after refunding items, I think it best to have it in a separate ticket. Thoughts on this are very welcome @spencern @rymorgan. I've added TODOs in the code in the meantime.

Sidenote --> I also noticed, the normal refund email doesn't show refund amount. (Adding a new ticket for this)

@spencern
Copy link
Contributor

spencern commented Sep 6, 2017

@joykare Are we not just updating copy on the existing return/refund email template? What new template do you need to create?

@joykare
Copy link
Contributor Author

joykare commented Sep 6, 2017

Was hoping to have the mailing in a new ticket, have this ticket merged first and hop onto that ticket. But if its critical for this ticket then I will do it here.

[Update] @spencern Email template done and uploaded.

@aaronjudd
Copy link
Contributor

Shippo + PayPal Express. Need to test with and safely check all payment types.

meteor.js?hash=814eae5b938a9a89ce7cf9174c98be4982f953d8:934 TypeError: Cannot read property 'support' of undefined
    at InvoiceContainer.hasRefundingEnabled (invoiceContainer.js:224)
    at InvoiceContainer.render (invoiceContainer.js:468)
    at modules.js?hash=034804c2473433a2e9b78c99244a582259ad3f15:108666
    at measureLifeCyclePerf (modules.js?hash=034804c2473433a2e9b78c99244a582259ad3f15:107946)
    at ReactCompositeComponentWrapper._renderValidatedComponentWithoutOwnerOrContext (modules.js?hash=034804c2473433a2e9b78c99244a582259ad3f15:108665)
    at ReactCompositeComponentWrapper._renderValidatedComponent (modules.js?hash=034804c2473433a2e9b78c99244a582259ad3f15:108692)
    at ReactCompositeComponentWrapper.performInitialMount (modules.js?hash=034804c2473433a2e9b78c99244a582259ad3f15:108232)
    at ReactCompositeComponentWrapper.mountComponent (modules.js?hash=034804c2473433a2e9b78c99244a582259ad3f15:108128)
    at Object.mountComponent (modules.js?hash=034804c2473433a2e9b78c99244a582259ad3f15:100889)
    at ReactCompositeComponentWrapper.performInitialMount (modules.js?hash=034804c2473433a2e9b78c99244a582259ad3f15:108241)
meteor.js?hash=814eae5b938a9a89ce7cf9174c98be4982f953d8:934 Exception from Tracker recompute function:
meteor.js?hash=814eae5b938a9a89ce7cf9174c98be4982f953d8:934 TypeError: Cannot read property '_currentElement' of null
    at ReactCompositeComponentWrapper._updateRenderedComponent (modules.js?hash=034804c2473433a2e9b78c99244a582259ad3f15:108615)
    at ReactCompositeComponentWrapper._performComponentUpdate (modules.js?hash=034804c2473433a2e9b78c99244a582259ad3f15:108594)
    at ReactCompositeComponentWrapper.updateComponent (modules.js?hash=034804c2473433a2e9b78c99244a582259ad3f15:108515)
    at ReactCompositeComponentWrapper.receiveComponent (modules.js?hash=034804c2473433a2e9b78c99244a582259ad3f15:108417)
    at Object.receiveComponent (modules.js?hash=034804c2473433a2e9b78c99244a582259ad3f15:100968)
    at ReactCompositeComponentWrapper._updateRenderedComponent (modules.js?hash=034804c2473433a2e9b78c99244a582259ad3f15:108624)
    at ReactCompositeComponentWrapper._performComponentUpdate (modules.js?hash=034804c2473433a2e9b78c99244a582259ad3f15:108594)
    at ReactCompositeComponentWrapper.updateComponent (modules.js?hash=034804c2473433a2e9b78c99244a582259ad3f15:108515)
    at ReactCompositeComponentWrapper.performUpdateIfNecessary (modules.js?hash=034804c2473433a2e9b78c99244a582259ad3f15:108431)
    at Object.performUpdateIfNecessary (modules.js?hash=034804c2473433a2e9b78c99244a582259ad3f15:101000)

dashboard_orders_and__marketplace__orders_line_items_refund_by_joykare_ pull_request__2590 _reactioncommerce_reaction

@aaronjudd aaronjudd self-requested a review September 6, 2017 17:03
Copy link
Contributor

@aaronjudd aaronjudd left a comment

Choose a reason for hiding this comment

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

Please test with all payment providers.

@joykare
Copy link
Contributor Author

joykare commented Sep 6, 2017

@aaronjudd the issue with hasRefundingEnabled wasnt introduced by this PR. There is a ticket to fix it (created by you). Checkout #2705

@aaronjudd
Copy link
Contributor

aaronjudd commented Sep 6, 2017

@joykare I've assigned #2705 to you then and I consider it to be a critical impact blocker for the release of marketplace. Seems to me, in reviewing, it should be a very quick fix.

@spencern
Copy link
Contributor

spencern commented Sep 6, 2017

@joykare I found a few more uses of "return" instead of "refund" but I went ahead and updated them so we can get this merged.
image

@spencern
Copy link
Contributor

spencern commented Sep 6, 2017

@zenweasel once you approve fixes for your requested changes we can go ahead and merge 🎉

@brent-hoover
Copy link
Collaborator

Bubbling up errors from Paypal, etc. seems to be working now
dashboard_orders

@brent-hoover brent-hoover merged commit d29acf7 into marketplace Sep 6, 2017
@brent-hoover brent-hoover deleted the joykare-orders-line-items-refund branch September 6, 2017 22:06
@spencern spencern mentioned this pull request Oct 11, 2017
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.