Merge pull request #2306 from UKGovernmentBEIS/feature/2931-feature-f…

…lag-for-new-import-service

(2931/5) Feature flag for new import service

CHANGELOG.md

@@ -25,6 +25,8 @@
 - update Import Row Error to expose csv row number as expected by the user
 - add a service that can import a file of multiple rows of csv that contain either an Actual,
   Refund, or Activity Comment
+- support both the new and the old actual importer, allowing switching between
+  them with a feature flag
 
 ## Release 141 - 2023-12-04
 

app/controllers/actuals/uploads_controller.rb

@@ -40,29 +40,60 @@ def update
     add_breadcrumb t("breadcrumb.report.upload_actuals"), new_report_actuals_upload_path(report)
 
     if upload.valid?
-      importer = Actual::Import.new(report: report, uploader: current_user)
-      importer.import(upload.rows)
-      @errors = importer.errors
+      if ROLLOUT.active?(:use_new_activity_actual_refund_comment_importer)
+        importer = Import::Csv::ActivityActualRefundComment::FileService.new(report: report, user: current_user, csv_rows: upload.rows)
+        import_result = importer.import!
+
+        @errors = importer.errors
+
+        if import_result
+          # the old import and the UI combine Actuals and Refunds, so we have to do the same
+          # once we have tested the import, we will come back and make the UI improvements
+          # to make the most of the new importer
+          imported_actuals_and_refunds = importer.imported_actuals + importer.imported_refunds
+
+          @total_actuals = total_transactions(imported_actuals_and_refunds)
+          @grouped_actuals = grouped_transactions(imported_actuals_and_refunds)
+          @success = true
+
+          flash.now[:notice] = t("action.actual.upload.success")
+        elsif @errors.empty?
+          flash.now[:error] = t("import.csv.activity_actual_refund_comment.errors.headers")
+        end
+      else
+        importer = Actual::Import.new(report: report, uploader: current_user)
+        importer.import(upload.rows)
+        @errors = importer.errors
 
-      if @errors.empty?
-        imported_actuals = importer.imported_actuals
+        if @errors.empty?
+          imported_actuals_and_refunds = importer.imported_actuals
 
-        @total_actuals = TotalPresenter.new(imported_actuals.sum(&:value)).value
-        @grouped_actuals = imported_actuals
-          .map { |actual| TransactionPresenter.new(actual) }
-          .group_by { |actual| ActivityPresenter.new(actual.parent_activity) }
+          @total_actuals = total_transactions(imported_actuals_and_refunds)
+          @grouped_actuals = grouped_transactions(imported_actuals_and_refunds)
+          @success = true
 
-        @success = true
-        flash.now[:notice] = t("action.actual.upload.success")
-      else
-        @invalid_with_comment = importer.invalid_with_comment
+          flash.now[:notice] = t("action.actual.upload.success")
+        else
+          @invalid_with_comment = importer.invalid_with_comment
+        end
       end
+
     else
       @errors = []
       flash.now[:error] = t("action.actual.upload.file_missing_or_invalid")
     end
   end
 
+  private def total_transactions(transactions)
+    TotalPresenter.new(transactions.sum(&:value)).value
+  end
+
+  private def grouped_transactions(transactions)
+    transactions
+      .map { |transaction| TransactionPresenter.new(transaction) }
+      .group_by { |transaction| ActivityPresenter.new(transaction.parent_activity) }
+  end
+
   private def report
     @_report ||= Report.find(params[:report_id])
   end

app/models/import/row_error.rb

@@ -1,11 +1,13 @@
 class Import::RowError < StandardError
-  attr_reader :row_number, :column, :value, :csv_row_number
+  attr_reader :row_number, :column, :value, :csv_row_number, :csv_row
 
   def initialize(column:, row_number:, value:, message:)
     @row_number = row_number
     @column = column
     @value = value
     @csv_row_number = row_number + 2
