From dbce7dc161e5d00c16a4a4fb70e768c70dd2bf83 Mon Sep 17 00:00:00 2001 From: Stephane CRIVISIER Date: Wed, 26 Sep 2018 12:01:14 +0200 Subject: [PATCH 1/9] Sort promotion codes --- .../app/controllers/spree/admin/promotion_codes_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/app/controllers/spree/admin/promotion_codes_controller.rb b/backend/app/controllers/spree/admin/promotion_codes_controller.rb index c8055a0d888..521eea7ea39 100644 --- a/backend/app/controllers/spree/admin/promotion_codes_controller.rb +++ b/backend/app/controllers/spree/admin/promotion_codes_controller.rb @@ -7,7 +7,7 @@ module Admin class PromotionCodesController < Spree::Admin::ResourceController def index @promotion = Spree::Promotion.accessible_by(current_ability, :read).find(params[:promotion_id]) - @promotion_codes = @promotion.promotion_codes + @promotion_codes = @promotion.promotion_codes.order(:value) respond_to do |format| format.html do From 044ff82d9362076287acf82f0279264e118dd289 Mon Sep 17 00:00:00 2001 From: Stephane CRIVISIER Date: Wed, 26 Sep 2018 12:02:48 +0200 Subject: [PATCH 2/9] Rename the spec The old name wasn't that explicit about what it is testing --- .../controllers/spree/admin/promotion_codes_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/spec/controllers/spree/admin/promotion_codes_controller_spec.rb b/backend/spec/controllers/spree/admin/promotion_codes_controller_spec.rb index 8bd550e071d..1a738dc3781 100644 --- a/backend/spec/controllers/spree/admin/promotion_codes_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/promotion_codes_controller_spec.rb @@ -11,7 +11,7 @@ let!(:code2) { create(:promotion_code, promotion: promotion) } let!(:code3) { create(:promotion_code, promotion: promotion) } - it "can create a promotion rule of a valid type" do + it "can create a CSV file with all promotion codes" do get :index, params: { promotion_id: promotion.id, format: 'csv' } expect(response).to be_successful parsed = CSV.parse(response.body, headers: true) From 400c88eb24d4c9fd64ca39fbae8b607527cbfaaf Mon Sep 17 00:00:00 2001 From: Stephane CRIVISIER Date: Wed, 26 Sep 2018 12:17:40 +0200 Subject: [PATCH 3/9] Add a link to the promotion's code list --- backend/app/views/spree/admin/promotions/edit.html.erb | 2 ++ core/config/locales/en.yml | 1 + 2 files changed, 3 insertions(+) diff --git a/backend/app/views/spree/admin/promotions/edit.html.erb b/backend/app/views/spree/admin/promotions/edit.html.erb index ea0892f1de8..945edae0570 100644 --- a/backend/app/views/spree/admin/promotions/edit.html.erb +++ b/backend/app/views/spree/admin/promotions/edit.html.erb @@ -6,6 +6,8 @@ <% content_for :page_actions do %>
  • <% if can?(:display, Spree::PromotionCode) %> + <%= link_to t('spree.view_promotion_code_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id), class: 'btn btn-primary' %> + <%= link_to t('spree.download_promotion_code_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id, format: :csv), class: 'btn btn-primary' %> <% end %> diff --git a/core/config/locales/en.yml b/core/config/locales/en.yml index e769b683a36..097541116f4 100644 --- a/core/config/locales/en.yml +++ b/core/config/locales/en.yml @@ -2094,6 +2094,7 @@ en: variant_to_be_received: Variant to be received variants: Variants version: Version + view_promotion_code_list: View Code List void: Void weight: Weight what_is_a_cvv: What is a (CVV) Credit Card Code? From 31c40720a5e2a4170db099d2d4a3d03bb2c10476 Mon Sep 17 00:00:00 2001 From: Stephane CRIVISIER Date: Wed, 26 Sep 2018 12:25:05 +0200 Subject: [PATCH 4/9] Add a form to create promotion code Let users explicitly create a new code on an existing promotion, without checking if it's a "single" or "multiple" promotion type because it's already possible do switch from a "single" to "multiple" using the `Spree::PromotionCodeBatch` creation process. This let the user creates a new code without using the "batch" method so that it can explicitly choose a new code without using random suffixes. --- .../spree/admin/promotion_codes_controller.rb | 18 +++++++++++ .../admin/promotion_codes/index.html.erb | 4 +++ .../spree/admin/promotion_codes/new.html.erb | 31 +++++++++++++++++++ backend/config/routes.rb | 2 +- .../admin/promotion_codes_controller_spec.rb | 6 ++++ core/config/locales/en.yml | 1 + 6 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 backend/app/views/spree/admin/promotion_codes/new.html.erb diff --git a/backend/app/controllers/spree/admin/promotion_codes_controller.rb b/backend/app/controllers/spree/admin/promotion_codes_controller.rb index 521eea7ea39..26e86dd5edb 100644 --- a/backend/app/controllers/spree/admin/promotion_codes_controller.rb +++ b/backend/app/controllers/spree/admin/promotion_codes_controller.rb @@ -20,6 +20,24 @@ def index end end end + + def new + @promotion = Spree::Promotion.accessible_by(current_ability, :read).find(params[:promotion_id]) + @promotion_code = @promotion.promotion_codes.new + end + + def create + @promotion = Spree::Promotion.accessible_by(current_ability, :read).find(params[:promotion_id]) + @promotion_code = @promotion.promotion_codes.new(value: params[:promotion_code][:value]) + + if @promotion_code.save + flash[:success] = flash_message_for(@promotion_code, :successfully_created) + redirect_to admin_promotion_promotion_codes_url(@promotion) + else + flash.now[:error] = @promotion_code.errors.full_messages.join(', ') + render_after_create_error + end + end end end end diff --git a/backend/app/views/spree/admin/promotion_codes/index.html.erb b/backend/app/views/spree/admin/promotion_codes/index.html.erb index dc643641d1b..5e35c39b0d4 100644 --- a/backend/app/views/spree/admin/promotion_codes/index.html.erb +++ b/backend/app/views/spree/admin/promotion_codes/index.html.erb @@ -4,6 +4,10 @@ <% content_for :page_actions do %>
  • + <% if can?(:create, Spree::PromotionCode) %> + <%= link_to t('spree.create_promotion_code'), new_admin_promotion_promotion_code_path(promotion_id: @promotion.id), class: 'btn btn-primary' %> + <% end %> + <%= link_to t('spree.download_promotion_code_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id, format: :csv), class: 'btn btn-primary' %>
    • <% end %> diff --git a/backend/app/views/spree/admin/promotion_codes/new.html.erb b/backend/app/views/spree/admin/promotion_codes/new.html.erb new file mode 100644 index 00000000000..923519296af --- /dev/null +++ b/backend/app/views/spree/admin/promotion_codes/new.html.erb @@ -0,0 +1,31 @@ +<% admin_breadcrumb link_to plural_resource_name(Spree::Promotion), spree.admin_promotions_path %> +<% admin_breadcrumb link_to(@promotion.name, spree.edit_admin_promotion_path(@promotion)) %> +<% admin_breadcrumb plural_resource_name(Spree::PromotionCode) %> + +<% content_for :page_actions do %> +
  • + <%= link_to t('spree.view_promotion_code_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id), class: 'btn btn-primary' %> + + <%= link_to t('spree.download_promotion_code_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id, format: :csv), class: 'btn btn-primary' %> +
    • +<% end %> + +<%= form_for [:admin, @promotion, @promotion_code], method: :post do |f| %> +
    + <%= render partial: 'spree/shared/error_messages', locals: { target: @promotion_code } %> + +
    +
    + <%= f.field_container :value do %> + <%= f.label :value, class: 'required' %> + <%= f.text_field :value, class: 'fullwidth', required: true %> + <% end %> +
    +
    + +
    + <%= button_tag t('spree.actions.create'), class: 'btn btn-primary' %> + <%= link_to t('spree.actions.cancel'), admin_promotion_promotion_codes_url(@promotion), class: 'button' %> +
    +
    +<% end %> diff --git a/backend/config/routes.rb b/backend/config/routes.rb index d635052e878..30d3fcf9a00 100644 --- a/backend/config/routes.rb +++ b/backend/config/routes.rb @@ -16,7 +16,7 @@ resources :promotions do resources :promotion_rules resources :promotion_actions - resources :promotion_codes, only: [:index] + resources :promotion_codes, only: [:index, :new, :create] resources :promotion_code_batches, only: [:index, :new, :create] do get '/download', to: "promotion_code_batches#download", defaults: { format: "csv" } end diff --git a/backend/spec/controllers/spree/admin/promotion_codes_controller_spec.rb b/backend/spec/controllers/spree/admin/promotion_codes_controller_spec.rb index 1a738dc3781..9802ae7adfa 100644 --- a/backend/spec/controllers/spree/admin/promotion_codes_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/promotion_codes_controller_spec.rb @@ -17,4 +17,10 @@ parsed = CSV.parse(response.body, headers: true) expect(parsed.entries.map(&:to_h)).to eq([{ "Code" => code1.value }, { "Code" => code2.value }, { "Code" => code3.value }]) end + + it "can create a new code" do + post :create, params: { value: "new_code" } + expect(response).to be_successful + expect(Spree::PromotionCode.where(promotion: promotion).count).to eql(4) + end end diff --git a/core/config/locales/en.yml b/core/config/locales/en.yml index 097541116f4..326f4ff073a 100644 --- a/core/config/locales/en.yml +++ b/core/config/locales/en.yml @@ -1146,6 +1146,7 @@ en: customer_returns: Customer Returns create: Create create_a_new_account: Create a new account + create_promotion_code: Create Promotion Code create_reimbursement: Create reimbursement create_one: Create One. created_at: Created At From 30db0da5b87e5b64773796221b9084cb9130f4a6 Mon Sep 17 00:00:00 2001 From: Stephane CRIVISIER Date: Wed, 26 Sep 2018 13:03:59 +0200 Subject: [PATCH 5/9] Setup PromotionCode management ACL --- .../spree/admin/promotion_codes_controller_spec.rb | 10 ++++++++-- core/lib/spree/permission_sets/promotion_management.rb | 1 + .../spree/permission_sets/promotion_management_spec.rb | 2 ++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/backend/spec/controllers/spree/admin/promotion_codes_controller_spec.rb b/backend/spec/controllers/spree/admin/promotion_codes_controller_spec.rb index 9802ae7adfa..7ff525aff10 100644 --- a/backend/spec/controllers/spree/admin/promotion_codes_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/promotion_codes_controller_spec.rb @@ -19,8 +19,14 @@ end it "can create a new code" do - post :create, params: { value: "new_code" } - expect(response).to be_successful + post :create, params: { promotion_id: promotion.id, promotion_code: { value: "new_code" } } + expect(response).to redirect_to(spree.admin_promotion_promotion_codes_path(promotion)) expect(Spree::PromotionCode.where(promotion: promotion).count).to eql(4) end + + it "cannot create an existing code" do + post :create, params: { promotion_id: promotion.id, promotion_code: { value: code1.value } } + expect(flash[:error]).not_to be_nil + expect(Spree::PromotionCode.where(promotion: promotion).count).to eql(3) + end end diff --git a/core/lib/spree/permission_sets/promotion_management.rb b/core/lib/spree/permission_sets/promotion_management.rb index 7e072848215..33268f4979b 100644 --- a/core/lib/spree/permission_sets/promotion_management.rb +++ b/core/lib/spree/permission_sets/promotion_management.rb @@ -8,6 +8,7 @@ def activate! can :manage, Spree::PromotionRule can :manage, Spree::PromotionAction can :manage, Spree::PromotionCategory + can :manage, Spree::PromotionCode end end end diff --git a/core/spec/models/spree/permission_sets/promotion_management_spec.rb b/core/spec/models/spree/permission_sets/promotion_management_spec.rb index af044cf5f2d..96786aec30b 100644 --- a/core/spec/models/spree/permission_sets/promotion_management_spec.rb +++ b/core/spec/models/spree/permission_sets/promotion_management_spec.rb @@ -16,6 +16,7 @@ it { is_expected.to be_able_to(:manage, Spree::PromotionRule) } it { is_expected.to be_able_to(:manage, Spree::PromotionAction) } it { is_expected.to be_able_to(:manage, Spree::PromotionCategory) } + it { is_expected.to be_able_to(:manage, Spree::PromotionCode) } end context "when not activated" do @@ -23,5 +24,6 @@ it { is_expected.not_to be_able_to(:manage, Spree::PromotionRule) } it { is_expected.not_to be_able_to(:manage, Spree::PromotionAction) } it { is_expected.not_to be_able_to(:manage, Spree::PromotionCategory) } + it { is_expected.not_to be_able_to(:manage, Spree::PromotionCode) } end end From b8f02dbfdac11ba661b163d71d612bcb1db84158 Mon Sep 17 00:00:00 2001 From: Stephane CRIVISIER Date: Sat, 20 Oct 2018 07:20:59 +0200 Subject: [PATCH 6/9] Fix indentation --- backend/app/views/spree/admin/promotion_codes/new.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/app/views/spree/admin/promotion_codes/new.html.erb b/backend/app/views/spree/admin/promotion_codes/new.html.erb index 923519296af..7aff01dcd94 100644 --- a/backend/app/views/spree/admin/promotion_codes/new.html.erb +++ b/backend/app/views/spree/admin/promotion_codes/new.html.erb @@ -17,8 +17,8 @@
    <%= f.field_container :value do %> - <%= f.label :value, class: 'required' %> - <%= f.text_field :value, class: 'fullwidth', required: true %> + <%= f.label :value, class: 'required' %> + <%= f.text_field :value, class: 'fullwidth', required: true %> <% end %>
    From 8bd16f70eafe697f31c6fb8cc2784515a794fa7e Mon Sep 17 00:00:00 2001 From: Stephane CRIVISIER Date: Sat, 20 Oct 2018 07:23:27 +0200 Subject: [PATCH 7/9] Use `errors.full_messages.to_sentence` `.full_messages.to_sentence` will do the same as `.full_messages.join(', ')` but uses an "and" as last divider --- .../app/controllers/spree/admin/promotion_codes_controller.rb | 2 +- backend/app/controllers/spree/admin/promotions_controller.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/app/controllers/spree/admin/promotion_codes_controller.rb b/backend/app/controllers/spree/admin/promotion_codes_controller.rb index 26e86dd5edb..48c65a9c110 100644 --- a/backend/app/controllers/spree/admin/promotion_codes_controller.rb +++ b/backend/app/controllers/spree/admin/promotion_codes_controller.rb @@ -34,7 +34,7 @@ def create flash[:success] = flash_message_for(@promotion_code, :successfully_created) redirect_to admin_promotion_promotion_codes_url(@promotion) else - flash.now[:error] = @promotion_code.errors.full_messages.join(', ') + flash.now[:error] = @promotion_code.errors.full_messages.to_sentence render_after_create_error end end diff --git a/backend/app/controllers/spree/admin/promotions_controller.rb b/backend/app/controllers/spree/admin/promotions_controller.rb index 57d1465f0ff..01ece74c5b3 100644 --- a/backend/app/controllers/spree/admin/promotions_controller.rb +++ b/backend/app/controllers/spree/admin/promotions_controller.rb @@ -24,7 +24,7 @@ def create flash[:success] = t('spree.promotion_successfully_created') redirect_to location_after_save else - flash[:error] = @promotion.errors.full_messages.join(", ") + flash[:error] = @promotion.errors.full_messages.to_sentence render action: 'new' end end From e3866a2bb9ccd89b855208727e7d23fc57ba607f Mon Sep 17 00:00:00 2001 From: Stephane CRIVISIER Date: Sun, 21 Oct 2018 10:28:30 +0200 Subject: [PATCH 8/9] Reword translations --- .../views/spree/admin/promotion_code_batches/index.html.erb | 2 +- .../app/views/spree/admin/promotion_codes/index.html.erb | 2 +- backend/app/views/spree/admin/promotion_codes/new.html.erb | 4 ++-- backend/app/views/spree/admin/promotions/edit.html.erb | 4 ++-- core/config/locales/en.yml | 6 +++--- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/backend/app/views/spree/admin/promotion_code_batches/index.html.erb b/backend/app/views/spree/admin/promotion_code_batches/index.html.erb index eab80daa864..bc558dcac7e 100644 --- a/backend/app/views/spree/admin/promotion_code_batches/index.html.erb +++ b/backend/app/views/spree/admin/promotion_code_batches/index.html.erb @@ -37,7 +37,7 @@ number_of_codes: promotion_code_batch.number_of_codes ) %> <%= link_to( - t('spree.download_promotion_code_list'), + t('spree.download_promotion_codes_list'), admin_promotion_promotion_code_batch_download_path( promotion_code_batch_id: promotion_code_batch.id, format: :csv diff --git a/backend/app/views/spree/admin/promotion_codes/index.html.erb b/backend/app/views/spree/admin/promotion_codes/index.html.erb index 5e35c39b0d4..97b9e8e005b 100644 --- a/backend/app/views/spree/admin/promotion_codes/index.html.erb +++ b/backend/app/views/spree/admin/promotion_codes/index.html.erb @@ -8,7 +8,7 @@ <%= link_to t('spree.create_promotion_code'), new_admin_promotion_promotion_code_path(promotion_id: @promotion.id), class: 'btn btn-primary' %> <% end %> - <%= link_to t('spree.download_promotion_code_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id, format: :csv), class: 'btn btn-primary' %> + <%= link_to t('spree.download_promotion_codes_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id, format: :csv), class: 'btn btn-primary' %> <% end %> diff --git a/backend/app/views/spree/admin/promotion_codes/new.html.erb b/backend/app/views/spree/admin/promotion_codes/new.html.erb index 7aff01dcd94..5a109730adb 100644 --- a/backend/app/views/spree/admin/promotion_codes/new.html.erb +++ b/backend/app/views/spree/admin/promotion_codes/new.html.erb @@ -4,9 +4,9 @@ <% content_for :page_actions do %>
  • - <%= link_to t('spree.view_promotion_code_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id), class: 'btn btn-primary' %> + <%= link_to t('spree.view_promotion_codes_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id), class: 'btn btn-primary' %> - <%= link_to t('spree.download_promotion_code_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id, format: :csv), class: 'btn btn-primary' %> + <%= link_to t('spree.download_promotion_codes_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id, format: :csv), class: 'btn btn-primary' %>
    • <% end %> diff --git a/backend/app/views/spree/admin/promotions/edit.html.erb b/backend/app/views/spree/admin/promotions/edit.html.erb index 945edae0570..bfcfaaa9970 100644 --- a/backend/app/views/spree/admin/promotions/edit.html.erb +++ b/backend/app/views/spree/admin/promotions/edit.html.erb @@ -6,9 +6,9 @@ <% content_for :page_actions do %>
  • <% if can?(:display, Spree::PromotionCode) %> - <%= link_to t('spree.view_promotion_code_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id), class: 'btn btn-primary' %> + <%= link_to t('spree.view_promotion_codes_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id), class: 'btn btn-primary' %> - <%= link_to t('spree.download_promotion_code_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id, format: :csv), class: 'btn btn-primary' %> + <%= link_to t('spree.download_promotion_codes_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id, format: :csv), class: 'btn btn-primary' %> <% end %> <% if can?(:display, Spree::PromotionCodeBatch) %> diff --git a/core/config/locales/en.yml b/core/config/locales/en.yml index 326f4ff073a..b4f8ece9391 100644 --- a/core/config/locales/en.yml +++ b/core/config/locales/en.yml @@ -1146,7 +1146,7 @@ en: customer_returns: Customer Returns create: Create create_a_new_account: Create a new account - create_promotion_code: Create Promotion Code + create_promotion_code: Create promotion code create_reimbursement: Create reimbursement create_one: Create One. created_at: Created At @@ -1201,7 +1201,7 @@ en: discount_rules: Discount rules dismiss_banner: No. Thanks! I'm not interested, do not display this message again display: Display - download_promotion_code_list: Download Code List + download_promotion_codes_list: Download codes list edit: Edit editing_country: Editing Country editing_adjustment_reason: Editing Adjustment Reason @@ -2095,7 +2095,7 @@ en: variant_to_be_received: Variant to be received variants: Variants version: Version - view_promotion_code_list: View Code List + view_promotion_codes_list: View codes list void: Void weight: Weight what_is_a_cvv: What is a (CVV) Credit Card Code? From e549cd7cc58882633e6a3c9bcb23e0e0fb281f46 Mon Sep 17 00:00:00 2001 From: Stephane CRIVISIER Date: Wed, 31 Oct 2018 17:31:06 +0100 Subject: [PATCH 9/9] Use `.build` instead of `.new` on promotion_codes --- .../app/controllers/spree/admin/promotion_codes_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/app/controllers/spree/admin/promotion_codes_controller.rb b/backend/app/controllers/spree/admin/promotion_codes_controller.rb index 48c65a9c110..6c19fa51ff6 100644 --- a/backend/app/controllers/spree/admin/promotion_codes_controller.rb +++ b/backend/app/controllers/spree/admin/promotion_codes_controller.rb @@ -23,12 +23,12 @@ def index def new @promotion = Spree::Promotion.accessible_by(current_ability, :read).find(params[:promotion_id]) - @promotion_code = @promotion.promotion_codes.new + @promotion_code = @promotion.promotion_codes.build end def create @promotion = Spree::Promotion.accessible_by(current_ability, :read).find(params[:promotion_id]) - @promotion_code = @promotion.promotion_codes.new(value: params[:promotion_code][:value]) + @promotion_code = @promotion.promotion_codes.build(value: params[:promotion_code][:value]) if @promotion_code.save flash[:success] = flash_message_for(@promotion_code, :successfully_created)