diff --git a/app/commands/decidim/direct_verifications/verification/create_import.rb b/app/commands/decidim/direct_verifications/verification/create_import.rb new file mode 100644 index 0000000..186523f --- /dev/null +++ b/app/commands/decidim/direct_verifications/verification/create_import.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module Decidim + module DirectVerifications + module Verification + class CreateImport < Rectify::Command + def initialize(form) + @form = form + @file = form.file + @organization = form.organization + @user = form.user + @action = form.action + end + + def call + return broadcast(:invalid) unless form.valid? + + case action + when :register + register_users_async + when :register_and_authorize + register_users_async + file.rewind + authorize_users_async + when :revoke + revoke_users_async + end + + broadcast(:ok) + end + + private + + attr_reader :form, :file, :organization, :user, :action + + def register_users_async + RegisterUsersJob.perform_later(file.read, organization, user, form.authorization_handler) + end + + def revoke_users_async + RevokeUsersJob.perform_later(file.read, organization, user, form.authorization_handler) + end + + def authorize_users_async + AuthorizeUsersJob.perform_later(file.read, organization, user, form.authorization_handler) + end + end + end + end +end diff --git a/app/commands/decidim/direct_verifications/verification/metadata_parser.rb b/app/commands/decidim/direct_verifications/verification/metadata_parser.rb index e5ee82e..e8cb541 100644 --- a/app/commands/decidim/direct_verifications/verification/metadata_parser.rb +++ b/app/commands/decidim/direct_verifications/verification/metadata_parser.rb @@ -9,8 +9,8 @@ class MetadataParser < BaseParser def header @header ||= begin header_row = lines[0].chomp - column_names = tokenize(header_row) - column_names.map(&:to_sym).map(&:downcase) + header_row = tokenize(header_row) + normalize_header(header_row) end end @@ -23,8 +23,8 @@ def parse_data(email, line, header) hash = {} header.each_with_index do |column, index| - value = tokens[index].strip - next if value.include?(email) + value = tokens[index] + next if value&.include?(email) hash[column] = value end @@ -34,7 +34,17 @@ def parse_data(email, line, header) private def tokenize(line) - CSV.parse(line)[0] + CSV.parse_line(line).map do |token| + token&.strip + end + end + + def normalize_header(line) + line.map do |field| + raise MissingHeaderError if field.nil? + + field.to_sym.downcase + end end end end diff --git a/app/commands/decidim/direct_verifications/verification/missing_header_error.rb b/app/commands/decidim/direct_verifications/verification/missing_header_error.rb new file mode 100644 index 0000000..296b39c --- /dev/null +++ b/app/commands/decidim/direct_verifications/verification/missing_header_error.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module Decidim + module DirectVerifications + module Verification + class MissingHeaderError < StandardError + end + end + end +end diff --git a/app/controllers/decidim/direct_verifications/verification/admin/direct_verifications_controller.rb b/app/controllers/decidim/direct_verifications/verification/admin/direct_verifications_controller.rb index 3d1571f..3661732 100644 --- a/app/controllers/decidim/direct_verifications/verification/admin/direct_verifications_controller.rb +++ b/app/controllers/decidim/direct_verifications/verification/admin/direct_verifications_controller.rb @@ -10,6 +10,8 @@ class DirectVerificationsController < Decidim::Admin::ApplicationController layout "decidim/admin/users" + I18N_SCOPE = "decidim.direct_verifications.verification.admin.direct_verifications" + def index enforce_permission_to :index, :authorization end @@ -18,29 +20,39 @@ def create enforce_permission_to :create, :authorization @userslist = params[:userslist] - @processor = UserProcessor.new(current_organization, current_user, session) + + @processor = UserProcessor.new(current_organization, current_user, session, instrumenter) @processor.emails = parser_class.new(@userslist).to_h @processor.authorization_handler = current_authorization_handler + @stats = UserStats.new(current_organization) @stats.authorization_handler = @processor.authorization_handler + register_users authorize_users revoke_users render(action: :index) && return if show_users_info + redirect_to direct_verifications_path + rescue MissingHeaderError => _e + flash[:error] = I18n.t("#{I18N_SCOPE}.create.missing_header") redirect_to direct_verifications_path end private + def instrumenter + @instrumenter ||= Instrumenter.new(current_user) + end + def register_users return unless params[:register] @processor.register_users flash[:warning] = t(".registered", count: @processor.emails.count, - registered: @processor.processed[:registered].count, - errors: @processor.errors[:registered].count) + registered: instrumenter.processed_count(:registered), + errors: instrumenter.errors_count(:registered)) end def authorize_users @@ -49,8 +61,8 @@ def authorize_users @processor.authorize_users flash[:notice] = t(".authorized", handler: t("#{@processor.authorization_handler}.name", scope: "decidim.authorization_handlers"), count: @processor.emails.count, - authorized: @processor.processed[:authorized].count, - errors: @processor.errors[:authorized].count) + authorized: instrumenter.processed_count(:authorized), + errors: instrumenter.errors_count(:authorized)) end def revoke_users @@ -59,8 +71,8 @@ def revoke_users @processor.revoke_users flash[:notice] = t(".revoked", handler: t("#{@processor.authorization_handler}.name", scope: "decidim.authorization_handlers"), count: @processor.emails.count, - revoked: @processor.processed[:revoked].count, - errors: @processor.errors[:revoked].count) + revoked: instrumenter.processed_count(:revoked), + errors: instrumenter.errors_count(:revoked)) end def show_users_info diff --git a/app/controllers/decidim/direct_verifications/verification/admin/imports_controller.rb b/app/controllers/decidim/direct_verifications/verification/admin/imports_controller.rb new file mode 100644 index 0000000..8f6321d --- /dev/null +++ b/app/controllers/decidim/direct_verifications/verification/admin/imports_controller.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +module Decidim + module DirectVerifications + module Verification + module Admin + class ImportsController < Decidim::Admin::ApplicationController + layout "decidim/admin/users" + helper_method :workflows, :current_authorization_handler + + def new + enforce_permission_to :create, :authorization + @form = form(CreateImportForm).instance + end + + def create + enforce_permission_to :create, :authorization + + defaults = { organization: current_organization, user: current_user } + form = form(CreateImportForm).from_params(params.merge(defaults)) + + CreateImport.call(form) do + on(:ok) do + flash[:notice] = t(".success") + end + + on(:invalid) do + flash[:alert] = t(".error") + end + end + + redirect_to new_import_path + end + + def workflows + workflows = configured_workflows & current_organization.available_authorizations.map.to_a + workflows.map do |workflow| + [t("#{workflow}.name", scope: "decidim.authorization_handlers"), workflow] + end + end + + def configured_workflows + return Decidim::DirectVerifications.config.manage_workflows if Decidim::DirectVerifications.config + + ["direct_verifications"] + end + + def current_authorization_handler + params[:authorization_handler] + end + end + end + end + end +end diff --git a/app/forms/decidim/direct_verifications/verification/create_import_form.rb b/app/forms/decidim/direct_verifications/verification/create_import_form.rb new file mode 100644 index 0000000..a68bf6b --- /dev/null +++ b/app/forms/decidim/direct_verifications/verification/create_import_form.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module Decidim + module DirectVerifications + module Verification + class CreateImportForm < Form + ACTIONS = { + "in" => :authorize, + "out" => :revoke, + "check" => :check + }.freeze + + attribute :file + attribute :organization, Decidim::Organization + attribute :user, Decidim::User + attribute :authorize, String + attribute :register, Boolean + attribute :authorization_handler, String + + validates :file, :organization, :user, :authorize, :authorization_handler, presence: true + validates :authorize, inclusion: { in: ACTIONS.keys } + + validate :available_authorization_handler + + def available_authorization_handler + return if authorization_handler.in?(organization.available_authorizations) + + errors.add(:authorization_handler, :inclusion) + end + + def action + if register && authorize == "in" + :register_and_authorize + elsif register + :register + else + ACTIONS[authorize] + end + end + end + end + end +end diff --git a/app/jobs/decidim/direct_verifications/authorize_users_job.rb b/app/jobs/decidim/direct_verifications/authorize_users_job.rb new file mode 100644 index 0000000..8f68c24 --- /dev/null +++ b/app/jobs/decidim/direct_verifications/authorize_users_job.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require "decidim/direct_verifications/instrumenter" + +module Decidim + module DirectVerifications + class AuthorizeUsersJob < BaseImportJob + class NullSession; end + + def process_users + emails.each do |email, data| + AuthorizeUser.new( + email, + data, + session, + organization, + instrumenter, + authorization_handler + ).call + end + end + + def type + :authorized + end + + private + + def session + NullSession.new + end + end + end +end diff --git a/app/jobs/decidim/direct_verifications/base_import_job.rb b/app/jobs/decidim/direct_verifications/base_import_job.rb new file mode 100644 index 0000000..5fc8c1a --- /dev/null +++ b/app/jobs/decidim/direct_verifications/base_import_job.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require "decidim/direct_verifications/instrumenter" + +module Decidim + module DirectVerifications + # This class implements the logic to import the user entries and sending an email notification + # with the result. The specifics to process the entries are meant to be implemented by + # subclasses which must implement the `#process_users` and `#type` methods. + class BaseImportJob < ApplicationJob + queue_as :default + + def perform(userslist, organization, current_user, authorization_handler) + @emails = Verification::MetadataParser.new(userslist).to_h + @organization = organization + @current_user = current_user + @instrumenter = Instrumenter.new(current_user) + @authorization_handler = authorization_handler + + process_users + send_email_notification + end + + private + + attr_reader :emails, :organization, :current_user, :instrumenter, :authorization_handler + + def send_email_notification + ImportMailer.finished_processing( + current_user, + instrumenter, + type, + authorization_handler + ).deliver_now + end + end + end +end diff --git a/app/jobs/decidim/direct_verifications/register_users_job.rb b/app/jobs/decidim/direct_verifications/register_users_job.rb new file mode 100644 index 0000000..8386087 --- /dev/null +++ b/app/jobs/decidim/direct_verifications/register_users_job.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require "decidim/direct_verifications/instrumenter" + +module Decidim + module DirectVerifications + class RegisterUsersJob < BaseImportJob + def process_users + emails.each do |email, data| + RegisterUser.new(email, data, organization, current_user, instrumenter).call + end + end + + def type + :registered + end + end + end +end diff --git a/app/jobs/decidim/direct_verifications/revoke_users_job.rb b/app/jobs/decidim/direct_verifications/revoke_users_job.rb new file mode 100644 index 0000000..b4ae3a5 --- /dev/null +++ b/app/jobs/decidim/direct_verifications/revoke_users_job.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Decidim + module DirectVerifications + class RevokeUsersJob < BaseImportJob + def process_users + emails.each do |email, _name| + RevokeUser.new(email, organization, instrumenter, authorization_handler).call + end + end + + def type + :revoked + end + end + end +end diff --git a/app/mailers/decidim/direct_verifications/import_mailer.rb b/app/mailers/decidim/direct_verifications/import_mailer.rb new file mode 100644 index 0000000..beaf80d --- /dev/null +++ b/app/mailers/decidim/direct_verifications/import_mailer.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Decidim + module DirectVerifications + class ImportMailer < Decidim::Admin::ApplicationMailer + include LocalisedMailer + + layout "decidim/mailer" + + I18N_SCOPE = "decidim.direct_verifications.verification.admin.imports.mailer" + + def finished_processing(user, instrumenter, type, handler) + @stats = Stats.from(instrumenter, type) + @organization = user.organization + @i18n_key = "#{I18N_SCOPE}.#{type}" + @handler = handler + + with_user(user) do + mail( + to: user.email, + subject: I18n.t("#{I18N_SCOPE}.subject") + ) + end + end + end + end +end diff --git a/app/mailers/decidim/direct_verifications/stats.rb b/app/mailers/decidim/direct_verifications/stats.rb new file mode 100644 index 0000000..d8e1869 --- /dev/null +++ b/app/mailers/decidim/direct_verifications/stats.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Decidim + module DirectVerifications + class Stats + attr_reader :count, :successful, :errors + + def self.from(instrumenter, type) + new( + count: instrumenter.emails_count(type), + successful: instrumenter.processed_count(type), + errors: instrumenter.errors_count(type) + ) + end + + def initialize(count:, successful:, errors:) + @count = count + @successful = successful + @errors = errors + end + end + end +end diff --git a/app/views/decidim/direct_verifications/import_mailer/finished_processing.html.erb b/app/views/decidim/direct_verifications/import_mailer/finished_processing.html.erb new file mode 100644 index 0000000..7c0eefe --- /dev/null +++ b/app/views/decidim/direct_verifications/import_mailer/finished_processing.html.erb @@ -0,0 +1,7 @@ +

