diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index ccaa53140b..546320930f 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1202,7 +1202,6 @@ Style/HashSyntax: - 'spec/queries/customers_with_balance_spec.rb' - 'spec/queries/outstanding_balance_spec.rb' - 'spec/queries/payments_requiring_action_spec.rb' - - 'spec/requests/admin/vouchers_controller_spec.rb' - 'spec/requests/api/v1/customers_spec.rb' - 'spec/requests/checkout/failed_checkout_spec.rb' - 'spec/requests/checkout/paypal_spec.rb' diff --git a/app/controllers/admin/vouchers_controller.rb b/app/controllers/admin/vouchers_controller.rb index 01ca689b91..33093bb07d 100644 --- a/app/controllers/admin/vouchers_controller.rb +++ b/app/controllers/admin/vouchers_controller.rb @@ -9,17 +9,9 @@ module Admin end def create - # The use of "safe_constantize" here will trigger a Brakeman error, it can safely be ignored - # as it's a false positive : https://github.com/openfoodfoundation/openfoodnetwork/pull/10821 - voucher_type = params[:vouchers_flat_rate][:voucher_type] - if Voucher::TYPES.include?(voucher_type) - @voucher = voucher_type.safe_constantize.create( - permitted_resource_params.merge(enterprise: @enterprise) - ) - else - @voucher.errors.add(:type) - return render_error - end + @voucher = Voucher.new( + permitted_resource_params.merge(enterprise: @enterprise) + ) if @voucher.save flash[:success] = I18n.t(:successfully_created, resource: "Voucher") @@ -27,6 +19,9 @@ module Admin else render_error end + rescue ActiveRecord::SubclassNotFound + @voucher.errors.add(:type) + render_error end private @@ -44,7 +39,7 @@ module Admin end def permitted_resource_params - params.require(:vouchers_flat_rate).permit(:code, :amount) + params.require(:voucher).permit(:code, :amount, :type) end end end diff --git a/app/views/admin/vouchers/new.html.haml b/app/views/admin/vouchers/new.html.haml index a27ea65d7a..6956f809bc 100644 --- a/app/views/admin/vouchers/new.html.haml +++ b/app/views/admin/vouchers/new.html.haml @@ -1,4 +1,4 @@ -= form_with model: @voucher, url: admin_enterprise_vouchers_path(@enterprise), html: { name: "voucher_form" } do |f| += form_with model: @voucher.becomes(Voucher), url: admin_enterprise_vouchers_path(@enterprise), html: { name: "voucher_form" } do |f| .row .sixteen.columns.alpha .four.columns.alpha.text-right @@ -17,11 +17,11 @@ = f.text_field :code, class: 'fullwidth' .row .alpha.four.columns - = f.label :voucher_type, t('.voucher_type') + = f.label :type, t('.voucher_type') .omega.eight.columns - = f.select :voucher_type, options_for_select(Voucher::TYPES.map { |type| [t(".#{type.demodulize.underscore}"), type] }, @voucher.class.to_s) + = f.select :type, options_for_select(Voucher::TYPES.map { |type| [t(".#{type.demodulize.underscore}"), type] }, @voucher.type) .row .alpha.four.columns = f.label :amount, t('.voucher_amount') .omega.eight.columns - = f.text_field :amount, value: @voucher.amount + = f.text_field :amount diff --git a/spec/requests/admin/vouchers_controller_spec.rb b/spec/requests/admin/vouchers_spec.rb similarity index 84% rename from spec/requests/admin/vouchers_controller_spec.rb rename to spec/requests/admin/vouchers_spec.rb index 60b0f34e17..86242e0ef1 100644 --- a/spec/requests/admin/vouchers_controller_spec.rb +++ b/spec/requests/admin/vouchers_spec.rb @@ -2,14 +2,14 @@ require "spec_helper" -describe Admin::VouchersController, type: :request do +describe "/admin/enterprises/:enterprise_id/vouchers", type: :request do let(:enterprise) { create(:supplier_enterprise, name: "Feedme") } let(:enterprise_user) { create(:user, enterprise_limit: 1) } before do Flipper.enable(:vouchers) - enterprise_user.enterprise_roles.build(enterprise: enterprise).save + enterprise_user.enterprise_roles.build(enterprise:).save sign_in enterprise_user end @@ -22,14 +22,14 @@ describe Admin::VouchersController, type: :request do end describe "POST /admin/enterprises/:enterprise_id/vouchers" do - subject(:create_voucher) { post admin_enterprise_vouchers_path(enterprise), params: params } + subject(:create_voucher) { post admin_enterprise_vouchers_path(enterprise), params: } let(:params) do { - vouchers_flat_rate: { - code: code, - amount: amount, - voucher_type: type + voucher: { + code:, + amount:, + type: } } end @@ -50,6 +50,15 @@ describe Admin::VouchersController, type: :request do end context "with a percentage rate voucher" do + let(:params) do + { + voucher: { + code:, + amount:, + type: + } + } + end let(:type) { "Vouchers::PercentageRate" } it "creates a new voucher" do diff --git a/spec/system/admin/vouchers_spec.rb b/spec/system/admin/vouchers_spec.rb index 68b7fe83f3..6c113ec693 100644 --- a/spec/system/admin/vouchers_spec.rb +++ b/spec/system/admin/vouchers_spec.rb @@ -50,9 +50,9 @@ describe ' context "with a flat rate voucher" do it 'creates a voucher' do # And I fill in the fields for a new voucher click save - fill_in 'vouchers_flat_rate_code', with: voucher_code - select "Flat", from: "vouchers_flat_rate_voucher_type" - fill_in 'vouchers_flat_rate_amount', with: amount + fill_in "Voucher Code", with: voucher_code + select "Flat", from: "Voucher Type" + fill_in "Amount", with: amount click_button 'Save' # Then I should get redirect to the entreprise voucher tab and see the created voucher @@ -64,9 +64,9 @@ describe ' context "with a percentage rate voucher" do it 'creates a voucher' do # And I fill in the fields for a new voucher click save - fill_in 'vouchers_flat_rate_code', with: voucher_code - select "Percentage (%)", from: "vouchers_flat_rate_voucher_type" - fill_in 'vouchers_flat_rate_amount', with: amount + fill_in "Voucher Code", with: voucher_code + select "Percentage (%)", from: "Voucher Type" + fill_in "Amount", with: amount click_button 'Save' # Then I should get redirect to the entreprise voucher tab and see the created voucher