From 20c28fd47b972d86f5d5475ee8bbea0949339564 Mon Sep 17 00:00:00 2001 From: Michael Becker <11881043-wandering_person@users.noreply.gitlab.com> Date: Thu, 16 Nov 2023 12:24:23 +0700 Subject: [PATCH] Ignore `last_edited_by_id` column on `vulnerabilities` table `last_edited_by_id` is always `nil` in production: ```sh [ gstg ] production> Vulnerability.where.not(last_edited_by_id: nil).count => 0 [ gstg ] production> ``` It was added [when the initial vulnerability table][1] was created, however appears to have never been used. This MR ignores the column for step 1 of the [3-MR drop column process][0] related to: https://gitlab.com/gitlab-org/gitlab/-/issues/268154 Changelog: deprecated [0]:https://docs.gitlab.com/ee/development/database/avoiding_downtime_in_migrations.html#dropping-columns [1]:https://gitlab.com/gitlab-org/gitlab/-/commit/8ad1881cc83fa970bf69103e2fcf1ea4175230ff --- app/models/vulnerability.rb | 3 ++- doc/api/project_vulnerabilities.md | 3 +-- doc/api/vulnerabilities.md | 6 +----- ee/app/models/ee/vulnerability.rb | 1 - ee/lib/ee/api/entities/vulnerability.rb | 2 +- ee/spec/lib/ee/api/entities/vulnerability_spec.rb | 11 ++++++++++- ee/spec/models/ee/vulnerability_spec.rb | 15 ++++++++++++++- 7 files changed, 29 insertions(+), 12 deletions(-) diff --git a/app/models/vulnerability.rb b/app/models/vulnerability.rb index 89a234b86af814..da607dcb7718cc 100644 --- a/app/models/vulnerability.rb +++ b/app/models/vulnerability.rb @@ -5,7 +5,8 @@ class Vulnerability < ApplicationRecord include EachBatch include IgnorableColumns - ignore_column %i[epic_id milestone_id last_edited_at start_date start_date_sourcing_milestone_id updated_by_id], + ignore_column %i[epic_id milestone_id last_edited_at last_edited_by_id + start_date start_date_sourcing_milestone_id updated_by_id], remove_with: '16.9', remove_after: '2024-01-19' diff --git a/doc/api/project_vulnerabilities.md b/doc/api/project_vulnerabilities.md index e1b0dedc0b9535..c3f03d0a073c36 100644 --- a/doc/api/project_vulnerabilities.md +++ b/doc/api/project_vulnerabilities.md @@ -11,6 +11,7 @@ type: reference, api > - `last_edited_at` [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/268154) in GitLab 16.7. > - `start_date` [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/268154) in GitLab 16.7. > - `updated_by_id` [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/268154) in GitLab 16.7. +> - `last_edited_by_id` [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/268154) in GitLab 16.7. WARNING: This API is in the process of being deprecated and considered unstable. @@ -82,7 +83,6 @@ Example response: "vulnerability_id": 103 }, "id": 103, - "last_edited_by_id": null, "project": { "created_at": "2020-04-07T13:54:25.634Z", "description": "", @@ -167,7 +167,6 @@ Example response: "vulnerability_id": 103 }, "id": 103, - "last_edited_by_id": null, "project": { "created_at": "2020-04-07T13:54:25.634Z", "description": "", diff --git a/doc/api/vulnerabilities.md b/doc/api/vulnerabilities.md index ed5638915e0d47..4820eb89e0d988 100644 --- a/doc/api/vulnerabilities.md +++ b/doc/api/vulnerabilities.md @@ -10,6 +10,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w > - `last_edited_at` [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/268154) in GitLab 16.7. > - `start_date` [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/268154) in GitLab 16.7. > - `updated_by_id` [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/268154) in GitLab 16.7. +> - `last_edited_by_id` [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/268154) in GitLab 16.7. NOTE: The former Vulnerabilities API was renamed to Vulnerability Findings API @@ -63,7 +64,6 @@ Example response: "full_name": "gitlab-examples / security / security-reports" }, "author_id": 1, - "last_edited_by_id": null, "closed_by_id": null, "due_date": null, "created_at": "2019-10-13T15:08:40.219Z", @@ -110,7 +110,6 @@ Example response: "full_name": "gitlab-examples / security / security-reports" }, "author_id": 1, - "last_edited_by_id": null, "closed_by_id": null, "due_date": null, "created_at": "2019-10-13T15:08:40.219Z", @@ -157,7 +156,6 @@ Example response: "full_name": "gitlab-examples / security / security-reports" }, "author_id": 1, - "last_edited_by_id": null, "closed_by_id": null, "due_date": null, "created_at": "2019-10-13T15:08:40.219Z", @@ -204,7 +202,6 @@ Example response: "full_name": "gitlab-examples / security / security-reports" }, "author_id": 1, - "last_edited_by_id": null, "closed_by_id": null, "due_date": null, "created_at": "2019-10-13T15:08:40.219Z", @@ -251,7 +248,6 @@ Example response: "full_name": "gitlab-examples / security / security-reports" }, "author_id": 1, - "last_edited_by_id": null, "closed_by_id": null, "due_date": null, "created_at": "2019-10-13T15:08:40.219Z", diff --git a/ee/app/models/ee/vulnerability.rb b/ee/app/models/ee/vulnerability.rb index 072f3ce6706ee4..f6ddaecc67889a 100644 --- a/ee/app/models/ee/vulnerability.rb +++ b/ee/app/models/ee/vulnerability.rb @@ -36,7 +36,6 @@ module Vulnerability belongs_to :project # keep this association named 'project' for correct work of markdown cache belongs_to :author, class_name: 'User' # keep this association named 'author' for correct work of markdown cache - belongs_to :last_edited_by, class_name: 'User' belongs_to :resolved_by, class_name: 'User' belongs_to :dismissed_by, class_name: 'User' belongs_to :confirmed_by, class_name: 'User' diff --git a/ee/lib/ee/api/entities/vulnerability.rb b/ee/lib/ee/api/entities/vulnerability.rb index dcfcde36b0ebc2..752741fa4d4efd 100644 --- a/ee/lib/ee/api/entities/vulnerability.rb +++ b/ee/lib/ee/api/entities/vulnerability.rb @@ -20,7 +20,6 @@ class Vulnerability < Grape::Entity expose :project_default_branch expose :author_id - expose :last_edited_by_id expose :resolved_by_id expose :dismissed_by_id expose :confirmed_by_id @@ -38,6 +37,7 @@ class Vulnerability < Grape::Entity expose(:last_edited_at) { |_settings, _options| nil } expose(:start_date) { |_settings, _options| nil } expose(:updated_by_id) { |_settings, _options| nil } + expose(:last_edited_by_id) { |_settings, _options| nil } end end end diff --git a/ee/spec/lib/ee/api/entities/vulnerability_spec.rb b/ee/spec/lib/ee/api/entities/vulnerability_spec.rb index 2785a8b4adae54..826272f9e65b20 100644 --- a/ee/spec/lib/ee/api/entities/vulnerability_spec.rb +++ b/ee/spec/lib/ee/api/entities/vulnerability_spec.rb @@ -30,7 +30,6 @@ expect(subject[:project_default_branch]).to eq(vulnerability.project_default_branch) expect(subject[:author_id]).to eq(vulnerability.author_id) - expect(subject[:last_edited_by_id]).to eq(vulnerability.last_edited_by_id) expect(subject[:resolved_by_id]).to eq(vulnerability.resolved_by_id) expect(subject[:dismissed_by_id]).to eq(vulnerability.dismissed_by_id) expect(subject[:confirmed_by_id]).to eq(vulnerability.confirmed_by_id) @@ -42,5 +41,15 @@ expect(subject[:resolved_at]).to eq(vulnerability.resolved_at) expect(subject[:dismissed_at]).to eq(vulnerability.dismissed_at) expect(subject[:confirmed_at]).to eq(vulnerability.confirmed_at) + + # These fields are deprecated and always returns nil + # TODO remove with https://gitlab.com/gitlab-org/gitlab/-/work_items/431990 + deprecated_fields = subject.to_h.slice(*%i[last_edited_at start_date updated_by_id last_edited_by_id]) + expect(deprecated_fields).to eq( + last_edited_at: nil, + start_date: nil, + updated_by_id: nil, + last_edited_by_id: nil + ) end end diff --git a/ee/spec/models/ee/vulnerability_spec.rb b/ee/spec/models/ee/vulnerability_spec.rb index d3c8a18756d828..9628a8698aa072 100644 --- a/ee/spec/models/ee/vulnerability_spec.rb +++ b/ee/spec/models/ee/vulnerability_spec.rb @@ -57,7 +57,6 @@ it { is_expected.to have_many(:related_issues).through(:issue_links).source(:issue) } it { is_expected.to have_many(:state_transitions).class_name('Vulnerabilities::StateTransition').inverse_of(:vulnerability) } it { is_expected.to belong_to(:author).class_name('User') } - it { is_expected.to belong_to(:last_edited_by).class_name('User') } it { is_expected.to belong_to(:resolved_by).class_name('User') } it { is_expected.to belong_to(:dismissed_by).class_name('User') } it { is_expected.to belong_to(:confirmed_by).class_name('User') } @@ -1049,4 +1048,18 @@ let(:record) { create(:vulnerability, :sast, :confirmed, :low, project: project) } let(:update_params) { { state: 'dismissed' } } end + + context 'when accessing ignored columns' do + # These fields are deprecated and always returns nil + # TODO remove with https://gitlab.com/gitlab-org/gitlab/-/work_items/431990 + it 'raises a NoMethodError' do + expect { vulnerability.epic }.to raise_error(NoMethodError) + expect { vulnerability.milestone }.to raise_error(NoMethodError) + expect { vulnerability.last_edited_at }.to raise_error(NoMethodError) + expect { vulnerability.last_edited_by }.to raise_error(NoMethodError) + expect { vulnerability.start_date }.to raise_error(NoMethodError) + expect { vulnerability.start_date_sourcing_milestone }.to raise_error(NoMethodError) + expect { vulnerability.updated_by }.to raise_error(NoMethodError) + end + end end -- GitLab