From 2ee144c27f78749aebc4fe97bc62a9fde6cf50c2 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Thu, 1 Jun 2017 17:19:42 +0900 Subject: [PATCH 1/2] FEATURE: Add DiscourseEvent trigger when a user logs in. * Also adds a event trigger when user logs in for the first time. --- app/controllers/users_email_controller.rb | 1 - app/models/user.rb | 8 ++++ lib/current_user.rb | 1 + spec/controllers/invites_controller_spec.rb | 19 +++++++- spec/controllers/session_controller_spec.rb | 17 ++++++- spec/controllers/users_controller_spec.rb | 47 ++++++++++++------- .../users_email_controller_spec.rb | 8 +++- spec/integration/omniauth_callbacks_spec.rb | 8 +++- 8 files changed, 86 insertions(+), 23 deletions(-) diff --git a/app/controllers/users_email_controller.rb b/app/controllers/users_email_controller.rb index a61fd57819f..474e370c781 100644 --- a/app/controllers/users_email_controller.rb +++ b/app/controllers/users_email_controller.rb @@ -45,4 +45,3 @@ class UsersEmailController < ApplicationController end end - diff --git a/app/models/user.rb b/app/models/user.rb index f3d7e325c6b..0743ff766dc 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -912,6 +912,14 @@ class User < ActiveRecord::Base DiscourseEvent.trigger(:user_logged_out, self) end + def logged_in + DiscourseEvent.trigger(:user_logged_in, self) + + if !self.seen_before? + DiscourseEvent.trigger(:user_first_logged_in, self) + end + end + protected def badge_grant diff --git a/lib/current_user.rb b/lib/current_user.rb index 79a04e4a71f..01c5570028b 100644 --- a/lib/current_user.rb +++ b/lib/current_user.rb @@ -16,6 +16,7 @@ module CurrentUser def log_on_user(user) current_user_provider.log_on_user(user,session,cookies) + user.logged_in end def log_off_user diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb index 90795927759..efb69302d5e 100644 --- a/spec/controllers/invites_controller_spec.rb +++ b/spec/controllers/invites_controller_spec.rb @@ -197,7 +197,12 @@ describe InvitesController do end it 'logs in the user' do - subject + events = DiscourseEvent.track_events { subject } + + expect(events.map { |event| event[:event_name] }).to include( + :user_logged_in, :user_first_logged_in + ) + expect(session[:current_user_id]).to eq(user.id) end @@ -378,10 +383,20 @@ describe InvitesController do before do Invite.expects(:redeem_from_token).with(invite.invite_key, user.email, nil, nil, topic.id).returns(user) - get :redeem_disposable_invite, email: user.email, token: invite.invite_key, topic: topic.id end it 'logs in user' do + events = DiscourseEvent.track_events do + get :redeem_disposable_invite, + email: user.email, + token: invite.invite_key, + topic: topic.id + end + + expect(events.map { |event| event[:event_name] }).to include( + :user_logged_in, :user_first_logged_in + ) + expect(session[:current_user_id]).to eq(user.id) end diff --git a/spec/controllers/session_controller_spec.rb b/spec/controllers/session_controller_spec.rb index 5645eca8221..679df2d59be 100644 --- a/spec/controllers/session_controller_spec.rb +++ b/spec/controllers/session_controller_spec.rb @@ -185,7 +185,14 @@ describe SessionController do sso.custom_fields["shop_url"] = "http://my_shop.com" sso.custom_fields["shop_name"] = "Sam" - get :sso_login, Rack::Utils.parse_query(sso.payload) + events = DiscourseEvent.track_events do + get :sso_login, Rack::Utils.parse_query(sso.payload) + end + + expect(events.map { |event| event[:event_name] }).to include( + :user_logged_in, :user_first_logged_in + ) + expect(response).to redirect_to('/a/') logged_on_user = Discourse.current_user_provider.new(request.env).current_user @@ -500,7 +507,13 @@ describe SessionController do describe 'success by username' do it 'logs in correctly' do - xhr :post, :create, login: user.username, password: 'myawesomepassword' + events = DiscourseEvent.track_events do + xhr :post, :create, login: user.username, password: 'myawesomepassword' + end + + expect(events.map { |event| event[:event_name] }).to include( + :user_logged_in, :user_first_logged_in + ) user.reload diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 8c2c33e48da..37303fb7686 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -152,10 +152,17 @@ describe UsersController do before do Guardian.any_instance.expects(:can_access_forum?).returns(true) EmailToken.expects(:confirm).with('asdfasdf').returns(user) - put :perform_account_activation, token: 'asdfasdf' end it 'correctly logs on user' do + events = DiscourseEvent.track_events do + put :perform_account_activation, token: 'asdfasdf' + end + + expect(events.map { |event| event[:event_name] }).to include( + :user_logged_in, :user_first_logged_in + ) + expect(response).to be_success expect(flash[:error]).to be_blank expect(session[:current_user_id]).to be_present @@ -266,9 +273,15 @@ describe UsersController do user = Fabricate(:user) user_auth_token = UserAuthToken.generate!(user_id: user.id) token = user.email_tokens.create(email: user.email).token - get :password_reset, token: token - put :password_reset, token: token, password: 'hg9ow8yhg98o' + + events = DiscourseEvent.track_events do + put :password_reset, token: token, password: 'hg9ow8yhg98o' + end + + expect(events.map { |event| event[:event_name] }).to include( + :user_logged_in, :user_first_logged_in + ) expect(response).to be_success expect(assigns[:error]).to be_blank @@ -386,22 +399,24 @@ describe UsersController do expect(session[:current_user_id]).to be_blank end - it 'does log in admin with valid token and SSO disabled' do - SiteSetting.enable_sso = false - token = admin.email_tokens.create(email: admin.email).token + context 'valid token' do + it 'does log in admin with SSO disabled' do + SiteSetting.enable_sso = false + token = admin.email_tokens.create(email: admin.email).token - get :admin_login, token: token - expect(response).to redirect_to('/') - expect(session[:current_user_id]).to eq(admin.id) - end + get :admin_login, token: token + expect(response).to redirect_to('/') + expect(session[:current_user_id]).to eq(admin.id) + end - it 'logs in admin with valid token and SSO enabled' do - SiteSetting.enable_sso = true - token = admin.email_tokens.create(email: admin.email).token + it 'logs in admin with SSO enabled' do + SiteSetting.enable_sso = true + token = admin.email_tokens.create(email: admin.email).token - get :admin_login, token: token - expect(response).to redirect_to('/') - expect(session[:current_user_id]).to eq(admin.id) + get :admin_login, token: token + expect(response).to redirect_to('/') + expect(session[:current_user_id]).to eq(admin.id) + end end end end diff --git a/spec/controllers/users_email_controller_spec.rb b/spec/controllers/users_email_controller_spec.rb index e43ad3befea..c10aad9aa07 100644 --- a/spec/controllers/users_email_controller_spec.rb +++ b/spec/controllers/users_email_controller_spec.rb @@ -35,7 +35,13 @@ describe UsersEmailController do it 'confirms with a correct token' do user.user_stat.update_columns(bounce_score: 42, reset_bounce_score_after: 1.week.from_now) - get :confirm, token: user.email_tokens.last.token + events = DiscourseEvent.track_events do + get :confirm, token: user.email_tokens.last.token + end + + expect(events.map { |event| event[:event_name] }).to include( + :user_logged_in, :user_first_logged_in + ) expect(response).to be_success expect(assigns(:update_result)).to eq(:complete) diff --git a/spec/integration/omniauth_callbacks_spec.rb b/spec/integration/omniauth_callbacks_spec.rb index d2616894f57..59b22b2ff45 100644 --- a/spec/integration/omniauth_callbacks_spec.rb +++ b/spec/integration/omniauth_callbacks_spec.rb @@ -49,7 +49,13 @@ RSpec.describe "OmniAuth Callbacks" do end it 'should return the right response' do - get "/auth/google_oauth2/callback.json" + events = DiscourseEvent.track_events do + get "/auth/google_oauth2/callback.json" + end + + expect(events.map { |event| event[:event_name] }).to include( + :user_logged_in, :user_first_logged_in + ) expect(response).to be_success From edbb876d1b499ab458b40896f837d113992cce71 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Thu, 1 Jun 2017 17:20:16 +0900 Subject: [PATCH 2/2] FIX: Discobot welcome post delay should start counting when a user logs in. --- plugins/discourse-narrative-bot/plugin.rb | 12 ++++ .../integration/discobot_welcome_post_spec.rb | 67 +++++++++++++++++++ .../discourse-narrative-bot/spec/user_spec.rb | 24 ++----- 3 files changed, 84 insertions(+), 19 deletions(-) create mode 100644 plugins/discourse-narrative-bot/spec/integration/discobot_welcome_post_spec.rb diff --git a/plugins/discourse-narrative-bot/plugin.rb b/plugins/discourse-narrative-bot/plugin.rb index 0cf53ee3b1e..7c951dbd90f 100644 --- a/plugins/discourse-narrative-bot/plugin.rb +++ b/plugins/discourse-narrative-bot/plugin.rb @@ -114,6 +114,18 @@ after_initialize do end self.add_model_callback(User, :after_commit, on: :create) do + if SiteSetting.discourse_narrative_bot_welcome_post_delay == 0 + self.enqueue_bot_welcome_post + end + end + + self.on(:user_first_logged_in) do |user| + if SiteSetting.discourse_narrative_bot_welcome_post_delay > 0 + user.enqueue_bot_welcome_post + end + end + + self.add_to_class(:user, :enqueue_bot_welcome_post) do return if SiteSetting.disable_discourse_narrative_bot_welcome_post delay = SiteSetting.discourse_narrative_bot_welcome_post_delay diff --git a/plugins/discourse-narrative-bot/spec/integration/discobot_welcome_post_spec.rb b/plugins/discourse-narrative-bot/spec/integration/discobot_welcome_post_spec.rb new file mode 100644 index 00000000000..d47ea393266 --- /dev/null +++ b/plugins/discourse-narrative-bot/spec/integration/discobot_welcome_post_spec.rb @@ -0,0 +1,67 @@ +require 'rails_helper' + +describe "Discobot welcome post" do + let(:user) { Fabricate(:user) } + + before do + SiteSetting.queue_jobs = true + SiteSetting.discourse_narrative_bot_enabled = true + end + + after do + Jobs::NarrativeInit.jobs.clear + end + + context 'when discourse_narrative_bot_welcome_post_delay is 0' do + it 'should not delay the welcome post' do + user + expect { sign_in(user) }.to_not change { Jobs::NarrativeInit.jobs.count } + end + end + + context 'When discourse_narrative_bot_welcome_post_delay is greater than 0' do + before do + SiteSetting.discourse_narrative_bot_welcome_post_delay = 5 + end + + context 'when user logs in normally' do + it 'should delay the welcome post until user logs in' do + expect { sign_in(user) }.to change { Jobs::NarrativeInit.jobs.count }.by(1) + expect(Jobs::NarrativeInit.jobs.first["args"].first["user_id"]).to eq(user.id) + end + end + + context 'when user redeems an invite' do + let(:invite) { Fabricate(:invite, invited_by: Fabricate(:admin), email: 'testing@gmail.com') } + + it 'should delay the welcome post until the user logs in' do + invite + + expect do + xhr :put, "/invites/show/#{invite.invite_key}", + username: 'somename', + name: 'testing', + password: 'asodaasdaosdhq' + end.to change { User.count }.by(1) + + expect(Jobs::NarrativeInit.jobs.first["args"].first["user_id"]).to eq(User.last.id) + end + end + + context 'when user redeems a disposable invite' do + it 'should delay the welcome post until the user logs in' do + token = Invite.generate_disposable_tokens(user).first + + expect do + xhr :get, "/invites/redeem/#{token}", + email: 'testing@gmail.com', + username: 'somename', + name: 'testing', + password: 'asodaasdaosdhq' + end.to change { User.count }.by(1) + + expect(Jobs::NarrativeInit.jobs.first["args"].first["user_id"]).to eq(User.last.id) + end + end + end +end diff --git a/plugins/discourse-narrative-bot/spec/user_spec.rb b/plugins/discourse-narrative-bot/spec/user_spec.rb index a18d48837ce..326631fc654 100644 --- a/plugins/discourse-narrative-bot/spec/user_spec.rb +++ b/plugins/discourse-narrative-bot/spec/user_spec.rb @@ -30,7 +30,7 @@ describe User do end end - describe 'enabled' do + context 'enabled' do before do SiteSetting.disable_discourse_narrative_bot_welcome_post = false end @@ -58,30 +58,16 @@ describe User do end end - describe 'when welcome message is delayed' do + describe 'when welcome message is configured to be delayed' do before do SiteSetting.discourse_narrative_bot_welcome_post_delay = 100 SiteSetting.queue_jobs = true end - it 'should delay the initialization of the new user track' do - Timecop.freeze do - user + it 'should delay the welcome post until user logs in' do + user - expect(Jobs::NarrativeInit.jobs.first['at']) - .to be_within(1.second).of(Time.zone.now.to_f + 100) - end - end - - it 'should delay sending the welcome message' do - SiteSetting.discourse_narrative_bot_welcome_post_type = 'welcome_message' - - Timecop.freeze do - user - - expect(Jobs::SendDefaultWelcomeMessage.jobs.first['at']) - .to be_within(1.second).of(Time.zone.now.to_f + 100) - end + expect(Jobs::NarrativeInit.jobs.count).to eq(0) end end end