Commit 5c6b2c4b authored by Lee Tickett's avatar Lee Tickett

Add CustomerRelations::IssueContact model

We need to remove usage of HasAndBelongsToMany. This MR changes
to use the preferred has_many through and introduces the joining
model.

Changelog: added
parent 44e31918
...@@ -7,7 +7,8 @@ class CustomerRelations::Contact < ApplicationRecord ...@@ -7,7 +7,8 @@ class CustomerRelations::Contact < ApplicationRecord
belongs_to :group, -> { where(type: Group.sti_name) }, foreign_key: 'group_id' belongs_to :group, -> { where(type: Group.sti_name) }, foreign_key: 'group_id'
belongs_to :organization, optional: true belongs_to :organization, optional: true
has_and_belongs_to_many :issues, join_table: :issue_customer_relations_contacts # rubocop: disable Rails/HasAndBelongsToMany has_many :issue_contacts, inverse_of: :contact
has_many :issues, through: :issue_contacts, inverse_of: :customer_relations_contacts
strip_attributes! :phone, :first_name, :last_name strip_attributes! :phone, :first_name, :last_name
......
# frozen_string_literal: true
class CustomerRelations::IssueContact < ApplicationRecord
self.table_name = "issue_customer_relations_contacts"
belongs_to :issue, optional: false, inverse_of: :customer_relations_contacts
belongs_to :contact, optional: false, inverse_of: :issue_contacts
validate :contact_belongs_to_issue_group
private
def contact_belongs_to_issue_group
return unless contact&.group_id
return unless issue&.project&.namespace_id
return if contact.group_id == issue.project.namespace_id
errors.add(:base, _('The contact does not belong to the same group as the issue.'))
end
end
...@@ -81,7 +81,8 @@ class Issue < ApplicationRecord ...@@ -81,7 +81,8 @@ class Issue < ApplicationRecord
has_and_belongs_to_many :self_managed_prometheus_alert_events, join_table: :issues_self_managed_prometheus_alert_events # rubocop: disable Rails/HasAndBelongsToMany has_and_belongs_to_many :self_managed_prometheus_alert_events, join_table: :issues_self_managed_prometheus_alert_events # rubocop: disable Rails/HasAndBelongsToMany
has_and_belongs_to_many :prometheus_alert_events, join_table: :issues_prometheus_alert_events # rubocop: disable Rails/HasAndBelongsToMany has_and_belongs_to_many :prometheus_alert_events, join_table: :issues_prometheus_alert_events # rubocop: disable Rails/HasAndBelongsToMany
has_many :prometheus_alerts, through: :prometheus_alert_events has_many :prometheus_alerts, through: :prometheus_alert_events
has_and_belongs_to_many :customer_relations_contacts, join_table: :issue_customer_relations_contacts, class_name: 'CustomerRelations::Contact' # rubocop: disable Rails/HasAndBelongsToMany has_many :issue_customer_relations_contacts, class_name: 'CustomerRelations::IssueContact', inverse_of: :issue
has_many :customer_relations_contacts, through: :issue_customer_relations_contacts, source: :contact, class_name: 'CustomerRelations::Contact', inverse_of: :issues
accepts_nested_attributes_for :issuable_severity, update_only: true accepts_nested_attributes_for :issuable_severity, update_only: true
accepts_nested_attributes_for :sentry_issue accepts_nested_attributes_for :sentry_issue
......
...@@ -33890,6 +33890,9 @@ msgstr "" ...@@ -33890,6 +33890,9 @@ msgstr ""
msgid "The connection will time out after %{timeout}. For repositories that take longer, use a clone/push combination." msgid "The connection will time out after %{timeout}. For repositories that take longer, use a clone/push combination."
msgstr "" msgstr ""
msgid "The contact does not belong to the same group as the issue."
msgstr ""
msgid "The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository." msgid "The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository."
msgstr "" msgstr ""
......
# frozen_string_literal: true
FactoryBot.define do
factory :issue_customer_relations_contact, class: 'CustomerRelations::IssueContact' do
issue { association(:issue, project: project) }
contact { association(:contact, group: group) }
transient do
group { association(:group) }
project { association(:project, group: group) }
end
trait :for_contact do
issue { association(:issue, project: project) }
contact { raise ArgumentError, '`contact` is manadatory' }
transient do
project { association(:project, group: contact.group) }
end
end
trait :for_issue do
issue { raise ArgumentError, '`issue` is manadatory' }
contact { association(:contact, group: issue.project.group) }
end
end
end
...@@ -22,6 +22,8 @@ RSpec.describe 'factories' do ...@@ -22,6 +22,8 @@ RSpec.describe 'factories' do
[:debian_project_component_file, :object_storage], [:debian_project_component_file, :object_storage],
[:debian_project_distribution, :object_storage], [:debian_project_distribution, :object_storage],
[:debian_file_metadatum, :unknown], [:debian_file_metadatum, :unknown],
[:issue_customer_relations_contact, :for_contact],
[:issue_customer_relations_contact, :for_issue],
[:package_file, :object_storage], [:package_file, :object_storage],
[:pages_domain, :without_certificate], [:pages_domain, :without_certificate],
[:pages_domain, :without_key], [:pages_domain, :without_key],
...@@ -72,6 +74,7 @@ RSpec.describe 'factories' do ...@@ -72,6 +74,7 @@ RSpec.describe 'factories' do
fork_network_member fork_network_member
group_member group_member
import_state import_state
issue_customer_relations_contact
milestone_release milestone_release
namespace namespace
project_broken_repo project_broken_repo
......
...@@ -60,6 +60,7 @@ issues: ...@@ -60,6 +60,7 @@ issues:
- incident_management_issuable_escalation_status - incident_management_issuable_escalation_status
- pending_escalations - pending_escalations
- customer_relations_contacts - customer_relations_contacts
- issue_customer_relations_contacts
work_item_type: work_item_type:
- issues - issues
events: events:
......
...@@ -6,7 +6,8 @@ RSpec.describe CustomerRelations::Contact, type: :model do ...@@ -6,7 +6,8 @@ RSpec.describe CustomerRelations::Contact, type: :model do
describe 'associations' do describe 'associations' do
it { is_expected.to belong_to(:group) } it { is_expected.to belong_to(:group) }
it { is_expected.to belong_to(:organization).optional } it { is_expected.to belong_to(:organization).optional }
it { is_expected.to have_and_belong_to_many(:issues) } it { is_expected.to have_many(:issue_contacts) }
it { is_expected.to have_many(:issues) }
end end
describe 'validations' do describe 'validations' do
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe CustomerRelations::IssueContact do
let_it_be(:issue_contact, reload: true) { create(:issue_customer_relations_contact) }
subject { issue_contact }
it { expect(subject).to be_valid }
describe 'associations' do
it { is_expected.to belong_to(:issue).required }
it { is_expected.to belong_to(:contact).required }
end
describe 'factory' do
let(:built) { build(:issue_customer_relations_contact) }
let(:stubbed) { build_stubbed(:issue_customer_relations_contact) }
let(:created) { create(:issue_customer_relations_contact) }
let(:group) { build(:group) }
let(:project) { build(:project, group: group) }
let(:issue) { build(:issue, project: project) }
let(:contact) { build(:contact, group: group) }
let(:for_issue) { build(:issue_customer_relations_contact, :for_issue, issue: issue) }
let(:for_contact) { build(:issue_customer_relations_contact, :for_contact, contact: contact) }
it 'uses objects from the same group', :aggregate_failures do
expect(stubbed.contact.group).to eq(stubbed.issue.project.group)
expect(built.contact.group).to eq(built.issue.project.group)
expect(created.contact.group).to eq(created.issue.project.group)
end
it 'builds using the same group', :aggregate_failures do
expect(for_issue.contact.group).to eq(group)
expect(for_contact.issue.project.group).to eq(group)
end
end
describe 'validation' do
let(:built) { build(:issue_customer_relations_contact, issue: create(:issue), contact: create(:contact)) }
it 'fails when the contact group does not match the issue group' do
expect(built).not_to be_valid
end
end
end
...@@ -34,7 +34,8 @@ RSpec.describe Issue do ...@@ -34,7 +34,8 @@ RSpec.describe Issue do
it { is_expected.to have_many(:issue_email_participants) } it { is_expected.to have_many(:issue_email_participants) }
it { is_expected.to have_many(:timelogs).autosave(true) } it { is_expected.to have_many(:timelogs).autosave(true) }
it { is_expected.to have_one(:incident_management_issuable_escalation_status) } it { is_expected.to have_one(:incident_management_issuable_escalation_status) }
it { is_expected.to have_and_belong_to_many(:customer_relations_contacts) } it { is_expected.to have_many(:issue_customer_relations_contacts) }
it { is_expected.to have_many(:customer_relations_contacts) }
describe 'versions.most_recent' do describe 'versions.most_recent' do
it 'returns the most recent version' do it 'returns the most recent version' do
......
...@@ -429,11 +429,11 @@ RSpec.describe 'getting an issue list for a project' do ...@@ -429,11 +429,11 @@ RSpec.describe 'getting an issue list for a project' do
end end
it 'avoids N+1 queries' do it 'avoids N+1 queries' do
create(:contact, group_id: group.id, issues: [issue_a]) create(:issue_customer_relations_contact, :for_issue, issue: issue_a)
control = ActiveRecord::QueryRecorder.new(skip_cached: false) { clean_state_query } control = ActiveRecord::QueryRecorder.new(skip_cached: false) { clean_state_query }
create(:contact, group_id: group.id, issues: [issue_a]) create(:issue_customer_relations_contact, :for_issue, issue: issue_a)
expect { clean_state_query }.not_to exceed_all_query_limit(control) expect { clean_state_query }.not_to exceed_all_query_limit(control)
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