From 01cf1404b5fae0250fd377ddac10b7e165be4f58 Mon Sep 17 00:00:00 2001 From: andrea longhi Date: Wed, 28 Feb 2024 15:43:03 +0100 Subject: [PATCH 01/15] Fix tax category filtering We need to whitelist the ransackable attributes used on the new admin tax categories page, or filtering won't work. --- core/app/models/spree/tax_category.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/app/models/spree/tax_category.rb b/core/app/models/spree/tax_category.rb index f1d75da1910..d4dc8f857ff 100644 --- a/core/app/models/spree/tax_category.rb +++ b/core/app/models/spree/tax_category.rb @@ -4,6 +4,8 @@ module Spree class TaxCategory < Spree::Base include Spree::SoftDeletable + self.allowed_ransackable_attributes = %w[name description] + after_discard do self.tax_rate_tax_categories = [] end From 8f8a4be1c4ce972a0d40d311111576828911b2f3 Mon Sep 17 00:00:00 2001 From: andrea longhi Date: Wed, 28 Feb 2024 10:45:43 +0100 Subject: [PATCH 02/15] Fix tax category params The former ones were not correct, probably just a leftover from a cut and paste. --- .../app/controllers/solidus_admin/tax_categories_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/admin/app/controllers/solidus_admin/tax_categories_controller.rb b/admin/app/controllers/solidus_admin/tax_categories_controller.rb index f5f0bc1378e..2567ee50228 100644 --- a/admin/app/controllers/solidus_admin/tax_categories_controller.rb +++ b/admin/app/controllers/solidus_admin/tax_categories_controller.rb @@ -34,7 +34,7 @@ def load_tax_category end def tax_category_params - params.require(:tax_category).permit(:tax_category_id, permitted_tax_category_attributes) + params.require(:tax_category).permit(:name, :description, :is_default, :tax_code) end end end From 187849644269ca7909f9fd29319cb00c14500798 Mon Sep 17 00:00:00 2001 From: andrea longhi Date: Mon, 4 Mar 2024 17:03:08 +0100 Subject: [PATCH 03/15] Rework modal component The component now behaves like a standard dialog: * closes on the current page, rather than redirecting elsewhere; * the close button is a standard dialog closing form; * the `open` attribute is removed, thus allowing ESC keypress to close the modal, and creating a focus trap when open. The previous behavior (i.e. have the modal open by default) is achieved via the new Stimulus controller, which on connect opens the dialog via JS. --- .../solidus_admin/ui/modal/component.html.erb | 14 +++++++------- .../components/solidus_admin/ui/modal/component.js | 7 +++++++ .../components/solidus_admin/ui/modal/component.rb | 2 +- 3 files changed, 15 insertions(+), 8 deletions(-) create mode 100644 admin/app/components/solidus_admin/ui/modal/component.js diff --git a/admin/app/components/solidus_admin/ui/modal/component.html.erb b/admin/app/components/solidus_admin/ui/modal/component.html.erb index c9ce14bbf80..8b408161258 100644 --- a/admin/app/components/solidus_admin/ui/modal/component.html.erb +++ b/admin/app/components/solidus_admin/ui/modal/component.html.erb @@ -14,13 +14,13 @@

<%= @title %>

