Fix Ship Order menu choice not working

[cyrillefr]

What? Why?

• Closes #12369

Error when trying to ship an order when everywhere in admin/orders/RXX/customer, admin/orders/RXXX/payments, admin/orders/RXXXX/adjustments, admin/orders/RXXXXX/return_authorizations Reason: there were not the ShipOrderComponent which enables a modal for shipping order. The same component this present when in the admin/orders/R52384XXX/edit page

• added ShipOrderComponent in the view that handles the dropdown (_order_links.html.haml)
• updated spec to test for a specific case

What should we test?

• As an admin, visit admin/orders.
• Choose an item that has yet to be shipped. (the ones that got to be captured)
• Edit it -> admin/orders/R52384XXX/edit
• Click anything a part 'ORDER DETAILS', eg CUSTOMER DETAILS or ADJUSTMENTS
• Select 'Ship Order' from the actions dropdown menu.
• the modal should appear without errors and be functional
• clicking on 'Confirm' should put the item in the shipped state.
• By going/clicking on 'Order Details', it should be displayed 'shipped'

Release notes

Changelog Category (reviewers may add a label for the release notes):

• [ ] User facing changes
• [ ] API changes (V0, V1, DFC or Webhook)
• [X] Technical changes only
• [ ] Feature toggled

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

[filipefurtad0]

Hey @cyrillefr ,

I've submitted a PR (here) with a spec reproducing this regression (it's currently in code review). In case you find it useful, please feel free to cherry pick it, for this PR :+1:

[cyrillefr]

Hello @filipefurtad0 ,

Thanks for the offer, I think your PR will be complementary to mine. Coverage will be top. It 's about to be merged, so I feel a bit insecure to change it now :)

[filipefurtad0]

Thanks for the offer, I think your PR will be complementary to mine. Coverage will be top. It 's about to be merged, so I feel a bit insecure to change it now :)

Thanks @cyrillefr - that's perfect as well: after merging, I can remove the pending from the tests :+1:

[mkllnk]

Oh, I just merged the other PR. So we have to resolve it here now.

[mkllnk]

Hm, the new specs seem to be failing. I shouldn't have merged them without testing against the solution.

[cyrillefr]

Hello @mkllnk ,

some words about my last changes. I have dropped my own spec to work with the shared examples made by @filipefurtad0.

I have added a click to the 'Order Details' part to check for the shipped status (it only writes there). To do so, I need to close the modal, which does not closes itself.

I then had a problem, from time to time: the mailing job was not enqueued. I think it is because when the modal closes, the mailing job did not have enough time to be enqueued. A little sleep(0.5) solved this flakyness.

But it is not a good practice, so I have looked for another solution. Which is to check at the end of the example if the job was enqueued. It requires some more precision about what kind of job to check, but is works well. I have run the shared examples 10 times (100 in total) and they pass. So I am confident there is no flakyness.

[mkllnk]

I need to close the modal, which does not closes itself.

But that's a bug, right? Shouldn't the modal close when you choose an action? @filipefurtad0

[filipefurtad0]

I need to close the modal, which does not closes itself.

But that's a bug, right? Shouldn't the modal close when you choose an action? @filipefurtad0

I've staged the PR, and I think you mean the confirmation modal - right? The dropdown closes when clicked, but the confirmation modal indeed requires a click/action from the user. Is this what you refer to @cyrillefr ? If so I think it is the correct behavior.

[mkllnk]

I meant this confirmation modal:

After clicking CONFIRM, the modal doesn't close for me. Why should it stay open?

[cyrillefr]

Hello @filipefurtad0 ,

Yes the modal I am referring to is the one on the screenshot, when you edit an order.

[filipefurtad0]

I meant this confirmation modal: Yes the modal I am referring to is the one on the screenshot, when you edit an order.

Ok, I see it now. I've pulled the code from this PR and compared it with master: indeed, in master the confirmation modal closes after selecting an option; this is not the case within this PR.

I agree with @mkllnk: we should keep the current behavior (in master), i.e., the modal should close after selecting one of the options.

[cyrillefr]

Hello @filipefurtad0 , I have checked and here are the details:

• dev/local mode

  • in this branch, the modal closes itself - only - if the subpage is 'Order Details'.
  • if another sub-page, it does not

• testing mode

  • subpage 'Order Details' the modal does not close itself at first, you need to click twice ...which differs from the dev/local.
  • does not closes in the other subpages

Which means this modal is dependent of the subpage.

@mkllnk , I 'll go check and update as soon as possible.

[cyrillefr]

Hello @mkllnk ,

Now there is no need to close the modal manually in the specs. I had noticed that in app/reflex/admin/orders_reflex.rb there was a kind of a choice between edit (the page of concern) and the index (rows) page. But did not pay enough attention. What happens here is that there is a redirection to the classical Rails controllers, now for every sub-page (before that, only for the edit page). So the behaviour is the same now for every sub-page. But ... the modal does still not close itself (there is a need of a modal:close call, like at line 72: cable_ready.dispatch_event(name: "modal:close") .

But because there is a redirection to a classical Rails controller, the page is reloaded and the reload make the modal go away.

Also , I would need a rerun of the checks since the specs in error works fine locally.

[filipefurtad0]

Hey @cyrillefr ,

Awesome, thanks so much for your work on this and the thorough description.

I've tested both as superadmin and hub admin, selecting the Ship Order from the Actions drop down, on the order edit page, for several orders in shipment_state = ready, on the tabs:

• Customer Details
• Order Details
• Payments
• Adjustments

In all cases, the shipping state changed to shipped, after confirming within the modal; Emails arrived as expected.

Great to finally have this one through :clap: :clap: :clap: Merging.

[dacook]

It looks like this fixes a user-facing bug, so I'm updating the label. Please correct me if wrong!