diff --git a/.rubocop.yml b/.rubocop.yml index 3428ac0b173..a69446c98b4 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -326,6 +326,7 @@ Naming/MethodParameterName: - id - to - _ + - "on" # Rubocop doesn't understand side-effects Style/IdenticalConditionalBranches: diff --git a/Gemfile b/Gemfile index 44bc66f5ea5..15e637dcc87 100644 --- a/Gemfile +++ b/Gemfile @@ -34,6 +34,7 @@ gem 'puma', '< 7', require: false gem 'i18n-tasks', '~> 0.9', require: false gem 'rspec_junit_formatter', require: false gem 'yard', require: false +gem 'db-query-matchers', require: false # Ensure the requirement is also updated in core/lib/spree/testing_support/factory_bot.rb gem 'factory_bot_rails', '>= 4.8', require: false diff --git a/admin/app/blueprints/solidus_admin/state_blueprint.rb b/admin/app/blueprints/solidus_admin/state_blueprint.rb new file mode 100644 index 00000000000..3e81791eca9 --- /dev/null +++ b/admin/app/blueprints/solidus_admin/state_blueprint.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module SolidusAdmin + class StateBlueprint < Blueprinter::Base + identifier :id + + field :name + + view :state_with_country do + field :state_with_country, name: :name + end + end +end diff --git a/admin/app/components/solidus_admin/base_component.rb b/admin/app/components/solidus_admin/base_component.rb index 5d6868fd197..e39c1707a4b 100644 --- a/admin/app/components/solidus_admin/base_component.rb +++ b/admin/app/components/solidus_admin/base_component.rb @@ -7,6 +7,7 @@ module SolidusAdmin # BaseComponent is the base class for all components in Solidus Admin. class BaseComponent < ViewComponent::Base include SolidusAdmin::ComponentsHelper + include SolidusAdmin::StimulusHelper include SolidusAdmin::VoidElementsHelper include Turbo::FramesHelper diff --git a/admin/app/components/solidus_admin/ui/button/component.rb b/admin/app/components/solidus_admin/ui/button/component.rb index 1110b4ef01f..4a0d9811381 100644 --- a/admin/app/components/solidus_admin/ui/button/component.rb +++ b/admin/app/components/solidus_admin/ui/button/component.rb @@ -91,6 +91,16 @@ def initialize( ] end + def self.submit(resource:, **attrs) + resource_name = resource.model_name.human + text = resource.new_record? ? t('.submit.create', resource_name:) : t('.submit.update', resource_name:) + new(text:, type: :submit, **attrs) + end + + def self.cancel + new(scheme: :secondary, text: t('.cancel')) + end + def call content = [] content << render(component('ui/icon').new(name: @icon, class: @icon_classes)) if @icon diff --git a/admin/app/components/solidus_admin/ui/button/component.yml b/admin/app/components/solidus_admin/ui/button/component.yml new file mode 100644 index 00000000000..5fa6738fcaa --- /dev/null +++ b/admin/app/components/solidus_admin/ui/button/component.yml @@ -0,0 +1,5 @@ +en: + cancel: "Cancel" + submit: + create: "Add %{resource_name}" + update: "Update %{resource_name}" diff --git a/admin/app/components/solidus_admin/ui/forms/field/component.rb b/admin/app/components/solidus_admin/ui/forms/field/component.rb index 3bfaf0b1045..341a786f36c 100644 --- a/admin/app/components/solidus_admin/ui/forms/field/component.rb +++ b/admin/app/components/solidus_admin/ui/forms/field/component.rb @@ -41,9 +41,9 @@ def self.select(form, method, choices, object: nil, hint: nil, tip: nil, size: : hint:, tip:, size: size, - name: "#{object_name}[#{method}]", + name: "#{object_name}[#{method}]#{'[]' if attributes[:multiple].present?}", choices:, - value: (object.public_send(method) if object.respond_to?(method)), + value: object.try(method), error: (errors.to_sentence.capitalize if errors), **attributes ) diff --git a/admin/app/components/solidus_admin/ui/forms/select/component.rb b/admin/app/components/solidus_admin/ui/forms/select/component.rb index b15fa866512..a7aaae408e2 100644 --- a/admin/app/components/solidus_admin/ui/forms/select/component.rb +++ b/admin/app/components/solidus_admin/ui/forms/select/component.rb @@ -108,7 +108,7 @@ def prepare_classes(size:) [&_.item_.remove-button]:has-[:disabled]:cursor-not-allowed" end - input_classes = ["[&_input]:has-[.item]:placeholder:invisible [&_input:disabled]:cursor-not-allowed [&_input:disabled]:bg-gray-50 + input_classes = ["[&_input]:has-[.item]:placeholder:opacity-0 [&_input:disabled]:cursor-not-allowed [&_input:disabled]:bg-gray-50 [&_input]:peer-invalid:placeholder:text-red-400"] unless @attributes[:multiple] diff --git a/admin/app/components/solidus_admin/zones/edit/component.html.erb b/admin/app/components/solidus_admin/zones/edit/component.html.erb new file mode 100644 index 00000000000..e2ddf102cf5 --- /dev/null +++ b/admin/app/components/solidus_admin/zones/edit/component.html.erb @@ -0,0 +1 @@ +<%= render component("zones/form").new(zone: @zone, form_url:, form_id:) %> diff --git a/admin/app/components/solidus_admin/zones/edit/component.rb b/admin/app/components/solidus_admin/zones/edit/component.rb new file mode 100644 index 00000000000..37fe6df7f7b --- /dev/null +++ b/admin/app/components/solidus_admin/zones/edit/component.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +class SolidusAdmin::Zones::Edit::Component < SolidusAdmin::Resources::Edit::Component +end diff --git a/admin/app/components/solidus_admin/zones/form/component.html.erb b/admin/app/components/solidus_admin/zones/form/component.html.erb new file mode 100644 index 00000000000..3fdfecb436b --- /dev/null +++ b/admin/app/components/solidus_admin/zones/form/component.html.erb @@ -0,0 +1,48 @@ +<%= turbo_frame_tag :resource_modal, target: "_top" do %> + <%= render component("ui/modal").new(title:) do |modal| %> + <%= form_for @zone, url: @form_url, html: { id: @form_id, **stimulus_controller, **stimulus_value(name: :kind, value: @zone.kind) } do |f| %> +
+ <%= render component("ui/forms/field").text_field(f, :name, class: "required") %> + + + <%= render component("ui/forms/field").text_field(f, :description, class: "required") %> + <%= render component("ui/forms/field").select( + f, + :kind, + [[t(".kinds.country"), :country], [t(".kinds.state"), :state]], + object: @zone, + value: @zone.kind || :state, + **stimulus_action("toggleKind", on: "change") + ) %> +
+ <% modal.with_actions do %> +
+ <%= render component("ui/button").cancel %> +
+ <%= render component("ui/button").submit(resource: @zone, form: @form_id) %> + <% end %> + <% end %> + <% end %> +<% end %> diff --git a/admin/app/components/solidus_admin/zones/form/component.js b/admin/app/components/solidus_admin/zones/form/component.js new file mode 100644 index 00000000000..2d13322122a --- /dev/null +++ b/admin/app/components/solidus_admin/zones/form/component.js @@ -0,0 +1,32 @@ +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + static targets = ["countriesWrapper", "statesWrapper", "countriesSelect", "statesSelect"] + static values = { + kind: { type: String, default: "state" } + } + + connect() { + this.toggleSelectsVisibility(); + this.toggleSelectDisabled(); + } + + toggleKind(event) { + if (!event.target.value) return; + + this.kindValue = event.target.value; + + this.toggleSelectsVisibility(); + this.toggleSelectDisabled(); + } + + toggleSelectsVisibility() { + this.countriesWrapperTarget.classList.toggle("hidden", this.kindValue === "state"); + this.statesWrapperTarget.classList.toggle("hidden", this.kindValue === "country"); + } + + toggleSelectDisabled() { + this.countriesSelectTarget.disabled = this.kindValue === "state"; + this.statesSelectTarget.disabled = this.kindValue === "country"; + } +} diff --git a/admin/app/components/solidus_admin/zones/form/component.rb b/admin/app/components/solidus_admin/zones/form/component.rb new file mode 100644 index 00000000000..f2b52b30f11 --- /dev/null +++ b/admin/app/components/solidus_admin/zones/form/component.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class SolidusAdmin::Zones::Form::Component < SolidusAdmin::BaseComponent + def initialize(zone:, form_url:, form_id:) + @zone = zone + @form_url = form_url + @form_id = form_id + end + + def title + @zone.new_record? ? t(".title.new") : t(".title.edit") + end +end diff --git a/admin/app/components/solidus_admin/zones/form/component.yml b/admin/app/components/solidus_admin/zones/form/component.yml new file mode 100644 index 00000000000..cb36d74fde8 --- /dev/null +++ b/admin/app/components/solidus_admin/zones/form/component.yml @@ -0,0 +1,7 @@ +en: + kinds: + country: "Country based" + state: "State based" + title: + edit: "Edit Zone" + new: "New Zone" diff --git a/admin/app/components/solidus_admin/zones/index/component.rb b/admin/app/components/solidus_admin/zones/index/component.rb index 04ff17ea1cd..33390df6bea 100644 --- a/admin/app/components/solidus_admin/zones/index/component.rb +++ b/admin/app/components/solidus_admin/zones/index/component.rb @@ -5,6 +5,10 @@ def model_class Spree::Zone end + def title + t('solidus_admin.zones.title') + end + def search_key :name_or_description_cont end @@ -13,15 +17,20 @@ def search_url solidus_admin.zones_path end - def row_url(zone) - spree.edit_admin_zone_path(zone) + def edit_path(zone) + solidus_admin.edit_zone_path(zone, **search_filter_params) + end + + def turbo_frames + %w[resource_modal] end def page_actions render component("ui/button").new( tag: :a, text: t('.add'), - href: spree.new_admin_zone_path, + href: solidus_admin.new_zone_path(**search_filter_params), + data: { turbo_frame: :resource_modal }, icon: "add-line", class: "align-self-end w-full", ) @@ -48,16 +57,46 @@ def filters def columns [ - :name, - :description, - { - header: :kind, - data: -> { component('ui/badge').new(name: _1.kind, color: _1.kind == 'country' ? :green : :blue) }, - }, - { - header: :zone_members, - data: -> { _1.zone_members.map(&:zoneable).map(&:name).to_sentence }, - }, + name_column, + description_column, + kind_column, + zone_members_column, ] end + + def name_column + { + header: :name, + data: ->(zone) do + link_to zone.name, edit_path(zone), + data: { turbo_frame: :resource_modal }, + class: 'body-link' + end + } + end + + def description_column + { + header: :description, + data: ->(zone) do + link_to zone.description, edit_path(zone), + data: { turbo_frame: :resource_modal }, + class: 'body-link' + end + } + end + + def kind_column + { + header: :kind, + data: -> { component('ui/badge').new(name: _1.kind, color: _1.kind == 'country' ? :green : :blue) }, + } + end + + def zone_members_column + { + header: :zone_members, + data: -> { _1.zone_members.map(&:zoneable).map(&:name).to_sentence }, + } + end end diff --git a/admin/app/components/solidus_admin/zones/new/component.html.erb b/admin/app/components/solidus_admin/zones/new/component.html.erb new file mode 100644 index 00000000000..e2ddf102cf5 --- /dev/null +++ b/admin/app/components/solidus_admin/zones/new/component.html.erb @@ -0,0 +1 @@ +<%= render component("zones/form").new(zone: @zone, form_url:, form_id:) %> diff --git a/admin/app/components/solidus_admin/zones/new/component.rb b/admin/app/components/solidus_admin/zones/new/component.rb new file mode 100644 index 00000000000..def8d85abd6 --- /dev/null +++ b/admin/app/components/solidus_admin/zones/new/component.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +class SolidusAdmin::Zones::New::Component < SolidusAdmin::Resources::New::Component +end diff --git a/admin/app/controllers/solidus_admin/resources_controller.rb b/admin/app/controllers/solidus_admin/resources_controller.rb index c181697d509..7ebc49c7eaa 100644 --- a/admin/app/controllers/solidus_admin/resources_controller.rb +++ b/admin/app/controllers/solidus_admin/resources_controller.rb @@ -21,6 +21,7 @@ class ResourcesController < SolidusAdmin::BaseController def index respond_to do |format| format.html { render index_component.new(page: @page) } + format.json { render json: blueprint.render(@page.records, view: blueprint_view) } end end @@ -77,7 +78,7 @@ def set_paginated_resources ).tap do |resources| instance_variable_set("@#{plural_resource_name}", resources) # sets @page instance variable in geared_pagination gem - set_page_and_extract_portion_from(resources, ordered_by: resources_sorting_options) + set_page_and_extract_portion_from(resources, ordered_by: resources_sorting_options, per_page:) end end @@ -89,6 +90,8 @@ def resources_collection resource_class.all end + def per_page; end + def set_resource @resource ||= resource_class.find(params[:id]).tap do |resource| instance_variable_set("@#{resource_name}", resource) diff --git a/admin/app/controllers/solidus_admin/states_controller.rb b/admin/app/controllers/solidus_admin/states_controller.rb new file mode 100644 index 00000000000..4fb8fb67dcb --- /dev/null +++ b/admin/app/controllers/solidus_admin/states_controller.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module SolidusAdmin + class StatesController < SolidusAdmin::ResourcesController + skip_before_action :authorize_solidus_admin_user! + + private + + def resource_class + Spree::State + end + + def resources_sorting_options + { name: :asc } + end + + def blueprint + SolidusAdmin::StateBlueprint + end + + def blueprint_view + view = params[:view]&.to_sym + blueprint.view?(view) ? view : :default + end + + def per_page + 100 + end + end +end diff --git a/admin/app/controllers/solidus_admin/zones_controller.rb b/admin/app/controllers/solidus_admin/zones_controller.rb index 11d64cd3a3a..93000037335 100644 --- a/admin/app/controllers/solidus_admin/zones_controller.rb +++ b/admin/app/controllers/solidus_admin/zones_controller.rb @@ -1,35 +1,15 @@ # frozen_string_literal: true module SolidusAdmin - class ZonesController < SolidusAdmin::BaseController - include SolidusAdmin::ControllerHelpers::Search - - def index - zones = apply_search_to( - Spree::Zone.order(id: :desc), - param: :q - ) - - set_page_and_extract_portion_from(zones) - - respond_to do |format| - format.html { render component('zones/index').new(page: @page) } - end - end - - def destroy - @zones = Spree::Zone.where(id: params[:id]) - - Spree::Zone.transaction { @zones.destroy_all } + class ZonesController < SolidusAdmin::ResourcesController + private - flash[:notice] = t('.success') - redirect_back_or_to zones_path, status: :see_other - end + def resource_class = Spree::Zone - private + def resources_collection = Spree::Zone.includes(zone_members: :zoneable) - def zone_params - params.require(:zone).permit(:zone_id, permitted_zone_attributes) + def permitted_resource_params + params.require(:zone).permit(:name, :description, state_ids: [], country_ids: []) end end end diff --git a/admin/app/helpers/solidus_admin/stimulus_helper.rb b/admin/app/helpers/solidus_admin/stimulus_helper.rb new file mode 100644 index 00000000000..e70b4d35781 --- /dev/null +++ b/admin/app/helpers/solidus_admin/stimulus_helper.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +# Simple shorthands and helpers for stimulus data attributes to avoid writing clumsy interpolations with `stimulus_id` +# Before: "data-#{stimulus_id}-target": "wrapper" +# After: stimulus_target("wrapper") + +module SolidusAdmin + module StimulusHelper + def stimulus_controller + { "data-controller": stimulus_id } + end + + def stimulus_action(action, on: nil) + action_construct = [] + action_construct << "#{on}->" if on.present? + action_construct << "#{stimulus_id}##{action}" + + { "data-action": action_construct.join } + end + + def stimulus_target(target) + { "data-#{stimulus_id}-target": target } + end + + def stimulus_value(name:, value:) + { "data-#{stimulus_id}-#{name}-value": value } + end + end +end diff --git a/admin/config/locales/zones.en.yml b/admin/config/locales/zones.en.yml index ec5882d4fd2..2d90c423a89 100644 --- a/admin/config/locales/zones.en.yml +++ b/admin/config/locales/zones.en.yml @@ -2,5 +2,9 @@ en: solidus_admin: zones: title: "Zones" + create: + success: "Zone was successfully created." destroy: success: "Zones were successfully removed." + update: + success: "Zone was successfully updated." diff --git a/admin/config/routes.rb b/admin/config/routes.rb index 4f70c26e766..ec215522965 100644 --- a/admin/config/routes.rb +++ b/admin/config/routes.rb @@ -9,6 +9,8 @@ get 'states', to: 'countries#states' end + resources :states, only: [:index], defaults: { format: :json } + admin_resources :products, only: [:index, :update, :destroy] do collection do put :discontinue @@ -78,7 +80,7 @@ admin_resources :shipping_categories, except: [:show] admin_resources :stock_locations, only: [:index, :destroy] admin_resources :stores, only: [:index, :destroy] - admin_resources :zones, only: [:index, :destroy] + admin_resources :zones, except: [:show] admin_resources :refund_reasons, except: [:show] admin_resources :reimbursement_types, only: [:index] admin_resources :return_reasons, except: [:show] diff --git a/admin/lib/solidus_admin/engine.rb b/admin/lib/solidus_admin/engine.rb index 82b1ec3e4ad..3e43bbd8b16 100644 --- a/admin/lib/solidus_admin/engine.rb +++ b/admin/lib/solidus_admin/engine.rb @@ -3,6 +3,7 @@ require "stimulus-rails" require "turbo-rails" require "view_component" +require "blueprinter" module SolidusAdmin class Engine < ::Rails::Engine diff --git a/admin/lib/solidus_admin/testing_support/feature_helpers.rb b/admin/lib/solidus_admin/testing_support/feature_helpers.rb index f082b7baa19..267ed2f2645 100644 --- a/admin/lib/solidus_admin/testing_support/feature_helpers.rb +++ b/admin/lib/solidus_admin/testing_support/feature_helpers.rb @@ -33,13 +33,23 @@ def select_row(text) end end - # Select an option from a "solidus-select" field + # Select options from a "solidus-select" field # - # @param value [String] which option to select + # @param value [String, Array] which option(s) to select # @param from [String] label of the select box def solidus_select(value, from:) - fill_in(from, with: value).send_keys(:return) - expect(find_field(from).ancestor(".control")).to have_text(value) + input = find_field(from, visible: :all) + control = input.ancestor(".control") + dropdown = control.sibling(".dropdown", visible: :all) + + # Make sure options are loaded + control.click + within(dropdown) { expect(first(".option", visible: :all)).to be } + + Array.wrap(value).each do |val| + input.fill_in(with: val).send_keys(:return) + expect(control).to have_text(val) + end end end end diff --git a/admin/solidus_admin.gemspec b/admin/solidus_admin.gemspec index 8ed9c0cedbb..a2c6f9f911c 100644 --- a/admin/solidus_admin.gemspec +++ b/admin/solidus_admin.gemspec @@ -28,6 +28,7 @@ Gem::Specification.new do |s| s.required_ruby_version = '>= 3.1.0' s.required_rubygems_version = '>= 1.8.23' + s.add_dependency 'blueprinter' s.add_dependency 'geared_pagination', '~> 1.1' s.add_dependency 'importmap-rails', ['>= 2.0', '< 3'] s.add_dependency 'solidus_backend' diff --git a/admin/spec/blueprints/solidus_admin/state_blueprint_spec.rb b/admin/spec/blueprints/solidus_admin/state_blueprint_spec.rb new file mode 100644 index 00000000000..59cb871b5d3 --- /dev/null +++ b/admin/spec/blueprints/solidus_admin/state_blueprint_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe SolidusAdmin::StateBlueprint, type: :blueprint do + let(:result) { described_class.render_as_hash(state, **options) } + let(:state) { create(:state) } + + describe "default" do + let(:options) { {} } + + it "returns correct data" do + expect(result).to eq(id: state.id, name: state.name) + end + end + + describe "state_with_country" do + let(:options) { { view: :state_with_country } } + + it "returns correct data" do + expect(result).to eq(id: state.id, name: state.state_with_country) + end + + it "uses cached result" do + described_class.render_as_hash(state, **options) + state.reload + expect { described_class.render_as_hash(state, **options) }.to make_database_queries(count: 0) + end + end +end diff --git a/admin/spec/components/solidus_admin/ui/button/component_spec.rb b/admin/spec/components/solidus_admin/ui/button/component_spec.rb index c37dd27dfa9..4eda5fab23a 100644 --- a/admin/spec/components/solidus_admin/ui/button/component_spec.rb +++ b/admin/spec/components/solidus_admin/ui/button/component_spec.rb @@ -8,4 +8,35 @@ render_preview(:overview) render_preview(:group) end + + describe ".submit" do + let(:component) { described_class.submit(resource:) } + + context "for a new resource" do + let(:resource) { build(:zone) } + + it "renders correct submit button" do + render_inline(component) + expect(page).to have_content("Add Zone") + end + end + + context "for an existing resource" do + let(:resource) { create(:zone) } + + it "renders correct submit button" do + render_inline(component) + expect(page).to have_content("Update Zone") + end + end + end + + describe ".cancel" do + let(:component) { described_class.cancel } + + it "renders Cancel button" do + render_inline(component) + expect(page).to have_content("Cancel") + end + end end diff --git a/admin/spec/features/properties_spec.rb b/admin/spec/features/properties_spec.rb index 59b7a2d1668..dda90f87642 100644 --- a/admin/spec/features/properties_spec.rb +++ b/admin/spec/features/properties_spec.rb @@ -50,35 +50,6 @@ end end - context "editing an existing property" do - let!(:property) { create(:property, name: "Color", presentation: "Cool Color") } - - it "updates the property" do - visit "/admin/properties" - click_on "Color" - - fill_in "Name", with: "Size" - fill_in "Presentation", with: "Cool Size" - click_on "Update Property" - - expect(page).to have_content("Property was successfully updated.") - expect(page).to have_content("Size") - expect(page).to have_content("Cool Size") - expect(Spree::Property.count).to eq(1) - end - - it "shows validation errors" do - visit "/admin/properties" - click_on "Color" - - fill_in "Name", with: "" - click_on "Update Property" - - expect(page).to have_content("can't be blank") - expect(Spree::Property.count).to eq(1) - end - end - context "editing an existing property" do let!(:property) { create(:property, name: "Color", presentation: "Cool Color") } diff --git a/admin/spec/features/zones_spec.rb b/admin/spec/features/zones_spec.rb index 41ef26457a4..8cac03c4132 100644 --- a/admin/spec/features/zones_spec.rb +++ b/admin/spec/features/zones_spec.rb @@ -5,6 +5,13 @@ describe "Zones", :js, type: :feature do before { sign_in create(:admin_user, email: 'admin@example.com') } + let(:states) do + [ + create(:state, name: "Alberta", country: create(:country, iso: "CA")), + create(:state, name: "Manitoba", country: create(:country, iso: "CA")) + ] + end + it "lists zones and allows deleting them" do create(:zone, name: "Europe") create(:zone, name: "North America") @@ -21,4 +28,65 @@ expect(Spree::Zone.count).to eq(1) expect(page).to be_axe_clean end + + context "creating new zone" do + before { states } + + it "creates a new zone" do + visit "/admin/zones" + click_on "Add new" + + fill_in "Name", with: "Canada" + solidus_select ["Alberta (Canada)", "Manitoba (Canada)"], from: "States" + fill_in "Description", with: "some Canada provinces" + click_on "Add Zone" + + expect(page).to have_content("Zone was successfully created.") + expect(page).to have_content("Alberta and Manitoba") + expect(page).to have_content("some Canada provinces") + end + + it "shows validation errors" do + visit "/admin/zones" + click_on "Add new" + + fill_in "Name", with: "" + click_on "Add Zone" + + expect(page).to have_content("can't be blank") + end + end + + context "editing an existing zone" do + before do + create(:zone, name: "CA", states:) + create(:country, iso: "US") + end + + it "updates the zone" do + visit "/admin/zones" + click_on "CA" + + fill_in "Name", with: "US" + fill_in "Description", with: "United States" + solidus_select "Country based", from: "Kind" + solidus_select "United States", from: "Countries" + click_on "Update Zone" + + expect(page).to have_content("Zone was successfully updated.") + expect(page).to have_content("US") + expect(page).to have_content("United States") + expect(page).to have_content("country") + end + + it "shows validation errors" do + visit "/admin/zones" + click_on "CA" + expect(page).to have_field("Name", with: "CA") + fill_in "Name", with: "" + click_on "Update Zone" + + expect(page).to have_content("can't be blank") + end + end end diff --git a/admin/spec/requests/solidus_admin/zones_spec.rb b/admin/spec/requests/solidus_admin/zones_spec.rb new file mode 100644 index 00000000000..7c75fb6e063 --- /dev/null +++ b/admin/spec/requests/solidus_admin/zones_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require "spec_helper" +require "solidus_admin/testing_support/shared_examples/crud_resource_requests" + +RSpec.describe "SolidusAdmin::ZonesController", type: :request do + include_examples "CRUD resource requests", "zone" do + let(:countries) { create_list(:country, 2) } + let(:resource_class) { Spree::Zone } + let(:valid_attributes) { { name: "Zone with countries", country_ids: countries.map(&:id) } } + let(:invalid_attributes) { { name: "" } } + + it "creates new zone members" do + expect { post solidus_admin.zones_path, params: { zone: valid_attributes } }.to change(Spree::ZoneMember, :count).by(countries.size) + end + + it "updates zone members" do + zone = create(:zone, :with_country) + expect { patch solidus_admin.zone_path(zone), params: { zone: valid_attributes } }.to change(Spree::ZoneMember, :count).by(1) + end + end + + context "N+1" do + before do + create_list(:zone, 2, :with_country) + create_list(:zone, 2, :with_state) + end + + let(:expected_count) do + [ + 1, # count zones + 1, # select zones + 1, # preload zone_members + 1, # preload countries + 1, # preload states + 1, # select stores + ].sum + end + + it "is optimized" do + expect { get solidus_admin.zones_path }.to make_database_queries(count: expected_count) + end + end +end diff --git a/admin/spec/spec_helper.rb b/admin/spec/spec_helper.rb index ba2144758ce..38bed2afc8f 100644 --- a/admin/spec/spec_helper.rb +++ b/admin/spec/spec_helper.rb @@ -74,6 +74,14 @@ require 'axe-rspec' require 'axe-capybara' +# DB Query Matchers +require "db-query-matchers" +DBQueryMatchers.configure do |config| + config.ignores = [/SHOW TABLES LIKE/] + config.ignore_cached = true + config.schemaless = true +end + RSpec.configure do |config| config.color = true config.infer_spec_type_from_file_location! diff --git a/api/app/views/spree/api/line_items/new.json.jbuilder b/api/app/views/spree/api/line_items/new.json.jbuilder index 75d1e7f1d31..94e2e9b3171 100644 --- a/api/app/views/spree/api/line_items/new.json.jbuilder +++ b/api/app/views/spree/api/line_items/new.json.jbuilder @@ -1,4 +1,4 @@ # frozen_string_literal: true -json.attributes(([*line_item_attributes] - [:id])) +json.attributes([*line_item_attributes] - [:id]) json.required_attributes([:variant_id, :quantity]) diff --git a/core/app/models/spree/classification.rb b/core/app/models/spree/classification.rb index 4966625014a..1fbd90be34a 100644 --- a/core/app/models/spree/classification.rb +++ b/core/app/models/spree/classification.rb @@ -8,6 +8,6 @@ class Classification < Spree::Base belongs_to :taxon, class_name: "Spree::Taxon", inverse_of: :classifications, touch: true, optional: true # For https://github.com/spree/spree/issues/3494 - validates_uniqueness_of :taxon_id, scope: :product_id, message: :already_linked + validates :taxon_id, uniqueness: { scope: :product_id, message: :already_linked } end end diff --git a/core/app/models/spree/inventory_unit.rb b/core/app/models/spree/inventory_unit.rb index ab1ec848108..80af85b3b89 100644 --- a/core/app/models/spree/inventory_unit.rb +++ b/core/app/models/spree/inventory_unit.rb @@ -24,7 +24,7 @@ def order=(_) raise "The order association has been removed from InventoryUnit. The order is now determined from the shipment." end - validates_presence_of :shipment, :line_item, :variant + validates :shipment, :line_item, :variant, presence: true before_destroy :ensure_can_destroy diff --git a/core/app/models/spree/role_user.rb b/core/app/models/spree/role_user.rb index c4aa044531c..ffa38a42bb5 100644 --- a/core/app/models/spree/role_user.rb +++ b/core/app/models/spree/role_user.rb @@ -8,7 +8,7 @@ class RoleUser < Spree::Base after_create :auto_generate_spree_api_key - validates_uniqueness_of :role_id, scope: :user_id + validates :role_id, uniqueness: { scope: :user_id } private diff --git a/core/app/models/spree/state.rb b/core/app/models/spree/state.rb index 24b2846e86f..fed9793c390 100644 --- a/core/app/models/spree/state.rb +++ b/core/app/models/spree/state.rb @@ -15,6 +15,7 @@ class State < Spree::Base ) end + self.allowed_ransackable_associations = %w[country] self.allowed_ransackable_attributes = %w[name] # table of { country.id => [ state.id , state.name ] }, arrays sorted by name @@ -36,7 +37,9 @@ def to_s end def state_with_country - "#{name} (#{country})" + Rails.cache.fetch("#{cache_key_with_version}/state_with_country") do + "#{name} (#{country})" + end end end end diff --git a/core/app/models/spree/stock_location.rb b/core/app/models/spree/stock_location.rb index 4ded3100981..b575f99d3db 100644 --- a/core/app/models/spree/stock_location.rb +++ b/core/app/models/spree/stock_location.rb @@ -22,8 +22,8 @@ class InvalidMovementError < StandardError; end has_many :shipping_method_stock_locations, dependent: :destroy has_many :shipping_methods, through: :shipping_method_stock_locations - validates_presence_of :name - validates_uniqueness_of :code, allow_blank: true, case_sensitive: false + validates :name, presence: true + validates :code, uniqueness: { allow_blank: true, case_sensitive: false } scope :active, -> { where(active: true) } scope :order_default, -> { order(default: :desc, position: :asc) } diff --git a/core/app/models/spree/store_credit.rb b/core/app/models/spree/store_credit.rb index 9f5fabc85ff..96de906af07 100644 --- a/core/app/models/spree/store_credit.rb +++ b/core/app/models/spree/store_credit.rb @@ -18,9 +18,9 @@ class Spree::StoreCredit < Spree::PaymentSource belongs_to :credit_type, class_name: 'Spree::StoreCreditType', foreign_key: 'type_id', optional: true has_many :store_credit_events - validates_presence_of :user_id, :category_id, :type_id, :created_by_id, :currency - validates_numericality_of :amount, { greater_than: 0 } - validates_numericality_of :amount_used, { greater_than_or_equal_to: 0 } + validates :user_id, :category_id, :type_id, :created_by_id, :currency, presence: true + validates :amount, numericality: { greater_than: 0 } + validates :amount_used, numericality: { greater_than_or_equal_to: 0 } validate :amount_used_less_than_or_equal_to_amount validate :amount_authorized_less_than_or_equal_to_amount diff --git a/core/app/models/spree/store_credit_event.rb b/core/app/models/spree/store_credit_event.rb index d5c6a70686e..8f7fe59cff1 100644 --- a/core/app/models/spree/store_credit_event.rb +++ b/core/app/models/spree/store_credit_event.rb @@ -9,7 +9,7 @@ class StoreCreditEvent < Spree::Base belongs_to :originator, polymorphic: true, optional: true belongs_to :store_credit_reason, class_name: 'Spree::StoreCreditReason', inverse_of: :store_credit_events, optional: true - validates_presence_of :store_credit_reason, if: :action_requires_reason? + validates :store_credit_reason, presence: { if: :action_requires_reason? } NON_EXPOSED_ACTIONS = [Spree::StoreCredit::ELIGIBLE_ACTION, Spree::StoreCredit::AUTHORIZE_ACTION] diff --git a/core/app/models/spree/tax_category.rb b/core/app/models/spree/tax_category.rb index 3b047cf8f46..7f9a7376516 100644 --- a/core/app/models/spree/tax_category.rb +++ b/core/app/models/spree/tax_category.rb @@ -11,7 +11,7 @@ class TaxCategory < Spree::Base end validates :name, presence: true - validates_uniqueness_of :name, case_sensitive: true, unless: :deleted_at + validates :name, uniqueness: { case_sensitive: true, unless: :deleted_at } has_many :tax_rate_tax_categories, class_name: 'Spree::TaxRateTaxCategory', diff --git a/core/app/models/spree/user_address.rb b/core/app/models/spree/user_address.rb index 70432f472ee..91e5b1bd597 100644 --- a/core/app/models/spree/user_address.rb +++ b/core/app/models/spree/user_address.rb @@ -5,8 +5,8 @@ class UserAddress < Spree::Base belongs_to :user, class_name: UserClassHandle.new, foreign_key: "user_id", inverse_of: :user_addresses belongs_to :address, class_name: "Spree::Address" - validates_uniqueness_of :address_id, scope: :user_id - validates_uniqueness_of :user_id, conditions: -> { default_shipping }, message: :default_address_exists, if: :default? + validates :address_id, uniqueness: { scope: :user_id } + validates :user_id, uniqueness: { conditions: -> { default_shipping }, message: :default_address_exists, if: :default? } scope :with_address_values, ->(address_attributes) do joins(:address).merge( diff --git a/core/app/models/spree/variant.rb b/core/app/models/spree/variant.rb index fd3755e51fd..958b50f3260 100644 --- a/core/app/models/spree/variant.rb +++ b/core/app/models/spree/variant.rb @@ -74,7 +74,7 @@ class Variant < Spree::Base validates :cost_price, numericality: { greater_than_or_equal_to: 0, allow_nil: true } validates :price, numericality: { greater_than_or_equal_to: 0, allow_nil: true } - validates_uniqueness_of :sku, allow_blank: true, case_sensitive: true, conditions: -> { where(deleted_at: nil) }, if: :enforce_unique_sku? + validates :sku, uniqueness: { allow_blank: true, case_sensitive: true, conditions: -> { where(deleted_at: nil) }, if: :enforce_unique_sku? } after_create :create_stock_items after_create :set_master_out_of_stock, unless: :is_master? diff --git a/core/app/models/spree/variant_property_rule_condition.rb b/core/app/models/spree/variant_property_rule_condition.rb index 7670d842a0a..6947082d7b4 100644 --- a/core/app/models/spree/variant_property_rule_condition.rb +++ b/core/app/models/spree/variant_property_rule_condition.rb @@ -5,6 +5,6 @@ class VariantPropertyRuleCondition < Spree::Base belongs_to :option_value, optional: true belongs_to :variant_property_rule, touch: true, optional: true - validates_uniqueness_of :option_value_id, scope: :variant_property_rule_id + validates :option_value_id, uniqueness: { scope: :variant_property_rule_id } end end diff --git a/core/app/models/spree/wallet_payment_source.rb b/core/app/models/spree/wallet_payment_source.rb index 16f5f298c3d..4b628420e60 100644 --- a/core/app/models/spree/wallet_payment_source.rb +++ b/core/app/models/spree/wallet_payment_source.rb @@ -5,8 +5,8 @@ class WalletPaymentSource < Spree::Base belongs_to :user, class_name: Spree::UserClassHandle.new, foreign_key: 'user_id', inverse_of: :wallet_payment_sources, optional: true belongs_to :payment_source, polymorphic: true, inverse_of: :wallet_payment_sources, optional: true - validates_presence_of :user - validates_presence_of :payment_source + validates :user, presence: true + validates :payment_source, presence: true validates :user_id, uniqueness: { scope: [:payment_source_type, :payment_source_id], message: :payment_source_already_exists diff --git a/core/config/locales/en.yml b/core/config/locales/en.yml index b73928cd973..ad8164abb08 100644 --- a/core/config/locales/en.yml +++ b/core/config/locales/en.yml @@ -435,8 +435,10 @@ en: weight: Weight width: Width spree/zone: + country_ids: Countries description: Description name: Name + state_ids: States errors: models: spree/address: diff --git a/core/lib/spree/testing_support/factories/zone_factory.rb b/core/lib/spree/testing_support/factories/zone_factory.rb index 7c3b99f58d9..eb436cd4acd 100644 --- a/core/lib/spree/testing_support/factories/zone_factory.rb +++ b/core/lib/spree/testing_support/factories/zone_factory.rb @@ -17,5 +17,9 @@ trait :with_country do countries { [create(:country)] } end + + trait :with_state do + states { [create(:state)] } + end end end diff --git a/legacy_promotions/app/models/spree/promotion/rules/product.rb b/legacy_promotions/app/models/spree/promotion/rules/product.rb index c8888bc71ad..2ebc1da0892 100644 --- a/legacy_promotions/app/models/spree/promotion/rules/product.rb +++ b/legacy_promotions/app/models/spree/promotion/rules/product.rb @@ -18,7 +18,7 @@ def preload_relations MATCH_POLICIES = %w(any all none) - validates_inclusion_of :preferred_match_policy, in: MATCH_POLICIES + validates :preferred_match_policy, inclusion: { in: MATCH_POLICIES } preference :match_policy, :string, default: MATCH_POLICIES.first diff --git a/legacy_promotions/app/models/spree/promotion/rules/taxon.rb b/legacy_promotions/app/models/spree/promotion/rules/taxon.rb index 16b459636d5..c1375258417 100644 --- a/legacy_promotions/app/models/spree/promotion/rules/taxon.rb +++ b/legacy_promotions/app/models/spree/promotion/rules/taxon.rb @@ -14,7 +14,7 @@ def preload_relations MATCH_POLICIES = %w(any all none) - validates_inclusion_of :preferred_match_policy, in: MATCH_POLICIES + validates :preferred_match_policy, inclusion: { in: MATCH_POLICIES } preference :match_policy, :string, default: MATCH_POLICIES.first def applicable?(promotable) diff --git a/legacy_promotions/app/models/spree/promotion_category.rb b/legacy_promotions/app/models/spree/promotion_category.rb index 302d15ecf3f..603a8e64d06 100644 --- a/legacy_promotions/app/models/spree/promotion_category.rb +++ b/legacy_promotions/app/models/spree/promotion_category.rb @@ -2,7 +2,7 @@ module Spree class PromotionCategory < Spree::Base - validates_presence_of :name + validates :name, presence: true has_many :promotions end end diff --git a/legacy_promotions/app/models/spree/promotion_code_batch.rb b/legacy_promotions/app/models/spree/promotion_code_batch.rb index 85a17be14be..2ef929f20c6 100644 --- a/legacy_promotions/app/models/spree/promotion_code_batch.rb +++ b/legacy_promotions/app/models/spree/promotion_code_batch.rb @@ -9,7 +9,7 @@ class CantProcessStartedBatch < StandardError has_many :promotion_codes, class_name: "Spree::PromotionCode", dependent: :destroy validates :number_of_codes, numericality: { greater_than: 0 } - validates_presence_of :base_code, :number_of_codes + validates :base_code, :number_of_codes, presence: true def finished? state == "completed"