<%= t( + @i18n_key, + count: @stats.count, + successful: @stats.successful, + errors: @stats.errors, + handler: @handler +) %>

diff --git a/app/views/decidim/direct_verifications/import_mailer/finished_processing.text.erb b/app/views/decidim/direct_verifications/import_mailer/finished_processing.text.erb new file mode 100644 index 0000000..8f239fa --- /dev/null +++ b/app/views/decidim/direct_verifications/import_mailer/finished_processing.text.erb @@ -0,0 +1,7 @@ +<%= t( + @i18n_key, + count: @stats.count, + successful: @stats.successful, + errors: @stats.errors, + handler: @handler +) %> diff --git a/app/views/decidim/direct_verifications/verification/admin/direct_verifications/index.html.erb b/app/views/decidim/direct_verifications/verification/admin/direct_verifications/index.html.erb index 79d9dbb..7154c54 100644 --- a/app/views/decidim/direct_verifications/verification/admin/direct_verifications/index.html.erb +++ b/app/views/decidim/direct_verifications/verification/admin/direct_verifications/index.html.erb @@ -7,7 +7,7 @@
-

<%= t("decidim.direct_verifications.verification.admin.new.info") %>

+

<%= t("decidim.direct_verifications.verification.admin.new.info_html", link: new_import_path) %>

<%= form_tag direct_verifications_path, multipart: true, class: "form" do %> <%= label_tag :userslist, t("admin.new.textarea", scope: "decidim.direct_verifications.verification") %> <%= text_area_tag :userslist, @userslist, rows: 10 %> diff --git a/app/views/decidim/direct_verifications/verification/admin/imports/new.html.erb b/app/views/decidim/direct_verifications/verification/admin/imports/new.html.erb new file mode 100644 index 0000000..60964d8 --- /dev/null +++ b/app/views/decidim/direct_verifications/verification/admin/imports/new.html.erb @@ -0,0 +1,50 @@ +
+
+

+ <%= t("admin.index.title", scope: "decidim.direct_verifications.verification") %> + <%= link_to t("admin.index.stats", scope: "decidim.direct_verifications.verification"), stats_path, class: "button tiny button--title" %> + <%= link_to t("admin.index.authorizations", scope: "decidim.direct_verifications.verification"), authorizations_path, class: "button tiny button--title" %> +

+
+
+

<%= t("decidim.direct_verifications.verification.admin.imports.new.info") %>

