From 4b0b4ad991127d7cc787d1b936c09585b6fb9113 Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Fri, 9 Sep 2022 10:31:41 +0100 Subject: [PATCH] Revert "Use a more simple layout on the order cycle checkout options form" This reverts commit 0dddaa6c2b9f1925adca6ff8deac8181691ae119. --- app/helpers/admin/order_cycles_helper.rb | 21 ++-- .../order_cycles/checkout_options.html.haml | 114 ++++++++++-------- .../controllers/select_all_controller.js | 19 --- config/locales/en.yml | 1 + .../stimulus/select_all_controller_test.js | 113 ----------------- 5 files changed, 72 insertions(+), 196 deletions(-) delete mode 100644 app/webpacker/controllers/select_all_controller.js delete mode 100644 spec/javascripts/stimulus/select_all_controller_test.js diff --git a/app/helpers/admin/order_cycles_helper.rb b/app/helpers/admin/order_cycles_helper.rb index 9ce47e6f4c..6bc3fea258 100644 --- a/app/helpers/admin/order_cycles_helper.rb +++ b/app/helpers/admin/order_cycles_helper.rb @@ -2,19 +2,18 @@ module Admin module OrderCyclesHelper - def order_cycle_distributors_payment_methods(order_cycle) - Spree::PaymentMethod. - joins(:distributors). - includes(:distributors). - available(:both). - where("distributor_id IN (?)", order_cycle.distributors.select(:id)) + def order_cycle_shared_payment_methods(order_cycle) + order_cycle.attachable_payment_methods. + where("distributor_id IN (?)", order_cycle.distributors.select(:id)). + group("spree_payment_methods.id"). + having("COUNT(DISTINCT(distributor_id)) > 1") end - def order_cycle_distributors_shipping_methods(order_cycle) - Spree::ShippingMethod. - joins(:distributors). - includes(:distributors). - where("distributor_id IN (?)", order_cycle.distributors.select(:id)) + def order_cycle_shared_shipping_methods(order_cycle) + order_cycle.attachable_shipping_methods. + where("distributor_id IN (?)", order_cycle.distributors.select(:id)). + group("spree_shipping_methods.id"). + having("COUNT(DISTINCT(distributor_id)) > 1") end end end diff --git a/app/views/admin/order_cycles/checkout_options.html.haml b/app/views/admin/order_cycles/checkout_options.html.haml index c3380c444a..9b87b807c0 100644 --- a/app/views/admin/order_cycles/checkout_options.html.haml +++ b/app/views/admin/order_cycles/checkout_options.html.haml @@ -3,8 +3,8 @@ - content_for :page_title do = t :edit_order_cycle -- payment_methods = order_cycle_distributors_payment_methods(@order_cycle) -- shipping_methods = order_cycle_distributors_shipping_methods(@order_cycle) +- shared_payment_methods = order_cycle_shared_payment_methods(@order_cycle) +- shared_shipping_methods = order_cycle_shared_shipping_methods(@order_cycle) = form_for [main_app, :admin, @order_cycle], html: { class: "order_cycle" } do |f| @@ -15,58 +15,66 @@ = hidden_field_tag "order_cycle[selected_shipping_method_ids][]", "" - .row - .three.columns -   - .ten.columns - %table.checkout-options - %thead - %tr - %th{ colspan: 2 }= t('.shipping_methods') - %tr{ "data-controller": "select-all" } - %td.text-center - %label - = check_box_tag nil, nil, nil, { "data-action": "change->select-all#toggleAll", "data-select-all-target": "all" } - = t(".select_all") - %td - - if shipping_methods.any? - - shipping_methods.each do |shipping_method| - %p - %label{ class: ("disabled" unless shipping_method.frontend?) } - = check_box_tag "order_cycle[selected_shipping_method_ids][]", - shipping_method.id, @order_cycle.shipping_methods.include?(shipping_method), - disabled: !shipping_method.frontend?, - id: "order_cycle_selected_shipping_method_ids_#{shipping_method.id}", - data: ({ "action" => "change->select-all#toggleCheckbox", "select-all-target" => "checkbox" } if shipping_method.frontend?) - = shipping_method.name - - unless shipping_method.frontend? - = "(#{t('.back_end')})" - - if @order_cycle.distributors.many? + %table.checkout-options + %thead + %tr + %th= t('.distributor') + %th= t('.shipping_methods') + %th= t('.payment_methods') + - @order_cycle.distributors.each do |distributor| + - payment_methods = @order_cycle.attachable_payment_methods.where("distributor_id = ?", distributor.id) - shared_payment_methods + - shipping_methods = @order_cycle.attachable_shipping_methods.where("distributor_id = ?", distributor.id) - shared_shipping_methods + %tr + %td= distributor.name + %td + - shipping_methods.each do |shipping_method| + %p + %label + = check_box_tag "order_cycle[selected_shipping_method_ids][]", + shipping_method.id, @order_cycle.shipping_methods.include?(shipping_method), + id: "order_cycle_selected_shipping_method_ids_#{shipping_method.id}" + = shipping_method.name + - distributor.shipping_methods.backend.each do |shipping_method| + %label.disabled + = check_box_tag nil, nil, false, disabled: true + = shipping_method.name + = "(#{t('.back_end')})" + - if distributor.shipping_methods.frontend.none? + %p + = t('.no_shipping_methods') + %td + - if distributor.payment_methods.available(:both).any? + %ul + - payment_methods.each do |payment_method| + %li= payment_method.name + - else + %p + = t('.no_payment_methods') + - if shared_payment_methods.any? || shared_shipping_methods.any? + %tr + %td= t('.shared') + %td + - shared_shipping_methods.each do |shared_shipping_method| + %p + %label + = check_box_tag "order_cycle[selected_shipping_method_ids][]", + shared_shipping_method.id, @order_cycle.shipping_methods.include?(shared_shipping_method), + id: "order_cycle_selected_shipping_method_ids_#{shared_shipping_method.id}" + = shared_shipping_method.name + %p + — + %em + = shared_shipping_method.distributors.where(id: @order_cycle.distributor_ids).map(&:name).join(", ") + %td + - if shared_payment_methods.any? + %ul + - shared_payment_methods.each do |shared_payment_method| + %li + = shared_payment_method.name + %p — - %small - %em - = shipping_method.distributors.map(&:name).join(", ") - - else - %p - = t('.no_shipping_methods') - %tr - %th{ colspan: 2 }= t('.payment_methods') - %tr - %td - %td - - if payment_methods.any? - %ul - - payment_methods.each do |payment_method| - %li - = payment_method.name - - if @order_cycle.distributors.many? - — - %small - %em - = payment_method.distributors.map(&:name).join(", ") - - else - %p - = t('.no_payment_methods') + %em + = shared_payment_method.distributors.where(id: @order_cycle.distributor_ids).map(&:name).join(", ") %div#save-bar %div.container diff --git a/app/webpacker/controllers/select_all_controller.js b/app/webpacker/controllers/select_all_controller.js deleted file mode 100644 index 30a00aee79..0000000000 --- a/app/webpacker/controllers/select_all_controller.js +++ /dev/null @@ -1,19 +0,0 @@ -import { Controller } from "stimulus"; - -export default class extends Controller { - static targets = ["all", "checkbox"]; - - connect() { - this.toggleCheckbox() - } - - toggleAll() { - this.checkboxTargets.forEach(checkbox => { - checkbox.checked = this.allTarget.checked; - }); - } - - toggleCheckbox() { - this.allTarget.checked = this.checkboxTargets.every(checkbox => checkbox.checked); - } -} diff --git a/config/locales/en.yml b/config/locales/en.yml index 0739d192c9..63e4d3f830 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1166,6 +1166,7 @@ en: back_end: "Back office only" cancel: "Cancel" checkout_options: "Checkout options" + distributor: "Distributor" no_payment_methods: Each distributor on this order cycle requires at least one payment method. no_shipping_methods: Each distributor on this order cycle requires at least one shipping method. payment_methods: "Payment Methods" diff --git a/spec/javascripts/stimulus/select_all_controller_test.js b/spec/javascripts/stimulus/select_all_controller_test.js deleted file mode 100644 index f7a17b884f..0000000000 --- a/spec/javascripts/stimulus/select_all_controller_test.js +++ /dev/null @@ -1,113 +0,0 @@ -/** - * @jest-environment jsdom - */ - -import { Application } from "stimulus"; -import select_all_controller from "../../../app/webpacker/controllers/select_all_controller"; - -describe("SelectAllController", () => { - beforeAll(() => { - const application = Application.start(); - application.register("select-all", select_all_controller); - }); - - beforeEach(() => { - document.body.innerHTML = ` -
- - - -
- `; - }); - - describe("#toggleAll", () => { - it("checks all checkboxes when it's checked and unchecks them all when unchecked", () => { - const selectAllCheckbox = document.getElementById("selectAllCheckbox"); - const checkboxA = document.getElementById("checkboxA"); - const checkboxB = document.getElementById("checkboxB"); - expect(selectAllCheckbox.checked).toBe(false); - expect(checkboxA.checked).toBe(false); - expect(checkboxB.checked).toBe(false); - - selectAllCheckbox.click() - - expect(selectAllCheckbox.checked).toBe(true); - expect(checkboxA.checked).toBe(true); - expect(checkboxB.checked).toBe(true); - - selectAllCheckbox.click() - - expect(selectAllCheckbox.checked).toBe(false); - expect(checkboxA.checked).toBe(false); - expect(checkboxB.checked).toBe(false); - }); - }); - - describe("#toggleCheckbox", () => { - it("checks the individual checkbox and checks the select all checkbox if all checkboxes are checked and vice versa", () => { - const selectAllCheckbox = document.getElementById("selectAllCheckbox"); - const checkboxA = document.getElementById("checkboxA"); - const checkboxB = document.getElementById("checkboxB"); - checkboxA.click() - expect(selectAllCheckbox.checked).toBe(false); - expect(checkboxA.checked).toBe(true); - expect(checkboxB.checked).toBe(false); - - checkboxB.click() - - expect(selectAllCheckbox.checked).toBe(true); - expect(checkboxA.checked).toBe(true); - expect(checkboxB.checked).toBe(true); - - checkboxB.click() - - expect(selectAllCheckbox.checked).toBe(false); - expect(checkboxA.checked).toBe(true); - expect(checkboxB.checked).toBe(false); - }); - }); - - describe("#connect", () => { - beforeEach(() => { - document.body.innerHTML = ` -
- - - -
- `; - }); - - it("checks the select all checkbox on page load if all checkboxes are checked", () => { - const selectAllCheckbox = document.getElementById("selectAllCheckbox"); - expect(selectAllCheckbox.checked).toBe(true); - }); - }); -});