From ad8c6c02fe0b5ffc27977361ec047420ca845b92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pau=20P=C3=A9rez=20Fabregat?= Date: Thu, 1 Oct 2020 14:32:40 +0200 Subject: [PATCH] support for CSV and multiple columns (#2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove not found `url` gem A `bundle install` complaints with: ``` Fetching gem metadata from https://rubygems.org/........ Fetching gem metadata from https://rubygems.org/. Resolving dependencies.... Your bundle is locked to url (0.3.2), but that version could not be found in any of the sources listed in your Gemfile. If you haven't changed sources, that means the author of url (0.3.2) has removed it. You'll need to update your bundle to a version other than url (0.3.2) that hasn't been removed in order to install. ``` It looks like that the gem that depends on this one had the same issue and drop it. * Gitignore common files * Extract email parsing into class And move Parser's specs to a new class-specific file. * Replace send calls to public method No reason to use `#send` if the method is public. * Improve a bit specs wording * Accept email data as a hash and store as metadata * Enable toggling the parser from config This adds a new parser and while making it backward compatible. It'll only be enabled if you configure with `processor = :metadata`. It does so by extracting specific entry parsing into two classes. Now we have them both encapsulated and we can extend the metadata one however we want without affecting current behaviour. * Read CSV format Well, a bit rudimentary still, but also normalize a bit the entry format. Less flexible => less complexity => less buggy. * Accept CSV header and unlimited columns This turns the entry parsers into full text parsers each with different features, not only at the entry level. While we keep the simple parser that deals only with names for backward compatibility, the new metadata parser deals with CSV and unlimited columns, while being more strict in its input format. * Refactor parsers naming So they better convey the use of the template method pattern and how things are design around inheritance. * Use stdlib's CSV interface Ruby's CSV parser can't compare to our naïve `split(",")`. * Increase #register_users test coverage * Doc Metadata mode and better config key Processor is already used by UserProcessor which is different from the parsers, which may lead to confusion. * Move email and name normalization to parser This is where it belongs. As a result, now the metadata parser doesn't force you to have the email in the second column. It'll just keep it out of the metadata JSON, that's all. * Remove unnecessary chomp The CSV lib already does so. * Memoize header row parsing We are hitting this computation for every single line from the CSV we process. * Fix flaky spec Hopefully. It's now consistently passing but I don't know why. I also took the chance to tidy up the shared examples hoping that'd make it easier to find to root cause of the flakyness. --- .gitignore | 4 + Gemfile.lock | 8 +- README.md | 15 +++ .../verification/base_parser.rb | 37 +++++++ .../verification/metadata_parser.rb | 42 ++++++++ .../verification/name_parser.rb | 40 +++++++ .../admin/direct_verifications_controller.rb | 20 ++-- .../tests/verification_controller_examples.rb | 16 ++- .../direct_verifications/user_processor.rb | 28 +++-- .../verification/engine.rb | 2 + .../verification/metadata_parser_spec.rb | 100 ++++++++++++++++++ .../verification/name_parser_spec.rb | 88 +++++++++++++++ .../direct_verifications_controller_spec.rb | 94 ++++++++++------ .../user_processor_spec.rb | 83 +++++++++++---- 14 files changed, 494 insertions(+), 83 deletions(-) create mode 100644 app/commands/decidim/direct_verifications/verification/base_parser.rb create mode 100644 app/commands/decidim/direct_verifications/verification/metadata_parser.rb create mode 100644 app/commands/decidim/direct_verifications/verification/name_parser.rb create mode 100644 spec/commands/decidim/direct_verifications/verification/metadata_parser_spec.rb create mode 100644 spec/commands/decidim/direct_verifications/verification/name_parser_spec.rb diff --git a/.gitignore b/.gitignore index 9c0646b..9bc06a8 100644 --- a/.gitignore +++ b/.gitignore @@ -18,3 +18,7 @@ spec/decidim_dummy_app # default development application development_app + +.byebug_history +.ruby-version +tags diff --git a/Gemfile.lock b/Gemfile.lock index da70ef3..3dcc29a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -109,10 +109,9 @@ GEM cells (>= 4.1.6, < 5.0.0) charlock_holmes (0.7.7) childprocess (3.0.0) - codecov (0.1.16) + codecov (0.2.11) json simplecov - url coercible (1.0.0) descendants_tracker (~> 0.0.1) coffee-rails (5.0.0) @@ -428,7 +427,7 @@ GEM thor (>= 0.14, < 2.0) jquery-tmpl-rails (1.1.0) rails (>= 3.1.0) - json (2.3.0) + json (2.3.1) jwt (2.2.1) kaminari (1.2.0) activesupport (>= 4.1.0) @@ -708,7 +707,6 @@ GEM unf_ext unf_ext (0.0.7.7) unicode-display_width (1.6.1) - url (0.3.2) valid_email2 (2.3.1) activemodel (>= 3.2) mail (~> 2.5) @@ -757,7 +755,7 @@ DEPENDENCIES web-console (~> 3.5) RUBY VERSION - ruby 2.6.3p62 + ruby 2.6.6p146 BUNDLED WITH 2.1.4 diff --git a/README.md b/README.md index 42f8c22..33e50a5 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,21 @@ With the detected list of emails admin have different options available: 3. Revoke the authorization for the list of users using any verification method available. 4. Check the status of the users in order to know if they are verified or registered. +### Metadata mode + +This mode provides extra capabilities over the default processing: + +* Reads CSV format with header (copy and paste it from your spreadsheet) +* Stores all columns except the email as authorization metadata + +This enables querying the authorization metadata however fits you best. + +To enable it create a new initializer called `config/initializers/decidim_direct_verifications.rb` with the following contents + +```rb +Rails.application.config.direct_verifications_parser = :metadata +``` + ## Installation Add this line to your application's Gemfile: diff --git a/app/commands/decidim/direct_verifications/verification/base_parser.rb b/app/commands/decidim/direct_verifications/verification/base_parser.rb new file mode 100644 index 0000000..e584161 --- /dev/null +++ b/app/commands/decidim/direct_verifications/verification/base_parser.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module Decidim + module DirectVerifications + module Verification + # Abstract class all concrete parsers should inherit from. They are expected to implement + # #header, #lines, and #parse_data methods. + class BaseParser + EMAIL_REGEXP = /([A-Z0-9+._-]+@[A-Z0-9._-]+\.[A-Z0-9_-]+)\b/i.freeze + + def initialize(txt) + @txt = txt + @emails = {} + end + + def to_h + lines.each do |line| + EMAIL_REGEXP.match(line) do |match| + email = normalize(match[0]) + emails[email] = parse_data(email, line, header) + end + end + + emails + end + + private + + attr_reader :txt, :emails + + def normalize(value) + value.to_s.downcase + 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 new file mode 100644 index 0000000..e5ee82e --- /dev/null +++ b/app/commands/decidim/direct_verifications/verification/metadata_parser.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require "csv" + +module Decidim + module DirectVerifications + module Verification + class MetadataParser < BaseParser + def header + @header ||= begin + header_row = lines[0].chomp + column_names = tokenize(header_row) + column_names.map(&:to_sym).map(&:downcase) + end + end + + def lines + @lines ||= StringIO.new(txt).readlines + end + + def parse_data(email, line, header) + tokens = tokenize(line) + + hash = {} + header.each_with_index do |column, index| + value = tokens[index].strip + next if value.include?(email) + + hash[column] = value + end + hash + end + + private + + def tokenize(line) + CSV.parse(line)[0] + end + end + end + end +end diff --git a/app/commands/decidim/direct_verifications/verification/name_parser.rb b/app/commands/decidim/direct_verifications/verification/name_parser.rb new file mode 100644 index 0000000..8f61ad5 --- /dev/null +++ b/app/commands/decidim/direct_verifications/verification/name_parser.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Decidim + module DirectVerifications + module Verification + class NameParser < BaseParser + LINE_DELIMITER = /[\r\n;,]/.freeze + NON_ALPHA_CHARS = /[^[:print:]]|[\"\$\<\>\|\\]/.freeze + + def header + nil + end + + def lines + txt.split(LINE_DELIMITER) + end + + def parse_data(email, line, _header) + name = parse_name(email, line) + name = strip_non_alpha_chars(name) + name.presence || fallback_name(email) + end + + private + + def strip_non_alpha_chars(str) + (str.presence || "").gsub(NON_ALPHA_CHARS, "").strip + end + + def parse_name(email, line) + line.split(email).first + end + + def fallback_name(email) + email.split("@").first + end + 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 3dc857f..b8a365f 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 @@ -20,11 +20,14 @@ def create @userlist = params[:userlist] @workflows = workflows + processor = UserProcessor.new(current_organization, current_user) - processor.emails = extract_emails_to_hash @userlist + processor.emails = parser_class.new(@userlist).to_h processor.authorization_handler = authorization_handler(params[:authorization_handler]) + stats = UserStats.new(current_organization) stats.authorization_handler = processor.authorization_handler + if params[:register] processor.register_users flash[:warning] = t(".registered", count: processor.emails.count, @@ -52,21 +55,18 @@ def create registered: stats.registered) render(action: :index) && return end + redirect_to direct_verifications_path end private - def extract_emails_to_hash(txt) - reg = /([A-Z0-9+._-]+@[A-Z0-9._-]+\.[A-Z0-9_-]+)\b/i - emails = {} - txt.split(/[\r\n;,]/).each do |line| - reg.match line do |m| - n = line.split(m[0]).first - emails[m[0]] = (n.presence || "").gsub(/[^[:print:]]|[\"\$\<\>\|\\]/, "").strip - end + def parser_class + if Rails.configuration.direct_verifications_parser == :metadata + MetadataParser + else + NameParser end - emails end def authorization_handler(authorization_handler) diff --git a/lib/decidim/direct_verifications/tests/verification_controller_examples.rb b/lib/decidim/direct_verifications/tests/verification_controller_examples.rb index 36dc6c2..b384c92 100644 --- a/lib/decidim/direct_verifications/tests/verification_controller_examples.rb +++ b/lib/decidim/direct_verifications/tests/verification_controller_examples.rb @@ -2,9 +2,11 @@ shared_examples_for "checking users" do |params| context "when check without mails" do + before { params[:userlist] = "" } + it "renders the index with info message" do - params[:userlist] = "" post :create, params: params + expect(flash[:info]).not_to be_empty expect(flash[:info]).to include("0 users detected") expect(subject).to render_template("decidim/direct_verifications/verification/admin/direct_verifications/index") @@ -12,9 +14,11 @@ end context "when check with mails" do + before { params[:userlist] = "mail@example.com" } + it "renders the index with info message" do - params[:userlist] = "mail@example.com" post :create, params: params + expect(flash[:info]).not_to be_empty expect(flash[:info]).to include("1 users detected") expect(subject).to render_template("decidim/direct_verifications/verification/admin/direct_verifications/index") @@ -26,6 +30,7 @@ context "when send valid emails" do it "creates warning message" do post :create, params: params + expect(flash[:warning]).not_to be_empty expect(flash[:warning]).to include("1 detected") expect(flash[:warning]).to include("0 errors") @@ -39,6 +44,7 @@ context "when send valid emails" do it "creates notice message" do post :create, params: params + expect(flash[:notice]).not_to be_empty expect(flash[:notice]).to include("1 detected") expect(flash[:notice]).to include("0 errors") @@ -50,14 +56,18 @@ shared_examples_for "revoking users" do |params| context "when send valid emails" do - it "creates notice message" do + before do create( :authorization, :granted, name: verification_type, user: authorized_user ) + end + + it "creates notice message" do post :create, params: params + expect(flash[:notice]).not_to be_empty expect(flash[:notice]).to include("1 detected") expect(flash[:notice]).to include("0 errors") diff --git a/lib/decidim/direct_verifications/user_processor.rb b/lib/decidim/direct_verifications/user_processor.rb index e684a0d..befa6e0 100644 --- a/lib/decidim/direct_verifications/user_processor.rb +++ b/lib/decidim/direct_verifications/user_processor.rb @@ -12,17 +12,19 @@ def initialize(organization, current_user) @emails = {} end - attr_reader :organization, :current_user, :errors, :processed, :emails - attr_accessor :authorization_handler - - def emails=(email_list) - @emails = email_list.map { |k, v| [k.to_s.downcase, v.presence || k.split("@").first] }.to_h - end + attr_reader :organization, :current_user, :errors, :processed + attr_accessor :authorization_handler, :emails def register_users - @emails.each do |email, name| + emails.each do |email, data| next if find_user(email) + name = if data.is_a?(Hash) + data[:name] + else + data + end + form = register_form(email, name) begin InviteUser.call(form) do @@ -41,9 +43,11 @@ def register_users end def authorize_users - @emails.each do |email, _name| + 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)) do @@ -61,7 +65,7 @@ def authorize_users end def revoke_users - @emails.each do |email, _name| + emails.each do |email, _name| if (u = find_user(email)) auth = authorization(u) next unless auth.granted? @@ -87,7 +91,7 @@ def find_user(email) end def register_form(email, name) - OpenStruct.new(name: name.presence || email.split("@").first, + OpenStruct.new(name: name.presence || fallback_name(email), email: email.downcase, organization: organization, admin: false, @@ -95,6 +99,10 @@ def register_form(email, name) invitation_instructions: "direct_invite") end + def fallback_name(email) + email.split("@").first + end + def authorization(user) Authorization.find_or_initialize_by( user: user, diff --git a/lib/decidim/direct_verifications/verification/engine.rb b/lib/decidim/direct_verifications/verification/engine.rb index b304bee..96c54d5 100644 --- a/lib/decidim/direct_verifications/verification/engine.rb +++ b/lib/decidim/direct_verifications/verification/engine.rb @@ -6,6 +6,8 @@ module Verification class Engine < ::Rails::Engine isolate_namespace Decidim::DirectVerifications::Verification + config.direct_verifications_parser = :name + paths["db/migrate"] = nil paths["lib/tasks"] = nil diff --git a/spec/commands/decidim/direct_verifications/verification/metadata_parser_spec.rb b/spec/commands/decidim/direct_verifications/verification/metadata_parser_spec.rb new file mode 100644 index 0000000..ac154ea --- /dev/null +++ b/spec/commands/decidim/direct_verifications/verification/metadata_parser_spec.rb @@ -0,0 +1,100 @@ +# frozen_string_literal: true + +require "spec_helper" + +module Decidim::DirectVerifications::Verification + describe MetadataParser do + subject { described_class.new(txt) } + + describe "#to_h" do + context "when the text is empty" do + let(:txt) { "" } + + it "returns an empty hash" do + expect(subject.to_h).to eq({}) + end + end + + context "when the text is invalid" do + let(:txt) do + <<-EMAILS.strip_heredoc + nonsense emails... + not_an@email + no em@il + EMAILS + end + + it "returns an empty hash" do + expect(subject.to_h).to eq({}) + end + end + + context "when the text is a CSV with headers" do + let(:txt) do + <<-ROWS.strip_heredoc + Name,Email,Department,Salary + Bob,bob@email.com,Engineering,1000 + Jane,jane@email.com,Sales,2000 + John,john@email.com,Management,5000 + ROWS + end + + it "returns the data in a hash with the email as key" do + expect(subject.to_h).to eq( + "bob@email.com" => { name: "Bob", department: "Engineering", salary: "1000" }, + "jane@email.com" => { name: "Jane", department: "Sales", salary: "2000" }, + "john@email.com" => { name: "John", department: "Management", salary: "5000" } + ) + end + + context "and the CSV includes quotes" do + let(:txt) do + <<-CSV.strip_heredoc + Name,Email,Type + use,test3@t.com,type + User, another@email.com,third@email.com@as.com + Test 1, test1@test.com, customer + \"Test\\| 4\", { name: "use", type: "type" }, + "a@b.co" => { name: "User", type: "third@email.com@as.com" }, + "test1@test.com" => { name: "Test 1", type: "customer" }, + "test4@test.com" => { name: "Test\\| 4", type: "producer" } + ) + end + end + + context "when the name is not specified" do + let(:txt) do + <<-EMAILS.strip_heredoc + email + em@il.com + EMAILS + end + + it "infers the name from the email" do + expect(subject.to_h).to eq("em@il.com" => {}) + end + end + + context "when entries are duplicate" do + let(:txt) do + <<-EMAILS.strip_heredoc + name,email + brandy,brandy@example.com + brandy,brandy@example.com + EMAILS + end + + it "keeps only one" do + expect(subject.to_h).to eq("brandy@example.com" => { name: "brandy" }) + end + end + end + end + end +end diff --git a/spec/commands/decidim/direct_verifications/verification/name_parser_spec.rb b/spec/commands/decidim/direct_verifications/verification/name_parser_spec.rb new file mode 100644 index 0000000..28cc676 --- /dev/null +++ b/spec/commands/decidim/direct_verifications/verification/name_parser_spec.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +require "spec_helper" + +module Decidim::DirectVerifications::Verification + describe NameParser do + subject { described_class.new(txt) } + + describe "#to_h" do + context "when the text is empty" do + let(:txt) { "" } + + it "returns an empty hash" do + expect(subject.to_h).to eq({}) + end + end + + context "when the text is invalid" do + let(:txt) do + <<-EMAILS.strip_heredoc + nonsense emails... + not_an@email + no em@il + EMAILS + end + + it "returns an empty hash" do + expect(subject.to_h).to eq({}) + end + end + + context "when the text is valid" do + let(:txt) do + <<-EMAILS.strip_heredoc + test1@test.com + test2@test.com + use test3@t.com + User \ranother@email.com,third@email.com@as.com + Test 1 test1@test.com + \"Test\\| 4\" , My other name + EMAILS + end + + it "returns the emails in a hash" do + expect(subject.to_h).to eq( + "a@b.co" => "User", + "another@email.com" => "another", + "dot.email@test.com" => "dot.email", + "my-other@email.org" => "My other name", + "my@email.net" => "My name", + "test1@test.com" => "Test 1", + "test2@test.com" => "test2", + "test3@t.com" => "use", + "test4@test.com" => "Test 4", + "third@email.com" => "third", + "with.dot@email.dot.com" => "My.Dot:Name" + ) + end + end + + context "when the name is not specified" do + let(:txt) do + <<-EMAILS.strip_heredoc + em@il.com + EMAILS + end + + it "infers the name from the email" do + expect(subject.to_h).to eq("em@il.com" => "em") + end + end + + context "when entries are duplicate" do + let(:txt) do + <<-EMAILS.strip_heredoc + brandy,brandy@example.com + brandy,brandy@example.com + EMAILS + end + + it "keeps only one" do + expect(subject.to_h).to eq("brandy@example.com" => "brandy") + end + end + end + end +end 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 7e163f8..5ee6ec5 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 @@ -44,8 +44,19 @@ module Decidim::DirectVerifications::Verification::Admin context "when register users with check" do params = { userlist: "mail@example.com", register: true } + it_behaves_like "checking users", params - it_behaves_like "registering users", params + + it "creates warning message" do + post :create, params: params + + expect(flash[:warning]).not_to be_empty + expect(flash[:warning]).to include("1 detected") + expect(flash[:warning]).to include("0 errors") + expect(flash[:warning]).to include("1 users") + expect(flash[:warning]).to include("registered") + end + it "renders the index with warning message" do post :create, params: params expect(subject).to render_template("decidim/direct_verifications/verification/admin/direct_verifications/index") @@ -54,49 +65,66 @@ module Decidim::DirectVerifications::Verification::Admin context "when register users with authorize" do params = { userlist: "mail@example.com", register: true, authorize: "in" } + it_behaves_like "registering users", params it_behaves_like "authorizing users", params + it "redirects with notice and warning messages" do post :create, params: params expect(subject).to redirect_to(action: :index) end - end - context "when register users with revoke" do - params = { userlist: "authorized@example.com", authorize: "out" } - it_behaves_like "revoking users", params - end - end + context "when the name is not specified" do + it "infers the name from the email" do + post :create, params: { + userlist: "Name,Email,Type\r\n\"\",brandy@example.com,consumer", + register: true, + authorize: "in" + } - describe "email extractor" do - it "converts empty text to empty hash" do - txt = "" - h = controller.send(:extract_emails_to_hash, txt) - expect(h).to eq({}) - end + user = Decidim::User.find_by(email: "brandy@example.com") + expect(user.name).to eq("brandy") + end + end + + context "when in metadata mode" do + 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 + + it "stores any extra columns as authorization metadata" do + post :create, params: { + userlist: "Name,Email,Type\r\nBrandy,brandy@example.com,consumer,2\r\nWhisky,whisky@example.com,producer,3", + register: true, + authorize: "in" + } - it "converts invalid text to empty hash" do - txt = "nonsense emails...\nnot_an@email\nno em@il" - h = controller.send(:extract_emails_to_hash, txt) - expect(h).to eq({}) + 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 the name is not specified" do + it "infers the name from the email" do + post :create, params: { + userlist: "Name,Email,Type\r\n\"\",brandy@example.com,consumer", + register: true, + authorize: "in" + } + + user = Decidim::User.find_by(email: "brandy@example.com") + expect(user.name).to eq("brandy") + end + end + end end - it "converts valid text to emails hash" do - txt = "test1@test.com\ntest2@test.com\nuse test3@t.com\nUser \ranother@email.com,third@email.com@as.com\nTest 1 test1@test.com\n\"Test\\| 4\" , My other name " - h = controller.send(:extract_emails_to_hash, txt) - expect(h).to eq( - "a@b.co" => "User", - "another@email.com" => "", - "dot.email@test.com" => "", - "my-other@email.org" => "My other name", - "my@email.net" => "My name", - "test1@test.com" => "Test 1", - "test2@test.com" => "", - "test3@t.com" => "use", - "test4@test.com" => "Test 4", - "third@email.com" => "", - "with.dot@email.dot.com" => "My.Dot:Name" - ) + context "when register users with revoke" do + params = { userlist: "authorized@example.com", authorize: "out" } + it_behaves_like "revoking users", params 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 c62ad28..3031cc0 100644 --- a/spec/lib/decidim/direct_verifications/user_processor_spec.rb +++ b/spec/lib/decidim/direct_verifications/user_processor_spec.rb @@ -13,20 +13,20 @@ module DirectVerifications end context "when emails are passed" do - it "has hash with automatic names" do - subject.send(:emails=, "em@il.com" => "") - expect(subject.emails).to eq("em@il.com" => "em") + it "uses the specified name" do + subject.emails = { "em@il.com" => "em", "em@il.net" => "A name" } + expect(subject.emails).to eq("em@il.com" => "em", "em@il.net" => "A name") end - it "has hash with passed names" do - subject.send(:emails=, "em@il.com" => "", "em@il.net" => "A name") - expect(subject.emails).to eq("em@il.com" => "em", "em@il.net" => "A name") + it "passes all the extra data when specified" do + subject.emails = { "em@il.com" => { name: "A name", type: "consumer" } } + expect(subject.emails).to eq("em@il.com" => { name: "A name", type: "consumer" }) end end context "when emails are not passed" do - it "has empty hash" do - subject.send(:emails=, {}) + it "uses an empty hash" do + subject.emails = {} expect(subject.emails).to eq({}) end end @@ -53,7 +53,7 @@ module DirectVerifications end end - context "when registers a valid users" do + context "when registering valid users" do before do subject.emails = ["em@il.com", "em@il.com", "em@il.net"] subject.register_users @@ -65,7 +65,35 @@ module DirectVerifications end end - context "when registers invalid users" do + context "when registering valid users with metadata" do + before do + subject.emails = { "em@il.com" => { name: "Brandy", type: "producer" } } + subject.register_users + end + + it "has no errors" do + expect(subject.processed[:registered].count).to eq(1) + expect(subject.errors[:registered].count).to eq(0) + end + end + + context "when registering users without name" do + before do + subject.emails = { "em@il.com" => { type: "producer" } } + subject.register_users + end + + it "has no errors" do + expect(subject.processed[:registered].count).to eq(1) + expect(subject.errors[:registered].count).to eq(0) + end + + it "infers the name from the email" do + expect(Decidim::User.find_by(email: "em@il.com").name).to eq("em") + end + end + + context "when registering invalid users" do before do subject.emails = ["em@il.org", ""] subject.register_users @@ -77,33 +105,44 @@ module DirectVerifications end end - context "when authorize confirmed users" do - before do + context "when authorizing confirmed users" do + it "has no errors" do subject.emails = { user.email => user.name } - # subject.register_users subject.authorize_users - end - it "has no errors" do expect(subject.processed[:authorized].count).to eq(1) expect(subject.errors[:authorized].count).to eq(0) end + + it "stores user data as authorization metadata" do + subject.emails = { user.email => { name: user.name, type: "consumer" } } + subject.authorize_users + + expect(Authorization.last.metadata).to eq("name" => user.name, "type" => "consumer") + end end - context "when authorize unconfirmed users" do - before do + context "when authorizing unconfirmed users" do + it "has no errors" do subject.emails = ["em@mail.com"] subject.register_users subject.authorize_users - end - it "has no errors" do expect(subject.processed[:authorized].count).to eq(1) expect(subject.errors[:authorized].count).to eq(0) end + + it "stores user data as authorization metadata" do + subject.emails = { "em@mail.com" => { type: "consumer" } } + subject.register_users + subject.authorize_users + + expect(Decidim::User.find_by(email: "em@mail.com").name).to eq("em") + expect(Authorization.last.metadata).to eq("type" => "consumer") + end end - context "when authorize unregistered users" do + context "when authorizing unregistered users" do before do subject.emails = ["em@mail.com"] subject.authorize_users @@ -115,7 +154,7 @@ module DirectVerifications end end - context "when revoke existing users" do + context "when revoking existing users" do before do subject.emails = { user.email => user.name } subject.authorize_users @@ -128,7 +167,7 @@ module DirectVerifications end end - context "when revoke non-existing users" do + context "when revoking non-existing users" do before do subject.emails = ["em@il.com"] subject.authorize_users