Changes from MR review

Mostly code cleanup

db/post_migrate/20200519201128_migrate_vulnerability_dismissal_feedback.rb

@@ -4,31 +4,12 @@
 # for more information on how to write migrations for GitLab.
 
 class MigrateVulnerabilityDismissalFeedback < ActiveRecord::Migration[6.0]
-  # Uncomment the following include if you require helper functions:
   include Gitlab::Database::MigrationHelpers
 
-  # Set this constant to true if this migration requires downtime.
   DOWNTIME = false
-
-  # When a migration requires downtime you **must** uncomment the following
-  # constant and define a short and easy to understand explanation as to why the
-  # migration requires downtime.
-  # DOWNTIME_REASON = ''
-
-  # When using the methods "add_concurrent_index", "remove_concurrent_index" or
-  # "add_column_with_default" you must disable the use of transactions
-  # as these methods can not run in an existing transaction.
-  # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure
-  # that either of them is the _only_ method called in the migration,
-  # any other changes should go in a separate migration.
-  # This ensures that upon failure _only_ the index creation or removing fails
-  # and can be retried or reverted easily.
-  #
-  # To disable transactions uncomment the following line and remove these
-  # comments:
   disable_ddl_transaction!
 
-  MIGRATION = 'UpdateVulnerabilitiesFromDismissalFeedback'.freeze
+  MIGRATION = 'UpdateVulnerabilitiesFromDismissalFeedback'
   BATCH_SIZE = 500
   DELAY_INTERVAL = 2.minutes.to_i
 

db/structure.sql

@@ -14011,6 +14011,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200519141534
 20200519171058
 20200519194042
+20200519201128
 20200520103514
 20200521022725
 20200521225327

ee/changelogs/unreleased/217600-migrate-dismissed_data-for-vulnerabilities.yml

 ---
-title: Migrate dismissal data to Vulnersabilities from Vulnerability Feedback
+title: Migrate dismissal data from Vulnerability Feedback to Vulnerabilities
 merge_request: 32444
 author:
 type: fixed

ee/lib/ee/gitlab/background_migration/update_vulnerabilities_from_dismissal_feedback.rb

@@ -23,7 +23,6 @@ module EE
           return unless project
           return if project.archived? || project.pending_delete?
 
-          #binding.pry
           update_vulnerability_from_dismissal_feedback(project.id)
         end
 

ee/spec/lib/ee/gitlab/background_migration/update_vulnerabilities_from_dismissal_feedback_spec.rb

@@ -17,20 +17,19 @@ describe Gitlab::BackgroundMigration::UpdateVulnerabilitiesFromDismissalFeedback
   let(:confidence) { Vulnerabilities::Occurrence::CONFIDENCE_LEVELS[:medium] }
   let(:report_type) { Vulnerabilities::Occurrence::REPORT_TYPES[:sast] }
 
-  let!(:user) { users.create!(id: 13, email: 'author@example.com', username: 'author', projects_limit: 10) }
-  let!(:project) { projects.create!(id: 123, namespace_id: namespace.id, name: 'gitlab', path: 'gitlab') }
+  let!(:user) { users.create!(email: 'author@example.com', username: 'author', projects_limit: 10) }
+  let!(:project) { projects.create!(namespace_id: namespace.id, name: 'gitlab', path: 'gitlab') }
 
   let(:namespace) do
-    namespaces.create!(id: 12, name: 'namespace', path: '/path', description: 'description')
+    namespaces.create!(name: 'namespace', path: '/path', description: 'description')
   end
 
   let(:scanner) do
-    scanners.create!(id: 6, project_id: project.id, external_id: 'clair', name: 'Security Scanner')
+    scanners.create!(project_id: project.id, external_id: 'clair', name: 'Security Scanner')
   end
 
   let(:identifier) do
