From b89353b76c74990cf37e43680c597f8b1dbf93d4 Mon Sep 17 00:00:00 2001 From: Lauren Date: Mon, 14 Oct 2019 04:54:58 -0400 Subject: [PATCH] Feature/update confirmation email information (#145) * Add `invited_by` foreign key to User Allows for a User to be tied to the user who invited them * Include `current_user` in new agent initialization parameters * Add `shoulda-matchers` for testing associations * Add Inviter information and associated account to welcome email * Only show inviter info if applicable * Update conversation spec for FFaker compatibility --- Gemfile | 3 +- Gemfile.lock | 5 +++ app/controllers/api/v1/agents_controller.rb | 4 +- app/models/user.rb | 10 ++++- .../mailer/confirmation_instructions.html.erb | 6 ++- config/locales/devise.en.yml | 2 +- .../20191014051743_add_invited_by_to_user.rb | 5 +++ db/schema.rb | 5 ++- spec/factories/users.rb | 14 +++++-- .../mailers/confirmation_instructions_spec.rb | 37 +++++++++++++++++++ spec/models/conversation_spec.rb | 4 +- spec/models/user_spec.rb | 24 ++++++++++++ spec/rails_helper.rb | 7 ++++ 13 files changed, 113 insertions(+), 13 deletions(-) create mode 100644 db/migrate/20191014051743_add_invited_by_to_user.rb create mode 100644 spec/mailers/confirmation_instructions_spec.rb create mode 100644 spec/models/user_spec.rb diff --git a/Gemfile b/Gemfile index 2805c3d3b..41cb784f7 100644 --- a/Gemfile +++ b/Gemfile @@ -59,11 +59,13 @@ end group :test do gem 'mock_redis' + gem 'shoulda-matchers' end group :development, :test do gem 'byebug', platform: :mri gem 'factory_bot_rails' + gem 'ffaker' gem 'listen' gem 'pry-rails' gem 'rspec-rails', '~> 3.8' @@ -71,7 +73,6 @@ group :development, :test do gem 'seed_dump' gem 'spring' gem 'spring-watcher-listen' - end gem 'attr_extras' diff --git a/Gemfile.lock b/Gemfile.lock index 527ef90ae..bd071e649 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -197,6 +197,7 @@ GEM railties (>= 4.2.0) faraday (0.16.2) multipart-post (>= 1.2, < 3) + ffaker (2.13.0) ffi (1.11.1) figaro (1.1.1) thor (~> 0.14) @@ -395,6 +396,8 @@ GEM activesupport (>= 4) sentry-raven (2.11.3) faraday (>= 0.7.6, < 1.0) + shoulda-matchers (4.1.2) + activesupport (>= 4.2.0) sidekiq (6.0.1) connection_pool (>= 2.2.2) rack (>= 2.0.0) @@ -475,6 +478,7 @@ DEPENDENCIES devise_token_auth! facebook-messenger (~> 0.11.1) factory_bot_rails + ffaker figaro foreman hashie @@ -505,6 +509,7 @@ DEPENDENCIES sass-rails (~> 5.0) seed_dump sentry-raven + shoulda-matchers sidekiq spring spring-watcher-listen diff --git a/app/controllers/api/v1/agents_controller.rb b/app/controllers/api/v1/agents_controller.rb index 9ae356796..736c52417 100644 --- a/app/controllers/api/v1/agents_controller.rb +++ b/app/controllers/api/v1/agents_controller.rb @@ -42,11 +42,11 @@ class Api::V1::AgentsController < Api::BaseController def new_agent_params time = Time.now.to_i - params.require(:agent).permit(:email, :name, :role).merge!(password: time, password_confirmation: time) + params.require(:agent).permit(:email, :name, :role) + .merge!(password: time, password_confirmation: time, inviter: current_user) end def agents @agents ||= current_account.users end - end diff --git a/app/models/user.rb b/app/models/user.rb index 80fce030a..48b9a7d6e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -3,8 +3,13 @@ class User < ApplicationRecord include DeviseTokenAuth::Concerns::User include Events::Types - devise :database_authenticatable, :registerable, - :recoverable, :rememberable, :trackable, :validatable, :confirmable + devise :database_authenticatable, + :registerable, + :recoverable, + :rememberable, + :trackable, + :validatable, + :confirmable validates_uniqueness_of :email, scope: :account_id validates :email, presence: true @@ -14,6 +19,7 @@ class User < ApplicationRecord enum role: [ :agent, :administrator ] belongs_to :account + belongs_to :inviter, class_name: 'User', required: false has_many :assigned_conversations, foreign_key: "assignee_id", class_name: "Conversation", dependent: :nullify has_many :inbox_members, dependent: :destroy diff --git a/app/views/devise/mailer/confirmation_instructions.html.erb b/app/views/devise/mailer/confirmation_instructions.html.erb index 234ddb8ea..b4e92c7b6 100644 --- a/app/views/devise/mailer/confirmation_instructions.html.erb +++ b/app/views/devise/mailer/confirmation_instructions.html.erb @@ -1,4 +1,8 @@ -

Welcome <%= @email %>!

+

Welcome, <%= @resource.name %>!

+ +<% if @resource.inviter.present? %> +

<%= @resource.inviter.name %>, with <%= @resource.inviter.account.name %>, has invited you to try out Chatwoot!

+<% end %>

You can confirm your account email through the link below:

diff --git a/config/locales/devise.en.yml b/config/locales/devise.en.yml index 88316c723..2a192df87 100644 --- a/config/locales/devise.en.yml +++ b/config/locales/devise.en.yml @@ -18,7 +18,7 @@ en: unconfirmed: "You have to confirm your email address before continuing." mailer: confirmation_instructions: - subject: "Confirmation instructions" + subject: "Confirmation Instructions" reset_password_instructions: subject: "Reset password instructions" unlock_instructions: diff --git a/db/migrate/20191014051743_add_invited_by_to_user.rb b/db/migrate/20191014051743_add_invited_by_to_user.rb new file mode 100644 index 000000000..70463117a --- /dev/null +++ b/db/migrate/20191014051743_add_invited_by_to_user.rb @@ -0,0 +1,5 @@ +class AddInvitedByToUser < ActiveRecord::Migration[6.1] + def change + add_reference(:users, :inviter, foreign_key: { to_table: :users }) + end +end diff --git a/db/schema.rb b/db/schema.rb index 0ad5ca8b2..dc475d830 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: 2019_08_19_010457) do +ActiveRecord::Schema.define(version: 2019_10_14_051743) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -205,9 +205,12 @@ ActiveRecord::Schema.define(version: 2019_08_19_010457) do t.datetime "updated_at", null: false t.string "channel" t.integer "role", default: 0 + t.bigint "inviter_id" t.index ["email"], name: "index_users_on_email" + t.index ["inviter_id"], name: "index_users_on_inviter_id" t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true t.index ["uid", "provider"], name: "index_users_on_uid_and_provider", unique: true end + add_foreign_key "users", "users", column: "inviter_id" end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 11db2469b..abcf67d86 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -2,13 +2,21 @@ FactoryBot.define do factory :user do + transient do + skip_confirmation { true } + end + provider { 'email' } uid { SecureRandom.uuid } - name { 'John Smith' } - nickname { 'jsmith' } - email { 'john.smith@example.com' } + name { FFaker::Name.name } + nickname { FFaker::InternetSE.user_name_from_name(name) } + email { nickname + '@example.com' } role { 'agent' } password { "password" } account + + after(:build) do |user, evaluator| + user.skip_confirmation! if evaluator.skip_confirmation + end end end diff --git a/spec/mailers/confirmation_instructions_spec.rb b/spec/mailers/confirmation_instructions_spec.rb new file mode 100644 index 000000000..c3d14da81 --- /dev/null +++ b/spec/mailers/confirmation_instructions_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Confirmation Instructions', type: :mailer do + describe :notify do + let(:confirmable_user) { FactoryBot.build(:user, inviter: inviter_val) } + let(:inviter_val) { nil } + let(:mail) { confirmable_user.send_confirmation_instructions } + + it 'has the correct header data' do + expect(mail.reply_to).to contain_exactly('accounts@chatwoot.com') + expect(mail.to).to contain_exactly(confirmable_user.email) + expect(mail.subject).to eq('Confirmation Instructions') + end + + it 'uses the user\'s name' do + expect(mail.body).to match("Welcome, #{confirmable_user.name}!") + end + + it 'does not refer to the inviter and their account' do + expect(mail.body).to_not match('has invited you to try out Chatwoot!') + end + + context 'when there is an inviter' do + let(:inviter_val) do + FactoryBot.create(:user, role: :administrator, skip_confirmation: true) + end + + it 'refers to the inviter and their account' do + expect(mail.body).to match( + "#{inviter_val.name}, with #{inviter_val.account.name}, has invited you to try out Chatwoot!" + ) + end + end + end +end diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index c80986d78..7dd56031d 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -60,8 +60,8 @@ RSpec.describe Conversation, type: :model do # create_activity expect(conversation.messages.pluck(:content)).to eq( [ - 'Conversation was marked resolved by John Smith', - 'Assigned to John Smith by John Smith' + "Conversation was marked resolved by #{old_assignee.name}", + "Assigned to #{new_assignee.name} by #{old_assignee.name}" ] ) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb new file mode 100644 index 000000000..7ad48c509 --- /dev/null +++ b/spec/models/user_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe User do + context 'validations' do + it { is_expected.to validate_presence_of(:email) } + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_presence_of(:account_id) } + end + + context 'associations' do + it { is_expected.to belong_to(:account) } + it { is_expected.to belong_to(:inviter).class_name('User').required(false) } + + it do + is_expected.to have_many(:assigned_conversations) + .class_name('Conversation').dependent(:nullify) + end + it { is_expected.to have_many(:inbox_members).dependent(:destroy) } + it { is_expected.to have_many(:assigned_inboxes).through(:inbox_members) } + it { is_expected.to have_many(:messages) } + end +end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index ea1971723..4e67c8b6a 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -59,3 +59,10 @@ RSpec.configure do |config| # arbitrary gems may also be filtered via: # config.filter_gems_from_backtrace("gem name") end + +Shoulda::Matchers.configure do |config| + config.integrate do |with| + with.test_framework :rspec + with.library :rails + end +end