From f2f4d217094b6ddeb4c847a074c5155aeb9fecea Mon Sep 17 00:00:00 2001 From: Orlando Del Aguila Date: Wed, 24 Feb 2021 20:04:21 -0600 Subject: [PATCH 01/10] dev(email-flow): remove columns from db --- db/migrate/20210224211702_remove_user_confirmation_email.rb | 6 ++++++ db/schema.rb | 5 ++--- 2 files changed, 8 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20210224211702_remove_user_confirmation_email.rb diff --git a/db/migrate/20210224211702_remove_user_confirmation_email.rb b/db/migrate/20210224211702_remove_user_confirmation_email.rb new file mode 100644 index 00000000..643fc3dd --- /dev/null +++ b/db/migrate/20210224211702_remove_user_confirmation_email.rb @@ -0,0 +1,6 @@ +class RemoveUserConfirmationEmail < ActiveRecord::Migration[6.1] + def change + remove_column :users, :active, :boolean, default: false + remove_column :users, :confirmation_token, :string, index: true + end +end diff --git a/db/schema.rb b/db/schema.rb index ce533841..71313041 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,8 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_02_17_192628) do +ActiveRecord::Schema.define(version: 2021_02_24_211702) do + # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -108,10 +109,8 @@ t.bigint "external_id" t.string "name" t.string "username" - t.string "confirmation_token" t.datetime "confirmed_at" t.datetime "confirmation_sent_at" - t.boolean "active", default: false t.datetime "activated_at" t.string "email_token" t.index ["email"], name: "index_users_on_email", unique: true From d6513355bc5243475929741337fa4f4bf3dae44e Mon Sep 17 00:00:00 2001 From: Orlando Del Aguila Date: Wed, 24 Feb 2021 20:13:15 -0600 Subject: [PATCH 02/10] dev(discourse): change discourse existing user flow (wip) --- app/jobs/link_discourse_account_job.rb | 3 +++ app/models/user.rb | 11 ++++++++--- spec/factories/users.rb | 2 -- spec/jobs/link_discourse_account_job_spec.rb | 3 +-- spec/models/user_spec.rb | 2 -- 5 files changed, 12 insertions(+), 9 deletions(-) diff --git a/app/jobs/link_discourse_account_job.rb b/app/jobs/link_discourse_account_job.rb index 36da7592..a2a7b192 100644 --- a/app/jobs/link_discourse_account_job.rb +++ b/app/jobs/link_discourse_account_job.rb @@ -13,6 +13,9 @@ def perform(user) discourse_user = discourse.find_user_by_email if discourse_user + # TODO: here's how we need to refactor this part + # Link the user, save external_id + # create an email_token for this user and send the welcome email. if user.confirmed? user.update(external_id: discourse_user["id"]) else diff --git a/app/models/user.rb b/app/models/user.rb index 88a971e5..3d223ad2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -6,12 +6,10 @@ # # id :bigint not null, primary key # activated_at :datetime -# active :boolean default(FALSE) # admin :boolean default(FALSE) # avatar_url :string # banned :boolean default(FALSE) # confirmation_sent_at :datetime -# confirmation_token :string # confirmed_at :datetime # custom_fields :jsonb # email :string @@ -64,8 +62,10 @@ def self.find_or_create_from_sso(payload) user[sso_attribute] = payload[sso_attribute] end - new_record = user.new_record? + # Since the user comes from SSO, we can confirm the accoun + user.confirm! + new_record = user.new_record? user.save [user, new_record] @@ -103,6 +103,11 @@ def admin? !!admin end + def confirm! + user.confirmed_at ||= DateTime.now + user.save! + end + def activate! update!(active: true, activated_at: DateTime.now) end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 48eeca97..7a79df18 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -6,12 +6,10 @@ # # id :bigint not null, primary key # activated_at :datetime -# active :boolean default(FALSE) # admin :boolean default(FALSE) # avatar_url :string # banned :boolean default(FALSE) # confirmation_sent_at :datetime -# confirmation_token :string # confirmed_at :datetime # custom_fields :jsonb # email :string diff --git a/spec/jobs/link_discourse_account_job_spec.rb b/spec/jobs/link_discourse_account_job_spec.rb index 96f573f0..8b7e4d33 100644 --- a/spec/jobs/link_discourse_account_job_spec.rb +++ b/spec/jobs/link_discourse_account_job_spec.rb @@ -18,7 +18,6 @@ user = FactoryBot.create(:user, external_id: nil, confirmed_at: DateTime.now) expect_any_instance_of(DiscourseService).to receive(:find_user_by_email).and_return({"id" => 10}) - expect_any_instance_of(DiscourseService).not_to receive(:invite_user) perform_enqueued_jobs { LinkDiscourseAccountJob.perform_later(user) } @@ -30,7 +29,7 @@ user = FactoryBot.create(:user, external_id: nil) expect_any_instance_of(DiscourseService).to receive(:find_user_by_email).and_return({"id" => 10}) - expect_any_instance_of(DiscourseService).not_to receive(:invite_user) + expect(UserMailer).to receive_message_chain(:confirmation_email, :deliver_later) perform_enqueued_jobs { LinkDiscourseAccountJob.perform_later(user) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f0324264..39266bab 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -6,12 +6,10 @@ # # id :bigint not null, primary key # activated_at :datetime -# active :boolean default(FALSE) # admin :boolean default(FALSE) # avatar_url :string # banned :boolean default(FALSE) # confirmation_sent_at :datetime -# confirmation_token :string # confirmed_at :datetime # custom_fields :jsonb # email :string From 2599a33fffc4a20e368632d2d044d1e2c97c4f8a Mon Sep 17 00:00:00 2001 From: Orlando Del Aguila Date: Thu, 25 Feb 2021 12:31:13 -0600 Subject: [PATCH 03/10] feat(discourse): refactor existing account link flow --- app/jobs/link_discourse_account_job.rb | 44 ++++++++------- app/services/discourse_service.rb | 7 ++- spec/jobs/link_discourse_account_job_spec.rb | 24 +++----- spec/services/discourse_service_spec.rb | 58 ++++++++++++++++++++ 4 files changed, 96 insertions(+), 37 deletions(-) diff --git a/app/jobs/link_discourse_account_job.rb b/app/jobs/link_discourse_account_job.rb index a2a7b192..22c7fcf4 100644 --- a/app/jobs/link_discourse_account_job.rb +++ b/app/jobs/link_discourse_account_job.rb @@ -13,33 +13,37 @@ def perform(user) discourse_user = discourse.find_user_by_email if discourse_user - # TODO: here's how we need to refactor this part - # Link the user, save external_id - # create an email_token for this user and send the welcome email. - if user.confirmed? - user.update(external_id: discourse_user["id"]) - else - # send confirmation email - User.send_confirmation_instructions(email: user.email) + user.external_id = discourse_user["id"] + user.username = discourse_user["username"] + + response = discourse.create_email_token + + if [true, "OK"].exclude?(response["success"]) + return Raven.capture_message( + "Couldn't create a Discourse invite", + extra: {user_id: user.id, user_email: user.email} + ) end - return - end + user.email_token = response["email_token"] + else + # Create discourse account + response = discourse.create_user - # Create discourse account - response = discourse.create_user + if [true, "OK"].exclude?(response["success"]) + return Raven.capture_message( + "Couldn't create a Discourse invite", + extra: {user_id: user.id, user_email: user.email} + ) + end - if [true, "OK"].exclude?(response["success"]) - return Raven.capture_message( - "Couldn't create a Discourse invite", - extra: {user_id: user.id, user_email: user.email} - ) + user.email_token = response["email_token"] + user.external_id = response["user_id"] + user.username = response["username"] end - user.username = response["username"] - user.email_token = response["email_token"] - user.external_id = response["user_id"] user.save! + logger.info("Discourse account #{id} linked with user id #{self.id}") UserMailer.welcome_email(user: user).deliver_later end diff --git a/app/services/discourse_service.rb b/app/services/discourse_service.rb index fe70b5dd..b0b86563 100644 --- a/app/services/discourse_service.rb +++ b/app/services/discourse_service.rb @@ -30,7 +30,7 @@ def create_user user_fields = {} USER_FIELDS_MAP.each { |key, sym| user_fields[key] = user.custom_fields.fetch(sym, "") } - password = SecureRandom.hex(rand(20...24)) + password = SecureRandom.hex(rand(24...32)) client.create_user({ email: user.email, @@ -43,6 +43,11 @@ def create_user }) end + # This is used to create an email login link + def create_email_token + client.post("/u/email-token.json", {login: user.email}) + end + # Check if username is available def check_username(username) client.check_username(username) diff --git a/spec/jobs/link_discourse_account_job_spec.rb b/spec/jobs/link_discourse_account_job_spec.rb index 8b7e4d33..aa3e8b33 100644 --- a/spec/jobs/link_discourse_account_job_spec.rb +++ b/spec/jobs/link_discourse_account_job_spec.rb @@ -14,28 +14,20 @@ describe "#perform" do context "existing account" do - it "it sets external_id to the one found on Discourse if user is confirmed" do - user = FactoryBot.create(:user, external_id: nil, confirmed_at: DateTime.now) - - expect_any_instance_of(DiscourseService).to receive(:find_user_by_email).and_return({"id" => 10}) - - perform_enqueued_jobs { LinkDiscourseAccountJob.perform_later(user) } - - user.reload - expect(user.external_id).to eq(10) - end - - it "it send confirmation email if user is on Discourse but not confirmed" do + it "it links user and sends welcome email" do user = FactoryBot.create(:user, external_id: nil) + email_token = SecureRandom.hex(20) - expect_any_instance_of(DiscourseService).to receive(:find_user_by_email).and_return({"id" => 10}) - - expect(UserMailer).to receive_message_chain(:confirmation_email, :deliver_later) + expect_any_instance_of(DiscourseService).to receive(:find_user_by_email).and_return({"id" => 10, "username" => "orlando"}) + expect_any_instance_of(DiscourseService).to receive(:create_email_token).and_return({"email_token" => email_token, "success" => "OK"}) + expect(UserMailer).to receive_message_chain(:welcome_email, :deliver_later) perform_enqueued_jobs { LinkDiscourseAccountJob.perform_later(user) } user.reload - expect(user.external_id).to be_nil + expect(user.external_id).to eq(10) + expect(user.email_token).to eq(email_token) + expect(user.username).to eq("orlando") end end diff --git a/spec/services/discourse_service_spec.rb b/spec/services/discourse_service_spec.rb index 5362e2d1..1ee0ece6 100644 --- a/spec/services/discourse_service_spec.rb +++ b/spec/services/discourse_service_spec.rb @@ -152,4 +152,62 @@ WebMock.disable_net_connect! end end + + describe ".create_email_token" do + it "creates a email token" do + user = FactoryBot.create(:user) + + discourse = DiscourseService.new(user) + + response = { + "success" => "OK", + "user_found" => true, + "email_token" => "15fa6c1c626cb97ccf18e5abd537260e" + } + + stub_discourse_request(:post, "u/email-token.json") + .to_return(status: 200, body: response.to_json, headers: {"Content-Type": "application/json"}) + + response = discourse.create_email_token + + expect(response["success"]).to eq("OK") + expect(response["user_found"]).to eq(true) + expect(response["email_token"]).to eq("15fa6c1c626cb97ccf18e5abd537260e") + end + + it "returns user_found false when no user is found" do + user = FactoryBot.create(:user, email: "notfound@example.com") + discourse = DiscourseService.new(user) + + response = { + "success" => "OK", + "user_found" => false, + "email_token" => nil + } + + stub_discourse_request(:post, "u/email-token.json", {login: user.email}) + .to_return(status: 200, body: response.to_json, headers: {"Content-Type": "application/json"}) + + response = discourse.create_email_token + + expect(response["success"]).to eq("OK") + expect(response["user_found"]).to eq(false) + expect(response["email_token"]).to be_falsy + end + + xit "creates a email token against the API" do + user = FactoryBot.create(:user, email: "orlando@debtcollective.org") + + WebMock.allow_net_connect! + + discourse = DiscourseService.new(user) + response = discourse.create_email_token + + expect(response["success"]).to eq("OK") + expect(response["user_found"]).to eq(true) + expect(response["email_token"]).to be_truthy + + WebMock.disable_net_connect! + end + end end From 6804793fe9fa4f388e6586a2d08580839cf2d05e Mon Sep 17 00:00:00 2001 From: Orlando Del Aguila Date: Thu, 25 Feb 2021 12:59:32 -0600 Subject: [PATCH 04/10] dev(user): fix confirm! method --- app/models/user.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 3d223ad2..d2e62fe5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -104,8 +104,8 @@ def admin? end def confirm! - user.confirmed_at ||= DateTime.now - user.save! + self.confirmed_at ||= DateTime.now + save! end def activate! From 14da913086809fde1f0d11902637ebe94467ad77 Mon Sep 17 00:00:00 2001 From: Orlando Del Aguila Date: Thu, 25 Feb 2021 13:17:26 -0600 Subject: [PATCH 05/10] fix(discourse): fix logger --- app/jobs/link_discourse_account_job.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/jobs/link_discourse_account_job.rb b/app/jobs/link_discourse_account_job.rb index 22c7fcf4..1aad0e65 100644 --- a/app/jobs/link_discourse_account_job.rb +++ b/app/jobs/link_discourse_account_job.rb @@ -43,7 +43,7 @@ def perform(user) end user.save! - logger.info("Discourse account #{id} linked with user id #{self.id}") + logger.info("Discourse account #{response["user_id"]} linked with user id #{user.id}") UserMailer.welcome_email(user: user).deliver_later end From dbada9e4536dc7986ac1ffac9d5e5cff3a433e4e Mon Sep 17 00:00:00 2001 From: Orlando Del Aguila Date: Mon, 1 Mar 2021 16:34:12 -0600 Subject: [PATCH 06/10] dev: deprecate old email confirmation code --- .../user_confirmations_controller.rb | 46 ------------------- app/models/user.rb | 4 +- spec/features/user_confirmations_spec.rb | 39 ---------------- spec/services/discourse_service_spec.rb | 20 ++++++++ 4 files changed, 22 insertions(+), 87 deletions(-) delete mode 100644 spec/features/user_confirmations_spec.rb diff --git a/app/controllers/user_confirmations_controller.rb b/app/controllers/user_confirmations_controller.rb index 90a6a055..c7ad256c 100644 --- a/app/controllers/user_confirmations_controller.rb +++ b/app/controllers/user_confirmations_controller.rb @@ -3,52 +3,6 @@ class UserConfirmationsController < ApplicationController layout "minimal" - # GET /user_confirmations?confirmation_token=abcdef - def index - @confirmation_token = params[:confirmation_token] - @user = User.find_by_confirmation_token(@confirmation_token) - - respond_to do |format| - if @user - format.html { render :index, status: :ok } - else - format.html { render :index, status: :not_found } - end - end - end - - # POST /user_confirmations - def create - user = User.send_confirmation_instructions(email: current_user&.email || params[:email]) - - respond_to do |format| - if user.errors.empty? - format.json { render json: {status: "success", message: "Confirmation email sent"}, status: :ok } - else - format.json { render json: {status: "failed", message: "Invalid confirmation token"}, status: :not_found } - end - end - end - - # POST /user_confirmations/confirm - def confirm - @user = User.confirm_by_token(user_confirmation_params[:confirmation_token]) - @user_confirmed = @user.errors.empty? - - respond_to do |format| - if @user_confirmed - # run the Discourse link account job to fetch and link with a Discourse account - LinkDiscourseAccountJob.perform_later(@user) - - format.html { render :confirm, notice: "Email confirmed", status: :ok } - format.json { render json: {status: "success", message: "Email confirmed"}, status: :ok } - else - format.html { render :confirm, notice: "Invalid confirmation token", status: :not_found } - format.json { render json: {status: "failed", message: "Invalid confirmation token"}, status: :not_found } - end - end - end - # GET /user_confirmations/confirm_email/:email_token def confirm_email_token @email_token = params[:email_token] diff --git a/app/models/user.rb b/app/models/user.rb index d2e62fe5..0256b8a9 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -62,8 +62,8 @@ def self.find_or_create_from_sso(payload) user[sso_attribute] = payload[sso_attribute] end - # Since the user comes from SSO, we can confirm the accoun - user.confirm! + # Since the user comes from SSO, we can confirm the account + user.confirmed_at ||= DateTime.now new_record = user.new_record? user.save diff --git a/spec/features/user_confirmations_spec.rb b/spec/features/user_confirmations_spec.rb deleted file mode 100644 index af7002a5..00000000 --- a/spec/features/user_confirmations_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" - -describe "UserConfirmations", type: :feature do - context "with valid token" do - it "validates user after completing flow" do - user = FactoryBot.create(:user, confirmation_token: "token") - - visit "/user_confirmations?confirmation_token=#{user.confirmation_token}" - - click_button "Confirm my email" - - user.reload - expect(user.confirmed_at.to_i).to be_within(100).of(DateTime.now.to_i) - end - end - - context "with invalid token" do - it "shows invalid token error" do - FactoryBot.create(:user) - - visit "/user_confirmations?confirmation_token=invalid" - - expect(page).to have_content("Invalid confirmation token") - end - end - - context "alredy validated user" do - it "shows already confirmed error" do - user = FactoryBot.create(:user, confirmation_token: "token", confirmed_at: DateTime.now) - - visit "/user_confirmations?confirmation_token=#{user.confirmation_token}" - - expect(page).to have_content("Your email has been already confirmed") - expect(page).to have_content("Click here to go to the login page") - end - end -end diff --git a/spec/services/discourse_service_spec.rb b/spec/services/discourse_service_spec.rb index 1ee0ece6..f64cd795 100644 --- a/spec/services/discourse_service_spec.rb +++ b/spec/services/discourse_service_spec.rb @@ -195,6 +195,26 @@ expect(response["email_token"]).to be_falsy end + it "creates an email token" do + user = FactoryBot.create(:user, email: "orlando@debtcollective.org") + + response = { + "success" => "OK", + "user_found" => true, + "email_token" => "29fa89dd95e1f5008fbad2b3fca9011e" + } + + stub_discourse_request(:post, "u/email-token.json", {login: user.email}) + .to_return(status: 200, body: response.to_json, headers: {"Content-Type": "application/json"}) + + discourse = DiscourseService.new(user) + response = discourse.create_email_token + + expect(response["success"]).to eq("OK") + expect(response["user_found"]).to eq(true) + expect(response["email_token"]).to be_truthy + end + xit "creates a email token against the API" do user = FactoryBot.create(:user, email: "orlando@debtcollective.org") From ffbcc3ae868bd9b2f854957727c5d6e439b8d771 Mon Sep 17 00:00:00 2001 From: Orlando Del Aguila Date: Mon, 1 Mar 2021 16:52:13 -0600 Subject: [PATCH 07/10] test: fix tests and remove deprecated methods --- .../user_confirmations_controller.rb | 2 +- app/jobs/link_discourse_account_job.rb | 2 +- app/models/user.rb | 39 +---------- .../confirm_email_token.html.erb | 2 +- app/views/user_confirmations/index.html.erb | 21 ------ .../user_confirmations_controller_spec.rb | 67 +------------------ 6 files changed, 9 insertions(+), 124 deletions(-) delete mode 100644 app/views/user_confirmations/index.html.erb diff --git a/app/controllers/user_confirmations_controller.rb b/app/controllers/user_confirmations_controller.rb index c7ad256c..da5a515c 100644 --- a/app/controllers/user_confirmations_controller.rb +++ b/app/controllers/user_confirmations_controller.rb @@ -23,7 +23,7 @@ def confirm_email @user = User.find_by_email_token!(@email_token) respond_to do |format| - if @user&.activate! + if @user&.confirm! email_login_url = "#{ENV["DISCOURSE_URL"]}/session/email-login/#{@user.email_token}" format.html { redirect_to email_login_url } diff --git a/app/jobs/link_discourse_account_job.rb b/app/jobs/link_discourse_account_job.rb index 1aad0e65..4b5d85b0 100644 --- a/app/jobs/link_discourse_account_job.rb +++ b/app/jobs/link_discourse_account_job.rb @@ -45,6 +45,6 @@ def perform(user) user.save! logger.info("Discourse account #{response["user_id"]} linked with user id #{user.id}") - UserMailer.welcome_email(user: user).deliver_later + user.send_welcome_email end end diff --git a/app/models/user.rb b/app/models/user.rb index 0256b8a9..fd29f7dc 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -71,45 +71,12 @@ def self.find_or_create_from_sso(payload) [user, new_record] end - def self.send_confirmation_instructions(email:) - user = User.find_by_email(email.downcase) - - if user - confirmation_token = user.confirmation_token || SecureRandom.hex(20) - user.update(confirmation_token: confirmation_token, confirmation_sent_at: DateTime.now) - UserMailer.confirmation_email(user: user).deliver_later - else - user = User.new - user.errors.add(:base, "User not found") - end - - user - end - - def self.confirm_by_token(confirmation_token) - user = User.find_by_confirmation_token(confirmation_token) - - if user - user.update(confirmation_token: nil, confirmed_at: DateTime.now) - else - user = User.new - user.errors.add(:base, "Invalid confirmation token") - end - - user - end - - def admin? - !!admin - end - def confirm! - self.confirmed_at ||= DateTime.now - save! + update!(confirmed_at: DateTime.now) end - def activate! - update!(active: true, activated_at: DateTime.now) + def send_welcome_email + UserMailer.welcome_email(user: self).deliver_later end # TODO: We are getting first_name and last_name from users when the join the union diff --git a/app/views/user_confirmations/confirm_email_token.html.erb b/app/views/user_confirmations/confirm_email_token.html.erb index 3c022190..56426fe1 100644 --- a/app/views/user_confirmations/confirm_email_token.html.erb +++ b/app/views/user_confirmations/confirm_email_token.html.erb @@ -1,5 +1,5 @@ <% if @user %> - <% if @user.active? %> + <% if @user.confirmed? %>