+ 
+      email, name, membership_phone, membership_type, membership_weight
+      ava@example.com, Ava Hawkins, +3476318371, consumer, 1
+      ewan@example.com, Ewan Cooper, +34653565765, worker, 1
+      tilly@example.com, Tilly Jones, +34653565761, collaborator, 0.67
+      ...
+
+ + <%= decidim_form_for(@form, url: imports_path, html: { class: "form" }) do |form| %> + + + + + + <%= label_tag :authorization_handler, t("admin.new.authorization_handler", scope: "decidim.direct_verifications.verification") %> + <%= select_tag :authorization_handler, options_for_select(workflows, current_authorization_handler) %> + + <%= form.file_field :file, label: t(".file") %> + <%= form.submit t(".submit"), class: "button" %> + <% end %> + +
+
+ +<%= javascript_include_tag "decidim/direct_verifications/verification/admin/direct_verifications_admin" %> diff --git a/config/initializers/mail_previews.rb b/config/initializers/mail_previews.rb new file mode 100644 index 0000000..1b218c2 --- /dev/null +++ b/config/initializers/mail_previews.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +Rails.application.configure do + config.action_mailer.preview_path = Decidim::DirectVerifications::Verification::Engine.root.join("spec/mailers") +end diff --git a/config/locales/ca.yml b/config/locales/ca.yml index 8bed42c..8bf0b1f 100644 --- a/config/locales/ca.yml +++ b/config/locales/ca.yml @@ -32,6 +32,7 @@ ca: create: authorized: "S'han verificat correctament %{authorized} usuaris utilitzant [%{handler}] (%{count} detectats, %{errors} errors)" info: "S'han detectat %{count} usuaris, dels quals %{registered} estan registrats, %{authorized} autoritzats utilitzant [%{handler}] (%{unconfirmed} sense confirmar)" + missing_header: Si us plau, proporcioneu una fila d'encapçalament registered: "S'han registrat correctament %{registered} usuaris (%{count} detectats, %{errors} errors)" revoked: S'ha revocat correctament la verificació de %{revoked} usuaris utilitzant [%{handler}] (%{count} detectats, %{errors} errors) gdpr_disclaimer: Feu-ho sota la vostra responsabilitat. Recordeu que heu de tenir el consentiment explícit dels vostres usuaris per registrar-los. En cas contrari, estareu infringint la regulació GDPR als països de la UE. diff --git a/config/locales/en.yml b/config/locales/en.yml index 695f652..56328e3 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -21,6 +21,24 @@ en: direct_verifications: verification: admin: + imports: + new: + info: Import a CSV file with a user entry per line copying the format from the example below + submit: Upload file + file: CSV file with users data + create: + success: File successfully uploaded. We'll email you when all users are imported. + error: There was an error importing the file + mailer: + subject: File import results + authorized: "%{successful} users have been successfully verified using + [%{handler}] (%{count} detected, %{errors} errors)" + info: "%{count} users detected, of which %{registered} are registered, + %{authorized} authorized using [%{handler}] (%{unconfirmed} unconfirmed)" + registered: "%{successful} users have been successfully registered (%{count} + detected, %{errors} errors) " + revoked: Verification from %{successful} users have been revoked using + [%{handler}] (%{count} detected, %{errors} errors) authorizations: index: created_at: Created at @@ -35,6 +53,7 @@ en: [%{handler}] (%{count} detected, %{errors} errors)" info: "%{count} users detected, of which %{registered} are registered, %{authorized} authorized using [%{handler}] (%{unconfirmed} unconfirmed)" + missing_header: Please, provide a header row registered: "%{registered} users have been successfully registered (%{count} detected, %{errors} errors) " revoked: Verification from %{revoked} users have been revoked using @@ -50,8 +69,7 @@ en: authorization_handler: Verification method authorize: Authorize users check: Check users status - info: Enter the emails here, one per line. If the emails are preceded - by a text, it will be interpreted as the user's name + info_html: You can import a CSV or enter the emails here, one per line. If the emails are preceded by a text, it will be interpreted as the user's name. register: Register users in the platform (if they exist they will be ignored) revoke: Revoke authorization from users submit: Send and process the list diff --git a/config/locales/es.yml b/config/locales/es.yml index ab7086d..92bc943 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -32,6 +32,7 @@ es: create: authorized: "Se han verificado correctamente %{authorized} usuarios usando [%{handler}] (%{count} detectados, %{errors} errores)" info: "Se han detectado %{count} usuarios, de los cuales %{registered} están registrados, %{authorized} autorizados usando [%{handler}] (%{unconfirmed} sin confirmar)" + missing_header: Por favor, proporcionad una fila de encabezamiento registered: "Se han registrado correctamente %{registered} usuarios (%{count} detectados, %{errors} errores)" revoked: Se ha revocado correctament la verificación de %{revoked} usuarios usando [%{handler}] (%{count} detectados, %{errors} errores) gdpr_disclaimer: Haga esto bajo su responsabilidad. Recuerde que debe contar con el consentimiento explícito de sus usuarios para poder registrarlos. De lo contrario, estará infringiendo la regulación GDPR en los países de la UE. diff --git a/lib/decidim/direct_verifications/authorize_user.rb b/lib/decidim/direct_verifications/authorize_user.rb index 0b430a9..f6e4124 100644 --- a/lib/decidim/direct_verifications/authorize_user.rb +++ b/lib/decidim/direct_verifications/authorize_user.rb @@ -3,13 +3,16 @@ module Decidim module DirectVerifications class AuthorizeUser - def initialize(email, data, session, organization, instrumenter) + # rubocop:disable Metrics/ParameterLists + def initialize(email, data, session, organization, instrumenter, authorization_handler) @email = email @data = data @session = session @organization = organization @instrumenter = instrumenter + @authorization_handler = authorization_handler end + # rubocop:enable Metrics/ParameterLists def call unless user @@ -31,7 +34,7 @@ def call private - attr_reader :email, :data, :session, :organization, :instrumenter + attr_reader :email, :data, :session, :organization, :instrumenter, :authorization_handler def valid_authorization? !authorization.granted? || authorization.expired? @@ -46,7 +49,7 @@ def authorization begin auth = Authorization.find_or_initialize_by( user: user, - name: :direct_verifications + name: authorization_handler ) auth.metadata = data auth diff --git a/lib/decidim/direct_verifications/instrumenter.rb b/lib/decidim/direct_verifications/instrumenter.rb new file mode 100644 index 0000000..2b8c0e5 --- /dev/null +++ b/lib/decidim/direct_verifications/instrumenter.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +module Decidim + module DirectVerifications + class Instrumenter + def initialize(current_user) + @current_user = current_user + @errors = { registered: Set.new, authorized: Set.new, revoked: Set.new } + @processed = { registered: Set.new, authorized: Set.new, revoked: Set.new } + end + + def add_processed(type, email) + @processed[type] << email + end + + def add_error(type, email) + @errors[type] << email + end + + def processed_count(key) + processed[key].size + end + + def errors_count(key) + errors[key].size + end + + def emails_count(key) + @processed[key].size + @errors[key].size + end + + def track(event, email, user = nil) + if user + add_processed event, email + log_action user + else + add_error event, email + end + end + + private + + attr_reader :current_user, :processed, :errors + + def log_action(user) + Decidim.traceability.perform_action!( + "invite", + user, + current_user, + extra: { + invited_user_role: "participant", + invited_user_id: user.id + } + ) + end + end + end +end diff --git a/lib/decidim/direct_verifications/register_user.rb b/lib/decidim/direct_verifications/register_user.rb index 018283e..1bd218a 100644 --- a/lib/decidim/direct_verifications/register_user.rb +++ b/lib/decidim/direct_verifications/register_user.rb @@ -3,20 +3,20 @@ module Decidim module DirectVerifications class RegisterUser - def initialize(email, name, organization, current_user, instrumenter) + def initialize(email, data, organization, current_user, instrumenter) @email = email - @name = name + @name = data.is_a?(Hash) ? data[:name] : data @organization = organization @current_user = current_user @instrumenter = instrumenter end def call - return if find_user + return if user InviteUser.call(form) do on(:ok) do - instrumenter.track(:registered, email, find_user) + instrumenter.track(:registered, email, user) end on(:invalid) do instrumenter.track(:registered, email) @@ -31,8 +31,8 @@ def call attr_reader :email, :name, :organization, :current_user, :instrumenter - def find_user - User.find_by(email: email, decidim_organization_id: organization.id) + def user + @user ||= User.find_by(email: email, decidim_organization_id: organization.id) end def form diff --git a/lib/decidim/direct_verifications/revoke_user.rb b/lib/decidim/direct_verifications/revoke_user.rb new file mode 100644 index 0000000..aaabc8c --- /dev/null +++ b/lib/decidim/direct_verifications/revoke_user.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module Decidim + module DirectVerifications + class RevokeUser + def initialize(email, organization, instrumenter, authorization_handler) + @email = email + @organization = organization + @instrumenter = instrumenter + @authorization_handler = authorization_handler + end + + def call + unless user + instrumenter.add_error :revoked, email + return + end + + return unless valid_authorization? + + Verification::DestroyUserAuthorization.call(authorization) do + on(:ok) do + instrumenter.add_processed :revoked, email + end + end + end + + private + + attr_reader :email, :organization, :instrumenter, :authorization_handler + + def user + @user ||= User.find_by(email: email, decidim_organization_id: organization.id) + end + + def authorization + @authorization ||= Authorization.find_by(user: user, name: authorization_handler) + end + + def valid_authorization? + authorization&.granted? + end + end + end +end diff --git a/lib/decidim/direct_verifications/user_processor.rb b/lib/decidim/direct_verifications/user_processor.rb index bf34dc6..e8c528b 100644 --- a/lib/decidim/direct_verifications/user_processor.rb +++ b/lib/decidim/direct_verifications/user_processor.rb @@ -2,18 +2,20 @@ require "decidim/direct_verifications/register_user" require "decidim/direct_verifications/authorize_user" +require "decidim/direct_verifications/revoke_user" +require "decidim/direct_verifications/instrumenter" module Decidim module DirectVerifications class UserProcessor - def initialize(organization, current_user, session) + def initialize(organization, current_user, session, instrumenter) @organization = organization @current_user = current_user @authorization_handler = :direct_verifications - @errors = { registered: [], authorized: [], revoked: [] } - @processed = { registered: [], authorized: [], revoked: [] } + @emails = {} @session = session + @instrumenter = instrumenter end attr_reader :organization, :current_user, :session, :errors, :processed @@ -21,82 +23,25 @@ def initialize(organization, current_user, session) def register_users emails.each do |email, data| - name = if data.is_a?(Hash) - data[:name] - else - data - end - RegisterUser.new(email, name, organization, current_user, self).call + RegisterUser.new(email, data, organization, current_user, instrumenter).call end end def authorize_users emails.each do |email, data| - AuthorizeUser.new(email, data, session, organization, self).call + AuthorizeUser.new(email, data, session, organization, instrumenter, authorization_handler).call end end 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, instrumenter, authorization_handler).call end end - def track(event, email, user = nil) - if user - add_processed event, email - log_action user - else - add_error event, email - end - end - - def add_error(type, email) - @errors[type] << email unless @errors[type].include? email - end - - def add_processed(type, email) - @processed[type] << email unless @processed[type].include? email - end - private - def log_action(user) - Decidim.traceability.perform_action!( - "invite", - user, - current_user, - extra: { - invited_user_role: "participant", - invited_user_id: user.id - } - ) - 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 + attr_reader :instrumenter end end end diff --git a/lib/decidim/direct_verifications/verification/admin_engine.rb b/lib/decidim/direct_verifications/verification/admin_engine.rb index d51c9a5..e6159c4 100644 --- a/lib/decidim/direct_verifications/verification/admin_engine.rb +++ b/lib/decidim/direct_verifications/verification/admin_engine.rb @@ -11,6 +11,7 @@ class AdminEngine < ::Rails::Engine resources :direct_verifications, only: [:index, :create, :stats] resources :stats, only: [:index] resources :authorizations, only: [:index, :destroy] + resources :imports, only: [:new, :create] root to: "direct_verifications#index" end diff --git a/spec/commands/decidim/direct_verifications/verification/create_import_spec.rb b/spec/commands/decidim/direct_verifications/verification/create_import_spec.rb new file mode 100644 index 0000000..44f36e2 --- /dev/null +++ b/spec/commands/decidim/direct_verifications/verification/create_import_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require "spec_helper" + +module Decidim + module DirectVerifications + module Verification + describe CreateImport do + subject(:command) { described_class.new(form) } + + let(:form) do + instance_double( + CreateImportForm, + file: file, + organization: organization, + user: user, + action: action, + authorization_handler: "direct_verifications" + ) + end + let(:filename) { file_fixture("users.csv") } + let(:file) { Rack::Test::UploadedFile.new(filename, "text/csv") } + let(:organization) { build(:organization) } + let(:user) { build(:user) } + let(:action) { :register } + + before do + allow(RegisterUsersJob).to receive(:perform_later) + allow(RevokeUsersJob).to receive(:perform_later) + end + + context "when the form is valid" do + before do + allow(form).to receive(:valid?).and_return(true) + end + + context "when the action is register" do + let(:action) { :register } + + it "calls the RegisterUsersJob job" do + command.call + expect(RegisterUsersJob).to have_received(:perform_later) + end + + it "does not call the RevokeUsersJob job" do + command.call + expect(RevokeUsersJob).not_to have_received(:perform_later) + end + + it "broadcasts ok" do + expect { command.call }.to broadcast(:ok) + end + end + + context "when the action is revoke" do + let(:action) { :revoke } + + it "does not call the RegisterUsersJob job" do + command.call + expect(RegisterUsersJob).not_to have_received(:perform_later) + end + + it "calls the RevokeUsersJob job" do + command.call + expect(RevokeUsersJob).to have_received(:perform_later) + end + + it "broadcasts ok" do + expect { command.call }.to broadcast(:ok) + end + end + end + + context "when the form is not valid" do + before do + allow(form).to receive(:valid?).and_return(false) + end + + it "does not call the RegisterUsersJob job" do + command.call + expect(RegisterUsersJob).not_to have_received(:perform_later) + end + + it "broadcasts ok" do + expect { command.call }.to broadcast(:invalid) + end + end + end + end + end +end diff --git a/spec/commands/decidim/direct_verifications/verification/metadata_parser_spec.rb b/spec/commands/decidim/direct_verifications/verification/metadata_parser_spec.rb index ac154ea..d987966 100644 --- a/spec/commands/decidim/direct_verifications/verification/metadata_parser_spec.rb +++ b/spec/commands/decidim/direct_verifications/verification/metadata_parser_spec.rb @@ -6,6 +6,16 @@ module Decidim::DirectVerifications::Verification describe MetadataParser do subject { described_class.new(txt) } + describe "#header" do + context "when the first line has empty columns" do + let(:txt) { "Melina.morrison@bccm.coop,MORRISON Melina,,,11" } + + it "raises an error" do + expect { subject.to_h }.to raise_error(MissingHeaderError) + end + end + end + describe "#to_h" do context "when the text is empty" do let(:txt) { "" } @@ -29,6 +39,21 @@ module Decidim::DirectVerifications::Verification end end + context "when the text has empty columns" do + let(:txt) do + <<-EMAILS.strip_heredoc + Name,Email,Department,Salary + Bob,bob@example.com,,1000 + EMAILS + end + + it "skips those columns" do + expect(subject.to_h).to eq( + "bob@example.com" => { department: nil, name: "Bob", salary: "1000" } + ) + end + end + context "when the text is a CSV with headers" do let(:txt) do <<-ROWS.strip_heredoc @@ -68,6 +93,21 @@ module Decidim::DirectVerifications::Verification end end + context "and the CSV includes trailing or leading whitespaces" do + let(:txt) do + <<-CSV.strip_heredoc + Name, Email, Type + Ava Hawkins, ava@example.com, collaborator + CSV + end + + it "returns the data in a hash with the email as key" do + expect(subject.to_h).to eq( + "ava@example.com" => { name: "Ava Hawkins", type: "collaborator" } + ) + end + end + context "when the name is not specified" do let(:txt) do <<-EMAILS.strip_heredoc diff --git a/spec/controllers/decidim/direct_verifications/verification/admin/direct_verifications_controller_spec.rb b/spec/controllers/decidim/direct_verifications/verification/admin/direct_verifications_controller_spec.rb index c4b4fdd..7ce1f70 100644 --- a/spec/controllers/decidim/direct_verifications/verification/admin/direct_verifications_controller_spec.rb +++ b/spec/controllers/decidim/direct_verifications/verification/admin/direct_verifications_controller_spec.rb @@ -14,6 +14,8 @@ module Decidim::DirectVerifications::Verification::Admin let(:verification_type) { "direct_verifications" } let(:authorized_user) { create(:user, email: "authorized@example.com", organization: organization) } + let(:i18n_scope) { "decidim.direct_verifications.verification.admin.direct_verifications" } + before do request.env["decidim.current_organization"] = user.organization sign_in user, scope: :user @@ -32,10 +34,13 @@ module Decidim::DirectVerifications::Verification::Admin describe "POST create" do context "when parameters are defaults" do params = { userslist: "" } + it_behaves_like "checking users", params + it "have no registered or authorized users" do perform_enqueued_jobs do post :create, params: params + expect(flash[:info]).to include("0 are registered") expect(flash[:info]).to include("0 authorized") expect(flash[:info]).to include("unconfirmed") @@ -62,7 +67,9 @@ module Decidim::DirectVerifications::Verification::Admin it "renders the index with warning message" do perform_enqueued_jobs do post :create, params: params - expect(subject).to render_template("decidim/direct_verifications/verification/admin/direct_verifications/index") + expect(subject).to render_template( + "decidim/direct_verifications/verification/admin/direct_verifications/index" + ) end end end @@ -81,12 +88,10 @@ module Decidim::DirectVerifications::Verification::Admin end context "when the name is not specified" do + let(:data) { "Name,Email,Type\r\n\"\",brandy@example.com,consumer" } + it "infers the name from the email" do - post :create, params: { - userslist: "Name,Email,Type\r\n\"\",brandy@example.com,consumer", - register: true, - authorize: "in" - } + post :create, params: { userslist: data, register: true, authorize: "in" } user = Decidim::User.find_by(email: "brandy@example.com") expect(user.name).to eq("brandy") @@ -94,6 +99,10 @@ module Decidim::DirectVerifications::Verification::Admin end context "when in metadata mode" do + let(:data) do + "Name,Email,Type\r\nBrandy,brandy@example.com,consumer,2\r\nWhisky,whisky@example.com,producer,3" + end + around do |example| original_processor = Rails.configuration.direct_verifications_parser Rails.configuration.direct_verifications_parser = :metadata @@ -102,36 +111,69 @@ module Decidim::DirectVerifications::Verification::Admin end it "stores any extra columns as authorization metadata" do - post :create, params: { - userslist: "Name,Email,Type\r\nBrandy,brandy@example.com,consumer,2\r\nWhisky,whisky@example.com,producer,3", - register: true, - authorize: "in" - } + post :create, params: { userslist: data, register: true, authorize: "in" } user = Decidim::User.find_by(email: "brandy@example.com") authorization = Decidim::Authorization.find_by(decidim_user_id: user.id) expect(authorization.metadata).to eq("name" => "Brandy", "type" => "consumer") end + context "when a column is empty" do + let(:data) { "Name,Email,Type,City\r\nBrandy,brandy@example.com,,Barcelona" } + + it "sets it nil" do + post :create, params: { userslist: data, register: true, authorize: "in" } + + user = Decidim::User.find_by(email: "brandy@example.com") + authorization = Decidim::Authorization.find_by(decidim_user_id: user.id) + + expect(authorization.metadata) + .to eq("name" => "Brandy", "type" => nil, "city" => "Barcelona") + end + end + context "when the name is not specified" do + let(:data) { "Name,Email,Type\r\n\"\",brandy@example.com,consumer" } + it "infers the name from the email" do - post :create, params: { - userslist: "Name,Email,Type\r\n\"\",brandy@example.com,consumer", - register: true, - authorize: "in" - } + post :create, params: { userslist: data, register: true, authorize: "in" } user = Decidim::User.find_by(email: "brandy@example.com") expect(user.name).to eq("brandy") end end + + context "when the first row has empty columns" do + let(:data) { "brandy@example.com,,consumer" } + + it "shows a user error" do + post :create, params: { userslist: data, register: true, authorize: "in" } + expect(flash[:error]).to eq(I18n.t("#{i18n_scope}.create.missing_header")) + end + end + + context "when no header is provided" do + let(:data) { "brandy@example.com,consumer" } + + it "works" do + post :create, params: { userslist: data, register: true, authorize: "in" } + expect(response).to redirect_to(direct_verifications_path) + end + end end end - context "when register users with revoke" do + context "when revoking users" do params = { userslist: "authorized@example.com", authorize: "out" } it_behaves_like "revoking users", params end + + context "when revoking users with another verification" do + params = { userslist: "authorized@example.com", authorize: "out", authorization_handler: "other" } + let(:verification_type) { "other" } + + it_behaves_like "revoking users", params + end end end end diff --git a/spec/controllers/decidim/direct_verifications/verification/admin/imports_controller_spec.rb b/spec/controllers/decidim/direct_verifications/verification/admin/imports_controller_spec.rb new file mode 100644 index 0000000..e745457 --- /dev/null +++ b/spec/controllers/decidim/direct_verifications/verification/admin/imports_controller_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require "spec_helper" + +module Decidim::DirectVerifications::Verification::Admin + describe ImportsController, type: :controller do + routes { Decidim::DirectVerifications::Verification::AdminEngine.routes } + + let(:organization) { create(:organization) } + let(:user) { create(:user, :admin, :confirmed, organization: organization) } + + before do + request.env["decidim.current_organization"] = organization + sign_in user + end + + describe "#new" do + it "authorizes the action" do + expect(controller).to receive(:allowed_to?).with(:create, :authorization, {}) + get :new + end + + it "renders the decidim/admin/users layout" do + get :new + expect(response).to render_template("layouts/decidim/admin/users") + end + end + + describe "#create" do + let(:filename) { file_fixture("users.csv") } + let(:file) { Rack::Test::UploadedFile.new(filename, "text/csv") } + + context "when the import is valid" do + it "authorizes the action" do + expect(controller).to receive(:allowed_to?).with(:create, :authorization, {}) + post :create, params: { file: file } + end + + it "redirects to :new" do + post :create, params: { file: file } + expect(response).to redirect_to(new_import_path) + end + end + + context "when the import is not valid" do + it "authorizes the action" do + expect(controller).to receive(:allowed_to?).with(:create, :authorization, {}) + post :create, params: {} + end + + it "redirects to :new" do + post :create, params: {} + expect(response).to redirect_to(new_import_path) + end + + it "displays an error" do + post :create, params: {} + expect(flash[:alert]).to eq(I18n.t("decidim.direct_verifications.verification.admin.imports.create.error")) + end + end + end + end +end diff --git a/spec/fixtures/files/users.csv b/spec/fixtures/files/users.csv new file mode 100644 index 0000000..32ff5ec --- /dev/null +++ b/spec/fixtures/files/users.csv @@ -0,0 +1,2 @@ +Name, Email, Type +Brandy, brandy@example.com, consumer diff --git a/spec/forms/decidim/direct_verifications/verification/create_import_form_spec.rb b/spec/forms/decidim/direct_verifications/verification/create_import_form_spec.rb new file mode 100644 index 0000000..b4e8c4a --- /dev/null +++ b/spec/forms/decidim/direct_verifications/verification/create_import_form_spec.rb @@ -0,0 +1,139 @@ +# frozen_string_literal: true + +require "spec_helper" + +module Decidim + module DirectVerifications + module Verification + describe CreateImportForm do + subject(:form) { described_class.from_params(attributes) } + + let(:organization) { build(:organization, available_authorizations: %w(direct_verifications)) } + let(:attributes) do + { + user: build(:user), + organization: organization, + file: double(File), + authorize: action, + authorization_handler: "direct_verifications" + } + end + let(:action) { "in" } + + context "when all attributes are provided" do + it { is_expected.to be_valid } + end + + context "when :register is provided" do + let(:attributes) do + { + user: build(:user), + organization: build(:organization), + file: double(File), + authorize: action, + register: "1" + } + end + + context "when action is in" do + let(:action) { "in" } + + it "returns it as :register_and_authorize" do + expect(form.action).to eq(:register_and_authorize) + end + end + + context "when action is out" do + let(:action) { "out" } + + it "returns it as :register" do + expect(form.action).to eq(:register) + end + end + + context "when action is check" do + let(:action) { "check" } + + it "returns it as :register" do + expect(form.action).to eq(:register) + end + end + + context "when action is unknown" do + let(:action) { "dummy" } + + it { is_expected.not_to be_valid } + end + end + + context "when :register is not provided" do + let(:attributes) do + { + user: build(:user), + organization: build(:organization), + file: double(File), + authorize: action + } + end + + context "when action is in" do + let(:action) { "in" } + + it "returns it as :authorize" do + expect(form.action).to eq(:authorize) + end + end + + context "when action is out" do + let(:action) { "out" } + + it "returns it as :revoke" do + expect(form.action).to eq(:revoke) + end + end + + context "when action is check" do + let(:action) { "check" } + + it "returns it as :check" do + expect(form.action).to eq(:check) + end + end + + context "when action is unknown" do + let(:action) { "dummy" } + + it { is_expected.not_to be_valid } + end + end + + context "when authorization handler is not provided" do + let(:attributes) do + { + user: build(:user), + organization: build(:organization), + file: double(File), + authorize: action + } + end + + it { is_expected.not_to be_valid } + end + + context "when authorization handler is not known" do + let(:attributes) do + { + user: build(:user), + organization: build(:organization), + file: double(File), + authorize: action, + authorization_handler: "unknown" + } + end + + it { is_expected.not_to be_valid } + end + end + end + end +end diff --git a/spec/jobs/decidim/direct_verifications/register_users_job_spec.rb b/spec/jobs/decidim/direct_verifications/register_users_job_spec.rb new file mode 100644 index 0000000..3e59d5b --- /dev/null +++ b/spec/jobs/decidim/direct_verifications/register_users_job_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require "spec_helper" + +module Decidim + module DirectVerifications + describe RegisterUsersJob, type: :job do + let(:userslist) { "Name,Email,Type\r\n\"\",brandy@example.com,consumer" } + let(:organization) { create(:organization) } + let!(:current_user) { create(:user, organization: organization) } + let(:mailer) { double(:mailer, deliver_now: true) } + + around do |example| + perform_enqueued_jobs { example.run } + end + + before do + allow(ImportMailer) + .to receive(:finished_processing) + .with(current_user, kind_of(Instrumenter), :registered, "direct_verifications") + .and_return(mailer) + end + + it "creates the user" do + expect { described_class.perform_later(userslist, organization, current_user, "direct_verifications") } + .to change(Decidim::User, :count).from(1).to(2) + + user = Decidim::User.find_by(email: "brandy@example.com") + expect(user.name).to eq("brandy") + end + + it "does not authorize the user" do + described_class.perform_later(userslist, organization, current_user, "direct_verifications") + + user = Decidim::User.find_by(email: "brandy@example.com") + authorization = Decidim::Authorization.find_by(decidim_user_id: user.id) + expect(authorization).to be_nil + end + + it "notifies the result by email" do + described_class.perform_later(userslist, organization, current_user, "direct_verifications") + expect(mailer).to have_received(:deliver_now) + end + end + end +end diff --git a/spec/jobs/decidim/direct_verifications/revoke_users_job_spec.rb b/spec/jobs/decidim/direct_verifications/revoke_users_job_spec.rb new file mode 100644 index 0000000..8748a60 --- /dev/null +++ b/spec/jobs/decidim/direct_verifications/revoke_users_job_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require "spec_helper" + +module Decidim + module DirectVerifications + describe RevokeUsersJob, type: :job do + let(:userslist) { "Name,Email,Type\r\n\"\",brandy@example.com,consumer" } + let(:organization) { create(:organization) } + let(:current_user) { create(:user, organization: organization) } + let(:mailer) { double(:mailer, deliver_now: true) } + + let(:user) { create(:user, organization: organization, email: "brandy@example.com") } + let!(:authorization) { create(:authorization, :granted, user: user, name: :direct_verifications) } + + around do |example| + perform_enqueued_jobs { example.run } + end + + before do + allow(ImportMailer) + .to receive(:finished_processing) + .with(current_user, kind_of(Instrumenter), :revoked, "direct_verifications") + .and_return(mailer) + end + + it "deletes the user authorization" do + described_class.perform_later(userslist, organization, current_user, "direct_verifications") + expect(Decidim::Authorization.find_by(id: authorization.id)).to be_nil + end + + it "notifies the result by email" do + described_class.perform_later(userslist, organization, current_user, "direct_verifications") + expect(mailer).to have_received(:deliver_now) + end + end + end +end diff --git a/spec/lib/decidim/direct_verifications/authorize_user_spec.rb b/spec/lib/decidim/direct_verifications/authorize_user_spec.rb index 0534b89..ea66487 100644 --- a/spec/lib/decidim/direct_verifications/authorize_user_spec.rb +++ b/spec/lib/decidim/direct_verifications/authorize_user_spec.rb @@ -5,7 +5,11 @@ module Decidim module DirectVerifications describe AuthorizeUser do - subject { described_class.new(email, data, session, organization, instrumenter) } + subject do + described_class.new(email, data, session, organization, instrumenter, authorization_handler) + end + + let(:authorization_handler) { :direct_verifications } describe "#call" do let(:data) { user.name } @@ -15,7 +19,7 @@ module DirectVerifications let(:user) { create(:user, organization: organization) } let(:email) { user.email } let(:session) { {} } - let(:instrumenter) { instance_double(UserProcessor, add_processed: true, add_error: true) } + let(:instrumenter) { instance_double(Instrumenter, add_processed: true, add_error: true) } context "when passing the user name" do let(:data) { user.name } @@ -108,7 +112,7 @@ module DirectVerifications let(:email) { "em@mail.com" } let(:data) { "Andy" } let(:session) { {} } - let(:instrumenter) { instance_double(UserProcessor, add_processed: true, add_error: true) } + let(:instrumenter) { instance_double(Instrumenter, add_processed: true, add_error: true) } it "tracks an error" do subject.call diff --git a/spec/lib/decidim/direct_verifications/instrumenter_spec.rb b/spec/lib/decidim/direct_verifications/instrumenter_spec.rb new file mode 100644 index 0000000..833841c --- /dev/null +++ b/spec/lib/decidim/direct_verifications/instrumenter_spec.rb @@ -0,0 +1,134 @@ +# frozen_string_literal: true + +require "spec_helper" + +module Decidim + module DirectVerifications + describe Instrumenter do + subject { described_class.new(current_user) } + + let(:current_user) { build_stubbed(:user) } + + shared_examples "an error event" do |type| + context "when the email does not exist" do + it "adds the email" do + subject.add_error(type, "email@example.com") + expect(subject.errors_count(type)).to eq(1) + end + end + + context "when the email already exists" do + before { subject.add_error(type, "email@example.com") } + + it "does not duplicate the email" do + subject.add_error(type, "email@example.com") + expect(subject.errors_count(type)).to eq(1) + end + end + end + + shared_examples "a process event" do |type| + context "when the email does not exist" do + it "adds the email" do + subject.add_processed(type, "email@example.com") + expect(subject.processed_count(type)).to eq(1) + end + end + + context "when the email already exists" do + before { subject.add_processed(type, "email@example.com") } + + it "does not duplicate the email" do + subject.add_processed(type, "email@example.com") + expect(subject.processed_count(type)).to eq(1) + end + end + end + + describe "#add_error" do + it_behaves_like "an error event", :registered + it_behaves_like "an error event", :authorized + it_behaves_like "an error event", :revoked + + context "when the provided type does not exist" do + let(:type) { :fake } + + it "raises due to NilClass" do + expect { subject.add_error(type, "email@example.com") }.to raise_error(NoMethodError) + end + end + end + + describe "#add_processed" do + it_behaves_like "an error event", :registered + it_behaves_like "an error event", :authorized + it_behaves_like "an error event", :revoked + + context "when the provided type does not exist" do + let(:type) { :fake } + + it "raises due to NilClass" do + expect { subject.add_processed(type, "email@example.com") }.to raise_error(NoMethodError) + end + end + end + + describe "#track" do + context "when a user is passed" do + let(:user) { build_stubbed(:user) } + + before { allow(Decidim.traceability).to receive(:perform_action!) } + + context "and the provided type exists" do + let(:type) { :registered } + + it "adds the process event" do + subject.track(type, "email@example.com", user) + expect(subject.processed_count(type)).to eq(1) + end + + it "logs the action" do + expect(Decidim.traceability).to receive(:perform_action!).with( + "invite", + user, + current_user, + extra: { + invited_user_role: "participant", + invited_user_id: user.id + } + ) + subject.track(type, "email@example.com", user) + end + end + + context "and the provided type does not exist" do + let(:type) { :fake } + + it "raises due to NilClass" do + expect { subject.track(type, "email@example.com", user) }.to raise_error(NoMethodError) + end + end + end + + context "when a user is not passed" do + context "and the provided type exists" do + let(:type) { :registered } + + it "adds the error event" do + subject.track(type, "email@example.com") + expect(subject.errors_count(type)).to eq(1) + end + end + + context "and the provided type does not exist" do + let(:type) { :fake } + + it "raises due to NilClass" do + expect { subject.track(type, "email@example.com") }.to raise_error(NoMethodError) + end + end + end + end + end + end +end diff --git a/spec/lib/decidim/direct_verifications/register_user_spec.rb b/spec/lib/decidim/direct_verifications/register_user_spec.rb index 9fdcb3b..f1dd803 100644 --- a/spec/lib/decidim/direct_verifications/register_user_spec.rb +++ b/spec/lib/decidim/direct_verifications/register_user_spec.rb @@ -9,7 +9,7 @@ module DirectVerifications let(:user) { build(:user) } let(:organization) { build(:organization) } - let(:instrumenter) { instance_double(UserProcessor, track: true) } + let(:instrumenter) { instance_double(Instrumenter, track: true) } let(:email) { "em@il.com" } let(:name) { "Joni" } diff --git a/spec/lib/decidim/direct_verifications/revoke_user_spec.rb b/spec/lib/decidim/direct_verifications/revoke_user_spec.rb new file mode 100644 index 0000000..d2ec651 --- /dev/null +++ b/spec/lib/decidim/direct_verifications/revoke_user_spec.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +require "spec_helper" + +module Decidim + module DirectVerifications + describe RevokeUser do + subject { described_class.new(email, organization, instrumenter, "direct_verifications") } + + let(:user) { create(:user, organization: organization) } + let(:email) { user.email } + + describe "#call" do + let(:organization) { build(:organization) } + let(:instrumenter) { instance_double(Instrumenter, add_processed: true, add_error: true) } + + context "when revoking existing users" do + let(:email) { user.email } + + before { create(:authorization, :granted, user: user, name: :direct_verifications) } + + it "tracks the operation" do + subject.call + expect(instrumenter).to have_received(:add_processed).with(:revoked, email) + end + + it "revokes the user authorization" do + expect(Verification::DestroyUserAuthorization).to receive(:call) + subject.call + end + end + + context "when revoking non-existing users" do + let(:email) { "em@mail.com" } + + it "tracks the error" do + subject.call + expect(instrumenter).to have_received(:add_error).with(:revoked, email) + end + + it "does not revoke the authorization" do + expect(Verification::DestroyUserAuthorization).not_to receive(:call) + subject.call + end + end + + context "when the authorization does not exist" do + it "does not track the operation" do + subject.call + expect(instrumenter).not_to have_received(:add_processed) + expect(instrumenter).not_to have_received(:add_error) + end + + it "does not revoke the authorization" do + expect(Verification::DestroyUserAuthorization).not_to receive(:call) + subject.call + end + end + + context "when the authorization is not granted" do + before { create(:authorization, :pending, user: user, name: :direct_verifications) } + + it "does not track the operation" do + subject.call + expect(instrumenter).not_to have_received(:add_processed) + expect(instrumenter).not_to have_received(:add_error) + end + + it "does not revoke the authorization" do + expect(Verification::DestroyUserAuthorization).not_to receive(:call) + subject.call + end + end + + context "when the authorization fails to be destroyed" do + let(:user) { create(:user, organization: organization) } + let(:email) { user.email } + let(:authorization) { create(:authorization, :granted, user: user, name: :direct_verifications) } + + before do + allow(authorization).to receive(:destroy!).and_raise(ActiveRecord::ActiveRecordError) + allow(Authorization) + .to receive(:find_by) + .with(user: user, name: "direct_verifications") + .and_return(authorization) + end + + it "lets lower-level exceptions pass through" do + expect { subject.call }.to raise_error(ActiveRecord::ActiveRecordError) + end + end + + context "when passing a non-default authorization handler" do + subject { described_class.new(email, organization, instrumenter, authorization_handler) } + + let(:authorization_handler) { :other_verification_method } + let(:user) { create(:user, organization: organization) } + + before { create(:authorization, :granted, user: user, name: authorization_handler) } + + it "tracks the operation" do + subject.call + expect(instrumenter).to have_received(:add_processed).with(:revoked, email) + end + + it "revokes the user authorization" do + expect(Verification::DestroyUserAuthorization).to receive(:call) + subject.call + end + end + 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..d7f2647 100644 --- a/spec/lib/decidim/direct_verifications/user_processor_spec.rb +++ b/spec/lib/decidim/direct_verifications/user_processor_spec.rb @@ -5,13 +5,14 @@ module Decidim module DirectVerifications describe UserProcessor do - subject { described_class.new(organization, user, session) } + subject { described_class.new(organization, user, session, instrumenter) } let(:user) { create(:user, :confirmed, :admin, organization: organization) } let(:session) { double(:session) } let(:organization) do create(:organization, available_authorizations: ["direct_verifications"]) end + let(:instrumenter) { Instrumenter.new(nil) } context "when emails are passed" do it "uses the specified name" do @@ -32,30 +33,6 @@ module DirectVerifications end end - context "when add processed" do - it "has unique emails per type" do - subject.add_processed(:registered, "em@il.com") - subject.add_processed(:registered, "em@il.com") - expect(subject.processed[:registered].count).to eq(1) - - subject.add_processed(:authorized, "em@il.com") - subject.add_processed(:authorized, "em@il.com") - expect(subject.processed[:authorized].count).to eq(1) - end - end - - context "when add errors" do - it "has unique emails per type" do - subject.add_error(:registered, "em@il.com") - subject.add_error(:registered, "em@il.com") - expect(subject.errors[:registered].count).to eq(1) - - subject.add_error(:authorized, "em@il.com") - subject.add_error(:authorized, "em@il.com") - expect(subject.errors[:authorized].count).to eq(1) - end - end - describe "#register_users" do context "when registering valid users" do before do @@ -64,8 +41,8 @@ module DirectVerifications end it "has no errors" do - expect(subject.processed[:registered].count).to eq(2) - expect(subject.errors[:registered].count).to eq(0) + expect(instrumenter.processed_count(:registered)).to eq(2) + expect(instrumenter.errors_count(:registered)).to eq(0) end end @@ -76,8 +53,8 @@ module DirectVerifications end it "has no errors" do - expect(subject.processed[:registered].count).to eq(1) - expect(subject.errors[:registered].count).to eq(0) + expect(instrumenter.processed_count(:registered)).to eq(1) + expect(instrumenter.errors_count(:registered)).to eq(0) end end end @@ -91,8 +68,8 @@ module DirectVerifications it "has no errors" do subject.authorize_users - expect(subject.processed[:authorized].count).to eq(1) - expect(subject.errors[:authorized].count).to eq(0) + expect(instrumenter.processed_count(:authorized)).to eq(1) + expect(instrumenter.errors_count(:authorized)).to eq(0) end end @@ -116,8 +93,8 @@ module DirectVerifications it "has no errors" do subject.authorize_users - expect(subject.processed[:authorized].count).to eq(1) - expect(subject.errors[:authorized].count).to eq(0) + expect(instrumenter.processed_count(:authorized)).to eq(1) + expect(instrumenter.errors_count(:authorized)).to eq(0) end end @@ -140,12 +117,13 @@ module DirectVerifications before do subject.emails = { user.email => user.name } subject.authorize_users - subject.revoke_users end it "has no errors" do - expect(subject.processed[:revoked].count).to eq(1) - expect(subject.errors[:revoked].count).to eq(0) + subject.revoke_users + + expect(instrumenter.processed_count(:revoked)).to eq(1) + expect(instrumenter.errors_count(:revoked)).to eq(0) end end @@ -153,12 +131,13 @@ module DirectVerifications before do subject.emails = ["em@il.com"] subject.authorize_users - subject.revoke_users end - it "has no errors" do - expect(subject.processed[:revoked].count).to eq(0) - expect(subject.errors[:revoked].count).to eq(1) + it "has errors" do + subject.revoke_users + + expect(instrumenter.processed_count(:revoked)).to eq(0) + expect(instrumenter.errors_count(:revoked)).to eq(1) end end end diff --git a/spec/mailers/decidim/direct_verifications/import_mailer_spec.rb b/spec/mailers/decidim/direct_verifications/import_mailer_spec.rb new file mode 100644 index 0000000..dd53fed --- /dev/null +++ b/spec/mailers/decidim/direct_verifications/import_mailer_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require "spec_helper" + +module Decidim + module DirectVerifications + describe ImportMailer, type: :mailer do + let(:user) { build(:user) } + + describe "#finished_processing" do + subject(:mail) { described_class.finished_processing(user, instrumenter, type, handler) } + + let(:instrumenter) { instance_double(Instrumenter, emails_count: 1, processed_count: 1, errors_count: 0) } + let(:type) { :registered } + let(:handler) { :direct_verifications } + + it "sends the email to the passed user" do + expect(mail.to).to eq([user.email]) + end + + it "renders the subject" do + expect(mail.subject).to eq(I18n.t("decidim.direct_verifications.verification.admin.imports.mailer.subject")) + end + + it "localizes the subject" do + user.locale = "ca" + expect(I18n).to receive(:with_locale).with("ca") + + mail.body + end + + it "sets the organization" do + expect(mail.body.encoded).to include(user.organization.name) + end + + context "when the type is :registered" do + let(:type) { :registered } + + context "when the import had no errors" do + it "shows the number of errors" do + expect(mail.body.encoded).to include( + "1 users have been successfully registered (1 detected, 0 errors)" + ) + end + end + + context "when the import had errors" do + let(:instrumenter) { instance_double(Instrumenter, emails_count: 1, processed_count: 0, errors_count: 1) } + + it "shows the number of errors" do + expect(mail.body.encoded).to include( + "0 users have been successfully registered (1 detected, 1 errors)" + ) + end + end + end + + context "when the type is :revoked" do + let(:type) { :revoked } + + context "when the import had no errors" do + it "shows the number of errors" do + expect(mail.body.encoded).to include( + "Verification from 1 users have been revoked using [direct_verifications] (1 detected, 0 errors)" + ) + end + end + + context "when the import had errors" do + let(:instrumenter) { instance_double(Instrumenter, emails_count: 1, processed_count: 0, errors_count: 1) } + + it "shows the number of errors" do + expect(mail.body.encoded).to include( + "Verification from 0 users have been revoked using [direct_verifications] (1 detected, 1 errors)" + ) + end + end + end + end + end + end +end diff --git a/spec/mailers/previews/import_mailer_preview.rb b/spec/mailers/previews/import_mailer_preview.rb new file mode 100644 index 0000000..7546c8a --- /dev/null +++ b/spec/mailers/previews/import_mailer_preview.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Decidim + module DirectVerifications + class ImportMailerPreview < ActionMailer::Preview + def finished_registration + user = User.first + + instrumenter = Instrumenter.new(user) + instrumenter.add_processed(:registered, "email@example.com") + + ImportMailer.finished_registration(user, instrumenter) + end + end + end +end diff --git a/spec/system/decidim/direct_verifications/admin/admin_imports_users_spec.rb b/spec/system/decidim/direct_verifications/admin/admin_imports_users_spec.rb new file mode 100644 index 0000000..0de5ae9 --- /dev/null +++ b/spec/system/decidim/direct_verifications/admin/admin_imports_users_spec.rb @@ -0,0 +1,136 @@ +# frozen_string_literal: true + +require "spec_helper" + +describe "Admin imports users", type: :system do + let(:organization) { create(:organization) } + let(:user) { create(:user, :admin, :confirmed, organization: organization) } + + let(:i18n_scope) { "decidim.direct_verifications.verification.admin" } + let(:filename) { file_fixture("users.csv") } + + before do + switch_to_host(organization.host) + login_as user, scope: :user + + organization.available_authorizations = %w(direct_verifications other_verification_method) + organization.save! + Decidim::DirectVerifications.configure do |config| + config.manage_workflows = %w(other_verification_method) + end + + visit decidim_admin_direct_verifications.new_import_path + end + + it "can be accessed from direct_verifications_path" do + visit decidim_admin_direct_verifications.direct_verifications_path + click_link "import a CSV" + expect(page).to have_current_path(decidim_admin_direct_verifications.new_import_path) + end + + context "when registering users" do + it "registers users through a CSV file" do + attach_file("CSV file with users data", filename) + + check(I18n.t("#{i18n_scope}.new.register")) + + perform_enqueued_jobs do + click_button("Upload file") + end + + expect(page).to have_admin_callout(I18n.t("#{i18n_scope}.imports.create.success")) + expect(page).to have_current_path(decidim_admin_direct_verifications.new_import_path) + + expect(ActionMailer::Base.deliveries.first.to).to contain_exactly("brandy@example.com") + expect(ActionMailer::Base.deliveries.first.subject).to eq("Invitation instructions") + + expect(ActionMailer::Base.deliveries.last.body.encoded).to include( + I18n.t("#{i18n_scope}.imports.mailer.registered", count: 1, successful: 1, errors: 0) + ) + end + end + + context "when registering and authorizing users" do + it "registers and authorizes users through a CSV file" do + attach_file("CSV file with users data", filename) + + check(I18n.t("#{i18n_scope}.new.register")) + choose(I18n.t("#{i18n_scope}.new.authorize")) + select( + "translation missing: en.decidim.authorization_handlers.other_verification_method.name", + from: "Verification method" + ) + + perform_enqueued_jobs do + click_button("Upload file") + end + + expect(page).to have_admin_callout(I18n.t("#{i18n_scope}.imports.create.success")) + expect(page).to have_current_path(decidim_admin_direct_verifications.new_import_path) + + click_link I18n.t("index.authorizations", scope: "decidim.direct_verifications.verification.admin") + expect(page).not_to have_content("Brandy") # In the authorizations page we only show those with :direct_verifications + + expect(ActionMailer::Base.deliveries.first.to).to contain_exactly("brandy@example.com") + expect(ActionMailer::Base.deliveries.first.subject).to eq("Invitation instructions") + + expect(ActionMailer::Base.deliveries.second.body.encoded).to include( + I18n.t( + "#{i18n_scope}.imports.mailer.registered", + count: 1, + successful: 1, + errors: 0 + ) + ) + + expect(ActionMailer::Base.deliveries.third.body.encoded).to include( + I18n.t( + "#{i18n_scope}.imports.mailer.authorized", + handler: :other_verification_method, + count: 1, + successful: 1, + errors: 0 + ) + ) + end + end + + context "when revoking users" do + let(:user_to_revoke) do + create(:user, name: "Brandy", email: "brandy@example.com", organization: organization) + end + + before do + create(:authorization, :granted, user: user_to_revoke, name: :other_verification_method) + end + + it "revokes users through a CSV file" do + attach_file("CSV file with users data", filename) + choose(I18n.t("#{i18n_scope}.new.revoke")) + select( + "translation missing: en.decidim.authorization_handlers.other_verification_method.name", + from: "Verification method" + ) + + perform_enqueued_jobs do + click_button("Upload file") + end + + expect(page).to have_admin_callout("successfully") + expect(page).to have_current_path(decidim_admin_direct_verifications.new_import_path) + + click_link I18n.t("index.authorizations", scope: "decidim.direct_verifications.verification.admin") + expect(page).not_to have_content("Brandy") + + expect(ActionMailer::Base.deliveries.last.body.encoded).to include( + I18n.t( + "#{i18n_scope}.imports.mailer.revoked", + handler: :other_verification_method, + count: 1, + successful: 1, + errors: 0 + ) + ) + end + end +end diff --git a/spec/system/decidim/direct_verifications/admin/direct_verifications_spec.rb b/spec/system/decidim/direct_verifications/admin/direct_verifications_spec.rb new file mode 100644 index 0000000..a515e43 --- /dev/null +++ b/spec/system/decidim/direct_verifications/admin/direct_verifications_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require "spec_helper" + +describe "Admin creates direct verifications", type: :system do + let(:organization) { create(:organization) } + let(:user) { create(:user, :admin, :confirmed, organization: organization) } + + let(:i18n_scope) { "decidim.direct_verifications.verification.admin.direct_verifications" } + + before do + switch_to_host(organization.host) + login_as user, scope: :user + + visit decidim_admin_direct_verifications.direct_verifications_path + end + + around do |example| + original_processor = Rails.configuration.direct_verifications_parser + Rails.configuration.direct_verifications_parser = :metadata + example.run + Rails.configuration.direct_verifications_parser = original_processor + end + + context "when registering users" do + context "and no header is provided" do + it "shows an error message" do + fill_in "Emails list", with: "brandy@example.com,,consumer" + check "register" + choose "authorize_in" + + click_button "Send and process the list" + + expect(page).to have_content(I18n.t("#{i18n_scope}.create.missing_header")) + end + end + end +end