Process authorization individually (#22)

* Extract AuthorizeUser command

* Test authorization validation

* Refactor AuthorizeUser command

* Test methods without #send

They've become public now so there's no need for `#send`.

* Remove authorization redundant specs and refactor

lib/decidim/direct_verifications/authorize_user.rb

@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+module Decidim
+  module DirectVerifications
+    class AuthorizeUser
+      def initialize(email, data, session, organization, instrumenter)
+        @email = email
+        @data = data
+        @session = session
+        @organization = organization
+        @instrumenter = instrumenter
+      end
+
+      def call
+        unless user
+          instrumenter.add_error :authorized, email
+          return
+        end
+
+        return unless valid_authorization?
+
+        Verification::ConfirmUserAuthorization.call(authorization, form, session) do
+          on(:ok) do
+            instrumenter.add_processed :authorized, email
+          end
+          on(:invalid) do
+            instrumenter.add_error :authorized, email
+          end
+        end
+      end
+
+      private
+
+      attr_reader :email, :data, :session, :organization, :instrumenter
+
+      def valid_authorization?
+        !authorization.granted? || authorization.expired?
+      end
+
+      def user
+        @user ||= User.find_by(email: email, decidim_organization_id: organization.id)
+      end
+
+      def authorization
+        @authorization ||=
+          begin
+            auth = Authorization.find_or_initialize_by(
+              user: user,
+              name: :direct_verifications
+            )
+            auth.metadata = data
+            auth
+          end
+      end
+
+      def form
+        Verification::DirectVerificationsForm.new(email: user.email, name: user.name)
+      end
+    end
+  end
+end

lib/decidim/direct_verifications/user_processor.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "decidim/direct_verifications/register_user"
+require "decidim/direct_verifications/authorize_user"
 
 module Decidim
   module DirectVerifications
@@ -31,23 +32,7 @@ def register_users
 
       def authorize_users
         emails.each do |email, data|
-          if (u = find_user(email))
-            auth = authorization(u)
-            auth.metadata = data
-
-            next unless !auth.granted? || auth.expired?
-
-            Verification::ConfirmUserAuthorization.call(auth, authorize_form(u), session) do
-              on(:ok) do
-                add_processed :authorized, email
-              end
-              on(:invalid) do
-                add_error :authorized, email
-              end
-            end
-          else
-            add_error :authorized, email
-          end
+          AuthorizeUser.new(email, data, session, organization, self).call
         end
       end
 
@@ -80,8 +65,6 @@ def track(event, email, user = nil)
         end
       end
 
-      private
-
       def add_error(type, email)
         @errors[type] << email unless @errors[type].include? email
       end
@@ -90,6 +73,8 @@ def add_processed(type, email)
         @processed[type] << email unless @processed[type].include? email
       end
 
+      private
+
       def log_action(user)
         Decidim.traceability.perform_action!(
           "invite",
@@ -112,10 +97,6 @@ def authorization(user)
           name: authorization_handler
         )
       end
-
-      def authorize_form(user)
-        Verification::DirectVerificationsForm.new(email: user.email, name: user.name)
-      end
     end
   end
 end

spec/lib/decidim/direct_verifications/authorize_user_spec.rb

@@ -0,0 +1,128 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+module Decidim
+  module DirectVerifications
+    describe AuthorizeUser do
+      subject { described_class.new(email, data, session, organization, instrumenter) }
+
+      describe "#call" do
+        let(:data) { user.name }
+
+        context "when authorizing confirmed users" do
+          let(:organization) { build(:organization) }
+          let(:user) { create(:user, organization: organization) }
+          let(:email) { user.email }
+          let(:session) { {} }
+          let(:instrumenter) { instance_double(UserProcessor, add_processed: true, add_error: true) }
+
+          context "when passing the user name" do
+            let(:data) { user.name }
+
+            it "tracks the operation" do
+              subject.call
+
+              expect(instrumenter).to have_received(:add_processed).with(:authorized, email)
+              expect(instrumenter).not_to have_received(:add_error)
+            end
+
+            it "authorizes the user" do
+              expect(Verification::ConfirmUserAuthorization).to receive(:call)
+              subject.call
+            end
+          end
+
+          context "when passing user data" do
+            let(:data) { { name: user.name, type: "consumer" } }
+
+            it "stores it as authorization metadata" do
+              subject.call
+              expect(Authorization.last.metadata).to eq("name" => user.name, "type" => "consumer")
+            end
+
+            it "authorizes the user" do
+              expect(Verification::ConfirmUserAuthorization).to receive(:call)
+              subject.call
+            end
+          end
+
+          context "when the authorization already exists" do
+            context "when the authorization is not granted" do
+              let!(:authorization) { create(:authorization, :pending, user: user, name: :direct_verifications) }
+
+              it "authorizes the user" do
+                expect(Verification::ConfirmUserAuthorization).to receive(:call)
+                subject.call
+              end
+            end
+
+            context "when the authorization is already granted and expired" do
+              let!(:authorization) { create(:authorization, :granted, user: user, name: :direct_verifications) }
+
+              before do
+                allow(authorization).to receive(:expired?).and_return(true)
+                allow(Decidim::Authorization).to receive(:find_or_initialize_by).and_return(authorization)
+              end
+
+              it "does not authorize the user" do
+                expect(Verification::ConfirmUserAuthorization).to receive(:call)
+                subject.call
+              end
+            end
+
+            context "when the authorization is already granted and not expired" do
+              let!(:authorization) { create(:authorization, :granted, user: user, name: :direct_verifications) }
+
+              before do
+                allow(authorization).to receive(:expired?).and_return(false)
+                allow(Decidim::Authorization).to receive(:find_or_initialize_by).and_return(authorization)
+              end
+
+              it "does not authorize the user" do
+                expect(Verification::ConfirmUserAuthorization).not_to receive(:call)
+                subject.call
+              end
+            end
+          end
+
+          context "when the user fails to be authorized" do
+            let(:form) { instance_double(Verification::DirectVerificationsForm, valid?: false) }
+            let(:data) { user.name }
+
+            before do
+              allow(Verification::DirectVerificationsForm)
+                .to receive(:new).with(email: user.email, name: user.name) { form }
+            end
+
+            it "tracks the error" do
+              subject.call
+              expect(instrumenter).to have_received(:add_error).with(:authorized, email)
+            end
+          end
+        end
+
+        context "when authorizing unregistered users" do
+          let(:organization) { build(:organization) }
+          let(:user) { nil }
+          let(:email) { "[email protected]" }
+          let(:data) { "Andy" }
+          let(:session) { {} }
+          let(:instrumenter) { instance_double(UserProcessor, add_processed: true, add_error: true) }
+
+          it "tracks an error" do
+            subject.call
+
+            expect(instrumenter).not_to have_received(:add_processed)
+            expect(instrumenter).to have_received(:add_error).with(:authorized, email)
+          end
+
+          it "does not authorize the user" do
+            expect(Verification::ConfirmUserAuthorization).not_to receive(:call)
+            subject.call
+          end
+        end
+      end
+    end
+  end
+end

spec/lib/decidim/direct_verifications/user_processor_spec.rb

@@ -34,22 +34,24 @@ module DirectVerifications
 
       context "when add processed" do
         it "has unique emails per type" do
-          subject.send(:add_processed, :registered, "[email protected]")
-          subject.send(:add_processed, :registered, "[email protected]")
+          subject.add_processed(:registered, "[email protected]")
+          subject.add_processed(:registered, "[email protected]")
           expect(subject.processed[:registered].count).to eq(1)
-          subject.send(:add_processed, :authorized, "[email protected]")
-          subject.send(:add_processed, :authorized, "[email protected]")
+
+          subject.add_processed(:authorized, "[email protected]")
+          subject.add_processed(:authorized, "[email protected]")
           expect(subject.processed[:authorized].count).to eq(1)
         end
       end
 
       context "when add errors" do
         it "has unique emails per type" do
-          subject.send(:add_error, :registered, "[email protected]")
-          subject.send(:add_error, :registered, "[email protected]")
+          subject.add_error(:registered, "[email protected]")
+          subject.add_error(:registered, "[email protected]")
           expect(subject.errors[:registered].count).to eq(1)
-          subject.send(:add_error, :authorized, "[email protected]")
-          subject.send(:add_error, :authorized, "[email protected]")
+
+          subject.add_error(:authorized, "[email protected]")
+          subject.add_error(:authorized, "[email protected]")
           expect(subject.errors[:authorized].count).to eq(1)
         end
       end
@@ -80,52 +82,57 @@ module DirectVerifications
         end
       end
 
-      context "when authorizing confirmed users" do
-        it "has no errors" do
-          subject.emails = { user.email => user.name }
-          subject.authorize_users
-
-          expect(subject.processed[:authorized].count).to eq(1)
-          expect(subject.errors[:authorized].count).to eq(0)
-        end
+      describe "#authorize_users" do
+        context "when authorizing confirmed users" do
+          before do
+            subject.emails = { user.email => user.name }
+          end
 
-        it "stores user data as authorization metadata" do
-          subject.emails = { user.email => { name: user.name, type: "consumer" } }
-          subject.authorize_users
+          it "has no errors" do
+            subject.authorize_users
 
-          expect(Authorization.last.metadata).to eq("name" => user.name, "type" => "consumer")
+            expect(subject.processed[:authorized].count).to eq(1)
+            expect(subject.errors[:authorized].count).to eq(0)
+          end
         end
-      end
 
-      context "when authorizing unconfirmed users" do
-        it "has no errors" do
-          subject.emails = ["[email protected]"]
-          subject.register_users
-          subject.authorize_users
+        context "when authorizing confirmed users with metadata" do
+          before do
+            subject.emails = { user.email => { name: user.name, type: "consumer" } }
+          end
 
-          expect(subject.processed[:authorized].count).to eq(1)
-          expect(subject.errors[:authorized].count).to eq(0)
+          it "stores user data as authorization metadata" do
+            subject.authorize_users
+            expect(Authorization.last.metadata).to eq("name" => user.name, "type" => "consumer")
+          end
         end
 
-        it "stores user data as authorization metadata" do
-          subject.emails = { "[email protected]" => { type: "consumer" } }
-          subject.register_users
-          subject.authorize_users
+        context "when authorizing unconfirmed users" do
+          before do
+            subject.emails = ["[email protected]"]
+            subject.register_users
+          end
 
-          expect(Decidim::User.find_by(email: "[email protected]").name).to eq("em")
-          expect(Authorization.last.metadata).to eq("type" => "consumer")
-        end
-      end
+          it "has no errors" do
+            subject.authorize_users
 
-      context "when authorizing unregistered users" do
-        before do
-          subject.emails = ["[email protected]"]
-          subject.authorize_users
+            expect(subject.processed[:authorized].count).to eq(1)
+            expect(subject.errors[:authorized].count).to eq(0)
+          end
         end
 
-        it "has errors" do
-          expect(subject.processed[:authorized].count).to eq(0)
-          expect(subject.errors[:authorized].count).to eq(1)
+        context "when authorizing unconfirmed users with metadata" do
+          before do
+            subject.emails = { "[email protected]" => { type: "consumer" } }
+            subject.register_users
+          end
+
+          it "stores user data as authorization metadata" do
+            subject.authorize_users
+
+            expect(Decidim::User.find_by(email: "[email protected]").name).to eq("em")
+            expect(Authorization.last.metadata).to eq("type" => "consumer")
+          end
         end
       end