Commit f6f49861 authored by Patrick Derichs's avatar Patrick Derichs

Add resource weight tracking

Revert schema changes

Apply schema changes

Add feature flag test

Add foreign keys

Remove explicit key removal
parent 1f6f0606
# frozen_string_literal: true
class ResourceWeightEvent < ApplicationRecord
validates :user, presence: true
validates :issue, presence: true
belongs_to :user
belongs_to :issue
scope :by_issue, ->(issue) { where(issue_id: issue.id) }
end
# frozen_string_literal: true
class CreateResourceWeightEvent < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
create_table :resource_weight_events do |t|
t.integer :user_id, null: false
t.integer :issue_id, null: false
t.integer :weight
t.datetime_with_timezone :created_at, null: false
t.index [:user_id], name: 'index_resource_weight_events_on_user_id'
t.index [:issue_id, :weight], name: 'index_resource_weight_events_on_issue_id_and_weight'
end
add_concurrent_foreign_key :resource_weight_events, :users, column: :user_id, on_delete: :cascade
add_concurrent_foreign_key :resource_weight_events, :issues, column: :issue_id, on_delete: :cascade
end
def down
drop_table :resource_weight_events
end
end
...@@ -3605,6 +3605,15 @@ ActiveRecord::Schema.define(version: 2019_12_18_225624) do ...@@ -3605,6 +3605,15 @@ ActiveRecord::Schema.define(version: 2019_12_18_225624) do
t.index ["user_id"], name: "index_resource_label_events_on_user_id" t.index ["user_id"], name: "index_resource_label_events_on_user_id"
end end
create_table "resource_weight_events", force: :cascade do |t|
t.integer "user_id", null: false
t.integer "issue_id", null: false
t.integer "weight"
t.datetime_with_timezone "created_at", null: false
t.index ["issue_id", "weight"], name: "index_resource_weight_events_on_issue_id_and_weight"
t.index ["user_id"], name: "index_resource_weight_events_on_user_id"
end
create_table "reviews", force: :cascade do |t| create_table "reviews", force: :cascade do |t|
t.integer "author_id" t.integer "author_id"
t.integer "merge_request_id", null: false t.integer "merge_request_id", null: false
...@@ -4745,6 +4754,8 @@ ActiveRecord::Schema.define(version: 2019_12_18_225624) do ...@@ -4745,6 +4754,8 @@ ActiveRecord::Schema.define(version: 2019_12_18_225624) do
add_foreign_key "resource_label_events", "labels", on_delete: :nullify add_foreign_key "resource_label_events", "labels", on_delete: :nullify
add_foreign_key "resource_label_events", "merge_requests", on_delete: :cascade add_foreign_key "resource_label_events", "merge_requests", on_delete: :cascade
add_foreign_key "resource_label_events", "users", on_delete: :nullify add_foreign_key "resource_label_events", "users", on_delete: :nullify
add_foreign_key "resource_weight_events", "issues", name: "fk_5eb5cb92a1", on_delete: :cascade
add_foreign_key "resource_weight_events", "users", name: "fk_bfc406b47c", on_delete: :cascade
add_foreign_key "reviews", "merge_requests", on_delete: :cascade add_foreign_key "reviews", "merge_requests", on_delete: :cascade
add_foreign_key "reviews", "projects", on_delete: :cascade add_foreign_key "reviews", "projects", on_delete: :cascade
add_foreign_key "reviews", "users", column: "author_id", on_delete: :nullify add_foreign_key "reviews", "users", column: "author_id", on_delete: :nullify
......
# frozen_string_literal: true
module EE
module WeightEventable
extend ActiveSupport::Concern
included do
has_many :resource_weight_events
end
end
end
...@@ -13,6 +13,7 @@ module EE ...@@ -13,6 +13,7 @@ module EE
include Elastic::ApplicationVersionedSearch include Elastic::ApplicationVersionedSearch
include UsageStatistics include UsageStatistics
include WeightEventable
scope :order_weight_desc, -> { reorder ::Gitlab::Database.nulls_last_order('weight', 'DESC') } scope :order_weight_desc, -> { reorder ::Gitlab::Database.nulls_last_order('weight', 'DESC') }
scope :order_weight_asc, -> { reorder ::Gitlab::Database.nulls_last_order('weight') } scope :order_weight_asc, -> { reorder ::Gitlab::Database.nulls_last_order('weight') }
......
...@@ -30,13 +30,25 @@ module EE ...@@ -30,13 +30,25 @@ module EE
def handle_weight_change_note def handle_weight_change_note
if issuable.previous_changes.include?('weight') if issuable.previous_changes.include?('weight')
create_weight_change_note note = create_weight_change_note
track_weight_change(note)
end end
end end
def create_weight_change_note def create_weight_change_note
::SystemNoteService.change_weight_note(issuable, issuable.project, current_user) ::SystemNoteService.change_weight_note(issuable, issuable.project, current_user)
end end
def track_weight_change(note)
return unless weight_changes_tracking_enabled?
EE::ResourceEvents::ChangeWeightService.new(issuable, current_user, note.created_at).execute
end
def weight_changes_tracking_enabled?
::Feature.enabled?(:track_resource_weight_change_events, issuable.project)
end
end end
end end
end end
# frozen_string_literal: true
module EE
module ResourceEvents
class ChangeWeightService
attr_reader :resource, :user, :event_created_at
def initialize(resource, user, created_at)
@resource = resource
@user = user
@event_created_at = created_at
end
def execute
create_event_by_issue if first_weight_event?
ResourceWeightEvent
.new(user: user, issue: resource, weight: resource.weight, created_at: event_created_at)
.save
end
private
def first_weight_event?
ResourceWeightEvent.by_issue(resource).none?
end
def create_event_by_issue
ResourceWeightEvent
.new(user: user, issue: resource, weight: resource.weight, created_at: resource.updated_at)
.save
end
end
end
end
---
title: Track resource weight changes
merge_request: 21515
author:
type: added
...@@ -116,6 +116,7 @@ describe Issue do ...@@ -116,6 +116,7 @@ describe Issue do
it { is_expected.to have_many(:vulnerability_links).class_name('Vulnerabilities::IssueLink').inverse_of(:issue) } it { is_expected.to have_many(:vulnerability_links).class_name('Vulnerabilities::IssueLink').inverse_of(:issue) }
it { is_expected.to have_many(:related_vulnerabilities).through(:vulnerability_links).source(:vulnerability) } it { is_expected.to have_many(:related_vulnerabilities).through(:vulnerability_links).source(:vulnerability) }
it { is_expected.to belong_to(:promoted_to_epic).class_name('Epic') } it { is_expected.to belong_to(:promoted_to_epic).class_name('Epic') }
it { is_expected.to have_many(:resource_weight_events) }
describe 'versions.most_recent' do describe 'versions.most_recent' do
it 'returns the most recent version' do it 'returns the most recent version' do
......
...@@ -35,12 +35,40 @@ describe Issuable::CommonSystemNotesService do ...@@ -35,12 +35,40 @@ describe Issuable::CommonSystemNotesService do
subject { described_class.new(project, user).execute(issuable, old_labels: [], is_update: false) } subject { described_class.new(project, user).execute(issuable, old_labels: [], is_update: false) }
it 'creates a system note for weight' do before do
issuable.weight = 5 issuable.weight = 5
issuable.save issuable.save
end
it 'creates a system note for weight' do
expect { subject }.to change { issuable.notes.count }.from(0).to(1) expect { subject }.to change { issuable.notes.count }.from(0).to(1)
expect(issuable.notes.last.note).to match('changed weight') expect(issuable.notes.last.note).to match('changed weight')
end end
context 'when resource weight event tracking is enabled' do
before do
stub_feature_flags(track_resource_weight_change_events: true)
end
it 'creates a resource weight event' do
subject
note = issuable.notes.last
event = issuable.resource_weight_events.last
expect(event.weight).to eq(5)
expect(event.created_at).to eq(note.created_at)
end
end
context 'when resource weight event tracking is disabled' do
before do
stub_feature_flags(track_resource_weight_change_events: false)
end
it 'does not created a resource weight event' do
expect { subject }.not_to change { ResourceWeightEvent.count }
end
end
end end
end end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe EE::ResourceEvents::ChangeWeightService do
let_it_be(:user) { create(:user) }
let(:issue) { create(:issue, weight: 3) }
let(:created_at_time) { Time.new(2019, 1, 1, 12, 30, 48) }
subject { described_class.new(issue, user, created_at_time).execute }
before do
ResourceWeightEvent.new(issue: issue, user: user).save!
end
it 'creates the expected event record' do
expect { subject }.to change { ResourceWeightEvent.count }.by(1)
record = ResourceWeightEvent.last
expect_event_record(record, weight: 3, created_at: created_at_time)
end
context 'when weight is nil' do
let(:issue) { create(:issue, weight: nil) }
it 'creates an event record' do
expect { subject }.to change { ResourceWeightEvent.count }.by(1)
record = ResourceWeightEvent.last
expect_event_record(record, weight: nil, created_at: created_at_time)
end
end
context 'when there is no existing weight event record' do
before do
ResourceWeightEvent.delete_all
end
it 'creates the expected event records' do
expect { subject }.to change { ResourceWeightEvent.count }.by(2)
record = ResourceWeightEvent.first
expect_event_record(record, weight: 3, created_at: issue.reload.updated_at)
record = ResourceWeightEvent.last
expect_event_record(record, weight: 3, created_at: created_at_time)
end
end
def expect_event_record(record, weight:, created_at:)
expect(record.issue).to eq(issue)
expect(record.user).to eq(user)
expect(record.weight).to eq(weight)
expect(record.created_at).to eq(created_at)
end
end
# frozen_string_literal: true
FactoryBot.define do
factory :resource_weight_event do
issue { create(:issue) }
user { issue&.author || create(:user) }
end
end
...@@ -8,6 +8,7 @@ issues: ...@@ -8,6 +8,7 @@ issues:
- milestone - milestone
- notes - notes
- resource_label_events - resource_label_events
- resource_weight_events
- sentry_issue - sentry_issue
- label_links - label_links
- labels - labels
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe ResourceWeightEvent, type: :model do
describe 'validations' do
it { is_expected.not_to allow_value(nil).for(:user) }
it { is_expected.not_to allow_value(nil).for(:issue) }
it { is_expected.to allow_value(nil).for(:weight) }
end
describe 'associations' do
it { is_expected.to belong_to(:user) }
it { is_expected.to belong_to(:issue) }
end
describe '.by_issue' do
let_it_be(:user1) { create(:user) }
let_it_be(:user2) { create(:user) }
let_it_be(:issue1) { create(:issue, author: user1) }
let_it_be(:issue2) { create(:issue, author: user1) }
let_it_be(:issue3) { create(:issue, author: user2) }
let_it_be(:event1) { create(:resource_weight_event, issue: issue1) }
let_it_be(:event2) { create(:resource_weight_event, issue: issue2) }
let_it_be(:event3) { create(:resource_weight_event, issue: issue1) }
it 'returns the expected records for an issue with events' do
events = ResourceWeightEvent.by_issue(issue1)
expect(events).to contain_exactly(event1, event3)
end
it 'returns the expected records for an issue with no events' do
events = ResourceWeightEvent.by_issue(issue3)
expect(events).to be_empty
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment