Fix assignment policies and authorize assignment views

src/api/app/models/assignment.rb

@@ -22,21 +22,20 @@ class Assignment < ApplicationRecord
   #### Validations macros
   validate :assignee do
     errors.add(:assignee, 'must be in confirmed state') unless assignee && assignee.state == 'confirmed'
-    errors.add(:assignee, 'must be a project or package collaborator') unless assignee_is_a_collaborator?
+    errors.add(:assignee, 'must have the role maintainer, bugowner, reviewer on the project or package') unless assignee_has_required_role_to_be_assigned?
   end
   validates :package, uniqueness: true
 
   #### Instance methods (public and then protected/private)
-  def assignee_is_a_collaborator?
-    return false if assignee.nil?
 
-    collaborators = (package.relationships + package.project.relationships).map(&:user)
-    return false if collaborators.empty?
+  private
 
-    collaborators.include?(assignee)
-  end
+  def assignee_has_required_role_to_be_assigned?
+    return false if assignee.nil?
 
-  private
+    roles = Role.where(title: %w[maintainer bugowner reviewer])
+    (package.relationships.where(role_id: roles.ids, user_id: assignee) + package.project.relationships.where(role_id: roles.ids, user_id: assignee)).any?
+  end
 
   def trigger_event_on_creation
     Event::AssignmentCreate.create(event_parameters)

src/api/app/policies/assignment_policy.rb

@@ -4,10 +4,10 @@
 class AssignmentPolicy < ApplicationPolicy
   def create?
     return false unless Flipper.enabled?(:foster_collaboration, user)
-
     return true if user.admin?
 
-    record.assignee_is_a_collaborator?
+    roles = Role.where(title: %w[maintainer bugowner reviewer])
+    (record.package.relationships.where(role_id: roles.ids, user_id: user) + record.package.project.relationships.where(role_id: roles.ids, user_id: user)).any?
   end
 
   def destroy?

src/api/app/views/webui/package/side_links/_assignments.html.haml

@@ -8,7 +8,7 @@
       %p.mb-0.link-danger
         %i.fas.fa-user-minus
         Unassign
-- else
+- elsif policy(Assignment.new(package: package)).create?
   .dropdown#assignment-search
     %button.btn.btn-sm.dropdown-toggle.ps-0.border-0{ data: { 'bs-toggle': 'dropdown', 'bs-auto-close': 'outside' }, aria: { expanded: 'false' } }
       %strong

src/api/spec/factories/relationship.rb

@@ -19,6 +19,10 @@
       factory :relationship_package_user_as_bugowner do
         role { Role.find_by_title('bugowner') }
       end
+
+      factory :relationship_package_user_as_reviewer do
+        role { Role.find_by_title('reviewer') }
+      end
     end
 
     factory :relationship_package_group do

src/api/spec/models/assignment_spec.rb

@@ -1,30 +1,39 @@
 RSpec.describe Assignment do
-  describe '#assignee_is_a_collaborator?' do
+  describe '#assignee_has_required_role_to_be_assigned?' do
     subject do
-      assignment.assignee_is_a_collaborator?
+      build(:assignment, package: package, assignee: user)
     end
 
-    context 'when the assignee is a package collaborator' do
-      let(:package) { create(:package_with_maintainer) }
-      let(:assignment) { create(:assignment, package: package, assignee: package.maintainers.first) }
+    let(:project) { create(:project_with_package) }
+    let(:package) { project.packages.first }
+    let(:user) { create(:confirmed_user) }
 
-      it { expect(subject).to be true }
+    context 'when the assignee is a package maintainer' do
+      let!(:relationship_package_maintainer) { create(:relationship_package_user, package: package, user: user) }
+
+      it { expect(subject).to be_valid }
+    end
+
+    context 'when the assignee is a project maintainer' do
+      let!(:relationship_project_maintainer) { create(:relationship_project_user, project: project, user: user) }
+
+      it { expect(subject).to be_valid }
     end
 
