Replace cable_ready responses with turbo_stream OFN-12794 #12896
Conversation
wandji20
force-pushed
the
wb-OFN-12794
branch
12 times, most recently
from
c1080b7
to
73011b0
Compare
wandji20
force-pushed
the
wb-OFN-12794
branch
7 times, most recently
from
597a989
to
779e899
Compare
| @@ -951,10 +951,12 @@ def new_order_with_distribution(distributor, order_cycle) | |||
| order.finalize! # ensure order has a payment to capture | |||
| order.payments << create(:check_payment, order:, amount: order.total) | |||
| order.payments.first.capture! | |||
| login_as_admin | |||
There was a problem hiding this comment.
I could not figure out why the spec @enterprise_user had no permission to perform order ship action.
| visit spree.edit_admin_order_path(order) | ||
| end | ||
|
|
||
| it "ships the order and shipment email is sent" do | ||
| login_as_admin |
There was a problem hiding this comment.
Why is this needed ? it should already be logged as an admin because it's done in the before block.
There was a problem hiding this comment.
These specs are in the context of an enterprise manager and we have an enterprise manager signed in this block
before(:each) do
@enterprise_user = create(:user)
@enterprise_user.enterprise_roles.build(enterprise: supplier1).save
@enterprise_user.enterprise_roles.build(enterprise: coordinator1).save
@enterprise_user.enterprise_roles.build(enterprise: distributor1).save
login_as @enterprise_user
end
```
I had some trouble figuring out why the permission failed for the signed in enterprise user when trying to ship an order.
So I was hoping to get help during review.
| @@ -972,6 +974,7 @@ def new_order_with_distribution(distributor, order_cycle) | |||
| end | |||
|
|
|||
| it "ships the order without sending email" do | |||
| login_as_admin | |||
| = render ModalComponent.new(id: "resend_confirmation", modal_class: 'tiny', close_button: false) do | ||
| = form_with(url: resend_confirmation_emails_admin_orders_path, method: :post, data: { turbo: true, controller: 'bulk-actions' }) do | ||
| .margin-bottom-30 | ||
| = t('.resend_confirmation_confirm_html') | ||
| %p.modal-actions.justify-space-around | ||
| %button.button.secondary{ type: "button", 'data-action': 'click->modal#close' } | ||
| = t('js.admin.modals.cancel') | ||
| %button.button.primary{ type: 'submit' } | ||
| = t('js.admin.modals.confirm') | ||
|
|
||
| = render ModalComponent.new(id: "send_invoice", modal_class: 'tiny', close_button: false) do | ||
| = form_with(url: send_invoices_admin_orders_path, method: :post, data: { turbo: true, controller: 'bulk-actions' }) do | ||
| .margin-bottom-30 | ||
| = t('.send_invoice_confirm_html') | ||
| %p.modal-actions.justify-space-around | ||
| %button.button.secondary{ type: "button", 'data-action': 'click->modal#close' } | ||
| = t('js.admin.modals.cancel') | ||
| %button.button.primary{ type: 'submit' } | ||
| = t('js.admin.modals.confirm') | ||
|
|
||
|
|
||
| = render ModalComponent.new(id: "cancel_orders", modal_class: 'tiny', close_button: false) do | ||
| = form_with(url: cancel_orders_admin_orders_path, method: :post, data: { turbo: true, controller: 'bulk-actions' }) do | ||
| .margin-bottom-30 | ||
| = t("js.admin.orders.cancel_the_order_html") | ||
| .margin-bottom-30 | ||
| = render partial: "spree/admin/orders/messages/cancel_orders" | ||
|
|
||
| %p.modal-actions.justify-space-around | ||
| %button.button.secondary{ type: "button", 'data-action': 'click->modal#close' } | ||
| = t('js.admin.modals.cancel') | ||
| %button.button.primary{ type: 'submit' } | ||
| = t('js.admin.modals.confirm') |
There was a problem hiding this comment.
Any reasons why you are not using ConfirmModalComponent here ?
There was a problem hiding this comment.
I did this simply because the confirm modal did not support handling forms without refactoring.
I will refactor to remove these duplications.
| fetch(e.target.getAttribute('data-url'), { | ||
| method: "POST", | ||
| headers: { | ||
| Accept: "text/vnd.turbo-stream.html", | ||
| "Content-Type": "application/json", | ||
| "X-CSRF-Token": csrfToken | ||
| }, | ||
| body: JSON.stringify(data), | ||
| }) | ||
| .then((response) => response.text()) | ||
| .then((html) => { | ||
| Turbo.renderStreamMessage(html); | ||
| }) | ||
| .catch((error) => console.error(error)); |
There was a problem hiding this comment.
Maybe I am missing something here, but it feels like we are replicating something that Turbo should be doing for us ? Is there a way to refactor this so we don't have to manually use fetch ?
There was a problem hiding this comment.
My main reason was to be able to send data (bulk_ids) when printing invoices.
Another option I could think of was to use a button_to tag for the Print invoices bulk action and update the builk_ids input in the underlying generated form as I have done for the bulk_actions with a confirmation modal.
…ert to resend email confirmation with turbo_stream
…ns with turbo_stream
wandji20
force-pushed
the
wb-OFN-12794
branch
2 times, most recently
from
9f153e6
to
74ebcb2
Compare
wandji20
force-pushed
the
wb-OFN-12794
branch
from
74ebcb2
to
ad13f2e
Compare
What? Why?
Closes Replace mrujs with turbo #12794
Since Turbo already covers everything cable_ready does, we can safely move from cable_ready and mrujs to turbo.
What should we test?
The overall functionality of the app should be retained.
Release notes
Replace mrujs with turbo
Changelog Category (reviewers may add a label for the release notes):
Replace mrujs with turbo
Dependencies
N/A
Documentation updates
N/A