+    # csv_row to stay compatible with the current importer UI so we can switch between the two
+    @csv_row = @csv_row_number
     super(message)
   end
 end

config/locales/import/csv/actiity_actual_refund_comment/errors.en.yml

@@ -7,6 +7,7 @@ en:
           default:
             required: Is required
             negative: Cannot be negative
+          headers: Invalid headers in the csv file, check the template for the correct headers
           financial_quarter: Must be 1, 2, 3 or 4
           financial_year: Must be a four digit year
           financial_value: Must be a financial value

spec/controllers/actuals/uploads_controller_spec.rb

@@ -115,4 +115,56 @@
       expect(roda_identifiers).to_not include(ineligible_project.roda_identifier)
     end
   end
+
+  describe "#update" do
+    let(:report) { create(:report, organisation: organisation, state: :active) }
+
+    context "the use new importer feature flag is false" do
+      before { allow(ROLLOUT).to receive(:active?).with(:use_new_activity_actual_refund_comment_importer).and_return(false) }
+
+      it "uses the original actuals importer" do
+        fake_import_file = instance_double(CsvFileUpload)
+        allow(fake_import_file).to receive(:rows).and_return([])
+        allow(fake_import_file).to receive(:valid?).and_return(true)
+        allow(CsvFileUpload).to receive(:new).and_return(fake_import_file)
+
+        importer = instance_double(Actual::Import, import: true)
+        allow(importer).to receive(:errors).and_return([])
+        allow(importer).to receive(:imported_actuals).and_return([])
+        allow(importer).to receive(:invalid_with_comment).and_return(false)
+        allow(Actual::Import).to receive(:new).and_return(importer)
+
+        allow(Import::Csv::ActivityActualRefundComment::FileService).to receive(:new)
+
+        patch :update, params: {report_id: report.id}
+
+        expect(importer).to have_received(:import)
+        expect(Import::Csv::ActivityActualRefundComment::FileService).not_to have_received(:new)
+      end
+    end
+
+    context "the use new importer feature flag is true" do
+      before { allow(ROLLOUT).to receive(:active?).with(:use_new_activity_actual_refund_comment_importer).and_return(true) }
+
+      it "uses the new file importer" do
+        fake_import_file = instance_double(CsvFileUpload)
+        allow(fake_import_file).to receive(:rows).and_return([])
+        allow(fake_import_file).to receive(:valid?).and_return(true)
+        allow(CsvFileUpload).to receive(:new).and_return(fake_import_file)
+
+        importer = instance_double(Import::Csv::ActivityActualRefundComment::FileService, import!: true)
+        allow(importer).to receive(:errors).and_return([])
+        allow(importer).to receive(:imported_actuals).and_return([])
+        allow(importer).to receive(:imported_refunds).and_return([])
+        allow(Import::Csv::ActivityActualRefundComment::FileService).to receive(:new).and_return(importer)
+
+        allow(Actual::Import).to receive(:new)
+
+        patch :update, params: {report_id: report.id}
+
+        expect(importer).to have_received(:import!)
+        expect(Actual::Import).not_to have_received(:new)
+      end
+    end
+  end
 end

spec/features/import/users_can_upload_actuals_refunds_and_actvitiy_comments_spec.rb

