Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a new promotion code inside an existing promotion #2872

Merged
merged 9 commits into from
Nov 10, 2018
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Copy link
Member

@kennyadsl kennyadsl Oct 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just wondering why we are not using ActiveRecord build here. Is there a specific reason? I think you can even do @promotion.build_promotion_code right?

Same in the create method where it's not a conventional create. I'm taking a look at scaffolded rails controller templates.

Let me know!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@promotion.build_promotion_code does not seems to exists :

irb(main):009:0> promotion = Spree::Promotion.last
irb(main):010:0> promotion.build_promotion_code
Traceback (most recent call last):
        1: from (irb):10
NoMethodError (undefined method `build_promotion_code' for #<Spree::Promotion:0x00007f625b3b2e18>)
Did you mean?  build_promotion_category

And new or build return the same object :

irb(main):011:0> promotion.promotion_codes.build
=> #<Spree::PromotionCode id: nil, promotion_id: 65, value: nil, created_at: nil, updated_at: nil, promotion_code_batch_id: nil>
irb(main):012:0> promotion.promotion_codes.new
=> #<Spree::PromotionCode id: nil, promotion_id: 65, value: nil, created_at: nil, updated_at: nil, promotion_code_batch_id: nil>

I've switched to 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])

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.to_sentence
render_after_create_error
end
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion backend/app/views/spree/admin/promotion_codes/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@

<% content_for :page_actions do %>
<li>
<%= link_to t('spree.download_promotion_code_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id, format: :csv), class: 'btn btn-primary' %>
<% if can?(:create, Spree::PromotionCode) %>
tvdeyen marked this conversation as resolved.
Show resolved Hide resolved
<%= 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_codes_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id, format: :csv), class: 'btn btn-primary' %>
</li>
<% end %>

Expand Down
31 changes: 31 additions & 0 deletions backend/app/views/spree/admin/promotion_codes/new.html.erb
Original file line number Diff line number Diff line change
@@ -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 %>
<li>
<%= 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_codes_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id, format: :csv), class: 'btn btn-primary' %>
</li>
<% end %>

<%= form_for [:admin, @promotion, @promotion_code], method: :post do |f| %>
<fieldset class="no-border-top">
<%= render partial: 'spree/shared/error_messages', locals: { target: @promotion_code } %>

<div class="row">
<div class="col-4">
<%= f.field_container :value do %>
<%= f.label :value, class: 'required' %>
<%= f.text_field :value, class: 'fullwidth', required: true %>
<% end %>
</div>
</div>

<div class="form-buttons filter-actions actions" data-hook="buttons">
<%= button_tag t('spree.actions.create'), class: 'btn btn-primary' %>
<%= link_to t('spree.actions.cancel'), admin_promotion_promotion_codes_url(@promotion), class: 'button' %>
</div>
</fieldset>
<% end %>
4 changes: 3 additions & 1 deletion backend/app/views/spree/admin/promotions/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
<% content_for :page_actions do %>
<li>
<% if can?(:display, Spree::PromotionCode) %>
<%= 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.view_promotion_codes_list'), admin_promotion_promotion_codes_path(promotion_id: @promotion.id), 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) %>
Expand Down
2 changes: 1 addition & 1 deletion backend/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,22 @@
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)
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: { 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
4 changes: 3 additions & 1 deletion core/config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1200,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
Expand Down Expand Up @@ -2094,6 +2095,7 @@ en:
variant_to_be_received: Variant to be received
variants: Variants
version: Version
view_promotion_codes_list: View codes list
void: Void
weight: Weight
what_is_a_cvv: What is a (CVV) Credit Card Code?
Expand Down
1 change: 1 addition & 0 deletions core/lib/spree/permission_sets/promotion_management.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@
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
it { is_expected.not_to be_able_to(:manage, Spree::Promotion) }
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