- <%= render component('ui/button').new( - tag: :a, - href: @close_path, - icon: 'close-line', - scheme: :ghost, - title: t('.close'), - ) %> +
+ <%= render component('ui/button').new( + icon: 'close-line', + scheme: :ghost, + title: t('.close'), + ) %> +
diff --git a/admin/app/components/solidus_admin/ui/modal/component.js b/admin/app/components/solidus_admin/ui/modal/component.js new file mode 100644 index 00000000000..3d9ec5c408e --- /dev/null +++ b/admin/app/components/solidus_admin/ui/modal/component.js @@ -0,0 +1,7 @@ +import { Controller } from "@hotwired/stimulus"; + +export default class extends Controller { + connect() { + this.element.showModal(); + } +} diff --git a/admin/app/components/solidus_admin/ui/modal/component.rb b/admin/app/components/solidus_admin/ui/modal/component.rb index 86f2401c04c..7dae58a192d 100644 --- a/admin/app/components/solidus_admin/ui/modal/component.rb +++ b/admin/app/components/solidus_admin/ui/modal/component.rb @@ -3,7 +3,7 @@ class SolidusAdmin::UI::Modal::Component < SolidusAdmin::BaseComponent renders_one :actions - def initialize(title:, close_path: nil, open: true, **attributes) + def initialize(title:, close_path: nil, open: false, **attributes) @title = title @close_path = close_path @attributes = attributes From dca0c68f54a246e679f31779d310567c86a0308e Mon Sep 17 00:00:00 2001 From: andrea longhi Date: Thu, 29 Feb 2024 11:47:28 +0100 Subject: [PATCH 04/15] Add turbo frames to page component This way we can inject arbitrary turbo frames into pages rendered via this component. --- .../solidus_admin/ui/pages/index/component.html.erb | 4 ++++ .../app/components/solidus_admin/ui/pages/index/component.rb | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/admin/app/components/solidus_admin/ui/pages/index/component.html.erb b/admin/app/components/solidus_admin/ui/pages/index/component.html.erb index e9774650a8c..69916f08dd4 100644 --- a/admin/app/components/solidus_admin/ui/pages/index/component.html.erb +++ b/admin/app/components/solidus_admin/ui/pages/index/component.html.erb @@ -37,4 +37,8 @@ <% end %> <% end %> <% end %> + + <% turbo_frames.each do |frame| %> + <%= turbo_frame_tag frame %> + <% end %> <% end %> diff --git a/admin/app/components/solidus_admin/ui/pages/index/component.rb b/admin/app/components/solidus_admin/ui/pages/index/component.rb index 15d31a4089e..6ee519537bb 100644 --- a/admin/app/components/solidus_admin/ui/pages/index/component.rb +++ b/admin/app/components/solidus_admin/ui/pages/index/component.rb @@ -98,4 +98,8 @@ def render_sidebar page_with_sidebar_aside { sidebar } if sidebar end + + def turbo_frames + [] + end end From ef061448635ff10005065107c4e46c06bafb8332 Mon Sep 17 00:00:00 2001 From: andrea longhi Date: Tue, 27 Feb 2024 18:30:49 +0100 Subject: [PATCH 05/15] Show link to new tax category page The correct method name is `page_actions`, for this reason it was previously not showing. Also, href is changed to a new route from the new admin. --- .../solidus_admin/tax_categories/index/component.rb | 4 ++-- admin/config/routes.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/admin/app/components/solidus_admin/tax_categories/index/component.rb b/admin/app/components/solidus_admin/tax_categories/index/component.rb index 47ff3fb60bf..fc14ed906c1 100644 --- a/admin/app/components/solidus_admin/tax_categories/index/component.rb +++ b/admin/app/components/solidus_admin/tax_categories/index/component.rb @@ -13,11 +13,11 @@ def search_url solidus_admin.tax_categories_path end - def actions + def page_actions render component("ui/button").new( tag: :a, text: t('.add'), - href: spree.new_admin_tax_category_path, + href: solidus_admin.new_tax_category_path, icon: "add-line", class: "align-self-end w-full", ) diff --git a/admin/config/routes.rb b/admin/config/routes.rb index c938ea27de6..f8f729d68f6 100644 --- a/admin/config/routes.rb +++ b/admin/config/routes.rb @@ -51,7 +51,7 @@ admin_resources :option_types, only: [:index, :destroy], sortable: true admin_resources :taxonomies, only: [:index, :destroy], sortable: true admin_resources :promotion_categories, only: [:index, :destroy] - admin_resources :tax_categories, only: [:index, :destroy] + admin_resources :tax_categories, only: [:new, :index, :destroy] admin_resources :tax_rates, only: [:index, :destroy] admin_resources :payment_methods, only: [:index, :destroy], sortable: true admin_resources :stock_items, only: [:index, :edit, :update] From 565e58ad724daaf3db0ff1c8d42d8f8d76a2eb9c Mon Sep 17 00:00:00 2001 From: andrea longhi Date: Mon, 4 Mar 2024 16:24:10 +0100 Subject: [PATCH 06/15] Update turbo-rails gem to v2.0 The new version adds cool new features such as page morphing and refreshes. Specifically, we're going to use turbo stream refreshes in a later commit. --- admin/solidus_admin.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/admin/solidus_admin.gemspec b/admin/solidus_admin.gemspec index 5878002997a..41867f53963 100644 --- a/admin/solidus_admin.gemspec +++ b/admin/solidus_admin.gemspec @@ -33,6 +33,6 @@ Gem::Specification.new do |s| s.add_dependency 'solidus_backend' s.add_dependency 'solidus_core', '> 4.2' s.add_dependency 'stimulus-rails', '~> 1.2' - s.add_dependency 'turbo-rails', '~> 1.4' + s.add_dependency 'turbo-rails', '~> 2.0' s.add_dependency 'view_component', '~> 3.9' end From 15899d4fd12d80c0c814701dc75c5625b71b06dd Mon Sep 17 00:00:00 2001 From: andrea longhi Date: Mon, 4 Mar 2024 17:36:56 +0100 Subject: [PATCH 07/15] Add TaxCategoriesController#new action The `new` action will render a modal dialog above a list of tax categories, like the `index` action. For this reason, the common controller code is extracted to a private method. --- .../tax_categories_controller.rb | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/admin/app/controllers/solidus_admin/tax_categories_controller.rb b/admin/app/controllers/solidus_admin/tax_categories_controller.rb index 2567ee50228..09ac37e3cbb 100644 --- a/admin/app/controllers/solidus_admin/tax_categories_controller.rb +++ b/admin/app/controllers/solidus_admin/tax_categories_controller.rb @@ -4,13 +4,18 @@ module SolidusAdmin class TaxCategoriesController < SolidusAdmin::BaseController include SolidusAdmin::ControllerHelpers::Search - def index - tax_categories = apply_search_to( - Spree::TaxCategory.order(created_at: :desc, id: :desc), - param: :q, - ) + def new + @tax_category = Spree::TaxCategory.new - set_page_and_extract_portion_from(tax_categories) + set_index_page + + respond_to do |format| + format.html { render component('tax_categories/new').new(page: @page, tax_category: @tax_category) } + end + end + + def index + set_index_page respond_to do |format| format.html { render component('tax_categories/index').new(page: @page) } @@ -36,5 +41,14 @@ def load_tax_category def tax_category_params params.require(:tax_category).permit(:name, :description, :is_default, :tax_code) end + + def set_index_page + tax_categories = apply_search_to( + Spree::TaxCategory.order(created_at: :desc, id: :desc), + param: :q, + ) + + set_page_and_extract_portion_from(tax_categories) + end end end From bcbd4ebd4180303a40736a4ae9ced067a864fd0c Mon Sep 17 00:00:00 2001 From: andrea longhi Date: Tue, 27 Feb 2024 19:03:14 +0100 Subject: [PATCH 08/15] Add New Tax Category component The component renders: 1) a list of tax categories in the background; 2) a dialog modal with the form for creating a new resource. The modal is wrapped into a turbo frame tag, so it can be enclosed in other pages (i.e. the tax categories index) asynchronously. --- .../tax_categories/index/component.rb | 6 +++- .../tax_categories/new/component.html.erb | 28 +++++++++++++++++++ .../tax_categories/new/component.rb | 12 ++++++++ .../tax_categories/new/component.yml | 8 ++++++ 4 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 admin/app/components/solidus_admin/tax_categories/new/component.html.erb create mode 100644 admin/app/components/solidus_admin/tax_categories/new/component.rb create mode 100644 admin/app/components/solidus_admin/tax_categories/new/component.yml diff --git a/admin/app/components/solidus_admin/tax_categories/index/component.rb b/admin/app/components/solidus_admin/tax_categories/index/component.rb index fc14ed906c1..c435fb1da7c 100644 --- a/admin/app/components/solidus_admin/tax_categories/index/component.rb +++ b/admin/app/components/solidus_admin/tax_categories/index/component.rb @@ -17,12 +17,16 @@ def page_actions render component("ui/button").new( tag: :a, text: t('.add'), - href: solidus_admin.new_tax_category_path, + href: solidus_admin.new_tax_category_path, data: { turbo_frame: :new_tax_category_modal }, icon: "add-line", class: "align-self-end w-full", ) end + def turbo_frames + %w[new_tax_category_modal] + end + def search_key :name_or_description_cont end diff --git a/admin/app/components/solidus_admin/tax_categories/new/component.html.erb b/admin/app/components/solidus_admin/tax_categories/new/component.html.erb new file mode 100644 index 00000000000..b1a97dcc9be --- /dev/null +++ b/admin/app/components/solidus_admin/tax_categories/new/component.html.erb @@ -0,0 +1,28 @@ +<%= turbo_frame_tag :new_tax_category_modal do %> + <%= render component("ui/modal").new(title: t(".title")) do |modal| %> + <%= form_for @tax_category, url: solidus_admin.tax_categories_path(page: params[:page], q: params[:q]), html: { id: form_id } do |f| %> +
+ <%= render component("ui/forms/field").text_field(f, :name) %> + <%= render component("ui/forms/field").text_field(f, :tax_code) %> + <%= render component("ui/forms/field").text_field(f, :description) %> + +
+ <% modal.with_actions do %> +
+ <%= render component("ui/button").new(scheme: :secondary, text: t('.cancel')) %> +
+ <%= render component("ui/button").new(form: form_id, type: :submit, text: t('.submit')) %> + <% end %> + <% end %> + <% end %> +<% end %> + +<%= render component("tax_categories/index").new(page: @page) %> diff --git a/admin/app/components/solidus_admin/tax_categories/new/component.rb b/admin/app/components/solidus_admin/tax_categories/new/component.rb new file mode 100644 index 00000000000..d61ef1e3dd5 --- /dev/null +++ b/admin/app/components/solidus_admin/tax_categories/new/component.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class SolidusAdmin::TaxCategories::New::Component < SolidusAdmin::TaxCategories::Index::Component + def initialize(page:, tax_category:) + @page = page + @tax_category = tax_category + end + + def form_id + dom_id(@tax_category, "#{stimulus_id}_new_tax_category_form") + end +end diff --git a/admin/app/components/solidus_admin/tax_categories/new/component.yml b/admin/app/components/solidus_admin/tax_categories/new/component.yml new file mode 100644 index 00000000000..24706db9edf --- /dev/null +++ b/admin/app/components/solidus_admin/tax_categories/new/component.yml @@ -0,0 +1,8 @@ +# Add your component translations here. +# Use the translation in the example in your template with `t(".hello")`. +en: + title: "New Tax Category" + cancel: "Cancel" + submit: "Add Tax Category" + hints: + is_default: "When checked, this tax category will be selected by default when creating new products or variants." From 5584f931e8045a77055a82d90c76b9374dfaa575 Mon Sep 17 00:00:00 2001 From: andrea longhi Date: Mon, 4 Mar 2024 18:02:00 +0100 Subject: [PATCH 09/15] Complete tax category creation process The `#create` controller action allows the creation of a new tax category. Failed creation re-renders the form with errors within the turbo frame tag, while successful creation is handled via a turbo stream refresh tag that reloads the page. Scroll position is preserved thanks to the custom Turbo meta tag that enables the feature globally on the new admin. Integration specs are added to test the complete feature. --- .../tax_categories_controller.rb | 29 ++++++++++++++ .../solidus_admin/application.html.erb | 1 + admin/config/locales/tax_categories.en.yml | 2 + admin/config/routes.rb | 2 +- admin/spec/features/tax_categories_spec.rb | 39 +++++++++++++++++++ 5 files changed, 72 insertions(+), 1 deletion(-) diff --git a/admin/app/controllers/solidus_admin/tax_categories_controller.rb b/admin/app/controllers/solidus_admin/tax_categories_controller.rb index 09ac37e3cbb..cc9e833a2ba 100644 --- a/admin/app/controllers/solidus_admin/tax_categories_controller.rb +++ b/admin/app/controllers/solidus_admin/tax_categories_controller.rb @@ -14,6 +14,35 @@ def new end end + def create + @tax_category = Spree::TaxCategory.new(tax_category_params) + + if @tax_category.save + respond_to do |format| + flash[:notice] = t('.success') + + format.html do + redirect_to solidus_admin.tax_categories_path, status: :see_other + end + + format.turbo_stream do + # we need to explicitly write the refresh tag for now. + # See https://github.com/hotwired/turbo-rails/issues/579 + render turbo_stream: '' + end + end + else + set_index_page + + respond_to do |format| + format.html do + page_component = component('tax_categories/new').new(page: @page, tax_category: @tax_category) + render page_component, status: :unprocessable_entity + end + end + end + end + def index set_index_page diff --git a/admin/app/views/layouts/solidus_admin/application.html.erb b/admin/app/views/layouts/solidus_admin/application.html.erb index 4fa30ebdfd8..246051776a2 100644 --- a/admin/app/views/layouts/solidus_admin/application.html.erb +++ b/admin/app/views/layouts/solidus_admin/application.html.erb @@ -11,6 +11,7 @@ <%= stylesheet_link_tag SolidusAdmin::Config.theme_path(session[:admin_light_theme]), media: '(prefers-color-scheme: light)', "data-turbo-track": "reload" %> <%= stylesheet_link_tag SolidusAdmin::Config.theme_path(session[:admin_dark_theme]), media: '(prefers-color-scheme: dark)', "data-turbo-track": "reload" %> <%= javascript_importmap_tags "solidus_admin/application", shim: false, importmap: SolidusAdmin.importmap %> + diff --git a/admin/config/locales/tax_categories.en.yml b/admin/config/locales/tax_categories.en.yml index 9162bb44069..c86f9400f42 100644 --- a/admin/config/locales/tax_categories.en.yml +++ b/admin/config/locales/tax_categories.en.yml @@ -4,3 +4,5 @@ en: title: "Tax Categories" destroy: success: "Tax categories were successfully removed." + create: + success: "Tax category was successfully created." diff --git a/admin/config/routes.rb b/admin/config/routes.rb index f8f729d68f6..37003949356 100644 --- a/admin/config/routes.rb +++ b/admin/config/routes.rb @@ -51,7 +51,7 @@ admin_resources :option_types, only: [:index, :destroy], sortable: true admin_resources :taxonomies, only: [:index, :destroy], sortable: true admin_resources :promotion_categories, only: [:index, :destroy] - admin_resources :tax_categories, only: [:new, :index, :destroy] + admin_resources :tax_categories, only: [:new, :index, :create, :destroy] admin_resources :tax_rates, only: [:index, :destroy] admin_resources :payment_methods, only: [:index, :destroy], sortable: true admin_resources :stock_items, only: [:index, :edit, :update] diff --git a/admin/spec/features/tax_categories_spec.rb b/admin/spec/features/tax_categories_spec.rb index cbc15b436c9..b91a24256dc 100644 --- a/admin/spec/features/tax_categories_spec.rb +++ b/admin/spec/features/tax_categories_spec.rb @@ -21,4 +21,43 @@ expect(Spree::TaxCategory.count).to eq(1) expect(page).to be_axe_clean end + + context "when creating a new tax category" do + let(:query) { "?page=1&q%5Bname_or_description_cont%5D=Cloth" } + + before do + visit "/admin/tax_categories#{query}" + click_on "Add new" + expect(page).to have_content("New Tax Category") + expect(page).to be_axe_clean + end + + it "opens a modal" do + expect(page).to have_selector("dialog") + within("dialog") { click_on "Cancel" } + expect(page).not_to have_selector("dialog") + expect(page.current_url).to include(query) + end + + context "with valid data" do + it "successfully creates a new tax category, keeping page and q params" do + fill_in "Name", with: "Clothing" + + click_on "Add Tax Category" + + expect(page).to have_content("Tax category was successfully created.") + expect(Spree::TaxCategory.find_by(name: "Clothing")).to be_present + expect(page.current_url).to include(query) + end + end + + context "with invalid data" do + it "fails to create a new tax category, keeping page and q params" do + click_on "Add Tax Category" + + expect(page).to have_content "can't be blank" + expect(page.current_url).to include(query) + end + end + end end From 0f26b3f9294190fac8de26e0bd12b08c0298880a Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Fri, 8 Mar 2024 16:43:34 +0100 Subject: [PATCH 10/15] Use configurable default promotion API attributes The API gem needs to know which attributes are available by default, and that is dependent on the promotion system that is being utilized. --- api/lib/spree/api_configuration.rb | 5 +---- .../lib/spree/core/null_promotion_configuration.rb | 4 ++++ core/lib/spree/core/promotion_configuration.rb | 14 ++++++++++++++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/api/lib/spree/api_configuration.rb b/api/lib/spree/api_configuration.rb index 3247c64dd39..f585af3603a 100644 --- a/api/lib/spree/api_configuration.rb +++ b/api/lib/spree/api_configuration.rb @@ -112,10 +112,7 @@ class ApiConfiguration < Preferences::Configuration :variant_id ] - preference :promotion_attributes, :array, default: [ - :id, :name, :description, :expires_at, :starts_at, :type, :usage_limit, - :advertise, :path - ] + preference :promotion_attributes, :array, default: Spree::Config.promotions.promotion_api_attributes preference :store_attributes, :array, default: [ :id, :name, :url, :meta_description, :meta_keywords, :seo_title, diff --git a/core/lib/spree/core/null_promotion_configuration.rb b/core/lib/spree/core/null_promotion_configuration.rb index e3b5a821a31..86b3693dbaf 100644 --- a/core/lib/spree/core/null_promotion_configuration.rb +++ b/core/lib/spree/core/null_promotion_configuration.rb @@ -21,6 +21,10 @@ class NullPromotionConfiguration < Spree::Preferences::Configuration # the standard promotion finder class # Spree::NullPromotionFinder. class_name_attribute :promotion_finder_class, default: 'Spree::NullPromotionFinder' + + # !@attribute [rw] promotion_api_attributes + # @return [Array] Attributes to be returned by the API for a promotion + preference :promotion_api_attributes, :array, default: [] end end end diff --git a/core/lib/spree/core/promotion_configuration.rb b/core/lib/spree/core/promotion_configuration.rb index b4c7ba7f208..97255268f7e 100644 --- a/core/lib/spree/core/promotion_configuration.rb +++ b/core/lib/spree/core/promotion_configuration.rb @@ -9,6 +9,20 @@ class PromotionConfiguration < Spree::Preferences::Configuration # @return [Integer] Promotions to show per-page in the admin (default: +15+) preference :promotions_per_page, :integer, default: 15 + # @!attribute [rw] promotion_attributes + # @return [Array] Attributes to be returned by the API for a promotion + preference :promotion_api_attributes, :array, default: [ + :id, + :name, + :description, + :expires_at, + :starts_at, + :type, + :usage_limit, + :advertise, + :path + ] + # promotion_chooser_class allows extensions to provide their own PromotionChooser class_name_attribute :promotion_chooser_class, default: 'Spree::PromotionChooser' From 70510e621af02db9208ca519b4d1cb638a18d746 Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Fri, 8 Mar 2024 17:04:26 +0100 Subject: [PATCH 11/15] Stub promotion handler in coupon codes spec When the legacy promotion system is extracted into its own gem, we won't have the luxury of factories any longer and will need to stub the API of a promotion handler ourselves. --- .../requests/spree/api/coupon_codes_spec.rb | 83 ++++++++++++++----- 1 file changed, 62 insertions(+), 21 deletions(-) diff --git a/api/spec/requests/spree/api/coupon_codes_spec.rb b/api/spec/requests/spree/api/coupon_codes_spec.rb index e08db7ba46e..c7324d9c58d 100644 --- a/api/spec/requests/spree/api/coupon_codes_spec.rb +++ b/api/spec/requests/spree/api/coupon_codes_spec.rb @@ -12,26 +12,36 @@ module Spree::Api before do stub_authentication! + expect(Spree::Config.promotions.coupon_code_handler_class).to receive(:new).and_return(handler) end describe '#create' do - let(:promo) { create(:promotion_with_item_adjustment, code: 'night_melody') } - let(:promo_code) { promo.codes.first } - before do allow_any_instance_of(Spree::Order).to receive_messages user: current_api_user end context 'when successful' do let(:order) { create(:order_with_line_items) } + let(:successful_application) do + double( + 'handler', + successful?: true, + success: "The coupon code was successfully applied to your order.", + error: nil, + status_code: "coupon_code_applied" + ) + end + + let(:handler) do + double('handler', apply: successful_application) + end it 'applies the coupon' do - post spree.api_order_coupon_codes_path(order), params: { coupon_code: promo_code.value } + post spree.api_order_coupon_codes_path(order), params: { coupon_code: "10OFF" } expect(response.status).to eq(200) - expect(order.reload.promotions).to eq([promo]) expect(json_response).to eq({ - "success" => I18n.t('spree.coupon_code_applied'), + "success" => "The coupon code was successfully applied to your order.", "error" => nil, "successful" => true, "status_code" => "coupon_code_applied" @@ -41,12 +51,24 @@ module Spree::Api context 'when unsuccessful' do let(:order) { create(:order) } + let(:unsuccessful_application) do + double( + 'handler', + successful?: false, + success: nil, + error: "This coupon code could not be applied to the cart at this time.", + status_code: "coupon_code_unknown_error" + ) + end + + let(:handler) do + double('handler', apply: unsuccessful_application) + end it 'returns an error' do - post spree.api_order_coupon_codes_path(order), params: { coupon_code: promo_code.value } + post spree.api_order_coupon_codes_path(order), params: { coupon_code: "10OFF" } expect(response.status).to eq(422) - expect(order.reload.promotions).to eq([]) expect(json_response).to eq({ "success" => nil, "error" => I18n.t('spree.coupon_code_unknown_error'), @@ -58,25 +80,30 @@ module Spree::Api end describe '#destroy' do - let(:promo) { - create(:promotion_with_item_adjustment, - code: 'tenoff', - per_code_usage_limit: 5, - adjustment_rate: 10) - } - - let(:promo_code) { promo.codes.first } let(:order) { create(:order_with_line_items, user: current_api_user) } - before do - post spree.api_order_coupon_codes_path(order), params: { coupon_code: promo_code.value } - delete spree.api_order_coupon_code_path(order, promo_code.value) + subject do + delete spree.api_order_coupon_code_path(order, "10OFF") end context 'when successful' do + let(:successful_removal) do + double( + 'handler', + successful?: true, + success: "The coupon code was successfully removed from this order.", + error: nil, + status_code: "coupon_code_removed" + ) + end + + let(:handler) do + double('handler', remove: successful_removal) + end + it 'removes the coupon' do + subject expect(response.status).to eq(200) - expect(order.reload.promotions).to eq([]) expect(json_response).to eq({ "success" => I18n.t('spree.coupon_code_removed'), "error" => nil, @@ -87,8 +114,22 @@ module Spree::Api end context 'when unsuccessful' do + let(:unsuccessful_removal) do + double( + 'handler', + successful?: false, + success: nil, + error: "The coupon code you are trying to remove is not present on this order.", + status_code: "coupon_code_not_present" + ) + end + + let(:handler) do + double('handler', remove: unsuccessful_removal) + end + it 'returns an error' do - delete spree.api_order_coupon_code_path(order, promo_code.value) + subject expect(response.status).to eq(422) expect(order.reload.promotions).to eq([]) From 9bcb60e581f94f39ebfa2dcaa7dd7abc4c2f0313 Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Tue, 12 Mar 2024 10:06:39 +0100 Subject: [PATCH 12/15] Legacy Promotions: Depend on API We need to move some specs from solidus_legacy_promotions into solidus_legacy_promotions. These specs use routes from the gem. Let's add api as a dependency. --- legacy_promotions/lib/solidus_legacy_promotions.rb | 1 + legacy_promotions/solidus_legacy_promotions.gemspec | 1 + legacy_promotions/spec/rails_helper.rb | 2 ++ 3 files changed, 4 insertions(+) diff --git a/legacy_promotions/lib/solidus_legacy_promotions.rb b/legacy_promotions/lib/solidus_legacy_promotions.rb index 3f5857cad20..11e190aa6e5 100644 --- a/legacy_promotions/lib/solidus_legacy_promotions.rb +++ b/legacy_promotions/lib/solidus_legacy_promotions.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "solidus_core" +require "solidus_api" require "solidus_support" module SolidusLegacyPromotions diff --git a/legacy_promotions/solidus_legacy_promotions.gemspec b/legacy_promotions/solidus_legacy_promotions.gemspec index 778d9fc367f..a31c951abd4 100644 --- a/legacy_promotions/solidus_legacy_promotions.gemspec +++ b/legacy_promotions/solidus_legacy_promotions.gemspec @@ -24,5 +24,6 @@ Gem::Specification.new do |s| s.required_rubygems_version = '>= 1.8.23' s.add_dependency 'solidus_core', s.version + s.add_dependency 'solidus_api', s.version s.add_dependency 'solidus_support' end diff --git a/legacy_promotions/spec/rails_helper.rb b/legacy_promotions/spec/rails_helper.rb index c86a7eba5e9..f8a29a778bb 100644 --- a/legacy_promotions/spec/rails_helper.rb +++ b/legacy_promotions/spec/rails_helper.rb @@ -20,6 +20,7 @@ require 'spree/testing_support/preferences' require 'spree/testing_support/rake' require 'spree/testing_support/job_helpers' +require 'spree/api/testing_support/helpers' require 'cancan/matchers' ActiveJob::Base.queue_adapter = :test @@ -48,4 +49,5 @@ config.include Spree::TestingSupport::JobHelpers config.include FactoryBot::Syntax::Methods + config.include Spree::Api::TestingSupport::Helpers, type: :request end From 17dac427d26316fada848efb8d8eb79124e81d2c Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Tue, 12 Mar 2024 09:39:03 +0100 Subject: [PATCH 13/15] Copy API feature spec to legacy_promotions We're keeping the non-promotion-related parts of this spec in core, but the promotion part has to go. For this spec, I don't think it makes sense to stub out all the things. --- api/spec/features/checkout_spec.rb | 18 +- .../spec/features/api/checkout_spec.rb | 192 ++++++++++++++++++ 2 files changed, 194 insertions(+), 16 deletions(-) create mode 100644 legacy_promotions/spec/features/api/checkout_spec.rb diff --git a/api/spec/features/checkout_spec.rb b/api/spec/features/checkout_spec.rb index f8638177fe0..21af915ed69 100644 --- a/api/spec/features/checkout_spec.rb +++ b/api/spec/features/checkout_spec.rb @@ -7,8 +7,6 @@ module Spree before do stub_spree_preferences(Spree::Api::Config, requires_authentication: false) end - let!(:promotion) { FactoryBot.create(:promotion, :with_order_adjustment, code: 'foo', weighted_order_adjustment_amount: 10) } - let(:promotion_code) { promotion.codes.first } let!(:store) { FactoryBot.create(:store) } let(:bill_address) { FactoryBot.create(:address) } let(:ship_address) { FactoryBot.create(:address) } @@ -62,14 +60,6 @@ def create_line_item(variant, quantity = 1) expect(response).to have_http_status(:created) end - def add_promotion(_promotion) - expect { - post "/api/orders/#{@order.number}/coupon_codes", - params: { coupon_code: promotion_code.value } - }.to change { @order.promotions.count }.by 1 - expect(response).to have_http_status(:ok) - end - def add_address(address, billing: true) address_type = billing ? :bill_address : :ship_address # It seems we are missing an order-scoped address api endpoint since we need @@ -103,8 +93,8 @@ def assert_order_expectations expect(@order.state).to eq 'complete' expect(@order.completed_at).to be_a ActiveSupport::TimeWithZone expect(@order.item_total).to eq 600.00 - expect(@order.total).to eq 600.00 - expect(@order.adjustment_total).to eq(-10.00) + expect(@order.total).to eq 610.00 + expect(@order.adjustment_total).to eq(0) expect(@order.shipment_total).to eq 10.00 expect(@order.user).to eq @user expect(@order.bill_address).to eq bill_address @@ -112,7 +102,6 @@ def assert_order_expectations expect(@order.payments.length).to eq 1 expect(@order.line_items.any? { |li| li.variant == variant_1 && li.quantity == 2 }).to eq true expect(@order.line_items.any? { |li| li.variant == variant_2 && li.quantity == 2 }).to eq true - expect(@order.promotions).to eq [promotion] end it "is able to checkout with individualized requests" do @@ -120,7 +109,6 @@ def assert_order_expectations create_order create_line_item(variant_1, 2) - add_promotion(promotion) create_line_item(variant_2, 2) add_address(bill_address) @@ -152,7 +140,6 @@ def assert_order_expectations } }) - add_promotion(promotion) add_payment advance @@ -180,7 +167,6 @@ def assert_order_expectations } }) - add_promotion(promotion) add_payment advance diff --git a/legacy_promotions/spec/features/api/checkout_spec.rb b/legacy_promotions/spec/features/api/checkout_spec.rb new file mode 100644 index 00000000000..3455f07d366 --- /dev/null +++ b/legacy_promotions/spec/features/api/checkout_spec.rb @@ -0,0 +1,192 @@ +# frozen_string_literal: true + +require 'rails_helper' + +module Spree + RSpec.describe 'Api Feature Specs', type: :request do + before do + stub_spree_preferences(Spree::Api::Config, requires_authentication: false) + end + let!(:promotion) { FactoryBot.create(:promotion, :with_order_adjustment, code: 'foo', weighted_order_adjustment_amount: 10) } + let(:promotion_code) { promotion.codes.first } + let!(:store) { FactoryBot.create(:store) } + let(:bill_address) { FactoryBot.create(:address) } + let(:ship_address) { FactoryBot.create(:address) } + let(:variant_1) { FactoryBot.create(:variant, price: 100.00) } + let(:variant_2) { FactoryBot.create(:variant, price: 200.00) } + let(:payment_method) { FactoryBot.create(:check_payment_method) } + let!(:shipping_method) do + FactoryBot.create(:shipping_method).tap do |shipping_method| + shipping_method.zones.first.zone_members.create!(zoneable: ship_address.country) + shipping_method.calculator.set_preference(:amount, 10.0) + end + end + + def parsed + JSON.parse(response.body) + end + + def login + expect { + post '/api/users', params: { + user: { + email: "featurecheckoutuser@example.com", + password: "featurecheckoutuser" + } + } + }.to change { Spree.user_class.count }.by 1 + expect(response).to have_http_status(:created) + @user = Spree.user_class.find(parsed['id']) + + # copied from api testing helpers support since we can't really sign in + allow(Spree::LegacyUser).to receive(:find_by).with(hash_including(:spree_api_key)) { @user } + end + + def create_order(order_params: {}) + expect { post '/api/orders', params: order_params }.to change { Order.count }.by 1 + expect(response).to have_http_status(:created) + @order = Order.find(parsed['id']) + expect(@order.email).to eq "featurecheckoutuser@example.com" + end + + def update_order(order_params: {}) + put "/api/orders/#{@order.number}", params: order_params + expect(response).to have_http_status(:ok) + end + + def create_line_item(variant, quantity = 1) + expect { + post "/api/orders/#{@order.number}/line_items", + params: { line_item: { variant_id: variant.id, quantity: quantity } } + }.to change { @order.line_items.count }.by 1 + expect(response).to have_http_status(:created) + end + + def add_promotion(_promotion) + expect { + post "/api/orders/#{@order.number}/coupon_codes", + params: { coupon_code: promotion_code.value } + }.to change { @order.promotions.count }.by 1 + expect(response).to have_http_status(:ok) + end + + def add_address(address, billing: true) + address_type = billing ? :bill_address : :ship_address + # It seems we are missing an order-scoped address api endpoint since we need + # to use update here. + expect { + update_order(order_params: { order: { address_type => address.as_json.except('id') } }) + }.to change { @order.reload.public_send(address_type) }.to address + end + + def add_payment + expect { + post "/api/orders/#{@order.number}/payments", + params: { payment: { payment_method_id: payment_method.id } } + }.to change { @order.reload.payments.count }.by 1 + expect(response).to have_http_status(:created) + expect(@order.payments.last.payment_method).to eq payment_method + end + + def advance + put "/api/checkouts/#{@order.number}/advance" + expect(response).to have_http_status(:ok) + end + + def complete + put "/api/checkouts/#{@order.number}/complete" + expect(response).to have_http_status(:ok) + end + + def assert_order_expectations + @order.reload + expect(@order.state).to eq 'complete' + expect(@order.completed_at).to be_a ActiveSupport::TimeWithZone + expect(@order.item_total).to eq 600.00 + expect(@order.total).to eq 600.00 + expect(@order.adjustment_total).to eq(-10.00) + expect(@order.shipment_total).to eq 10.00 + expect(@order.user).to eq @user + expect(@order.bill_address).to eq bill_address + expect(@order.ship_address).to eq ship_address + expect(@order.payments.length).to eq 1 + expect(@order.line_items.any? { |li| li.variant == variant_1 && li.quantity == 2 }).to eq true + expect(@order.line_items.any? { |li| li.variant == variant_2 && li.quantity == 2 }).to eq true + expect(@order.promotions).to eq [promotion] + end + + it "is able to checkout with individualized requests" do + login + create_order + + create_line_item(variant_1, 2) + add_promotion(promotion) + create_line_item(variant_2, 2) + + add_address(bill_address) + add_address(ship_address, billing: false) + + add_payment + + advance + complete + + assert_order_expectations + end + + it "is able to checkout with the create request" do + login + + create_order(order_params: { + order: { + bill_address: bill_address.as_json.except('id'), + ship_address: ship_address.as_json.except('id'), + line_items: { + 0 => { variant_id: variant_1.id, quantity: 2 }, + 1 => { variant_id: variant_2.id, quantity: 2 } + }, + # Would like to do this, but it puts the payment in a complete state, + # which the order does not like when transitioning from confirm to complete + # since it looks to process pending payments. + # payments: [ { payment_method: payment_method.name, state: "pending" } ], + } + }) + + add_promotion(promotion) + add_payment + + advance + complete + + assert_order_expectations + end + + it "is able to checkout with the update request" do + login + + create_order + update_order(order_params: { + order: { + bill_address: bill_address.as_json.except('id'), + ship_address: ship_address.as_json.except('id'), + line_items: { + 0 => { variant_id: variant_1.id, quantity: 2 }, + 1 => { variant_id: variant_2.id, quantity: 2 } + }, + # Would like to do this, but it puts the payment in a complete state, + # which the order does not like when transitioning from confirm to complete + # since it looks to process pending payments. + # payments: [ { payment_method: payment_method.name, state: "pending" } ], + } + }) + + add_promotion(promotion) + add_payment + + advance + complete + + assert_order_expectations + end + end +end From 37f27e4aa69f91b691cd0ebd13c89517765ab51b Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Tue, 12 Mar 2024 09:44:52 +0100 Subject: [PATCH 14/15] Move promotion application spec to legacy_promotions This spec is not feasible with the promotion system extracted within core. Moving. --- .../spree/api/promotion_application_spec.rb | 50 ------------------- .../spree/api/promotion_application_spec.rb | 48 ++++++++++++++++++ 2 files changed, 48 insertions(+), 50 deletions(-) delete mode 100644 api/spec/requests/spree/api/promotion_application_spec.rb create mode 100644 legacy_promotions/spec/requests/spree/api/promotion_application_spec.rb diff --git a/api/spec/requests/spree/api/promotion_application_spec.rb b/api/spec/requests/spree/api/promotion_application_spec.rb deleted file mode 100644 index bda24996eb6..00000000000 --- a/api/spec/requests/spree/api/promotion_application_spec.rb +++ /dev/null @@ -1,50 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -module Spree::Api - describe 'Promotion application', type: :request do - before do - stub_authentication! - end - - context "with an available promotion" do - let!(:order) { create(:order_with_line_items, line_items_count: 1) } - let!(:promotion) do - promotion = create(:promotion, name: "10% off", code: "10off") - calculator = Spree::Calculator::FlatPercentItemTotal.create(preferred_flat_percent: "10") - action = Spree::Promotion::Actions::CreateItemAdjustments.create(calculator: calculator) - promotion.actions << action - promotion - end - - it "can apply a coupon code to the order" do - expect(order.total).to eq(110.00) - post spree.api_order_coupon_codes_path(order), params: { coupon_code: "10off", order_token: order.guest_token } - expect(response.status).to eq(200) - expect(order.reload.total).to eq(109.00) - expect(json_response["success"]).to eq("The coupon code was successfully applied to your order.") - expect(json_response["error"]).to be_blank - expect(json_response["successful"]).to be true - expect(json_response["status_code"]).to eq("coupon_code_applied") - end - - context "with an expired promotion" do - before do - promotion.starts_at = 2.weeks.ago - promotion.expires_at = 1.week.ago - promotion.save - end - - it "fails to apply" do - post spree.api_order_coupon_codes_path(order), params: { coupon_code: "10off", order_token: order.guest_token } - expect(response.status).to eq(422) - expect(json_response["success"]).to be_blank - expect(json_response["error"]).to eq("The coupon code is expired") - expect(json_response["successful"]).to be false - expect(json_response["status_code"]).to eq("coupon_code_expired") - end - end - end - end -end diff --git a/legacy_promotions/spec/requests/spree/api/promotion_application_spec.rb b/legacy_promotions/spec/requests/spree/api/promotion_application_spec.rb new file mode 100644 index 00000000000..eeb618d13ce --- /dev/null +++ b/legacy_promotions/spec/requests/spree/api/promotion_application_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Promotion application', type: :request do + before do + stub_authentication! + end + + context "with an available promotion" do + let!(:order) { create(:order_with_line_items, line_items_count: 1) } + let!(:promotion) do + promotion = create(:promotion, name: "10% off", code: "10off") + calculator = Spree::Calculator::FlatPercentItemTotal.create(preferred_flat_percent: "10") + action = Spree::Promotion::Actions::CreateItemAdjustments.create(calculator: calculator) + promotion.actions << action + promotion + end + + it "can apply a coupon code to the order" do + expect(order.total).to eq(110.00) + post spree.api_order_coupon_codes_path(order), params: { coupon_code: "10off", order_token: order.guest_token } + expect(response.status).to eq(200) + expect(order.reload.total).to eq(109.00) + expect(json_response["success"]).to eq("The coupon code was successfully applied to your order.") + expect(json_response["error"]).to be_blank + expect(json_response["successful"]).to be true + expect(json_response["status_code"]).to eq("coupon_code_applied") + end + + context "with an expired promotion" do + before do + promotion.starts_at = 2.weeks.ago + promotion.expires_at = 1.week.ago + promotion.save + end + + it "fails to apply" do + post spree.api_order_coupon_codes_path(order), params: { coupon_code: "10off", order_token: order.guest_token } + expect(response.status).to eq(422) + expect(json_response["success"]).to be_blank + expect(json_response["error"]).to eq("The coupon code is expired") + expect(json_response["successful"]).to be false + expect(json_response["status_code"]).to eq("coupon_code_expired") + end + end + end +end From 8d16e2dc593f58b85932d90c67a45a897975c2bb Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Tue, 12 Mar 2024 09:49:51 +0100 Subject: [PATCH 15/15] Fix coupon codes spec This spec was previously already fixed to not refer to any promotion-system-related things, but I forgot this one line in #5686. --- api/spec/requests/spree/api/coupon_codes_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/api/spec/requests/spree/api/coupon_codes_spec.rb b/api/spec/requests/spree/api/coupon_codes_spec.rb index c7324d9c58d..993d5852422 100644 --- a/api/spec/requests/spree/api/coupon_codes_spec.rb +++ b/api/spec/requests/spree/api/coupon_codes_spec.rb @@ -132,7 +132,6 @@ module Spree::Api subject expect(response.status).to eq(422) - expect(order.reload.promotions).to eq([]) expect(json_response).to eq({ "success" => nil, "error" => I18n.t('spree.coupon_code_not_present'),