From 8dc37c6bee8ed86a4c965d242fa99cf37404366a Mon Sep 17 00:00:00 2001 From: Niklas Date: Wed, 4 Sep 2024 19:51:29 +0000 Subject: [PATCH 1/4] Add merge_after attribute to MergeRequest Changelog: added --- app/graphql/types/merge_request_type.rb | 5 +++++ app/models/merge_request.rb | 2 ++ ...40829165121_add_merge_after_to_merge_requests.rb | 9 +++++++++ db/schema_migrations/20240829165121 | 1 + db/structure.sql | 1 + doc/api/graphql/reference/index.md | 1 + doc/api/merge_requests.md | 13 +++++++++++++ lib/api/entities/merge_request_basic.rb | 1 + spec/graphql/types/merge_request_type_spec.rb | 2 +- spec/lib/api/entities/merge_request_basic_spec.rb | 2 +- spec/models/merge_request_spec.rb | 9 +++++++++ 11 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20240829165121_add_merge_after_to_merge_requests.rb create mode 100644 db/schema_migrations/20240829165121 diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb index 8daef281b603aa..a18c474e4f2979 100644 --- a/app/graphql/types/merge_request_type.rb +++ b/app/graphql/types/merge_request_type.rb @@ -100,6 +100,11 @@ class MergeRequestType < BaseObject method: :public_merge_status, null: true, description: 'Merge status of the merge request.' + field :merge_after, ::Types::TimeType, + null: true, + description: 'Date after which the merge request can be merged.', + alpha: { milestone: '17.4' } + field :detailed_merge_status, ::Types::MergeRequests::DetailedMergeStatusEnum, null: true, calls_gitaly: true, description: 'Detailed merge status of the merge request.' diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 93ec0430866808..0c4dcc997b8cdc 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -488,6 +488,8 @@ def public_merge_status .merge(ResourceStateEvent.merged_with_no_event_source) } + scope :in_merge_time, -> { where(merge_after: ..Time.zone.now) } + def self.total_time_to_merge join_metrics .where( diff --git a/db/migrate/20240829165121_add_merge_after_to_merge_requests.rb b/db/migrate/20240829165121_add_merge_after_to_merge_requests.rb new file mode 100644 index 00000000000000..62215be5daf002 --- /dev/null +++ b/db/migrate/20240829165121_add_merge_after_to_merge_requests.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddMergeAfterToMergeRequests < Gitlab::Database::Migration[2.2] + milestone '17.4' + + def change + add_column :merge_requests, :merge_after, :datetime_with_timezone, default: nil + end +end diff --git a/db/schema_migrations/20240829165121 b/db/schema_migrations/20240829165121 new file mode 100644 index 00000000000000..d2827293142ebb --- /dev/null +++ b/db/schema_migrations/20240829165121 @@ -0,0 +1 @@ +6b436b203f748a008f7183984e0cf3f99aacab8b1d5bf24d9aee638b2eb770d8 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 54a815ca16c76e..d690fb19d50c86 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13263,6 +13263,7 @@ CREATE TABLE merge_requests ( head_pipeline_id bigint, imported_from smallint DEFAULT 0 NOT NULL, retargeted boolean DEFAULT false NOT NULL, + merge_after timestamp with time zone, CONSTRAINT check_970d272570 CHECK ((lock_version IS NOT NULL)) ); diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 779de53af3077c..02a1652ddf21cb 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -25494,6 +25494,7 @@ Defines which user roles, users, or groups can merge into a protected branch. | `iid` | [`String!`](#string) | Internal ID of the merge request. | | `inProgressMergeCommitSha` | [`String`](#string) | Commit SHA of the merge request if merge is in progress. | | `labels` | [`LabelConnection`](#labelconnection) | Labels of the merge request. (see [Connections](#connections)) | +| `mergeAfter` **{warning-solid}** | [`Time`](#time) | **Introduced** in GitLab 17.4. **Status**: Experiment. Date after which the merge request can be merged. | | `mergeCommitSha` | [`String`](#string) | SHA of the merge request commit (set once merged). | | `mergeError` | [`String`](#string) | Error message due to a merge error. | | `mergeOngoing` | [`Boolean!`](#boolean) | Indicates if a merge is currently occurring. | diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 9aa9bf6fe5d3de..c8f86761561c8b 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -17,6 +17,7 @@ DETAILS: > - `with_merge_status_recheck` [changed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/115948) in GitLab 15.11 [with a flag](../administration/feature_flags.md) named `restrict_merge_status_recheck` to be ignored for requests from users insufficient permissions. Disabled by default. > - `approvals_before_merge` [deprecated](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/119503) in GitLab 16.0. > - `prepared_at` [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/122001) in GitLab 16.1. +> - `merge_after` [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165092) in GitLab 17.4. All API calls to non-public information require authentication. @@ -118,6 +119,7 @@ Example response: "web_url": "https://gitlab.com/DouweM" }, "merged_at": "2018-09-07T11:16:17.520Z", + "merge_after": "2018-09-07T11:16:00.000Z", "prepared_at": "2018-09-04T11:16:17.520Z", "closed_by": null, "closed_at": null, @@ -367,6 +369,7 @@ Example response: "web_url": "https://gitlab.com/DouweM" }, "merged_at": "2018-09-07T11:16:17.520Z", + "merge_after": "2018-09-07T11:16:00.000Z", "prepared_at": "2018-09-04T11:16:17.520Z", "closed_by": null, "closed_at": null, @@ -553,6 +556,7 @@ Example response: "web_url": "https://gitlab.com/DouweM" }, "merged_at": "2018-09-07T11:16:17.520Z", + "merge_after": "2018-09-07T11:16:00.000Z", "prepared_at": "2018-09-04T11:16:17.520Z", "closed_by": null, "closed_at": null, @@ -747,6 +751,7 @@ Example response: "merged_by": null, // Deprecated and will be removed in API v5. Use `merge_user` instead. "merge_user": null, "merged_at": null, + "merge_after": "2018-09-07T11:16:00.000Z", "prepared_at": "2018-09-04T11:16:17.520Z", "closed_by": null, "closed_at": null, @@ -1134,6 +1139,7 @@ Example response: "merged_by": null, "merge_user": null, "merged_at": null, + "merge_after": "2018-09-07T11:16:00.000Z", "closed_by": null, "closed_at": null, "target_branch": "master", @@ -1243,6 +1249,7 @@ Example response: "merged_by": null, "merge_user": null, "merged_at": null, + "merge_after": "2018-09-07T11:16:00.000Z", "closed_by": null, "closed_at": null, "target_branch": "master", @@ -1787,6 +1794,7 @@ Example response: "web_url": "https://gitlab.com/DouweM" }, "merged_at": "2018-09-07T11:16:17.520Z", + "merge_after": "2018-09-07T11:16:00.000Z", "prepared_at": "2018-09-04T11:16:17.520Z", "closed_by": null, "closed_at": null, @@ -1960,6 +1968,7 @@ Example response: "web_url": "https://gitlab.com/DouweM" }, "merged_at": "2018-09-07T11:16:17.520Z", + "merge_after": "2018-09-07T11:16:00.000Z", "prepared_at": "2018-09-04T11:16:17.520Z", "closed_by": null, "closed_at": null, @@ -2153,6 +2162,7 @@ Example response: "web_url": "https://gitlab.com/DouweM" }, "merged_at": "2018-09-07T11:16:17.520Z", + "merge_after": "2018-09-07T11:16:00.000Z", "prepared_at": "2018-09-04T11:16:17.520Z", "closed_by": null, "closed_at": null, @@ -2354,6 +2364,7 @@ Example response: "web_url": "https://gitlab.com/DouweM" }, "merged_at": "2018-09-07T11:16:17.520Z", + "merge_after": "2018-09-07T11:16:00.000Z", "prepared_at": "2018-09-04T11:16:17.520Z", "closed_by": null, "closed_at": null, @@ -2742,6 +2753,7 @@ Example response: "web_url": "https://gitlab.com/DouweM" }, "merged_at": "2018-09-07T11:16:17.520Z", + "merge_after": "2018-09-07T11:16:00.000Z", "prepared_at": "2018-09-04T11:16:17.520Z", "closed_by": null, "closed_at": null, @@ -2906,6 +2918,7 @@ Example response: "web_url": "https://gitlab.com/DouweM" }, "merged_at": "2018-09-07T11:16:17.520Z", + "merge_after": "2018-09-07T11:16:00.000Z", "prepared_at": "2018-09-04T11:16:17.520Z", "closed_by": null, "closed_at": null, diff --git a/lib/api/entities/merge_request_basic.rb b/lib/api/entities/merge_request_basic.rb index fce85ff1c49b85..42a5fd2034ff59 100644 --- a/lib/api/entities/merge_request_basic.rb +++ b/lib/api/entities/merge_request_basic.rb @@ -64,6 +64,7 @@ class MergeRequestBasic < IssuableEntity merge_request.public_merge_status end expose :detailed_merge_status + expose :merge_after expose :diff_head_sha, as: :sha expose :merge_commit_sha expose :squash_commit_sha diff --git a/spec/graphql/types/merge_request_type_spec.rb b/spec/graphql/types/merge_request_type_spec.rb index ebd3cd5f4fceec..773565955f7d4c 100644 --- a/spec/graphql/types/merge_request_type_spec.rb +++ b/spec/graphql/types/merge_request_type_spec.rb @@ -37,7 +37,7 @@ squash_on_merge available_auto_merge_strategies has_ci mergeable commits committers commits_without_merge_commits squash security_auto_fix default_squash_commit_message auto_merge_strategy merge_user award_emoji prepared_at codequality_reports_comparer supports_lock_on_merge - mergeability_checks + mergeability_checks merge_after allows_multiple_assignees allows_multiple_reviewers retargeted name ] diff --git a/spec/lib/api/entities/merge_request_basic_spec.rb b/spec/lib/api/entities/merge_request_basic_spec.rb index 09f064c4cdd001..d0a056547f7b8f 100644 --- a/spec/lib/api/entities/merge_request_basic_spec.rb +++ b/spec/lib/api/entities/merge_request_basic_spec.rb @@ -20,7 +20,7 @@ def present(obj) expected_fields = %i[ merged_by merge_user merged_at closed_by closed_at target_branch user_notes_count upvotes downvotes author assignees assignee reviewers source_project_id target_project_id labels draft work_in_progress - milestone merge_when_pipeline_succeeds merge_status detailed_merge_status sha merge_commit_sha + milestone merge_when_pipeline_succeeds merge_status detailed_merge_status merge_after sha merge_commit_sha squash_commit_sha discussion_locked should_remove_source_branch force_remove_source_branch prepared_at reference references web_url time_stats squash task_completion_status has_conflicts blocking_discussions_resolved imported imported_from diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 8ef7220667569b..ff70e0f3c8d9f2 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -345,6 +345,15 @@ end end end + + describe '.in_merge_time' do + let_it_be(:mr_mergeable) { create(:merge_request, :unique_branches, merge_after: 1.hour.ago) } + let_it_be(:mr_non_mergeable) { create(:merge_request, :unique_branches, merge_after: 1.hour.from_now) } + + it 'filters by merge_after' do + expect(described_class.in_merge_time).to contain_exactly(mr_mergeable) + end + end end describe '#squash?' do -- GitLab From 737199c0e176b2842fc24d37560a95d56cc09c87 Mon Sep 17 00:00:00 2001 From: Niklas Date: Tue, 10 Sep 2024 10:35:55 +0000 Subject: [PATCH 2/4] Remove unused scope --- app/models/merge_request.rb | 2 -- spec/models/merge_request_spec.rb | 9 --------- 2 files changed, 11 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 0c4dcc997b8cdc..93ec0430866808 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -488,8 +488,6 @@ def public_merge_status .merge(ResourceStateEvent.merged_with_no_event_source) } - scope :in_merge_time, -> { where(merge_after: ..Time.zone.now) } - def self.total_time_to_merge join_metrics .where( diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index ff70e0f3c8d9f2..8ef7220667569b 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -345,15 +345,6 @@ end end end - - describe '.in_merge_time' do - let_it_be(:mr_mergeable) { create(:merge_request, :unique_branches, merge_after: 1.hour.ago) } - let_it_be(:mr_non_mergeable) { create(:merge_request, :unique_branches, merge_after: 1.hour.from_now) } - - it 'filters by merge_after' do - expect(described_class.in_merge_time).to contain_exactly(mr_mergeable) - end - end end describe '#squash?' do -- GitLab From f7f9472dbb9e856bb670da07823e5cffe31a7cec Mon Sep 17 00:00:00 2001 From: Niklas Date: Wed, 11 Sep 2024 19:51:14 +0000 Subject: [PATCH 3/4] Move merge_after to separate table --- .../concerns/resolves_merge_requests.rb | 1 + app/graphql/types/merge_request_type.rb | 4 +++ app/models/merge_request.rb | 3 +++ app/models/merge_request/merge_schedule.rb | 7 +++++ db/docs/merge_request_merge_schedules.yml | 10 +++++++ ...65121_add_merge_after_to_merge_requests.rb | 9 ------- ...54_create_merge_request_merge_schedules.rb | 14 ++++++++++ db/schema_migrations/20240829165121 | 1 - db/schema_migrations/20240911181854 | 1 + db/structure.sql | 26 ++++++++++++++++++- lib/api/entities/merge_request_basic.rb | 6 ++++- .../factories/merge_request_merge_schedule.rb | 7 +++++ .../merge_request/merge_schedule_spec.rb | 11 ++++++++ spec/models/merge_request_spec.rb | 1 + 14 files changed, 89 insertions(+), 12 deletions(-) create mode 100644 app/models/merge_request/merge_schedule.rb create mode 100644 db/docs/merge_request_merge_schedules.yml delete mode 100644 db/migrate/20240829165121_add_merge_after_to_merge_requests.rb create mode 100644 db/migrate/20240911181854_create_merge_request_merge_schedules.rb delete mode 100644 db/schema_migrations/20240829165121 create mode 100644 db/schema_migrations/20240911181854 create mode 100644 spec/factories/merge_request_merge_schedule.rb create mode 100644 spec/models/merge_request/merge_schedule_spec.rb diff --git a/app/graphql/resolvers/concerns/resolves_merge_requests.rb b/app/graphql/resolvers/concerns/resolves_merge_requests.rb index d356e1685722e3..7e21e3617f4d99 100644 --- a/app/graphql/resolvers/concerns/resolves_merge_requests.rb +++ b/app/graphql/resolvers/concerns/resolves_merge_requests.rb @@ -61,6 +61,7 @@ def preloads commit_count: [:metrics], diff_stats_summary: [:metrics], approved_by: [:approved_by_users], + merge_after: [:merge_schedule], milestone: [:milestone], security_auto_fix: [:author], head_pipeline: [:merge_request_diff, { head_pipeline: [:merge_request] }], diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb index a18c474e4f2979..e261b55300f069 100644 --- a/app/graphql/types/merge_request_type.rb +++ b/app/graphql/types/merge_request_type.rb @@ -352,6 +352,10 @@ def merge_user object.metrics&.merged_by || object.merge_user end + def merge_after + object.merge_schedule&.merge_after + end + def detailed_merge_status ::MergeRequests::Mergeability::DetailedMergeStatusService.new(merge_request: object).execute end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 93ec0430866808..40044859c0548c 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -66,6 +66,8 @@ class MergeRequest < ApplicationRecord has_one :predictions, inverse_of: :merge_request delegate :suggested_reviewers, to: :predictions + has_one :merge_schedule, class_name: 'MergeRequest::MergeSchedule', inverse_of: :merge_request + belongs_to :latest_merge_request_diff, class_name: 'MergeRequestDiff' manual_inverse_association :latest_merge_request_diff, :merge_request @@ -378,6 +380,7 @@ def public_merge_status preload_routables.preload( :assignees, :author, :unresolved_notes, :labels, :milestone, :timelogs, :latest_merge_request_diff, :reviewers, + :merge_schedule, target_project: :project_feature, metrics: [:latest_closed_by, :merged_by] ) diff --git a/app/models/merge_request/merge_schedule.rb b/app/models/merge_request/merge_schedule.rb new file mode 100644 index 00000000000000..f468f1e4138a5f --- /dev/null +++ b/app/models/merge_request/merge_schedule.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +# rubocop:disable Gitlab/BoundedContexts -- following existing pattern +class MergeRequest::MergeSchedule < ApplicationRecord # rubocop:disable Style/ClassAndModuleChildren -- following existing pattern + belongs_to :merge_request, optional: false +end +# rubocop:enable Gitlab/BoundedContexts diff --git a/db/docs/merge_request_merge_schedules.yml b/db/docs/merge_request_merge_schedules.yml new file mode 100644 index 00000000000000..0f2472a104cecc --- /dev/null +++ b/db/docs/merge_request_merge_schedules.yml @@ -0,0 +1,10 @@ +--- +table_name: merge_request_merge_schedules +classes: +- MergeRequest::MergeSchedule +feature_categories: +- code_review_workflow +description: Stores timestamps for scheduled merges +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165092 +milestone: '17.4' +gitlab_schema: gitlab_main diff --git a/db/migrate/20240829165121_add_merge_after_to_merge_requests.rb b/db/migrate/20240829165121_add_merge_after_to_merge_requests.rb deleted file mode 100644 index 62215be5daf002..00000000000000 --- a/db/migrate/20240829165121_add_merge_after_to_merge_requests.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -class AddMergeAfterToMergeRequests < Gitlab::Database::Migration[2.2] - milestone '17.4' - - def change - add_column :merge_requests, :merge_after, :datetime_with_timezone, default: nil - end -end diff --git a/db/migrate/20240911181854_create_merge_request_merge_schedules.rb b/db/migrate/20240911181854_create_merge_request_merge_schedules.rb new file mode 100644 index 00000000000000..970901331a62ab --- /dev/null +++ b/db/migrate/20240911181854_create_merge_request_merge_schedules.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class CreateMergeRequestMergeSchedules < Gitlab::Database::Migration[2.2] + milestone '17.4' + + def change + create_table :merge_request_merge_schedules do |t| # rubocop:disable Migration/EnsureFactoryForTable -- factory exists in spec/factories/merge_request_merge_schedule.rb + t.references :merge_request, foreign_key: { on_delete: :cascade }, index: false, null: false + t.datetime_with_timezone :merge_after + + t.index :merge_request_id, unique: true + end + end +end diff --git a/db/schema_migrations/20240829165121 b/db/schema_migrations/20240829165121 deleted file mode 100644 index d2827293142ebb..00000000000000 --- a/db/schema_migrations/20240829165121 +++ /dev/null @@ -1 +0,0 @@ -6b436b203f748a008f7183984e0cf3f99aacab8b1d5bf24d9aee638b2eb770d8 \ No newline at end of file diff --git a/db/schema_migrations/20240911181854 b/db/schema_migrations/20240911181854 new file mode 100644 index 00000000000000..75e2bbe9721c9a --- /dev/null +++ b/db/schema_migrations/20240911181854 @@ -0,0 +1 @@ +6dd9484b5c7d61943dae2ed684c8d84d0c0319e009c26d1bcbfcbe0ae69939e7 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index d690fb19d50c86..adb87ec0256084 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13105,6 +13105,21 @@ CREATE SEQUENCE merge_request_diffs_id_seq ALTER SEQUENCE merge_request_diffs_id_seq OWNED BY merge_request_diffs.id; +CREATE TABLE merge_request_merge_schedules ( + id bigint NOT NULL, + merge_request_id bigint NOT NULL, + merge_after timestamp with time zone +); + +CREATE SEQUENCE merge_request_merge_schedules_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE merge_request_merge_schedules_id_seq OWNED BY merge_request_merge_schedules.id; + CREATE TABLE merge_request_metrics ( merge_request_id bigint NOT NULL, latest_build_started_at timestamp without time zone, @@ -13263,7 +13278,6 @@ CREATE TABLE merge_requests ( head_pipeline_id bigint, imported_from smallint DEFAULT 0 NOT NULL, retargeted boolean DEFAULT false NOT NULL, - merge_after timestamp with time zone, CONSTRAINT check_970d272570 CHECK ((lock_version IS NOT NULL)) ); @@ -21873,6 +21887,8 @@ ALTER TABLE ONLY merge_request_diff_details ALTER COLUMN merge_request_diff_id S ALTER TABLE ONLY merge_request_diffs ALTER COLUMN id SET DEFAULT nextval('merge_request_diffs_id_seq'::regclass); +ALTER TABLE ONLY merge_request_merge_schedules ALTER COLUMN id SET DEFAULT nextval('merge_request_merge_schedules_id_seq'::regclass); + ALTER TABLE ONLY merge_request_metrics ALTER COLUMN id SET DEFAULT nextval('merge_request_metrics_id_seq'::regclass); ALTER TABLE ONLY merge_request_predictions ALTER COLUMN merge_request_id SET DEFAULT nextval('merge_request_predictions_merge_request_id_seq'::regclass); @@ -24214,6 +24230,9 @@ ALTER TABLE ONLY merge_request_diff_files ALTER TABLE ONLY merge_request_diffs ADD CONSTRAINT merge_request_diffs_pkey PRIMARY KEY (id); +ALTER TABLE ONLY merge_request_merge_schedules + ADD CONSTRAINT merge_request_merge_schedules_pkey PRIMARY KEY (id); + ALTER TABLE ONLY merge_request_metrics ADD CONSTRAINT merge_request_metrics_pkey PRIMARY KEY (id); @@ -28901,6 +28920,8 @@ CREATE INDEX index_merge_request_diffs_on_project_id ON merge_request_diffs USIN CREATE UNIQUE INDEX index_merge_request_diffs_on_unique_merge_request_id ON merge_request_diffs USING btree (merge_request_id) WHERE (diff_type = 2); +CREATE UNIQUE INDEX index_merge_request_merge_schedules_on_merge_request_id ON merge_request_merge_schedules USING btree (merge_request_id); + CREATE INDEX index_merge_request_metrics_on_first_deployed_to_production_at ON merge_request_metrics USING btree (first_deployed_to_production_at); CREATE INDEX index_merge_request_metrics_on_latest_closed_at ON merge_request_metrics USING btree (latest_closed_at) WHERE (latest_closed_at IS NOT NULL); @@ -35215,6 +35236,9 @@ ALTER TABLE zoekt_tasks ALTER TABLE ONLY ml_models ADD CONSTRAINT fk_rails_51e87f7c50 FOREIGN KEY (project_id) REFERENCES projects(id); +ALTER TABLE ONLY merge_request_merge_schedules + ADD CONSTRAINT fk_rails_5294434bc3 FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE; + ALTER TABLE ONLY elastic_group_index_statuses ADD CONSTRAINT fk_rails_52b9969b12 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; diff --git a/lib/api/entities/merge_request_basic.rb b/lib/api/entities/merge_request_basic.rb index 42a5fd2034ff59..4348d292eb4b1a 100644 --- a/lib/api/entities/merge_request_basic.rb +++ b/lib/api/entities/merge_request_basic.rb @@ -64,7 +64,11 @@ class MergeRequestBasic < IssuableEntity merge_request.public_merge_status end expose :detailed_merge_status - expose :merge_after + + expose :merge_after do |merge_request, _options| + merge_request.merge_schedule&.merge_after + end + expose :diff_head_sha, as: :sha expose :merge_commit_sha expose :squash_commit_sha diff --git a/spec/factories/merge_request_merge_schedule.rb b/spec/factories/merge_request_merge_schedule.rb new file mode 100644 index 00000000000000..cdeafb8e09e5b4 --- /dev/null +++ b/spec/factories/merge_request_merge_schedule.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :merge_request_merge_schedule, class: 'MergeRequest::MergeSchedule' do + merge_request + end +end diff --git a/spec/models/merge_request/merge_schedule_spec.rb b/spec/models/merge_request/merge_schedule_spec.rb new file mode 100644 index 00000000000000..7d8a1ffee5cddb --- /dev/null +++ b/spec/models/merge_request/merge_schedule_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequest::MergeSchedule, feature_category: :code_review_workflow do + subject { create(:merge_request_merge_schedule) } + + describe 'associations' do + it { is_expected.to belong_to(:merge_request).required } + end +end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 8ef7220667569b..5a0ec84ecfce79 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -36,6 +36,7 @@ it { is_expected.to have_many(:reviews).inverse_of(:merge_request) } it { is_expected.to have_many(:reviewed_by_users).through(:reviews).source(:author) } it { is_expected.to have_one(:cleanup_schedule).inverse_of(:merge_request) } + it { is_expected.to have_one(:merge_schedule).class_name('MergeRequest::MergeSchedule').inverse_of(:merge_request) } it { is_expected.to have_many(:created_environments).class_name('Environment').inverse_of(:merge_request) } it { is_expected.to have_many(:assignment_events).class_name('ResourceEvents::MergeRequestAssignmentEvent').inverse_of(:merge_request) } -- GitLab From 1fa346b3686bbb105ccf1c3b24cf15dc982c98e2 Mon Sep 17 00:00:00 2001 From: Niklas Date: Thu, 12 Sep 2024 20:12:47 +0000 Subject: [PATCH 4/4] Set sharding key for merge schedules --- app/models/merge_request.rb | 2 +- app/models/merge_request/merge_schedule.rb | 7 ------ app/models/merge_requests/merge_schedule.rb | 15 ++++++++++++ db/docs/merge_request_merge_schedules.yml | 6 +++-- ...54_create_merge_request_merge_schedules.rb | 3 +++ ...request_merge_schedules_sharding_key_fk.rb | 17 ++++++++++++++ db/schema_migrations/20240911181855 | 1 + db/structure.sql | 8 ++++++- .../import_export/base/relation_factory.rb | 2 +- .../factories/merge_request_merge_schedule.rb | 2 +- spec/lib/gitlab/import_export/all_models.yml | 3 +++ .../merge_request/merge_schedule_spec.rb | 11 --------- spec/models/merge_request_spec.rb | 2 +- .../merge_requests/merge_schedule_spec.rb | 23 +++++++++++++++++++ 14 files changed, 77 insertions(+), 25 deletions(-) delete mode 100644 app/models/merge_request/merge_schedule.rb create mode 100644 app/models/merge_requests/merge_schedule.rb create mode 100644 db/migrate/20240911181855_create_merge_request_merge_schedules_sharding_key_fk.rb create mode 100644 db/schema_migrations/20240911181855 delete mode 100644 spec/models/merge_request/merge_schedule_spec.rb create mode 100644 spec/models/merge_requests/merge_schedule_spec.rb diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 40044859c0548c..699af3ec43cd25 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -66,7 +66,7 @@ class MergeRequest < ApplicationRecord has_one :predictions, inverse_of: :merge_request delegate :suggested_reviewers, to: :predictions - has_one :merge_schedule, class_name: 'MergeRequest::MergeSchedule', inverse_of: :merge_request + has_one :merge_schedule, class_name: 'MergeRequests::MergeSchedule', inverse_of: :merge_request belongs_to :latest_merge_request_diff, class_name: 'MergeRequestDiff' manual_inverse_association :latest_merge_request_diff, :merge_request diff --git a/app/models/merge_request/merge_schedule.rb b/app/models/merge_request/merge_schedule.rb deleted file mode 100644 index f468f1e4138a5f..00000000000000 --- a/app/models/merge_request/merge_schedule.rb +++ /dev/null @@ -1,7 +0,0 @@ -# frozen_string_literal: true - -# rubocop:disable Gitlab/BoundedContexts -- following existing pattern -class MergeRequest::MergeSchedule < ApplicationRecord # rubocop:disable Style/ClassAndModuleChildren -- following existing pattern - belongs_to :merge_request, optional: false -end -# rubocop:enable Gitlab/BoundedContexts diff --git a/app/models/merge_requests/merge_schedule.rb b/app/models/merge_requests/merge_schedule.rb new file mode 100644 index 00000000000000..5749768b5a10d8 --- /dev/null +++ b/app/models/merge_requests/merge_schedule.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module MergeRequests + class MergeSchedule < ApplicationRecord + self.table_name = 'merge_request_merge_schedules' + + belongs_to :merge_request, optional: false, inverse_of: :merge_schedule + + before_validation :set_sharding_key + + def set_sharding_key + self.project_id = merge_request&.target_project&.id + end + end +end diff --git a/db/docs/merge_request_merge_schedules.yml b/db/docs/merge_request_merge_schedules.yml index 0f2472a104cecc..7923501b06b70f 100644 --- a/db/docs/merge_request_merge_schedules.yml +++ b/db/docs/merge_request_merge_schedules.yml @@ -1,10 +1,12 @@ --- table_name: merge_request_merge_schedules classes: -- MergeRequest::MergeSchedule +- MergeRequests::MergeSchedule feature_categories: - code_review_workflow description: Stores timestamps for scheduled merges introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165092 milestone: '17.4' -gitlab_schema: gitlab_main +gitlab_schema: gitlab_main_cell +sharding_key: + project_id: projects diff --git a/db/migrate/20240911181854_create_merge_request_merge_schedules.rb b/db/migrate/20240911181854_create_merge_request_merge_schedules.rb index 970901331a62ab..2c694ceead3ec7 100644 --- a/db/migrate/20240911181854_create_merge_request_merge_schedules.rb +++ b/db/migrate/20240911181854_create_merge_request_merge_schedules.rb @@ -8,7 +8,10 @@ def change t.references :merge_request, foreign_key: { on_delete: :cascade }, index: false, null: false t.datetime_with_timezone :merge_after + t.bigint :project_id, null: false + t.index :merge_request_id, unique: true + t.index :project_id end end end diff --git a/db/migrate/20240911181855_create_merge_request_merge_schedules_sharding_key_fk.rb b/db/migrate/20240911181855_create_merge_request_merge_schedules_sharding_key_fk.rb new file mode 100644 index 00000000000000..143248ee3c2562 --- /dev/null +++ b/db/migrate/20240911181855_create_merge_request_merge_schedules_sharding_key_fk.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class CreateMergeRequestMergeSchedulesShardingKeyFk < Gitlab::Database::Migration[2.2] + milestone '17.4' + + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_request_merge_schedules, :projects, column: :project_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :merge_request_merge_schedules, column: :project_id + end + end +end diff --git a/db/schema_migrations/20240911181855 b/db/schema_migrations/20240911181855 new file mode 100644 index 00000000000000..b3cf9c49b4c858 --- /dev/null +++ b/db/schema_migrations/20240911181855 @@ -0,0 +1 @@ +4e56708b920c40ba9c6050f2441f313715e3db134ad6114fb9091829506736c1 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index adb87ec0256084..d9b5da79c31949 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13108,7 +13108,8 @@ ALTER SEQUENCE merge_request_diffs_id_seq OWNED BY merge_request_diffs.id; CREATE TABLE merge_request_merge_schedules ( id bigint NOT NULL, merge_request_id bigint NOT NULL, - merge_after timestamp with time zone + merge_after timestamp with time zone, + project_id bigint NOT NULL ); CREATE SEQUENCE merge_request_merge_schedules_id_seq @@ -28922,6 +28923,8 @@ CREATE UNIQUE INDEX index_merge_request_diffs_on_unique_merge_request_id ON merg CREATE UNIQUE INDEX index_merge_request_merge_schedules_on_merge_request_id ON merge_request_merge_schedules USING btree (merge_request_id); +CREATE INDEX index_merge_request_merge_schedules_on_project_id ON merge_request_merge_schedules USING btree (project_id); + CREATE INDEX index_merge_request_metrics_on_first_deployed_to_production_at ON merge_request_metrics USING btree (first_deployed_to_production_at); CREATE INDEX index_merge_request_metrics_on_latest_closed_at ON merge_request_metrics USING btree (latest_closed_at) WHERE (latest_closed_at IS NOT NULL); @@ -34030,6 +34033,9 @@ ALTER TABLE ONLY operations_strategies ALTER TABLE ONLY lfs_objects_projects ADD CONSTRAINT fk_a56e02279c FOREIGN KEY (lfs_object_id) REFERENCES lfs_objects(id) ON DELETE RESTRICT NOT VALID; +ALTER TABLE ONLY merge_request_merge_schedules + ADD CONSTRAINT fk_a5ff9339a9 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; + ALTER TABLE ONLY merge_requests ADD CONSTRAINT fk_a6963e8447 FOREIGN KEY (target_project_id) REFERENCES projects(id) ON DELETE CASCADE; diff --git a/lib/gitlab/import_export/base/relation_factory.rb b/lib/gitlab/import_export/base/relation_factory.rb index a48d282307416a..952aef5625548f 100644 --- a/lib/gitlab/import_export/base/relation_factory.rb +++ b/lib/gitlab/import_export/base/relation_factory.rb @@ -9,7 +9,7 @@ class RelationFactory IMPORTED_OBJECT_MAX_RETRIES = 5 - OVERRIDES = { user_contributions: :user }.freeze + OVERRIDES = { user_contributions: :user, merge_schedule: 'MergeRequests::MergeSchedule' }.freeze EXISTING_OBJECT_RELATIONS = %i[].freeze # This represents all relations that have unique key on `project_id` or `group_id` diff --git a/spec/factories/merge_request_merge_schedule.rb b/spec/factories/merge_request_merge_schedule.rb index cdeafb8e09e5b4..2be331efe15912 100644 --- a/spec/factories/merge_request_merge_schedule.rb +++ b/spec/factories/merge_request_merge_schedule.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true FactoryBot.define do - factory :merge_request_merge_schedule, class: 'MergeRequest::MergeSchedule' do + factory :merge_request_merge_schedule, class: 'MergeRequests::MergeSchedule' do merge_request end end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index baa29e4e3ce681..246feda51d5324 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -227,6 +227,7 @@ merge_requests: - merge_request_context_commits - merge_request_context_commit_diff_files - merge_request_stage_events +- merge_schedule - events - merge_requests_closing_issues - cached_closes_issues @@ -293,6 +294,8 @@ merge_request_diff_files: merge_request_context_commits: - merge_request - diff_files +merge_schedule: +- merge_request cleanup_schedule: - merge_request ci_pipelines: diff --git a/spec/models/merge_request/merge_schedule_spec.rb b/spec/models/merge_request/merge_schedule_spec.rb deleted file mode 100644 index 7d8a1ffee5cddb..00000000000000 --- a/spec/models/merge_request/merge_schedule_spec.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe MergeRequest::MergeSchedule, feature_category: :code_review_workflow do - subject { create(:merge_request_merge_schedule) } - - describe 'associations' do - it { is_expected.to belong_to(:merge_request).required } - end -end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 5a0ec84ecfce79..2fab1f07887caf 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -36,7 +36,7 @@ it { is_expected.to have_many(:reviews).inverse_of(:merge_request) } it { is_expected.to have_many(:reviewed_by_users).through(:reviews).source(:author) } it { is_expected.to have_one(:cleanup_schedule).inverse_of(:merge_request) } - it { is_expected.to have_one(:merge_schedule).class_name('MergeRequest::MergeSchedule').inverse_of(:merge_request) } + it { is_expected.to have_one(:merge_schedule).class_name('MergeRequests::MergeSchedule').inverse_of(:merge_request) } it { is_expected.to have_many(:created_environments).class_name('Environment').inverse_of(:merge_request) } it { is_expected.to have_many(:assignment_events).class_name('ResourceEvents::MergeRequestAssignmentEvent').inverse_of(:merge_request) } diff --git a/spec/models/merge_requests/merge_schedule_spec.rb b/spec/models/merge_requests/merge_schedule_spec.rb new file mode 100644 index 00000000000000..a869032f118cb3 --- /dev/null +++ b/spec/models/merge_requests/merge_schedule_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequests::MergeSchedule, feature_category: :code_review_workflow do + subject { create(:merge_request_merge_schedule) } + + describe 'associations' do + it { is_expected.to belong_to(:merge_request).required } + end + + describe 'callbacks' do + let(:merge_request) { create(:merge_request) } + let(:schedule) do + create(:merge_request_merge_schedule, merge_request: merge_request, + project_id: merge_request.target_project.id + 1) + end + + it 'overrides project_id to the correct sharding key' do + expect(schedule.project_id).to eq(merge_request.target_project.id) + end + end +end -- GitLab