Skip to content

Save verification details conditionally after saving a replicable

Aakriti Gupta requested to merge ag-remove-autosave into master

What does this MR do and why?

Closes #341122 (closed)

We want to store verification details only for replicables that qualify for verification, e.g. for MR diffs only externally stored diffs are verified. This will help us speed up the queries used for gathering records for verification.

So, instead of autosaving child records, we check for the same filters used in available_verifiables and save the child object.

Please note: this MR is dependent on !69301 (merged) being merged. I will be rebasing it on master, once !69301 (merged) is merged.

Screenshots or screen recordings

These are strongly recommended to assist reviewers and reduce the time to merge your change.

How to set up and validate locally

  1. Test if conditional autosaving works
  • with used filters (files stored externally)
    • create MR, and MR diff, check if the MR diffs detail object is created
MergeRequest.new(target_branch: "master", source_branch: "dev-1", target_project_id: Project.first.id, source_project_id: Project.first.id, author_id: User.first.id, title: "New MR").save!

mr = MergeRequest.last

MergeRequestDiff.new(stored_externally: true, state: :collected, merge_request_id: mr.id).save!
diff = MergeRequestDiff.last

MergeRequestDiffDetail.where(merge_request_diff_id: diff.id).count
  • without filters
    • create MR
    • set stored_externally to false
    • check if MR diff detail object skipped?
MergeRequest.new(target_branch: "master", source_branch: "dev-2", target_project_id: Project.first.id, source_project_id: Project.first.id, author_id: User.first.id, title: "New MR").save!

mr = MergeRequest.last

MergeRequestDiff.new(stored_externally: false, merge_request_id: mr.id).save!
diff = MergeRequestDiff.last

MergeRequestDiffDetail.where(merge_request_diff_id: diff.id).count
  1. (Optional because not in scope) Does MR verification work?
    • use the new MRs created in the previous step, make sure stored_externally is true for some and they are included in with_files scope. You can find these with MergeRequestDiff.available_verifiables or you can update the current records with MergeRequestDiff.with_files.update_all(stored_externally: true) to fit the filters.
    • does the new MR diff detail object get updated with verification details?
    • is the bar on /admin/geo/nodes green for MR diffs for primary?

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Aakriti Gupta

Merge request reports