Fix validation errors on impersonations or transferring users (decidi…

…m#13731)

decidim-admin/app/controllers/decidim/admin/conflicts_controller.rb

@@ -47,7 +47,7 @@ def update
 
           on(:invalid) do
             flash.now[:alert] = I18n.t("error", scope: "decidim.admin.conflicts.transfer")
-            redirect_to decidim.root_path
+            render action: "edit"
           end
         end
       end

decidim-admin/app/controllers/decidim/admin/impersonations_controller.rb

@@ -29,6 +29,7 @@ def create
 
         @form = form(ImpersonateUserForm).from_params(
           user:,
+          name: params[:impersonate_user][:name],
           handler_name:,
           reason: params[:impersonate_user][:reason],
           authorization: Decidim::AuthorizationHandler.handler_for(

decidim-admin/app/forms/decidim/admin/impersonate_user_form.rb

@@ -14,13 +14,18 @@ class ImpersonateUserForm < Form
       attribute :handler_name, String
 
       validates :user, presence: true
+      validates :name, presence: true, unless: :persisted_user?
       validates :reason, presence: true, unless: :managed_user?
 
       private
 
       def managed_user?
         user && user.managed?
       end
+
+      def persisted_user?
+        user&.persisted?
+      end
     end
   end
 end

decidim-admin/app/forms/decidim/admin/transfer_user_form.rb

@@ -14,6 +14,21 @@ class TransferUserForm < Form
 
       validates :current_user, presence: true
       validates :conflict, presence: true
+      validates :email, presence: true
+      validate :unique_email
+
+      private
+
+      def unique_email
+        return if conflict.blank?
+        return true if Decidim::UserBaseEntity.where(
+          organization: context.current_organization,
+          email:
+        ).where.not(id: [conflict.current_user.id, conflict.managed_user_id]).empty?
+
+        errors.add :email, :taken
+        false
+      end
     end
   end
 end

decidim-admin/app/views/decidim/admin/conflicts/edit.html.erb

@@ -27,18 +27,28 @@
   </div>
 </div>
 
-<%= decidim_form_for(@form, url: conflict_path, method: :put, html: { class: "form" }) do |f| %>
-  <div class="card">
-    <div class="card-section">
-      <div class="row column">
-        <%= f.text_field :reason, label: t("reason", scope: "decidim.admin.conflicts.transfer") %>
+<div class="item__edit item__edit-1col">
+  <div class="item__edit-form">
+    <%= decidim_form_for(@form, url: conflict_path, method: :put, html: { class: "form form-defaults" }) do |f| %>
+      <div class="form__wrapper">
+        <div class="card pt-4">
+          <div class="card-section">
+            <div class="row column">
 
-        <%= f.text_field :email, label: t("email", scope: "decidim.admin.conflicts.transfer") %>
+              <%= f.text_field :reason, label: t("reason", scope: "decidim.admin.conflicts.transfer") %>
+            </div>
+
+            <div class="row column">
+
+              <%= f.text_field :email, label: t("email", scope: "decidim.admin.conflicts.transfer") %>
+            </div>
+          </div>
+        </div>
       </div>
-    </div>
-  </div>
 
-  <div class="form__wrapper-block flex-col-reverse md:flex-row justify-between">
-    <%= f.submit t("title", scope: "decidim.admin.conflicts.transfer"), class: "button button__sm button__secondary" %>
+      <div class="form__wrapper-block flex-col-reverse md:flex-row justify-between">
+        <%= f.submit t("title", scope: "decidim.admin.conflicts.transfer"), class: "button button__sm button__secondary" %>
+      </div>
+    <% end %>
   </div>
-<% end %>
+</div>

decidim-admin/spec/commands/decidim/admin/impersonate_user_spec.rb

@@ -19,9 +19,11 @@ module Decidim::Admin
     end
     let(:extra_params) do
       {
-        reason: "Need it"
+        reason: "Need it",
+        name:
       }
     end
+    let(:name) { "Rigoberto" }
     let(:form) do
       ImpersonateUserForm.from_params(
         form_params.merge(extra_params)
@@ -87,12 +89,24 @@ module Decidim::Admin
       let(:user) { create(:user, organization:) }
 
       it_behaves_like "the impersonate user command"
+
+      context "and no name is passed to the form" do
+        let(:name) { nil }
+
+        it_behaves_like "the impersonate user command"
+      end
     end
 
     context "when passed an existing managed user" do
       let(:user) { create(:user, :managed, organization:) }
 
       it_behaves_like "the impersonate user command"
+
+      context "and no name is passed to the form" do
+        let(:name) { nil }
+
+        it_behaves_like "the impersonate user command"
+      end
     end
 
     context "when passed a new managed user" do
@@ -103,6 +117,14 @@ module Decidim::Admin
       it "creates the user in DB" do
         expect { subject.call }.to change { Decidim::User.managed.count }.by(1)
       end
+
+      context "and no name is passed to the form" do
+        let(:name) { nil }
+
+        it "is not valid" do
+          expect { subject.call }.to broadcast(:invalid)
+        end
+      end
     end
   end
 end

decidim-admin/spec/forms/decidim/admin/transfer_user_form_spec.rb

@@ -16,7 +16,9 @@ module Admin
       let(:organization) { create(:organization) }
       let(:current_user) { create(:user, :admin, organization:) }
       let(:new_user) { create(:user, organization:) }
+      let(:other_user) { create(:user, organization:) }
       let(:managed_user) { create(:user, managed: true, organization:) }
+      let(:email) { new_user.email }
 
       let(:conflict) do
         Decidim::Verifications::Conflict.create(current_user: new_user, managed_user:)
@@ -25,14 +27,39 @@ module Admin
       let(:attributes) do
         {
           current_user:,
-          conflict:
+          conflict:,
+          email:
         }
       end
 
       context "when form is valid" do
         it { is_expected.to be_valid }
       end
 
+      context "when the email is the used by the managed_user" do
+        let(:email) { managed_user.email }
+
+        it { is_expected.to be_valid }
+      end
+
+      context "when email is blank" do
+        let(:email) { nil }
+
+        it { is_expected.to be_invalid }
+      end
+
+      context "when email belongs to an existing user different of the emails of the conflicting users" do
+        let(:email) { other_user.email }
+
+        it { is_expected.to be_invalid }
+      end
+
+      context "when email does not belong to an existing user" do
+        let(:email) { "[email protected]" }
+
+        it { is_expected.to be_valid }
+      end
+
       context "when no current_user is passed" do
         let(:current_user) { nil }
 

decidim-admin/spec/shared/manage_impersonations_examples.rb

@@ -23,18 +23,29 @@
   end
 
   shared_examples_for "creating a managed user" do
+    let(:name) { "Rigoberto" }
+
     before do
       navigate_to_impersonations_page
 
       click_on "Manage new participant"
 
-      fill_in_the_impersonation_form(document_number, name: "Rigoberto")
+      fill_in_the_impersonation_form(document_number, name:)
     end
 
     it "shows a success message" do
       expect(page).to have_content("successfully")
     end
 
+    context "when no name is provided" do
+      let(:name) { "" }
+
+      it "shows a validation error message" do
+        expect(page).to have_no_content("successfully")
+        expect(page).to have_content("There are errors on the form")
+      end
+    end
+
     context "when authorization data is invalid" do
       let(:document_number) { "123456789Y" }
 

decidim-admin/spec/system/admin_manages_conflicts_spec.rb

@@ -0,0 +1,88 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+describe "Admin manages conflicts" do
+  let(:organization) { create(:organization) }
+
+  let!(:user) { create(:user, :admin, :confirmed, organization:) }
+  let!(:other_user) { create(:user, :admin, :confirmed, organization:, email: "[email protected]") }
+
+  let!(:conflictive_user) { create(:user, :admin, :confirmed, organization:, email: "[email protected]") }
+  let!(:managed_user) { create(:user, managed: true, organization:, email: "[email protected]") }
+
+  let!(:conflict) { create(:conflict, current_user: conflictive_user, managed_user:) }
+
+  before do
+    switch_to_host(organization.host)
+    login_as user, scope: :user
+    visit decidim_admin.root_path
+    click_on "Participants"
+    click_on "Verification conflicts"
+    click_on "Transfer"
+  end
+
+  context "when resolving a conflict" do
+    context "when no mail is passed" do
+      before do
+        click_on "Transfer"
+      end
+
+      it "the transfer cannot be sent" do
+        expect(page).to have_content("There is an error")
+      end
+    end
+
+    context "when a mail is passed" do
+      let(:email) { "[email protected]" }
+
+      before do
+        fill_in "Email", with: email
+        click_on "Transfer"
+      end
+
+      context "when the email is not in use by any other user" do
+        it "the transfer is successful" do
+          expect(page).to have_content("The current transfer has been successfully completed.")
+        end
+
+        it "the email of the managed user is replaced with the email passed by the form" do
+          expect(managed_user.reload.email).to eq("[email protected]")
+        end
+      end
+
+      context "when the email is the email of the conflictive user" do
+        let(:email) { "[email protected]" }
+
+        it "the transfer is successful" do
+          expect(page).to have_content("The current transfer has been successfully completed.")
+        end
+
+        it "the email of the managed user is replaced with the email of the conflictive one" do
+          expect(managed_user.reload.email).to eq("[email protected]")
+        end
+      end
+
+      context "when the email is the email of the managed user" do
+        let(:email) { "[email protected]" }
+
+        it "the transfer is successful" do
+          expect(page).to have_content("The current transfer has been successfully completed.")
+        end
+
+        it "the managed user keeps its email" do
+          expect(managed_user.reload.email).to eq("[email protected]")
+        end
+      end
+
+      context "when the email is the email of an already existing user" do
+        let(:email) { "[email protected]" }
+
+        it "the transfer fails" do
+          expect(page).to have_no_content("The current transfer has been successfully completed.")
+          expect(page).to have_content("There was a problem transferring the current participant to managed participant")
+        end
+      end
+    end
+  end
+end