@@ -0,0 +1,103 @@
+RSpec.feature "users can upload actuals, refunds and activity comments" do
+  let(:organisation) { create(:partner_organisation) }
+  let(:user) { create(:partner_organisation_user, organisation: organisation) }
+
+  let!(:project) { create(:project_activity, :newton_funded, organisation: organisation) }
+  let!(:another_project) { create(:project_activity, :newton_funded, organisation: organisation, parent: project.parent) }
+  let!(:yet_another_project) { create(:project_activity, :newton_funded, organisation: organisation, parent: project.parent) }
+
+  let!(:report) { create(:report, :active, fund: project.associated_fund, organisation: organisation) }
+
+  before do
+    authenticate!(user: user)
+    visit report_actuals_path(report)
+    click_link t("page_content.actuals.button.upload")
+
+    allow(ROLLOUT).to receive(:active?).with(:use_new_activity_actual_refund_comment_importer).and_return(true)
+  end
+
+  context "when the import is successful" do
+    let(:uploaded_csv) do
+      csv_data = <<~CSV
+        Activity RODA Identifier                | Financial Quarter | Financial Year | Actual Value | Refund Value | Comment
+        #{project.roda_identifier}              | 1                 | 2020           | 20000        | 0            |
+        #{another_project.roda_identifier}      | 1                 | 2020           | 0            | 30000        | Refund comment
+        #{yet_another_project.roda_identifier}  | 1                 | 2020           | 0            | 0            | Activity comment
+      CSV
+      upload_csv(csv_data)
+    end
+
+    it "shows the actuals and refunds that were imported" do
+      attach_file "report[actual_csv]", uploaded_csv.path
+      click_button t("action.actual.upload.button")
+
+      expect(Actual.count).to be 1
+      expect(Refund.count).to be 1
+
+      expect(page).to have_text(t("action.actual.upload.success"))
+
+      expect(page).to have_content(project.roda_identifier)
+      expect(page).to have_content("£20,000.00")
+
+      expect(page).to have_content(another_project.roda_identifier)
+      expect(page).to have_content("-£30,000.00")
+    end
+
+    it "allows the user to see the activity comment" do
+      attach_file "report[actual_csv]", uploaded_csv.path
+      click_button t("action.actual.upload.button")
+      visit report_comments_path(report)
+
+      expect(page).to have_content("Activity comment")
+      expect(page).to have_content(yet_another_project.roda_identifier)
+    end
+  end
+
+  context "when the import is unsuccessful" do
+    it "shows an error when the uploaded file does not contain the required headers" do
+      csv_data = <<~CSV
+        Incorrect Header           | Not correct header |
+        #{project.roda_identifier} | 1                  |
+      CSV
+      uploaded_csv = upload_csv(csv_data)
+
+      attach_file "report[actual_csv]", uploaded_csv.path
+      click_button t("action.actual.upload.button")
+
+      expect(page).to have_content("Invalid headers in the csv file")
+    end
+
+    it "shows the error when the uploaded actual is not valid" do
+      csv_data = <<~CSV
+        Activity RODA Identifier                | Financial Quarter | Financial Year | Actual Value | Refund Value | Comment
+        #{project.roda_identifier}              | 1                 | 2020           | -20000        | 0            |
+      CSV
+      uploaded_csv = upload_csv(csv_data)
+
+      attach_file "report[actual_csv]", uploaded_csv.path
+      click_button t("action.actual.upload.button")
+
+      expect(page).to have_content("Cannot be negative")
+    end
+
+    it "shows the error when the uploaded refund is not valid" do
+      csv_data = <<~CSV
+        Activity RODA Identifier                | Financial Quarter | Financial Year | Actual Value | Refund Value | Comment
+        #{project.roda_identifier}              | 1                 | 2020           | 0            | -30000       |
+      CSV
+      uploaded_csv = upload_csv(csv_data)
+
+      attach_file "report[actual_csv]", uploaded_csv.path
+      click_button t("action.actual.upload.button")
+
+      expect(page).to have_content("Refund must have a comment")
+    end
+  end
+
+  def upload_csv(content)
+    file = Tempfile.new("test.csv")
+    file.write(content.gsub(/ *\| */, ","))
+    file.close
+    file
+  end
+end

spec/models/import/row_error_spec.rb

@@ -34,6 +34,12 @@
     end
   end
 
+  describe "#csv_row" do
+    it "returns csv_row_number to maintain compatibility with the existing importer" do
+      expect(subject.csv_row).to eql subject.csv_row_number
+    end
+  end
+
   describe "#message" do
     it "returns the supplied message" do
       expect(subject.message).to eql "The message"