From 5cc20be32a9ec829454097c0c93a56d70decc0d3 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Thu, 1 Apr 2021 21:02:48 +0200
Subject: [PATCH] Extract RevokeUser command

And I added more tests covering missing cases while at it.
---
 .../direct_verifications/revoke_user.rb       | 46 +++++++++++++++++++
 .../direct_verifications/user_processor.rb    | 28 +----------
 .../user_processor_spec.rb                    | 36 ++++++++++++++-
 3 files changed, 82 insertions(+), 28 deletions(-)
 create mode 100644 lib/decidim/direct_verifications/revoke_user.rb

diff --git a/lib/decidim/direct_verifications/revoke_user.rb b/lib/decidim/direct_verifications/revoke_user.rb
new file mode 100644
index 0000000..cce4423
--- /dev/null
+++ b/lib/decidim/direct_verifications/revoke_user.rb
@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+module Decidim
+  module DirectVerifications
+    class RevokeUser
+      def initialize(email, organization, instrumenter)
+        @email = email
+        @organization = organization
+        @instrumenter = instrumenter
+      end
+
+      def call
+        if (u = find_user)
+          auth = authorization(u)
+          return unless auth.granted?
+
+          Verification::DestroyUserAuthorization.call(auth) do
+            on(:ok) do
+              instrumenter.add_processed :revoked, email
+            end
+            on(:invalid) do
+              add_error :revoked, email
+            end
+          end
+        else
+          instrumenter.add_error :revoked, email
+        end
+      end
+
+      private
+
+      attr_reader :email, :organization, :instrumenter
+
+      def find_user
+        User.find_by(email: email, decidim_organization_id: organization.id)
+      end
+
+      def authorization(user)
+        Authorization.find_or_initialize_by(
+          user: user,
+          name: :direct_verifications
+        )
+      end
+    end
+  end
+end
diff --git a/lib/decidim/direct_verifications/user_processor.rb b/lib/decidim/direct_verifications/user_processor.rb
index bf34dc6..5eb3e3b 100644
--- a/lib/decidim/direct_verifications/user_processor.rb
+++ b/lib/decidim/direct_verifications/user_processor.rb
@@ -2,6 +2,7 @@
 
 require "decidim/direct_verifications/register_user"
 require "decidim/direct_verifications/authorize_user"
+require "decidim/direct_verifications/revoke_user"
 
 module Decidim
   module DirectVerifications
@@ -38,21 +39,7 @@ def authorize_users
 
       def revoke_users
         emails.each do |email, _name|
-          if (u = find_user(email))
-            auth = authorization(u)
-            next unless auth.granted?
-
-            Verification::DestroyUserAuthorization.call(auth) do
-              on(:ok) do
-                add_processed :revoked, email
-              end
-              on(:invalid) do
-                add_error :revoked, email
-              end
-            end
-          else
-            add_error :revoked, email
-          end
+          RevokeUser.new(email, organization, self).call
         end
       end
 
@@ -86,17 +73,6 @@ def log_action(user)
           }
         )
       end
-
-      def find_user(email)
-        User.find_by(email: email, decidim_organization_id: @organization.id)
-      end
-
-      def authorization(user)
-        Authorization.find_or_initialize_by(
-          user: user,
-          name: authorization_handler
-        )
-      end
     end
   end
 end
diff --git a/spec/lib/decidim/direct_verifications/user_processor_spec.rb b/spec/lib/decidim/direct_verifications/user_processor_spec.rb
index 6133854..9ff83a8 100644
--- a/spec/lib/decidim/direct_verifications/user_processor_spec.rb
+++ b/spec/lib/decidim/direct_verifications/user_processor_spec.rb
@@ -140,10 +140,11 @@ module DirectVerifications
         before do
           subject.emails = { user.email => user.name }
           subject.authorize_users
-          subject.revoke_users
         end
 
         it "has no errors" do
+          subject.revoke_users
+
           expect(subject.processed[:revoked].count).to eq(1)
           expect(subject.errors[:revoked].count).to eq(0)
         end
@@ -153,12 +154,43 @@ module DirectVerifications
         before do
           subject.emails = ["em@il.com"]
           subject.authorize_users
+        end
+
+        it "has errors" do
+          subject.revoke_users
+
+          expect(subject.processed[:revoked].count).to eq(0)
+          expect(subject.errors[:revoked].count).to eq(1)
+        end
+      end
+
+      context "when the authorization does not exist" do
+        before do
+          create(:user, email: "em@il.com", organization: organization)
+          subject.emails = ["em@il.com"]
+        end
+
+        it "has no errors" do
           subject.revoke_users
+
+          expect(subject.processed[:revoked].count).to eq(0)
+          expect(subject.errors[:revoked].count).to eq(0)
+        end
+      end
+
+      context "when the authorization is not granted" do
+        let(:user) { create(:user, email: "em@il.com", organization: organization) }
+
+        before do
+          subject.emails = ["em@il.com"]
+          create(:authorization, :pending, user: user, name: :direct_verifications)
         end
 
         it "has no errors" do
+          subject.revoke_users
+
           expect(subject.processed[:revoked].count).to eq(0)
-          expect(subject.errors[:revoked].count).to eq(1)
+          expect(subject.errors[:revoked].count).to eq(0)
         end
       end
     end