-    identifiers.create!(id: 7,
-      project_id: 123,
+    identifiers.create!(project_id: project.id,
       fingerprint: 'd432c2ad2953e8bd587a3a43b3ce309b5b0154c7',
       external_type: 'SECURITY_ID',
       external_id: 'SECURITY_0',
@@ -39,11 +38,11 @@ describe Gitlab::BackgroundMigration::UpdateVulnerabilitiesFromDismissalFeedback
 
   context 'vulnerability has been dismissed' do
     let!(:vulnerability) { vulnerabilities.create!(vuln_params) }
-    let!(:pipeline) { pipelines.create!(id: 234, project_id: project.id, ref: 'master', sha: 'adf43c3a', status: :success, user_id: user.id) }
+    let!(:pipeline) { pipelines.create!(project_id: project.id, ref: 'master', sha: 'adf43c3a', status: :success, user_id: user.id) }
 
     let!(:vulnerability_occurrence) do
       vulnerability_occurrences.create!(
-        id: 1, report_type: vulnerability.report_type, name: 'finding_1',
+        report_type: vulnerability.report_type, name: 'finding_1',
         primary_identifier_id: identifier.id, uuid: 'abc', project_fingerprint: 'abc123',
         location_fingerprint: 'abc456', project_id: project.id, scanner_id: scanner.id, severity: severity,
         confidence: confidence, metadata_version: 'sast:1.0', raw_metadata: '{}', vulnerability_id: vulnerability.id)
@@ -74,7 +73,7 @@ describe Gitlab::BackgroundMigration::UpdateVulnerabilitiesFromDismissalFeedback
     end
 
     context 'project is archived' do
-      let!(:project) { projects.create!(id: 123, namespace_id: namespace.id, name: 'gitlab', path: 'gitlab', archived: true) }
+      let!(:project) { projects.create!(namespace_id: namespace.id, name: 'gitlab', path: 'gitlab', archived: true) }
 
       it 'vulnerability dismissed_by_id should remain nil' do
         expect(vulnerability.dismissed_by_id).to eq(nil)
@@ -90,7 +89,7 @@ describe Gitlab::BackgroundMigration::UpdateVulnerabilitiesFromDismissalFeedback
     end
 
     context 'project is set to be deleted' do
-      let!(:project) { projects.create!(id: 123, namespace_id: namespace.id, name: 'gitlab', path: 'gitlab', pending_delete: true) }
+      let!(:project) { projects.create!(namespace_id: namespace.id, name: 'gitlab', path: 'gitlab', pending_delete: true) }
 
       it 'vulnerability dismissed_by_id should remain nil' do
         expect(vulnerability.dismissed_by_id).to eq(nil)
@@ -107,7 +106,7 @@ describe Gitlab::BackgroundMigration::UpdateVulnerabilitiesFromDismissalFeedback
   end
 
   context 'has not been dismissed' do
-    let!(:vulnerability) { vulnerabilities.create!(vuln_params.merge({state: 1})) }
+    let!(:vulnerability) { vulnerabilities.create!(vuln_params.merge({ state: 1 })) }
 
     it 'vulnerability should not have a dismissed_by_id' do
       expect(vulnerability.dismissed_by_id).to be_nil

ee/spec/migrations/migrate_vulnerability_dismissal_feedback_spec.rb

@@ -15,7 +15,7 @@ describe MigrateVulnerabilityDismissalFeedback, :migration, :sidekiq do
 
   let(:vulnerabilities) { table(:vulnerabilities) }
 
-  let(:dismissed_state) {  Gitlab::BackgroundMigration::UpdateVulnerabilitiesFromDismissalFeedback::VULNERABILITY_DISMISSED }
+  let(:dismissed_state) { Gitlab::BackgroundMigration::UpdateVulnerabilitiesFromDismissalFeedback::VULNERABILITY_DISMISSED }
   let(:severity) { Vulnerabilities::Occurrence::SEVERITY_LEVELS[:unknown] }
   let(:confidence) { Vulnerabilities::Occurrence::CONFIDENCE_LEVELS[:medium] }
   let(:report_type) { Vulnerabilities::Occurrence::REPORT_TYPES[:sast] }