-    context 'when the assignee is a project collaborator' do
-      let(:maintainer) { create(:confirmed_user) }
-      let(:project) { create(:project_with_package, maintainer: maintainer) }
-      let(:package) { project.packages.first }
-      let(:assignment) { create(:assignment, package: package, assignee: maintainer) }
+    context 'when the assignee is a package bugowner' do
+      let!(:bugowner_relationship) { create(:relationship_package_user_as_bugowner, user: user, package: package) }
 
-      it { expect(subject).to be true }
+      it { expect(subject).to be_valid }
     end
 
-    context 'when the assignee is not a collaborator' do
-      let(:iggy) { create(:confirmed_user) }
-      let(:assignment) { build(:assignment, assignee: iggy) }
+    context 'when the assignee is a package reviewer' do
+      let!(:reviewer_relationship) { create(:relationship_package_user_as_reviewer, user: user, package: package) }
+
+      it { expect(subject).to be_valid }
+    end
 
-      it { expect(subject).to be false }
+    context 'when the assignee has no role' do
+      it { expect(subject).not_to be_valid }
     end
   end
 end

src/api/spec/policies/assignment_policy_spec.rb

@@ -6,33 +6,53 @@
       Flipper.enable(:foster_collaboration)
     end
 
-    context 'a package collaborator' do
-      let(:package_collaborator) { package.maintainers.first }
+    context 'as a package maintainer' do
+      let(:package_maintainer) { package.maintainers.first }
       let(:package) { create(:package_with_maintainer) }
-      let(:assignment) { create(:assignment, assigner: package_collaborator, assignee: package_collaborator, package: package) }
+      let(:assignment) { build(:assignment, assigner: package_maintainer, package: package) }
 
-      it { is_expected.to permit(package_collaborator, assignment) }
+      it { is_expected.to permit(package_maintainer, assignment) }
     end
 
-    context 'a project collaborator' do
-      let(:project_collaborator) { create(:confirmed_user) }
-      let(:project) { create(:project_with_package, maintainer: project_collaborator) }
+    context 'as project maintainer' do
+      let(:project_maintainer) { create(:confirmed_user) }
+      let(:project) { create(:project_with_package, maintainer: project_maintainer) }
       let(:package) { project.packages.first }
-      let(:assignment) { build(:assignment, assigner: project_collaborator, assignee: project_collaborator, package: package) }
+      let(:assignment) { build(:assignment, assigner: project_maintainer, package: package) }
 
-      it { is_expected.to permit(project_collaborator, assignment) }
+      it { is_expected.to permit(project_maintainer, assignment) }
     end
 
-    context 'an admin' do
+    context 'as a admin user' do
       let(:admin_user) { create(:admin_user) }
-      let(:assignment) { build(:assignment) }
+      let(:package) { create(:package) }
+      let(:assignment) { build(:assignment, assigner: admin_user, package: package) }
 
       it { is_expected.to permit(admin_user, assignment) }
     end
 
-    context 'any other user' do
+    context 'as a bugowner' do
       let(:user) { create(:confirmed_user) }
-      let(:assignment) { build(:assignment) }
+      let!(:bugowner_relationship) { create(:relationship_package_user_as_bugowner, user: user, package: package) }
+      let(:package) { create(:package) }
+      let(:assignment) { build(:assignment, assigner: user, package: package) }
+
+      it { is_expected.to permit(user, assignment) }
+    end
+
+    context 'as a reviewer' do
+      let(:user) { create(:confirmed_user) }
+      let!(:reviewer_relationship) { create(:relationship_package_user_as_reviewer, user: user, package: package) }
+      let(:package) { create(:package) }
+      let(:assignment) { build(:assignment, assigner: user, package: package) }
+
+      it { is_expected.to permit(user, assignment) }
+    end
+
+    context 'a user without any role' do
+      let(:user) { create(:confirmed_user) }
+      let(:package) { create(:package) }
+      let(:assignment) { build(:assignment, assigner: user, package: package) }
 
       it { is_expected.not_to permit(user, assignment) }
     end