Skip to content

Commit b8f149d

Browse files
committed
Clarify that URL uniqueness applies to stored identifiers
GitLab payloads do carry an instance-global id, but it was never persisted; only the URL is stored on every record. Reword the migration note and mark gitlab_id as unsafe for lookups.
1 parent 08759be commit b8f149d

3 files changed

Lines changed: 10 additions & 5 deletions

File tree

db/migrate/20260608120000_add_unique_index_to_integration_html_urls.rb

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
# frozen_string_literal: true
22

3-
# The webhook URL is the only globally unique identifier for a pull request,
4-
# merge request or issue: per-project sequential numbers (GitLab's iid) repeat
5-
# across repositories. Enforce that uniqueness at the database level so a
6-
# missed lookup site or a concurrent webhook can never split one entity across
7-
# two rows. Surviving duplicates are folded into the most recent row first;
3+
# The webhook URL is the only globally unique identifier stored for a pull
4+
# request, merge request or issue: GitLab records hold only the per-project
5+
# iid, which repeats across repositories, and GitHub records created from
6+
# comment payloads carry no github_id at all. Enforce the URL's uniqueness at
7+
# the database level so a missed lookup site or a concurrent webhook can never
8+
# split one entity across two rows. Surviving duplicates are folded into the most recent row first;
89
# none are expected, since every write path already keys on the URL.
910
class AddUniqueIndexToIntegrationHtmlUrls < ActiveRecord::Migration[8.1]
1011
disable_ddl_transaction!

modules/gitlab_integration/app/models/gitlab_issue.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ class GitlabIssue < ApplicationRecord
5555
def self.find_by_gitlab_identifiers(url:, id: nil, initialize: false)
5656
raise ArgumentError, "needs an url" if url.blank?
5757

58+
# gitlab_id holds GitLab's per-project iid, which repeats across
59+
# repositories, so the URL is the only identifier safe to look up by.
5860
found = find_by(gitlab_html_url: url)
5961
found || (new(gitlab_id: id, gitlab_html_url: url) if initialize)
6062
end

modules/gitlab_integration/app/models/gitlab_merge_request.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ class GitlabMergeRequest < ApplicationRecord
5858
def self.find_by_gitlab_identifiers(url:, id: nil, initialize: false)
5959
raise ArgumentError, "needs an url" if url.blank?
6060

61+
# gitlab_id holds GitLab's per-project iid, which repeats across
62+
# repositories, so the URL is the only identifier safe to look up by.
6163
found = find_by(gitlab_html_url: url)
6264
found || (new(gitlab_id: id, gitlab_html_url: url) if initialize)
6365
end

0 commit comments

Comments
 (0)