Commit 6857e1d0 authored by charlieablett's avatar charlieablett

Avoid title length overflows

- More terse deletion logic
parent 57adce0c
......@@ -70,19 +70,20 @@ WITH data AS (
# create backup records
BackupLabel.insert_all!(duplicate_labels.map { |label| label.except("row_number") })
ApplicationRecord.connection.execute(<<-SQL.squish)
DELETE FROM labels
WHERE labels.id IN (#{duplicate_labels.map { |dup| dup["id"] }.join(", ")});
SQL
Label.where(id: duplicate_labels.pluck("id")).delete_all
end
end
def rename_partial_duplicates(start_id, stop_id)
# We need to ensure that the new title (with `_duplicate#{ID}`) doesn't exceed the limit.
# Truncate the original title (if needed) to 245 characters minus the length of the ID
# then add `_duplicate#{ID}`
soft_duplicates = ApplicationRecord.connection.execute(<<-SQL.squish)
WITH data AS (
SELECT
*,
title || '_' || 'duplicate' || id AS new_title,
substring(title from 1 for 245 - length(id::text)) || '_duplicate' || id::text as new_title,
#{RENAME} AS restore_action,
row_number() OVER (PARTITION BY project_id, title ORDER BY id) AS row_number
FROM labels
......@@ -95,7 +96,7 @@ WITH data AS (
BackupLabel.insert_all!(soft_duplicates.map { |label| label.except("row_number") })
ApplicationRecord.connection.execute(<<-SQL.squish)
UPDATE labels SET title = title || '_' || 'duplicate' || extract(epoch from now())
UPDATE labels SET title = substring(title from 1 for 245 - length(id::text)) || '_duplicate' || id::text
WHERE labels.id IN (#{soft_duplicates.map { |dup| dup["id"] }.join(", ")});
SQL
end
......
......@@ -259,7 +259,7 @@ Ties are broken arbitrarily.
In specific circumstances it was possible to create labels with duplicate titles in the same
namespace.
To resolve the duplication, [in GitLab 13.0](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/21384)
To resolve the duplication, [in GitLab 13.1](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/21384)
and later, some duplicate labels have `_duplicate<number>` appended to their titles.
You can safely change these labels' titles if you prefer.
......
......@@ -198,8 +198,8 @@ describe RemoveDuplicateLabelsFromProject do
it 'modifies the titles of the partial duplicates' do
migration.up
expect(second_label.reload.title).to match(/#{label_title}_duplicate/)
expect(fourth_label.reload.title).to match(/#{other_title}_duplicate/)
expect(second_label.reload.title).to match(/#{label_title}_duplicate#{second_label.id}$/)
expect(fourth_label.reload.title).to match(/#{other_title}_duplicate#{fourth_label.id}$/)
end
it 'restores renamed records on rollback' do
......@@ -213,6 +213,22 @@ describe RemoveDuplicateLabelsFromProject do
expect(second_label.reload.attributes).to include(second_label_attributes)
expect(fourth_label.reload.attributes).to include(fourth_label_attributes)
end
context 'when the labels have a long title that might overflow' do
let(:long_title) { "a" * 255 }
before do
first_label.update_attribute(:title, long_title)
second_label.update_attribute(:title, long_title)
end
it 'keeps the length within the limit' do
migration.up
expect(second_label.reload.title).to eq("#{"a" * 244}_duplicate#{second_label.id}")
expect(second_label.title.length).to eq 255
end
end
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment