FIX: Don't show that an existing user is invited_by another user (#27665)
If an existing user (John) accepts an invite created by Kenny to a group, John may be seen as invited by Kenny, despite already having an account on the site. This fix removes the bug by excluding invites that determine the invited_by after the user's creation date. The delay buffer in the query accounts for invites that also create the user at the same time.
This commit is contained in:
parent
eadda77edf
commit
55bf0e21fb
|
@ -602,8 +602,20 @@ class User < ActiveRecord::Base
|
|||
end
|
||||
|
||||
def invited_by
|
||||
# this is unfortunate, but when an invite is redeemed,
|
||||
# any user created by the invite is created *after*
|
||||
# the invite's redeemed_at
|
||||
invite_redemption_delay = 5.seconds
|
||||
used_invite =
|
||||
Invite.with_deleted.joins(:invited_users).where("invited_users.user_id = ?", self.id).first
|
||||
Invite
|
||||
.with_deleted
|
||||
.joins(:invited_users)
|
||||
.where(
|
||||
"invited_users.user_id = ? AND invited_users.redeemed_at <= ?",
|
||||
self.id,
|
||||
self.created_at + invite_redemption_delay,
|
||||
)
|
||||
.first
|
||||
used_invite.try(:invited_by)
|
||||
end
|
||||
|
||||
|
|
|
@ -4,3 +4,8 @@ Fabricator(:invite) do
|
|||
invited_by(fabricator: :user)
|
||||
email "iceking@ADVENTURETIME.ooo"
|
||||
end
|
||||
|
||||
Fabricator(:invited_group) do
|
||||
group
|
||||
invite
|
||||
end
|
||||
|
|
|
@ -649,6 +649,17 @@ RSpec.describe InviteRedeemer do
|
|||
InviteRedeemer.new(invite: invite_link, redeeming_user: redeeming_user).redeem
|
||||
end.not_to change { User.count }
|
||||
end
|
||||
|
||||
it "does not set the redeeming user's invited_by since the user is already present" do
|
||||
redeeming_user.update(created_at: Time.now - 6.seconds)
|
||||
group = Fabricate(:group)
|
||||
group.add_owner(invite_link.invited_by)
|
||||
InvitedGroup.create(group_id: group.id, invite_id: invite_link.id)
|
||||
|
||||
expect do
|
||||
InviteRedeemer.new(invite: invite_link, redeeming_user: redeeming_user).redeem
|
||||
end.not_to change { redeeming_user.invited_by }
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -3235,11 +3235,26 @@ RSpec.describe User do
|
|||
describe "#invited_by" do
|
||||
it "returns even if invites was trashed" do
|
||||
invite = Fabricate(:invite, invited_by: Fabricate(:user))
|
||||
Fabricate(:invited_user, invite: invite, user: user)
|
||||
Fabricate(:invited_user, invite: invite, user: user, redeemed_at: Time.now)
|
||||
invite.trash!
|
||||
|
||||
expect(user.invited_by).to eq(invite.invited_by)
|
||||
end
|
||||
|
||||
it "does not return invites that are not redeemed yet" do
|
||||
invite = Fabricate(:invite, invited_by: Fabricate(:user))
|
||||
Fabricate(:invited_user, invite: invite, user: user, redeemed_at: nil)
|
||||
invite.trash!
|
||||
|
||||
expect(user.invited_by).to eq(nil)
|
||||
end
|
||||
|
||||
it "excludes invites redeemed after user creation" do
|
||||
invite = Fabricate(:invite, invited_by: Fabricate(:user))
|
||||
Fabricate(:invited_user, invite: invite, user: user, redeemed_at: user.created_at + 6.second)
|
||||
|
||||
expect(user.invited_by).to eq(nil)
|
||||
end
|
||||
end
|
||||
|
||||
describe "#username_equals_to?" do
|
||||
|
|
|
@ -0,0 +1,47 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
describe "User Invites", type: :system do
|
||||
fab!(:admin)
|
||||
fab!(:user)
|
||||
fab!(:invite) { Fabricate(:invite, email: nil, domain: nil, max_redemptions_allowed: 1) }
|
||||
fab!(:invited_group) { Fabricate(:invited_group, invite: invite) }
|
||||
let(:user_menu) { PageObjects::Components::UserMenu.new }
|
||||
|
||||
it "can redeem invite as existing user and not have invited_by" do
|
||||
sign_in(user)
|
||||
visit "/invites/#{invite.invite_key}"
|
||||
|
||||
assert_selector(".invite-page .login-welcome-header")
|
||||
assert_selector(".invite-form", text: "You were invited by:")
|
||||
assert_selector(".invite-form .user-detail .name-line", text: invite.invited_by.username)
|
||||
|
||||
find(".invite-form .btn-primary").click
|
||||
expect(page).to have_current_path("/")
|
||||
|
||||
sign_in(admin)
|
||||
visit "/u/#{user.username}/summary"
|
||||
expect(page).not_to have_css(".invited-by")
|
||||
end
|
||||
|
||||
it "includes invited_by user in the invitee's summary" do
|
||||
visit "/invites/#{invite.invite_key}"
|
||||
assert_selector(".invite-form .user-detail .name-line", text: invite.invited_by.username)
|
||||
|
||||
fill_in "Email", with: "boaty@mcboatface.com"
|
||||
fill_in "Password", with: "boatyMcBoatface"
|
||||
fill_in "Username", with: "boaty"
|
||||
find(".invitation-cta__accept:not([disabled])", wait: 10)
|
||||
|
||||
assert_selector(".login-title", text: "Welcome to Discourse!")
|
||||
|
||||
find(".invite-form .btn-primary").click
|
||||
wait_for { User.find_by_username("boaty").present? }
|
||||
|
||||
User.find_by_username("boaty").activate
|
||||
|
||||
sign_in(admin)
|
||||
visit "/u/boaty/summary"
|
||||
assert_selector("dt.invited-by", text: "Invited By")
|
||||
assert_selector("dd.invited-by", text: invite.invited_by.username)
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue