[#57707] add unit tests for remove user from group

modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/add_user_to_group_command.rb

@@ -50,7 +50,7 @@ def call(auth_strategy:, user:, group:)
                 info "Adding #{user} to #{group} through #{url}"
 
                 response = http.post(UrlBuilder.url(@storage.uri, "ocs/v1.php/cloud/users", user, "groups"),
-                                     form: { "groupid" => CGI.escapeURIComponent(group) })
+                                     form: { "groupid" => group })
 
                 handle_response(response)
               end

modules/storages/spec/common/storages/peripherals/storage_interaction/nextcloud/remove_user_from_group_command_spec.rb

@@ -0,0 +1,170 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+require "spec_helper"
+require_module_spec_helper
+
+RSpec.describe Storages::Peripherals::StorageInteraction::Nextcloud::RemoveUserFromGroupCommand, :webmock do
+  let(:storage) { create(:nextcloud_storage_with_local_connection, :as_automatically_managed, username: "vcr") }
+  let(:auth_strategy) { Storages::Peripherals::Registry.resolve("nextcloud.authentication.userless").call }
+
+  describe "basic command setup" do
+    it "is registered as commands.remove_user_from_group" do
+      expect(Storages::Peripherals::Registry
+               .resolve("#{storage}.commands.remove_user_from_group")).to eq(described_class)
+    end
+
+    it "responds to #call with correct parameters" do
+      expect(described_class).to respond_to(:call)
+
+      method = described_class.method(:call)
+      expect(method.parameters).to contain_exactly(%i[keyreq storage],
+                                                   %i[keyreq auth_strategy],
+                                                   %i[keyreq user],
+                                                   %i[keyreq group])
+    end
+  end
+
+  shared_examples_for "failing request" do |error_code:|
+    it "returns a failure" do
+      result = described_class.call(storage:, auth_strategy:, user:, group:)
+      expect(result).to be_failure
+
+      error = result.errors
+      expect(error.code).to eq(error_code)
+      expect(error.data.source).to eq(described_class)
+    end
+  end
+
+  context "if user exists in target group", vcr: "nextcloud/remove_user_from_group_success" do
+    let(:user) { "[email protected]" }
+    let(:group) { "Sith Assassins" }
+
+    before do
+      create_group(group)
+      add_user_to_group(user, group)
+
+      # There is a bug in the group folder API that does not allow to remove a user from a group,
+      # if this is its last group
+      create_group("Sith Assassins Backup")
+      add_user_to_group(user, "Sith Assassins Backup")
+    end
+
+    after do
+      remove_group(group)
+      remove_group("Sith Assassins Backup")
+    end
+
+    it "returns a success" do
+      members = group_members(group)
+      expect(members).to include(user)
+
+      result = described_class.call(storage:, auth_strategy:, user:, group:)
+      expect(result).to be_success
+
+      members = group_members(group)
+      expect(members).not_to include(user)
+    end
+  end
+
+  context "if user does not exist in target group", vcr: "nextcloud/remove_user_from_group_no_user" do
+    let(:user) { "[email protected]" }
+    let(:group) { "Sith Assassins" }
+
+    before do
+      create_group(group)
+    end
+
+    after do
+      remove_group(group)
+    end
+
+    it "returns a success" do
+      members = group_members(group)
+      expect(members).not_to include(user)
+
+      result = described_class.call(storage:, auth_strategy:, user:, group:)
+      expect(result).to be_success
+
+      members = group_members(group)
+      expect(members).not_to include(user)
+    end
+  end
+
+  context "if target group does not exist", vcr: "nextcloud/remove_user_from_group_not_existing_group" do
+    let(:user) { "[email protected]" }
+    let(:group) { "Sith Assassins" }
+
+    it_behaves_like "failing request", error_code: :group_does_not_exist
+  end
+
+  context "if user does not exist", vcr: "nextcloud/remove_user_from_group_not_existing_user" do
+    let(:user) { "this is not the user you are looking for" }
+    let(:group) { "Sith Assassins" }
+
+    before do
+      create_group(group)
+    end
+
+    after do
+      remove_group(group)
+    end
+
+    it_behaves_like "failing request", error_code: :user_does_not_exist
+  end
+
+  private
+
+  def auth = Storages::Peripherals::StorageInteraction::Authentication[auth_strategy]
+
+  def create_group(group)
+    auth.call(storage:, http_options: { headers: { "OCS-APIRequest" => "true" } }) do |http|
+      http.post(Storages::UrlBuilder.url(storage.uri, "ocs/v1.php/cloud/groups"),
+                form: { "groupid" => group })
+    end
+  end
+
+  def remove_group(group)
+    auth.call(storage:, http_options: { headers: { "OCS-APIRequest" => "true" } }) do |http|
+      http.delete(Storages::UrlBuilder.url(storage.uri, "/ocs/v1.php/cloud/groups", group))
+    end
+  end
+
+  def add_user_to_group(user, group)
+    Storages::Peripherals::StorageInteraction::Nextcloud::AddUserToGroupCommand
+      .call(storage:, auth_strategy:, user:, group:)
+  end
+
+  def group_members(group)
+    Storages::Peripherals::StorageInteraction::Nextcloud::GroupUsersQuery
+      .call(storage:, auth_strategy:, group:)
+      .result
+  end
+end

modules/storages/spec/common/storages/peripherals/storage_interaction/nextcloud/set_permissions_command_spec.rb

@@ -32,7 +32,6 @@
 require_module_spec_helper
 
 RSpec.describe Storages::Peripherals::StorageInteraction::Nextcloud::SetPermissionsCommand, :webmock do
-  let(:user) { create(:user) }
   let(:storage) { create(:nextcloud_storage_with_local_connection, :as_automatically_managed, username: "vcr") }
   let(:auth_strategy) { Storages::Peripherals::Registry.resolve("nextcloud.authentication.userless").call }
 
@@ -78,7 +77,7 @@
   end
 
   context "if a user does not exist",
-          skip: "When setting permissions for a user that does not exists, nextcloud's response doesn't contain the" \
+          skip: "When setting permissions for a user that does not exists, nextcloud's response doesn't contain the " \
                 "needed information. We need to work around this by maybe having a separate request fetching ACLs " \
                 "after setting them.",
           vcr: "nextcloud/set_permissions_invalid_user_id" do