Merge branch 'fix-member-roles-api' into 'master'

Fix issues with Member Roles API

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/140088



Merged-by: Miranda Fluharty <mfluharty@gitlab.com>
Approved-by: Daniel Tian <dtian@gitlab.com>
Approved-by: Miranda Fluharty <mfluharty@gitlab.com>
Approved-by: mo khan <mo@mokhan.ca>
Approved-by: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Reviewed-by: mo khan <mo@mokhan.ca>
Co-authored-by: Alex Buijs <abuijs@gitlab.com>

ee/app/graphql/resolvers/member_roles/permission_list_resolver.rb

@@ -7,10 +7,10 @@ class PermissionListResolver < BaseResolver
 
       def resolve
         MemberRole.all_customizable_permissions.map do |permission, definition|
-          definition[:requirement] = definition[:requirement]&.to_s&.upcase
-          permission = permission.to_s.upcase
+          requirement = definition[:requirement]&.to_s&.upcase
+          value = permission.to_s.upcase
 
-          definition.merge(value: permission)
+          definition.merge(value: value, requirement: requirement)
         end
       end
     end

ee/app/models/members/member_role.rb

@@ -122,9 +122,9 @@ def validate_requirements
       next unless requirement # skipping permissions that have no requirement
       next if self[requirement] # the requierement is met
 
-      errors.add(permission,
-        format(s_("MemberRole|%{requirement} has to be enabled in order to enable %{permission}."),
-          requirement: requirement, permission: permission)
+      errors.add(:base,
+        format(s_("MemberRole|%{requirement} has to be enabled in order to enable %{permission}"),
+          requirement: requirement.to_s.humanize, permission: permission.to_s.humanize)
       )
     end
   end

ee/spec/features/groups/member_roles_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Member Roles', :js, feature_category: :permissions do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:group) { create(:group) }
+
+  let(:name) { 'My custom role' }
+  let(:permissions) { { read_vulnerability: { name: 'read_vulnerability' } } }
+  let(:permission) { :read_vulnerability }
+  let(:permission_name) { permission.to_s.humanize }
+  let(:access_level) { 'Developer' }
+
+  before_all do
+    group.add_owner(user)
+  end
+
+  before do
+    stub_licensed_features(custom_roles: true)
+  end
+
+  def create_role(access_level, name, permissions)
+    click_button 'Add new role'
+    select access_level, from: 'Base role to use as template'
+    fill_in 'Role name', with: name
+    permissions.each do |permission|
+      page.check permission
+    end
+    click_button 'Create new role'
+  end
+
+  def created_role(name, id, access_level, permissions)
+    [name, id, access_level, *permissions].join(' ')
+  end
+
+  describe 'adding a new custom role' do
+    before do
+      allow(Gitlab::CustomRoles::Definition).to receive(:all).and_return(permissions)
+
+      sign_in(user)
+      visit group_settings_roles_and_permissions_path(group)
+    end
+
+    it 'creates a new custom role' do
+      create_role(access_level, name, [permission_name])
+
+      created_member_role = MemberRole.find_by(
+        name: name,
+        base_access_level: Gitlab::Access.options[access_level],
+        permission => true)
+
+      expect(created_member_role).not_to be_nil
+
+      role = created_role(name, created_member_role.id, access_level, [permission_name])
+      expect(page).to have_content(role)
+    end
+
+    context 'when the permission has a requirement' do
+      let(:permissions) do
+        { admin_vulnerability: { name: 'admin_vulnerability', requirement: 'read_vulnerability' },
+          read_vulnerability: { name: 'read_vulnerability' } }
+      end
+
+      let(:permission) { :admin_vulnerability }
+      let(:requirement) { permissions[permission][:requirement] }
+      let(:requirement_name) { requirement.to_s.humanize }
+
+      context 'when the requirement has not been met' do
+        it 'show an error message' do
+          create_role(access_level, name, [permission_name])
+
+          created_member_role = MemberRole.find_by(
+            name: name,
+            base_access_level: Gitlab::Access.options[access_level],
+            permission => true)
+
+          expect(created_member_role).to be_nil
+          expect(page).to have_content("#{requirement_name} has to be enabled in order to enable #{permission_name}")
+        end
+      end
+
+      context 'when the requirement has been met' do
+        it 'creates the custom role' do
+          create_role(access_level, name, [permission_name, requirement_name])
+
+          created_member_role = MemberRole.find_by(
+            name: name,
+            base_access_level: Gitlab::Access.options[access_level],
+            permission => true,
+            requirement => true)
+
+          expect(created_member_role).not_to be_nil
+
+          role = created_role(name, created_member_role.id, access_level, [permission_name, requirement_name])
+          expect(page).to have_content(role)
+        end
+      end
+    end
+  end
+end

ee/spec/models/members/member_role_spec.rb

@@ -138,8 +138,8 @@
           member_role.admin_vulnerability = true
 
           expect(member_role).not_to be_valid
-          expect(member_role.errors[:admin_vulnerability])
-            .to include(s_("MemberRole|read_vulnerability has to be enabled in order to enable admin_vulnerability."))
+          expect(member_role.errors[:base])
+            .to include(s_("MemberRole|Read vulnerability has to be enabled in order to enable Admin vulnerability"))
         end
       end
     end

locale/gitlab.pot

@@ -29704,7 +29704,7 @@ msgstr ""
 msgid "MemberInviteEmail|Invitation to join the %{project_or_group} %{project_or_group_name}"
 msgstr ""
-msgid "MemberRole|%{requirement} has to be enabled in order to enable %{permission}."
+msgid "MemberRole|%{requirement} has to be enabled in order to enable %{permission}"
 msgstr ""
 msgid "MemberRole|Actions"