Address reviewer comments

- Modify specs to exclude time data
- Use label ID instead of epoch
- Remove duplicate test

db/migrate/20200305082754_remove_duplicate_labels_from_project.rb

@@ -9,9 +9,6 @@ class RemoveDuplicateLabelsFromProject < ActiveRecord::Migration[6.0]
   disable_ddl_transaction!
 
   class BackupLabel < Label
-    include BulkInsertSafe
-    include EachBatch
-
     self.table_name = 'backup_labels'
   end
 
@@ -85,7 +82,7 @@ WHERE labels.id IN (#{duplicate_labels.map { |dup| dup["id"] }.join(", ")});
 WITH data AS (
   SELECT
      *,
-     title || '_' || 'duplicate' || extract(epoch from now()) AS new_title,
+     title || '_' || 'duplicate' || id AS new_title,
      #{RENAME} AS restore_action,
      row_number() OVER (PARTITION BY project_id, title ORDER BY id) AS row_number
   FROM labels

db/migrate/20200305082858_add_uniqueness_index_to_label_title_and_project.rb

@@ -10,12 +10,12 @@ class AddUniquenessIndexToLabelTitleAndProject < ActiveRecord::Migration[6.0]
   PROJECT_AND_TITLE = [:project_id, :title]
 
   def up
-    remove_concurrent_index :labels, PROJECT_AND_TITLE if index_exists? :labels, PROJECT_AND_TITLE
-    add_concurrent_index :labels, PROJECT_AND_TITLE, where: "labels.group_id = null", unique: true
+    add_concurrent_index :labels, PROJECT_AND_TITLE, where: "labels.group_id IS NULL", unique: true, name: "index_labels_on_project_id_and_title_unique"
+    remove_concurrent_index :labels, PROJECT_AND_TITLE, name: "index_labels_on_project_id_and_title"
   end
 
   def down
-    remove_concurrent_index :labels, PROJECT_AND_TITLE if index_exists? :labels, PROJECT_AND_TITLE
-    add_concurrent_index :labels, PROJECT_AND_TITLE, where: "labels.group_id = null", unique: false
+    add_concurrent_index :labels, PROJECT_AND_TITLE, where: "labels.group_id IS NULL", unique: false, name: "index_labels_on_project_id_and_title"
+    remove_concurrent_index :labels, PROJECT_AND_TITLE, name: "index_labels_on_project_id_and_title_unique"
   end
 end

db/structure.sql

@@ -19398,7 +19398,7 @@ CREATE INDEX index_labels_on_group_id_and_title ON public.labels USING btree (gr
 
 CREATE INDEX index_labels_on_project_id ON public.labels USING btree (project_id);
 
-CREATE UNIQUE INDEX index_labels_on_project_id_and_title ON public.labels USING btree (project_id, title) WHERE (group_id = NULL::integer);
+CREATE UNIQUE INDEX index_labels_on_project_id_and_title_unique ON public.labels USING btree (project_id, title) WHERE (group_id IS NULL);
 
 CREATE INDEX index_labels_on_template ON public.labels USING btree (template) WHERE template;
 

spec/migrations/remove_duplicate_labels_from_project_spec.rb

@@ -92,15 +92,15 @@ describe RemoveDuplicateLabelsFromProject do
       end
 
       it 'restores removed records on rollback' do
-        second_label_attributes = second_label.attributes
-        fourth_label_attributes = fourth_label.attributes
+        second_label_attributes = modified_attributes(second_label)
+        fourth_label_attributes = modified_attributes(fourth_label)
 
         migration.up
 
         migration.down
 
-        expect(second_label.attributes).to match(second_label_attributes)
-        expect(fourth_label.attributes).to match(fourth_label_attributes)
+        expect(second_label.attributes).to include(second_label_attributes)
+        expect(fourth_label.attributes).to include(fourth_label_attributes)
       end
     end
 
@@ -154,12 +154,12 @@ describe RemoveDuplicateLabelsFromProject do
       end
 
       it 'restores removed records on rollback' do
-        third_label_attributes = third_label.attributes
+        third_label_attributes = modified_attributes(third_label)
 
         migration.up
         migration.down
 
-        expect(third_label.attributes).to match(third_label_attributes)
+        expect(third_label.attributes).to include(third_label_attributes)
       end
     end
   end
@@ -203,16 +203,20 @@ describe RemoveDuplicateLabelsFromProject do
       end
 
       it 'restores renamed records on rollback' do
-        second_label_attributes = second_label.attributes
-        fourth_label_attributes = fourth_label.attributes
+        second_label_attributes = modified_attributes(second_label)
+        fourth_label_attributes = modified_attributes(fourth_label)
 
         migration.up
 
         migration.down
 
-        expect(second_label.reload.attributes).to match(second_label_attributes)
-        expect(fourth_label.reload.attributes).to match(fourth_label_attributes)
+        expect(second_label.reload.attributes).to include(second_label_attributes)
+        expect(fourth_label.reload.attributes).to include(fourth_label_attributes)
       end
     end
   end
+
+  def modified_attributes(label)
+    label.attributes.except('created_at', 'updated_at')
+  end
 end