Commit 7186a649 authored by Jan Provaznik's avatar Jan Provaznik Committed by Igor Drozdov

Move requirements into its own module

Because there will be multiple resources needed for requirements
it's better to keep them all together in a separate module.

requirements table is not renamed because renaming this would cause
downtime and deployment issues.
parent 9d870883
...@@ -26,7 +26,7 @@ class RequirementsFinder ...@@ -26,7 +26,7 @@ class RequirementsFinder
attr_reader :current_user, :params attr_reader :current_user, :params
def init_collection def init_collection
return Requirement.none unless Ability.allowed?(current_user, :read_requirement, project) return RequirementsManagement::Requirement.none unless Ability.allowed?(current_user, :read_requirement, project)
project.requirements project.requirements
end end
...@@ -50,7 +50,7 @@ class RequirementsFinder ...@@ -50,7 +50,7 @@ class RequirementsFinder
end end
def sort(items) def sort(items)
sorts = Requirement.simple_sorts.keys sorts = RequirementsManagement::Requirement.simple_sorts.keys
sort = sorts.include?(params[:sort]) ? params[:sort] : 'id_desc' sort = sorts.include?(params[:sort]) ? params[:sort] : 'id_desc'
items.order_by(sort) items.order_by(sort)
......
...@@ -25,8 +25,8 @@ module Resolvers ...@@ -25,8 +25,8 @@ module Resolvers
# At this point we need the `id` of the project to query for issues, so # At this point we need the `id` of the project to query for issues, so
# make sure it's loaded and not `nil` before continuing. # make sure it's loaded and not `nil` before continuing.
project = object.respond_to?(:sync) ? object.sync : object project = object.respond_to?(:sync) ? object.sync : object
return Requirement.none if project.nil? return RequirementsManagement::Requirement.none if project.nil?
return Requirement.none unless Feature.enabled?(:requirements_management, project, default_enabled: true) return RequirementsManagement::Requirement.none unless Feature.enabled?(:requirements_management, project, default_enabled: true)
args[:project_id] = project.id args[:project_id] = project.id
args[:iids] ||= [args[:iid]].compact args[:iids] ||= [args[:iid]].compact
......
...@@ -59,7 +59,7 @@ module EE ...@@ -59,7 +59,7 @@ module EE
has_many :audit_events, as: :entity has_many :audit_events, as: :entity
has_many :designs, inverse_of: :project, class_name: 'DesignManagement::Design' has_many :designs, inverse_of: :project, class_name: 'DesignManagement::Design'
has_many :path_locks has_many :path_locks
has_many :requirements has_many :requirements, inverse_of: :project, class_name: 'RequirementsManagement::Requirement'
# the rationale behind vulnerabilities and vulnerability_findings can be found here: # the rationale behind vulnerabilities and vulnerability_findings can be found here:
# https://gitlab.com/gitlab-org/gitlab/issues/10252#terminology # https://gitlab.com/gitlab-org/gitlab/issues/10252#terminology
......
...@@ -34,7 +34,7 @@ module EE ...@@ -34,7 +34,7 @@ module EE
has_many :reviews, foreign_key: :author_id, inverse_of: :author has_many :reviews, foreign_key: :author_id, inverse_of: :author
has_many :epics, foreign_key: :author_id has_many :epics, foreign_key: :author_id
has_many :requirements, foreign_key: :author_id has_many :requirements, foreign_key: :author_id, inverse_of: :author, class_name: 'RequirementsManagement::Requirement'
has_many :assigned_epics, foreign_key: :assignee_id, class_name: "Epic" has_many :assigned_epics, foreign_key: :assignee_id, class_name: "Epic"
has_many :path_locks, dependent: :destroy # rubocop: disable Cop/ActiveRecordDependent has_many :path_locks, dependent: :destroy # rubocop: disable Cop/ActiveRecordDependent
has_many :vulnerability_feedback, foreign_key: :author_id, class_name: 'Vulnerabilities::Feedback' has_many :vulnerability_feedback, foreign_key: :author_id, class_name: 'Vulnerabilities::Feedback'
......
# frozen_string_literal: true
class Requirement < ApplicationRecord
include CacheMarkdownField
include StripAttribute
include AtomicInternalId
include Sortable
cache_markdown_field :title, pipeline: :single_line
strip_attributes :title
belongs_to :author, class_name: 'User'
belongs_to :project
has_internal_id :iid, scope: :project, init: ->(s) { s&.project&.requirements&.maximum(:iid) }
validates :author, :project, :title, presence: true
validates :title, length: { maximum: Issuable::TITLE_LENGTH_MAX }
validates :title_html, length: { maximum: Issuable::TITLE_HTML_LENGTH_MAX }, allow_blank: true
enum state: { opened: 1, archived: 2 }
scope :for_iid, -> (iid) { where(iid: iid) }
scope :for_state, -> (state) { where(state: state) }
scope :counts_by_state, -> { group(:state).count }
def self.simple_sorts
super.except('name_asc', 'name_desc')
end
# In the next iteration we will support also group-level requirements
# so it's better to use resource_parent instead of project directly
def resource_parent
project
end
end
# frozen_string_literal: true
module RequirementsManagement
class Requirement < ApplicationRecord
include CacheMarkdownField
include StripAttribute
include AtomicInternalId
include Sortable
# the expected name for this table is `requirements_management_requirements`,
# but to avoid downtime and deployment issues `requirements` is still used
# https://gitlab.com/gitlab-org/gitlab/-/merge_requests/30052#note_329556542
self.table_name = 'requirements'
cache_markdown_field :title, pipeline: :single_line
strip_attributes :title
belongs_to :author, inverse_of: :requirements, class_name: 'User'
belongs_to :project, inverse_of: :requirements
has_internal_id :iid, scope: :project, init: ->(s) { s&.project&.requirements&.maximum(:iid) }
validates :author, :project, :title, presence: true
validates :title, length: { maximum: Issuable::TITLE_LENGTH_MAX }
validates :title_html, length: { maximum: Issuable::TITLE_HTML_LENGTH_MAX }, allow_blank: true
enum state: { opened: 1, archived: 2 }
scope :for_iid, -> (iid) { where(iid: iid) }
scope :for_state, -> (state) { where(state: state) }
scope :counts_by_state, -> { group(:state).count }
def self.simple_sorts
super.except('name_asc', 'name_desc')
end
# In the next iteration we will support also group-level requirements
# so it's better to use resource_parent instead of project directly
def resource_parent
project
end
end
end
# frozen_string_literal: true
class RequirementPolicy < BasePolicy
delegate { @subject.resource_parent }
end
# frozen_string_literal: true
module RequirementsManagement
class RequirementPolicy < BasePolicy
delegate { @subject.resource_parent }
end
end
...@@ -7,7 +7,7 @@ module EE ...@@ -7,7 +7,7 @@ module EE
def migrate_records def migrate_records
migrate_epics migrate_epics
migrate_requirements migrate_requirements_management_requirements
migrate_vulnerabilities_feedback migrate_vulnerabilities_feedback
migrate_reviews migrate_reviews
super super
...@@ -20,7 +20,7 @@ module EE ...@@ -20,7 +20,7 @@ module EE
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def migrate_requirements def migrate_requirements_management_requirements
user.requirements.update_all(author_id: ghost_user.id) user.requirements.update_all(author_id: ghost_user.id)
end end
......
# frozen_string_literal: true # frozen_string_literal: true
FactoryBot.define do FactoryBot.define do
factory :requirement do factory :requirement, class: 'RequirementsManagement::Requirement' do
project project
author author
title { generate(:title) } title { generate(:title) }
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
describe Requirement do describe RequirementsManagement::Requirement do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
describe RequirementPolicy do describe RequirementsManagement::RequirementPolicy do
let_it_be(:owner) { create(:user) } let_it_be(:owner) { create(:user) }
let_it_be(:admin) { create(:admin) } let_it_be(:admin) { create(:admin) }
let_it_be(:reporter) { create(:user) } let_it_be(:reporter) { create(:user) }
......
...@@ -29,7 +29,7 @@ describe 'Creating a Requirement' do ...@@ -29,7 +29,7 @@ describe 'Creating a Requirement' do
'or you don\'t have permission to perform this action'] 'or you don\'t have permission to perform this action']
it 'does not create requirement' do it 'does not create requirement' do
expect { post_graphql_mutation(mutation, current_user: current_user) }.not_to change(Requirement, :count) expect { post_graphql_mutation(mutation, current_user: current_user) }.not_to change(RequirementsManagement::Requirement, :count)
end end
end end
...@@ -69,7 +69,7 @@ describe 'Creating a Requirement' do ...@@ -69,7 +69,7 @@ describe 'Creating a Requirement' do
errors: ["Title can't be blank"] errors: ["Title can't be blank"]
it 'does not create the requirement' do it 'does not create the requirement' do
expect { post_graphql_mutation(mutation, current_user: current_user) }.not_to change(Requirement, :count) expect { post_graphql_mutation(mutation, current_user: current_user) }.not_to change(RequirementsManagement::Requirement, :count)
end end
end end
......
...@@ -53,7 +53,7 @@ describe Users::MigrateToGhostUserService do ...@@ -53,7 +53,7 @@ describe Users::MigrateToGhostUserService do
let!(:user) { create(:user) } let!(:user) { create(:user) }
let(:service) { described_class.new(user) } let(:service) { described_class.new(user) }
include_examples "migrating a deleted user's associated records to the ghost user", Requirement, [:author] do include_examples "migrating a deleted user's associated records to the ghost user", RequirementsManagement::Requirement, [:author] do
let(:created_record) { create(:requirement, author: user) } let(:created_record) { create(:requirement, author: user) }
end end
end end
......
...@@ -21,7 +21,7 @@ describe Requirements::CreateService do ...@@ -21,7 +21,7 @@ describe Requirements::CreateService do
end end
it 'creates new requirement' do it 'creates new requirement' do
expect { subject }.to change { Requirement.count }.by(1) expect { subject }.to change { RequirementsManagement::Requirement.count }.by(1)
end end
it 'uses only permitted params' do it 'uses only permitted params' do
......
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