What does this MR do and why?

PoC read and write from new commits table

This is a PoC to evaluate changes needed to support only writing to the new table merge_request_diff_commits_b5377a7a34 before we drop the old table.

References

Screenshots or screen recordings

How to set up and validate locally

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Eugenia Grieff (7e537b46) at 18 Mar 15:24

... and 2 more commits

@marc_shaw Thanks, LGTM 👍

Summary

• Adds descriptive help text and a "How does automatic rebase work?" documentation link for the "Enable automatic rebase prior to merge" checkbox in project merge request settings.
• Uses safe_format + tag_pair for i18n-safe interpolation (following the modern pattern used in e.g. _pages.html.haml).
• Replaces the previous icon-only help link with an inline text link per Pajamas contextual help guidelines.

Related issues

Relates to #524048

What does this MR do?

Adds a help_text with an inline docs link to the automatic rebase checkbox in Settings > Merge requests > Merge options, so users can quickly learn what the setting does and access the documentation.

The help text reads:

Automatically rebases the source branch onto the target branch before merging. How does automatic rebase work?

@lauraXD @garyh Would you mind taking the backend and Merge Requests backend reviews please? Thanks!

Eugenia Grieff (3c1c2b31) at 18 Mar 09:57

Eugenia Grieff (7da13cd2) at 18 Mar 09:57

... and 1 more commit

What does this MR do and why?

Renames Gitlab::MergeRequests::DiffVersion to Gitlab::MergeRequests::DiffResolver and moves the compare early-return logic from the three rapid diffs methods in MergeRequest (first_diffs_slice, diffs_for_streaming, diffs_for_streaming_by_changed_paths) into DiffResolver#resolve.

This keeps the MR model methods as slim one-liner delegations and centralizes diff resolution logic in a single class, per the discussion with @patrickbajao.

References

• Closes #591242
• Related: !220188, !222414

What does this MR do and why?

Renames Gitlab::MergeRequests::DiffVersion to Gitlab::MergeRequests::DiffResolver and moves the compare early-return logic from the three rapid diffs methods in MergeRequest (first_diffs_slice, diffs_for_streaming, diffs_for_streaming_by_changed_paths) into DiffResolver#resolve.

This keeps the MR model methods as slim one-liner delegations and centralizes diff resolution logic in a single class, per the discussion with @patrickbajao.

References

• Closes #591242
• Related: !220188, !222414

Eugenia Grieff (6e2b5756) at 17 Mar 16:55

Eugenia Grieff (b05b21c7) at 17 Mar 16:55

... and 1 more commit

What does this MR do and why?

This MR updates the actor for all mr_reports_tab FF reference to be project instead of current_user.

There is no UI/UX changes, everything continues to work expected:

mr_reports_tab true	mr_reports_tab false

mr_reports_tab true

• Has "Reports" tab
• Display "View reports"
• Widget is not expandable

mr_reports_tab false

• No "Reports" tab
• Widget is expandable

References

Screenshots or screen recordings

How to set up and validate locally

1. Clone this repo and create a MR similar to this: gitlab-org/govern/threat-insights-demos/frontend/mr-reports-tab!1
2. Trigger a pipeline and it will create some findings

Enable and disable FF:

http://gdk.test:3000/rails/features/mr_reports_tab

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #592500

@sming-gitlab Sure, LGTM 👍

What does this MR do and why?

This MR updates the actor for all mr_reports_tab FF reference to be project instead of current_user.

There is no UI/UX changes, everything continues to work expected:

mr_reports_tab true	mr_reports_tab false

mr_reports_tab true

• Has "Reports" tab
• Display "View reports"
• Widget is not expandable

mr_reports_tab false

• No "Reports" tab
• Widget is expandable

References

Screenshots or screen recordings

How to set up and validate locally

1. Clone this repo and create a MR similar to this: gitlab-org/govern/threat-insights-demos/frontend/mr-reports-tab!1
2. Trigger a pipeline and it will create some findings

Enable and disable FF:

http://gdk.test:3000/rails/features/mr_reports_tab

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #592500

Eugenia Grieff (1d708855) at 17 Mar 11:43

The approach in !226904 (closed) didn't help in the end 😞 I added more details in #592062 (comment 3166114140)

Regardless of that, I wonder if this could have been caught by a spec / integration test.

I think the challenge is not having data with a size large enough to trigger these edge cases. I've seen cases where AR preloading caused problems that were caught only after deployment, so probably there is something to improve.

@lauraXD @manuelgrabowski Unfortunately, the approach in !227632 didn't work because preloading users is still needed. Without it, CachedCommit#to_hash in MergeRequestDiff#load_commits calls every column on the MergeRequestDiffCommit record, triggering individual SQL loads for commit and author fields (all delegated to merge_request_commits_metadata via has_commit_metadata?).

Batching the preloading (attempted in !226904 (closed)) doesn't work either because load_commits collects everything into the CommitCollection array before returning, so we still hold the same peak number of objects.

However, I found a possible solution to the problem affecting this worker: use an existing SQL query that fetches the commit SHAs and build the CommitRequest objects from them (I created !227632 to try this).

I can assign it to @lauraXD for review today, or let me know if I should pick someone else instead 🙇

What does this MR do and why?

Optimise merge_request_commits_metadata preloading for MR diff commits

Introduce Preloaders::MergeRequestDiffCommitsMetadataPreloader to batch-load merge_request_commits_metadata records by ID, avoiding the scoped belongs_to lambda overhead and memory spikes when loading all metadata in a single query for large diffs.

The preloader supports a with_users: flag that mirrors the existing with_users association extension, preloading :commit_author and :committer on metadata records, and directly on commits that lack a metadata record (fallback for un-backfilled rows).

References

Related to #592902

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

What does this MR do and why?

Optimise calls to MergeRequestDiff#commits and MergeRequest#commits so callers that don't need author/committer data can opt out of the with_users preload.

The following list of callers is updated (behind FF skip_merge_request_commits_user_preload) to skip preloading commit users' data:

Note: As far as I can tell, all other calls either explicitly need user records (committers for approval rules, audit logs) or use load_from_gitaly: true (which bypasses with_users entirely, so preload_users doesn't apply). I'll investigate further if there is some obscure path that I missed, but I think it's best to keep this MR scoped to as few changes as possible to make validation easier.

References

Related to #592902

#592062 (comment 3138432832)

How to set up and validate locally

1. Enable the feature flag in Rails console:

Feature.enable(:skip_merge_request_commits_user_preload)

2. Test the four affected code paths:

a) MergeRequest#changed_paths (rules:changes):

• Create/use an MR with CI rules that use rules:changes
• Verify the pipeline triggers correctly based on changed files
• Check that only commit SHAs are loaded (no author/committer data)

b) ContextCommitsFinder (SHA deduplication):

• Open an MR and try adding context commits (previously merged commits) in the commits tab
• Search for commits by SHA
• Verify search works and doesn't duplicate existing commits

c) Issues::CloseService (timestamp only):

• Create an issue and an MR that references it with "Closes #issue-number"
• Merge the MR
• Check that issue.metrics.first_mentioned_in_commit_at is set correctly

d) SystemNotes::IssuablesService (membership check):

• Reference a commit from within an MR
• Verify cross-reference notes are created/suppressed correctly

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.