diff --git a/app/contracts/groups/base_contract.rb b/app/contracts/groups/base_contract.rb index 07088522fa1..c54b69527a1 100644 --- a/app/contracts/groups/base_contract.rb +++ b/app/contracts/groups/base_contract.rb @@ -38,7 +38,6 @@ module Groups attribute :name attribute :lastname attribute :parent_id - attribute :organizational_unit validate :validate_unique_users validate :validate_users_not_in_other_department diff --git a/app/contracts/groups/create_contract.rb b/app/contracts/groups/create_contract.rb index fbb3c6fda7b..c3842bc1340 100644 --- a/app/contracts/groups/create_contract.rb +++ b/app/contracts/groups/create_contract.rb @@ -31,6 +31,7 @@ module Groups class CreateContract < BaseContract attribute :type + attribute :organizational_unit validate :type_is_group diff --git a/lib/api/v3/groups/group_representer.rb b/lib/api/v3/groups/group_representer.rb index dc2fd37c708..a7f561c9760 100644 --- a/lib/api/v3/groups/group_representer.rb +++ b/lib/api/v3/groups/group_representer.rb @@ -39,6 +39,14 @@ module API "Group" end + property :organizational_unit, + render_nil: true + + associated_resource :parent, + v3_path: :group, + representer: GroupRepresenter, + skip_render: ->(*) { represented.parent_id.nil? } + link :delete, cache_if: -> { current_user.admin? } do { diff --git a/lib/api/v3/groups/groups_api.rb b/lib/api/v3/groups/groups_api.rb index 6b43b830faf..ee46a425a13 100644 --- a/lib/api/v3/groups/groups_api.rb +++ b/lib/api/v3/groups/groups_api.rb @@ -46,7 +46,7 @@ module API route_param :id, type: Integer, desc: "Group ID" do after_validation do - @group = Group.visible(current_user).find(params[:id]) + @group = Group.visible(current_user).includes(:group_detail).find(params[:id]) end get &::API::V3::Utilities::Endpoints::Show diff --git a/spec/contracts/groups/update_contract_spec.rb b/spec/contracts/groups/update_contract_spec.rb index 68d6dd91159..31e4a2866c6 100644 --- a/spec/contracts/groups/update_contract_spec.rb +++ b/spec/contracts/groups/update_contract_spec.rb @@ -51,6 +51,12 @@ RSpec.describe Groups::UpdateContract do it_behaves_like "contract is invalid", type: :error_readonly end + + describe "organizational_unit" do + it "is not a writable attribute" do + expect(contract.writable_attributes).not_to include("organizational_unit") + end + end end end diff --git a/spec/requests/api/v3/groups/group_resource_spec.rb b/spec/requests/api/v3/groups/group_resource_spec.rb index 2a952790066..c6f3f52d775 100644 --- a/spec/requests/api/v3/groups/group_resource_spec.rb +++ b/spec/requests/api/v3/groups/group_resource_spec.rb @@ -81,6 +81,35 @@ RSpec.describe "API v3 Group resource", content_type: :json do end end + context "when the group is a regular group" do + it "includes organizationalUnit as false and no parent link" do + expect(subject.body) + .to be_json_eql(false.to_json) + .at_path("organizationalUnit") + + expect(subject.body) + .not_to have_json_path("_links/parent") + end + end + + context "when the group is a department with a parent" do + let(:parent_department) { create(:department) } + let(:group) do + create(:department, parent_id: parent_department.id, member_with_roles: { project => role }) + end + let(:members) { [] } + + it "includes organizationalUnit as true and the parent link" do + expect(subject.body) + .to be_json_eql(true.to_json) + .at_path("organizationalUnit") + + expect(subject.body) + .to be_json_eql(api_v3_paths.group(parent_department.id).to_json) + .at_path("_links/parent/href") + end + end + context "requesting nonexistent group" do let(:get_path) { api_v3_paths.group 9999 } @@ -148,6 +177,29 @@ RSpec.describe "API v3 Group resource", content_type: :json do end end + context "when creating a department" do + let(:body) do + { + name: "New department", + organizationalUnit: true + }.to_json + end + + it "responds with 201 and sets organizationalUnit" do + expect(response).to have_http_status(:created) + + expect(response.body) + .to be_json_eql(true.to_json) + .at_path("organizationalUnit") + end + + it "persists the organizational_unit flag" do + response + group = Group.find_by(name: "New department") + expect(group.organizational_unit?).to be true + end + end + context "when the user is allowed and the input is invalid" do current_user { admin } @@ -607,6 +659,63 @@ RSpec.describe "API v3 Group resource", content_type: :json do end end + describe "PATCH api/v3/groups/:id (department properties)" do + current_user { admin } + + context "when attempting to change organizational_unit" do + let(:department) { create(:department) } + + let(:body) do + { organizationalUnit: false }.to_json + end + + before do + perform_enqueued_jobs do + patch api_v3_paths.group(department.id), body + end + end + + it "does not change organizational_unit" do + expect(department.reload.organizational_unit?).to be true + end + + it "responds with 422 but ignores the change" do + expect(last_response).to have_http_status(:unprocessable_entity) + end + end + + context "when setting a parent on a department" do + let(:parent_department) { create(:department) } + let(:department) { create(:department) } + + let(:body) do + { + _links: { + parent: { href: api_v3_paths.group(parent_department.id) } + } + }.to_json + end + + before do + perform_enqueued_jobs do + patch api_v3_paths.group(department.id), body + end + end + + it "responds with 200" do + expect(last_response).to have_http_status(:ok) + end + + it "sets the parent" do + expect(last_response.body) + .to be_json_eql(api_v3_paths.group(parent_department.id).to_json) + .at_path("_links/parent/href") + + expect(department.reload.parent_id).to eq(parent_department.id) + end + end + end + describe "DELETE /api/v3/groups/:id" do let(:path) { api_v3_paths.group(group.id) } let(:other_project) { create(:project) }