From ea05a68df7eee15c9db55c26d55efb21bb17247f Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Tue, 17 Dec 2019 10:13:49 -0500 Subject: [PATCH] FIX: Use updated_at date to denote expired invites (#8521) --- app/models/invite.rb | 6 +++--- app/models/invite_redeemer.rb | 2 +- config/locales/client.en.yml | 2 +- .../20191211152404_add_index_to_invites.rb | 7 +++++++ spec/models/invite_redeemer_spec.rb | 16 ++++++++++++++++ spec/models/invite_spec.rb | 16 ++++++++++++---- 6 files changed, 40 insertions(+), 9 deletions(-) create mode 100644 db/migrate/20191211152404_add_index_to_invites.rb diff --git a/app/models/invite.rb b/app/models/invite.rb index 17a998d4a2a..d5f210456c0 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -55,7 +55,7 @@ class Invite < ActiveRecord::Base end def expired? - created_at < SiteSetting.invite_expiry_days.days.ago + updated_at < SiteSetting.invite_expiry_days.days.ago end # link_valid? indicates whether the invite link can be used to log in to the site @@ -191,7 +191,7 @@ class Invite < ActiveRecord::Base end def self.find_pending_invites_from(inviter, offset = 0) - find_all_invites_from(inviter, offset).where('invites.user_id IS NULL').order('invites.created_at DESC') + find_all_invites_from(inviter, offset).where('invites.user_id IS NULL').order('invites.updated_at DESC') end def self.find_redeemed_invites_from(inviter, offset = 0) @@ -244,7 +244,7 @@ class Invite < ActiveRecord::Base end def self.rescind_all_expired_invites_from(user) - Invite.where('invites.user_id IS NULL AND invites.email IS NOT NULL AND invited_by_id = ? AND invites.created_at < ?', + Invite.where('invites.user_id IS NULL AND invites.email IS NOT NULL AND invited_by_id = ? AND invites.updated_at < ?', user.id, SiteSetting.invite_expiry_days.days.ago).find_each do |invite| invite.trash!(user) end diff --git a/app/models/invite_redeemer.rb b/app/models/invite_redeemer.rb index 755241e00d1..f3eb839cdcb 100644 --- a/app/models/invite_redeemer.rb +++ b/app/models/invite_redeemer.rb @@ -88,7 +88,7 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_f end def mark_invite_redeemed - count = Invite.where('id = ? AND redeemed_at IS NULL AND created_at >= ?', + count = Invite.where('id = ? AND redeemed_at IS NULL AND updated_at >= ?', invite.id, SiteSetting.invite_expiry_days.days.ago) .update_all('redeemed_at = CURRENT_TIMESTAMP') diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index a8eabf6cd78..153c3124748 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1223,7 +1223,7 @@ en: search: "type to search invites..." title: "Invites" user: "Invited User" - sent: "Sent" + sent: "Last Sent" none: "No invites to display." truncated: one: "Showing the first invite." diff --git a/db/migrate/20191211152404_add_index_to_invites.rb b/db/migrate/20191211152404_add_index_to_invites.rb new file mode 100644 index 00000000000..9d7b3414836 --- /dev/null +++ b/db/migrate/20191211152404_add_index_to_invites.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddIndexToInvites < ActiveRecord::Migration[6.0] + def change + add_index :invites, [:invited_by_id] + end +end diff --git a/spec/models/invite_redeemer_spec.rb b/spec/models/invite_redeemer_spec.rb index f0f2798ba74..6098adddcdf 100644 --- a/spec/models/invite_redeemer_spec.rb +++ b/spec/models/invite_redeemer_spec.rb @@ -70,6 +70,7 @@ describe InviteRedeemer do inviter = invite.invited_by inviter.admin = true user = invite_redeemer.redeem + invite.reload expect(user.name).to eq(name) expect(user.username).to eq(username) @@ -153,5 +154,20 @@ describe InviteRedeemer do another_user = another_invite_redeemer.redeem expect(another_user).to eq(nil) end + + it "should correctly update the invite redeemed_at date" do + SiteSetting.invite_expiry_days = 2 + invite.update!(created_at: 10.days.ago) + + inviter = invite.invited_by + inviter.admin = true + user = invite_redeemer.redeem + invite.reload + + expect(user.invited_by).to eq(inviter) + expect(inviter.notifications.count).to eq(1) + expect(invite.redeemed_at).not_to eq(nil) + end + end end diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index 573f73c9d73..15a6b9d0c01 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -109,7 +109,6 @@ describe Invite do expect(new_invite.invite_key).not_to eq(invite.invite_key) expect(new_invite.topics).to eq([topic]) end - end context 'when adding a duplicate' do @@ -137,7 +136,7 @@ describe Invite do it 'returns a new invite if the other has expired' do SiteSetting.invite_expiry_days = 1 - invite.update!(created_at: 2.days.ago) + invite.update!(updated_at: 2.days.ago) new_invite = Invite.invite_by_email( 'iceking@adventuretime.ooo', inviter, topic @@ -159,6 +158,15 @@ describe Invite do end end + it 'resets expiry of a resent invite' do + SiteSetting.invite_expiry_days = 2 + invite.update!(updated_at: 10.days.ago) + expect(invite).to be_expired + + invite.resend_invite + expect(invite).not_to be_expired + end + it 'correctly marks invite emailed_status for email invites' do expect(invite.emailed_status).to eq(Invite.emailed_status_types[:sending]) @@ -214,7 +222,7 @@ describe Invite do it 'wont redeem an expired invite' do SiteSetting.invite_expiry_days = 10 - invite.update_column(:created_at, 20.days.ago) + invite.update_column(:updated_at, 20.days.ago) expect(invite.redeem).to be_blank end @@ -493,7 +501,7 @@ describe Invite do invite_1 = Fabricate(:invite, invited_by: user) invite_2 = Fabricate(:invite, invited_by: user) expired_invite = Fabricate(:invite, invited_by: user) - expired_invite.update!(created_at: 2.days.ago) + expired_invite.update!(updated_at: 2.days.ago) Invite.rescind_all_expired_invites_from(user) invite_1.reload invite_2.reload