Your email has been already confirmed

<%= link_to "Click here to go to the login page", discourse_url('/login') %>

diff --git a/app/views/user_confirmations/index.html.erb b/app/views/user_confirmations/index.html.erb deleted file mode 100644 index ae36c830..00000000 --- a/app/views/user_confirmations/index.html.erb +++ /dev/null @@ -1,21 +0,0 @@ -<% if @user %> - <% if @user.confirmed? %> -

Your email has been already confirmed

- -

<%= link_to "Click here to go to the login page", discourse_url('/login') %>

- <% else %> -

Please click the button below to confirm your email

- - <%= form_with(url: confirm_user_confirmations_path, method: 'post', local: true) do %> - <%= fields_for(:user_confirmation) do |f|%> - <%= f.hidden_field(:confirmation_token, value: @confirmation_token) %> - -
- <%= f.submit "Confirm my email", id: "button-submit", class: "button primary", data: { disable_with: "Loading..."} %> -
- <% end %> - <% end %> - <% end %> -<% else %> -

Invalid confirmation token

-<% end %> diff --git a/spec/controllers/user_confirmations_controller_spec.rb b/spec/controllers/user_confirmations_controller_spec.rb index 2bd649fe..173cee09 100644 --- a/spec/controllers/user_confirmations_controller_spec.rb +++ b/spec/controllers/user_confirmations_controller_spec.rb @@ -3,67 +3,6 @@ require "rails_helper" RSpec.describe UserConfirmationsController, type: :controller do - describe "GET #index" do - it "returns a success response if token is found" do - user = FactoryBot.create(:user_with_confirmation_token) - - get :index, params: {confirmation_token: user.confirmation_token} - - expect(response.status).to eq(200) - end - - it "returns not found if token is invalid" do - get :index, params: {confirmation_token: SecureRandom.hex(20)} - - expect(response.status).to eq(404) - end - end - - describe "POST #create" do - # This endpoint only supports json - before(:each) do - request.accept = "application/json" - end - - it "creates and send user confirmation instructions" do - user = FactoryBot.create(:user) - - expect(UserMailer).to receive_message_chain(:confirmation_email, :deliver_later) - - post :create, params: {email: user.email} - - expect(response.status).to eq(200) - end - - it "returns not found if token is invalid" do - post :create, params: {email: Faker::Internet.email} - - expect(response.status).to eq(404) - end - end - - describe "POST #confirm" do - it "confirms user email if token is valid" do - user = FactoryBot.create(:user_with_confirmation_token) - - expect(LinkDiscourseAccountJob).to receive_message_chain(:perform_later) - - post :confirm, params: {user_confirmation: {confirmation_token: user.confirmation_token}} - user.reload - - expect(response.status).to eq(200) - expect(user.confirmed_at).to be_within(1.second).of DateTime.now - expect(user.confirmed?).to eq(true) - expect(user.confirmation_token).to be_nil - end - - it "returns not found if token is invalid" do - post :confirm, params: {user_confirmation: {confirmation_token: SecureRandom.hex(20)}} - - expect(response.status).to eq(404) - end - end - describe "GET #confirm_email_token" do render_views @@ -75,7 +14,7 @@ expect(response.status).to eq(200) expect(response.body).to include("Confirm my email") - expect(user.active?).to eq(false) + expect(user.confirmed?).to eq(false) end it "returns not found if token is invalid" do @@ -91,8 +30,8 @@ user.reload expect(response.status).to eq(302) - expect(user.active?).to eq(true) - expect(user.activated_at).to be_within(1.second).of DateTime.now + expect(user.confirmed?).to eq(true) + expect(user.confirmed_at).to be_within(1.second).of DateTime.now end it "returns not found if token is invalid" do From 534b9685c7d78b9e35964fa638843d7277687a70 Mon Sep 17 00:00:00 2001 From: Orlando Del Aguila Date: Mon, 1 Mar 2021 16:58:41 -0600 Subject: [PATCH 08/10] dev: fix logger --- app/jobs/link_discourse_account_job.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/jobs/link_discourse_account_job.rb b/app/jobs/link_discourse_account_job.rb index 4b5d85b0..804cad5d 100644 --- a/app/jobs/link_discourse_account_job.rb +++ b/app/jobs/link_discourse_account_job.rb @@ -26,6 +26,7 @@ def perform(user) end user.email_token = response["email_token"] + logger.info("Discourse account #{discourse_user["id"]} linked with user id #{user.id}") else # Create discourse account response = discourse.create_user @@ -40,10 +41,10 @@ def perform(user) user.email_token = response["email_token"] user.external_id = response["user_id"] user.username = response["username"] + logger.info("Discourse account created. discourse_id: #{response["user_id"]} user_id: #{user.id}") end user.save! - logger.info("Discourse account #{response["user_id"]} linked with user id #{user.id}") user.send_welcome_email end From 779051bd0e83d9f33c9f0450a708fe43b791765a Mon Sep 17 00:00:00 2001 From: Orlando Del Aguila Date: Mon, 1 Mar 2021 17:03:14 -0600 Subject: [PATCH 09/10] dev: remove user.activated_at column --- app/models/user.rb | 1 - db/migrate/20210301230150_remove_activated_at_from_user.rb | 5 +++++ db/schema.rb | 3 +-- spec/factories/users.rb | 1 - spec/models/user_spec.rb | 1 - 5 files changed, 6 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20210301230150_remove_activated_at_from_user.rb diff --git a/app/models/user.rb b/app/models/user.rb index fd29f7dc..cf7e98f6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -5,7 +5,6 @@ # Table name: users # # id :bigint not null, primary key -# activated_at :datetime # admin :boolean default(FALSE) # avatar_url :string # banned :boolean default(FALSE) diff --git a/db/migrate/20210301230150_remove_activated_at_from_user.rb b/db/migrate/20210301230150_remove_activated_at_from_user.rb new file mode 100644 index 00000000..ce4e1a99 --- /dev/null +++ b/db/migrate/20210301230150_remove_activated_at_from_user.rb @@ -0,0 +1,5 @@ +class RemoveActivatedAtFromUser < ActiveRecord::Migration[6.1] + def change + remove_column :users, :activated_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 71313041..6f41490e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_02_24_211702) do +ActiveRecord::Schema.define(version: 2021_03_01_230150) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -111,7 +111,6 @@ t.string "username" t.datetime "confirmed_at" t.datetime "confirmation_sent_at" - t.datetime "activated_at" t.string "email_token" t.index ["email"], name: "index_users_on_email", unique: true end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 7a79df18..2bda633c 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -5,7 +5,6 @@ # Table name: users # # id :bigint not null, primary key -# activated_at :datetime # admin :boolean default(FALSE) # avatar_url :string # banned :boolean default(FALSE) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 39266bab..f1271783 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -5,7 +5,6 @@ # Table name: users # # id :bigint not null, primary key -# activated_at :datetime # admin :boolean default(FALSE) # avatar_url :string # banned :boolean default(FALSE) From fe5b87a764f8fc5a0cd2285d21da6cc1c6d8e847 Mon Sep 17 00:00:00 2001 From: Orlando Del Aguila Date: Tue, 2 Mar 2021 13:29:54 -0600 Subject: [PATCH 10/10] test: improve test description --- spec/jobs/link_discourse_account_job_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/jobs/link_discourse_account_job_spec.rb b/spec/jobs/link_discourse_account_job_spec.rb index aa3e8b33..70fc585a 100644 --- a/spec/jobs/link_discourse_account_job_spec.rb +++ b/spec/jobs/link_discourse_account_job_spec.rb @@ -13,8 +13,8 @@ end describe "#perform" do - context "existing account" do - it "it links user and sends welcome email" do + context "existing discourse account" do + it "persist discourse user information on the user record" do user = FactoryBot.create(:user, external_id: nil) email_token = SecureRandom.hex(20) @@ -31,7 +31,7 @@ end end - context "new account" do + context "new discourse account" do it "creates a discourse account and updates the user record" do user = FactoryBot.create(:user, external_id: nil) response = {