Merge pull request #11391 from rioug/11362-fix-error-creating-percentage-voucher

[Vouchers] fix error when creating a percentage voucher

.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'

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

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

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

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