Skip to content

Fix NoMethodError in issuable import CSV base service

Eugenia Grieff requested to merge 246857-fix-issuable-import-csv-service into master

What does this MR do?

Related to #246857 (closed)

In !46361 (merged) we refactored Issues::ImportCsvService and moved its content to Issuable::ImportCsv::BaseService so it could be reused to import Requirements.

In that process, we added a method to check for the presence of headers in the CSV file but missed the scenario of an empty file that would result in a NoMethodError as following:

Failures:
  1) RequirementsManagement::ImportRequirementsCsvWorker#perform behaves like an idempotent worker performs multiple times sequentially without raising an exception
     Failure/Error: expect { subject }.not_to raise_error
       expected no Exception, got #<NoMethodError: undefined method `downcase' for nil:NilClass> with backtrace:
         # ./app/services/issuable/import_csv/base_service.rb:53:in `verify_headers!'
         # ./app/services/issuable/import_csv/base_service.rb:41:in `with_csv_lines'
         # ./app/services/issuable/import_csv/base_service.rb:23:in `process_csv'
         # ./app/services/issuable/import_csv/base_service.rb:14:in `execute'

This MR adds an extra step that checks for the presence of lines in the CSV data and a new test case for empty files.

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Related to #246857 (closed)

Edited by Eugenia Grieff

Merge request reports

Loading