From 3094459cd979e95067e5299e08fc94cf3f8ea0d3 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Tue, 9 Jun 2020 20:49:32 +0530 Subject: [PATCH] FEATURE: multiple use invite links (#9813) --- .../app/components/invite-link-panel.js | 98 +++++++ .../discourse/app/controllers/invites-show.js | 74 ++++-- .../app/controllers/user-invited-show.js | 28 +- .../discourse/app/models/invite.js | 11 +- .../javascripts/discourse/app/models/user.js | 11 + .../discourse/app/routes/user-invited-show.js | 51 +++- .../components/future-date-input.hbs | 2 +- .../components/generated-invite-link.hbs | 6 +- .../components/invite-link-panel.hbs | 59 +++++ .../discourse/app/templates/invites/show.hbs | 28 +- .../app/templates/user-invited-show.hbs | 38 ++- app/assets/stylesheets/common/base/login.scss | 3 + .../components/share-and-invite-modal.scss | 21 +- app/assets/stylesheets/desktop/user.scss | 4 + app/controllers/invites_controller.rb | 63 +++-- app/controllers/users_controller.rb | 27 +- app/jobs/regular/anonymize_user.rb | 2 +- app/models/invite.rb | 178 ++++++++++--- app/models/invite_redeemer.rb | 50 ++-- app/models/invited_user.rb | 26 ++ app/models/user.rb | 5 +- app/serializers/invite_link_serializer.rb | 9 + app/serializers/invite_serializer.rb | 10 +- app/serializers/invited_serializer.rb | 7 + .../invited_user_record_serializer.rb | 60 +++++ app/serializers/invited_user_serializer.rb | 55 +--- app/services/user_merger.rb | 2 +- config/locales/client.en.yml | 23 +- config/locales/server.en.yml | 6 + config/routes.rb | 1 + config/site_settings.yml | 5 + ...8102014_add_bulk_invite_link_to_invites.rb | 17 ++ .../20200430072846_create_invited_users.rb | 15 ++ ...e_invite_redeemed_data_to_invited_users.rb | 26 ++ lib/badge_queries.rb | 3 +- lib/guardian.rb | 4 + .../jobs/send_default_welcome_message_spec.rb | 7 +- spec/fabricators/invited_user_fabricator.rb | 6 + spec/models/email_token_spec.rb | 2 +- spec/models/invite_redeemer_spec.rb | 71 ++++-- spec/models/invite_spec.rb | 142 +++++++++-- spec/requests/invites_controller_spec.rb | 241 ++++++++++++------ spec/requests/users_controller_spec.rb | 80 ++++-- spec/services/user_anonymizer_spec.rb | 8 +- spec/services/user_merger_spec.rb | 5 +- spec/services/username_changer_spec.rb | 4 +- .../acceptance/invite-accept-test.js | 23 +- .../invite-show-user-fields-test.js | 14 + 48 files changed, 1280 insertions(+), 351 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/components/invite-link-panel.js create mode 100644 app/assets/javascripts/discourse/app/templates/components/invite-link-panel.hbs create mode 100644 app/models/invited_user.rb create mode 100644 app/serializers/invite_link_serializer.rb create mode 100644 app/serializers/invited_user_record_serializer.rb create mode 100644 db/migrate/20200428102014_add_bulk_invite_link_to_invites.rb create mode 100644 db/migrate/20200430072846_create_invited_users.rb create mode 100644 db/post_migrate/20200602153813_migrate_invite_redeemed_data_to_invited_users.rb create mode 100644 spec/fabricators/invited_user_fabricator.rb diff --git a/app/assets/javascripts/discourse/app/components/invite-link-panel.js b/app/assets/javascripts/discourse/app/components/invite-link-panel.js new file mode 100644 index 00000000000..e2691e36fac --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/invite-link-panel.js @@ -0,0 +1,98 @@ +import I18n from "I18n"; +import Component from "@ember/component"; +import Group from "discourse/models/group"; +import { readOnly } from "@ember/object/computed"; +import { action } from "@ember/object"; +import discourseComputed from "discourse-common/utils/decorators"; +import Invite from "discourse/models/invite"; + +export default Component.extend({ + inviteModel: readOnly("panel.model.inviteModel"), + userInvitedShow: readOnly("panel.model.userInvitedShow"), + isStaff: readOnly("currentUser.staff"), + maxRedemptionAllowed: 5, + inviteExpiresAt: moment() + .add(1, "month") + .format("YYYY-MM-DD"), + + willDestroyElement() { + this._super(...arguments); + + this.reset(); + }, + + @discourseComputed("isStaff", "inviteModel.saving", "maxRedemptionAllowed") + disabled(isStaff, saving, canInviteTo, maxRedemptionAllowed) { + if (saving) return true; + if (!isStaff) return true; + if (maxRedemptionAllowed < 2) return true; + + return false; + }, + + groupFinder(term) { + return Group.findAll({ term, ignore_automatic: true }); + }, + + errorMessage: I18n.t("user.invited.invite_link.error"), + + reset() { + this.set("maxRedemptionAllowed", 5); + + this.inviteModel.setProperties({ + groupNames: null, + error: false, + saving: false, + finished: false, + inviteLink: null + }); + }, + + @action + generateMultipleUseInviteLink() { + if (this.disabled) { + return; + } + + const groupNames = this.get("inviteModel.groupNames"); + const maxRedemptionAllowed = this.maxRedemptionAllowed; + const inviteExpiresAt = this.inviteExpiresAt; + const userInvitedController = this.userInvitedShow; + const model = this.inviteModel; + model.setProperties({ saving: true, error: false }); + + return model + .generateMultipleUseInviteLink( + groupNames, + maxRedemptionAllowed, + inviteExpiresAt + ) + .then(result => { + model.setProperties({ + saving: false, + finished: true, + inviteLink: result + }); + + if (userInvitedController) { + Invite.findInvitedBy( + this.currentUser, + userInvitedController.filter + ).then(inviteModel => { + userInvitedController.setProperties({ + model: inviteModel, + totalInvites: inviteModel.invites.length + }); + }); + } + }) + .catch(e => { + if (e.jqXHR.responseJSON && e.jqXHR.responseJSON.errors) { + this.set("errorMessage", e.jqXHR.responseJSON.errors[0]); + } else { + this.set("errorMessage", I18n.t("user.invited.invite_link.error")); + } + model.setProperties({ saving: false, error: true }); + }); + } +}); diff --git a/app/assets/javascripts/discourse/app/controllers/invites-show.js b/app/assets/javascripts/discourse/app/controllers/invites-show.js index b421b770583..a38ec6e1a93 100644 --- a/app/assets/javascripts/discourse/app/controllers/invites-show.js +++ b/app/assets/javascripts/discourse/app/controllers/invites-show.js @@ -1,16 +1,18 @@ import I18n from "I18n"; import { isEmpty } from "@ember/utils"; -import { alias, notEmpty } from "@ember/object/computed"; +import { alias, notEmpty, or, readOnly } from "@ember/object/computed"; import Controller from "@ember/controller"; import discourseComputed from "discourse-common/utils/decorators"; import getUrl from "discourse-common/lib/get-url"; import DiscourseURL from "discourse/lib/url"; import { ajax } from "discourse/lib/ajax"; +import { emailValid } from "discourse/lib/utilities"; import PasswordValidation from "discourse/mixins/password-validation"; import UsernameValidation from "discourse/mixins/username-validation"; import NameValidation from "discourse/mixins/name-validation"; import UserFieldsValidation from "discourse/mixins/user-fields-validation"; import { findAll as findLoginMethods } from "discourse/models/login-method"; +import EmberObject from "@ember/object"; export default Controller.extend( PasswordValidation, @@ -18,7 +20,7 @@ export default Controller.extend( NameValidation, UserFieldsValidation, { - invitedBy: alias("model.invited_by"), + invitedBy: readOnly("model.invited_by"), email: alias("model.email"), accountUsername: alias("model.username"), passwordRequired: notEmpty("accountPassword"), @@ -26,6 +28,21 @@ export default Controller.extend( errorMessage: null, userFields: null, inviteImageUrl: getUrl("/images/envelope.svg"), + isInviteLink: readOnly("model.is_invite_link"), + submitDisabled: or( + "emailValidation.failed", + "usernameValidation.failed", + "passwordValidation.failed", + "nameValidation.failed", + "userFieldsValidation.failed" + ), + rejectedEmails: null, + + init() { + this._super(...arguments); + + this.rejectedEmails = []; + }, @discourseComputed welcomeTitle() { @@ -44,21 +61,6 @@ export default Controller.extend( return findLoginMethods().length > 0; }, - @discourseComputed( - "usernameValidation.failed", - "passwordValidation.failed", - "nameValidation.failed", - "userFieldsValidation.failed" - ) - submitDisabled( - usernameFailed, - passwordFailed, - nameFailed, - userFieldsFailed - ) { - return usernameFailed || passwordFailed || nameFailed || userFieldsFailed; - }, - @discourseComputed fullnameRequired() { return ( @@ -66,6 +68,35 @@ export default Controller.extend( ); }, + @discourseComputed("email", "rejectedEmails.[]") + emailValidation(email, rejectedEmails) { + // If blank, fail without a reason + if (isEmpty(email)) { + return EmberObject.create({ + failed: true + }); + } + + if (rejectedEmails.includes(email)) { + return EmberObject.create({ + failed: true, + reason: I18n.t("user.email.invalid") + }); + } + + if (emailValid(email)) { + return EmberObject.create({ + ok: true, + reason: I18n.t("user.email.ok") + }); + } + + return EmberObject.create({ + failed: true, + reason: I18n.t("user.email.invalid") + }); + }, + actions: { submit() { const userFields = this.userFields; @@ -80,6 +111,7 @@ export default Controller.extend( url: `/invites/show/${this.get("model.token")}.json`, type: "PUT", data: { + email: this.email, username: this.accountUsername, name: this.accountName, password: this.accountPassword, @@ -97,6 +129,14 @@ export default Controller.extend( DiscourseURL.redirectTo(result.redirect_to); } } else { + if ( + result.errors && + result.errors.email && + result.errors.email.length > 0 && + result.values + ) { + this.rejectedEmails.pushObject(result.values.email); + } if ( result.errors && result.errors.password && diff --git a/app/assets/javascripts/discourse/app/controllers/user-invited-show.js b/app/assets/javascripts/discourse/app/controllers/user-invited-show.js index eb3a42fd145..7e8f67ebe28 100644 --- a/app/assets/javascripts/discourse/app/controllers/user-invited-show.js +++ b/app/assets/javascripts/discourse/app/controllers/user-invited-show.js @@ -1,5 +1,5 @@ import I18n from "I18n"; -import { equal, reads, gte } from "@ember/object/computed"; +import { equal, reads } from "@ember/object/computed"; import Controller from "@ember/controller"; import Invite from "discourse/models/invite"; import discourseDebounce from "discourse/lib/debounce"; @@ -35,21 +35,30 @@ export default Controller.extend({ }, INPUT_DELAY), inviteRedeemed: equal("filter", "redeemed"), + invitePending: equal("filter", "pending"), + + @discourseComputed("filter") + inviteLinks(filter) { + return filter === "links" && this.currentUser.staff; + }, @discourseComputed("filter") showBulkActionButtons(filter) { return ( filter === "pending" && this.model.invites.length > 4 && - this.currentUser.get("staff") + this.currentUser.staff ); }, canInviteToForum: reads("currentUser.can_invite_to_forum"), - canBulkInvite: reads("currentUser.admin"), + canSendInviteLink: reads("currentUser.staff"), - showSearch: gte("totalInvites", 10), + @discourseComputed("totalInvites", "inviteLinks") + showSearch(totalInvites, inviteLinks) { + return totalInvites >= 10 && !inviteLinks; + }, @discourseComputed("invitesCount.total", "invitesCount.pending") pendingLabel(invitesCountTotal, invitesCountPending) { @@ -73,6 +82,17 @@ export default Controller.extend({ } }, + @discourseComputed("invitesCount.total", "invitesCount.links") + linksLabel(invitesCountTotal, invitesCountLinks) { + if (invitesCountTotal > 50) { + return I18n.t("user.invited.links_tab_with_count", { + count: invitesCountLinks + }); + } else { + return I18n.t("user.invited.links_tab"); + } + }, + actions: { rescind(invite) { invite.rescind(); diff --git a/app/assets/javascripts/discourse/app/models/invite.js b/app/assets/javascripts/discourse/app/models/invite.js index 7eb142c0abb..347ca71d680 100644 --- a/app/assets/javascripts/discourse/app/models/invite.js +++ b/app/assets/javascripts/discourse/app/models/invite.js @@ -10,7 +10,7 @@ const Invite = EmberObject.extend({ rescind() { ajax("/invites", { type: "DELETE", - data: { email: this.email } + data: { id: this.id } }); this.set("rescinded", true); }, @@ -42,7 +42,14 @@ Invite.reopenClass({ if (!isNone(search)) data.search = search; data.offset = offset || 0; - return ajax(userPath(`${user.username_lower}/invited.json`), { + let path; + if (filter === "links") { + path = userPath(`${user.username_lower}/invite_links.json`); + } else { + path = userPath(`${user.username_lower}/invited.json`); + } + + return ajax(path, { data }).then(result => { result.invites = result.invites.map(i => Invite.create(i)); diff --git a/app/assets/javascripts/discourse/app/models/user.js b/app/assets/javascripts/discourse/app/models/user.js index a6356182f33..113b592c277 100644 --- a/app/assets/javascripts/discourse/app/models/user.js +++ b/app/assets/javascripts/discourse/app/models/user.js @@ -654,6 +654,17 @@ const User = RestModel.extend({ }); }, + generateMultipleUseInviteLink( + group_names, + max_redemptions_allowed, + expires_at + ) { + return ajax("/invites/link", { + type: "POST", + data: { group_names, max_redemptions_allowed, expires_at } + }); + }, + @observes("muted_category_ids") updateMutedCategories() { this.set("mutedCategories", Category.findByIds(this.muted_category_ids)); diff --git a/app/assets/javascripts/discourse/app/routes/user-invited-show.js b/app/assets/javascripts/discourse/app/routes/user-invited-show.js index 3cd83f79ceb..7af69e14dfd 100644 --- a/app/assets/javascripts/discourse/app/routes/user-invited-show.js +++ b/app/assets/javascripts/discourse/app/routes/user-invited-show.js @@ -30,18 +30,51 @@ export default DiscourseRoute.extend({ actions: { showInvite() { + const panels = [ + { + id: "invite", + title: "user.invited.single_user", + model: { + inviteModel: this.currentUser, + userInvitedShow: this.controllerFor("user-invited-show") + } + } + ]; + + if (this.get("currentUser.staff")) { + panels.push({ + id: "invite-link", + title: "user.invited.multiple_user", + model: { + inviteModel: this.currentUser, + userInvitedShow: this.controllerFor("user-invited-show") + } + }); + } + showModal("share-and-invite", { modalClass: "share-and-invite", - panels: [ - { - id: "invite", - title: "user.invited.create", - model: { - inviteModel: this.currentUser, - userInvitedShow: this.controllerFor("user-invited-show") - } + panels + }); + }, + + editInvite(inviteKey) { + const inviteLink = `${Discourse.BaseUrl}/invites/${inviteKey}`; + this.currentUser.setProperties({ finished: true, inviteLink }); + const panels = [ + { + id: "invite-link", + title: "user.invited.generate_link", + model: { + inviteModel: this.currentUser, + userInvitedShow: this.controllerFor("user-invited-show") } - ] + } + ]; + + showModal("share-and-invite", { + modalClass: "share-and-invite", + panels }); } } diff --git a/app/assets/javascripts/discourse/app/templates/components/future-date-input.hbs b/app/assets/javascripts/discourse/app/templates/components/future-date-input.hbs index 14da87147f3..6e6b117e087 100644 --- a/app/assets/javascripts/discourse/app/templates/components/future-date-input.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/future-date-input.hbs @@ -31,7 +31,7 @@
{{d-icon "far-clock"}} - {{input placeholder="--:--" type="time" value=time disabled=timeInputDisabled}} + {{input placeholder="--:--" type="time" class="time-input" value=time disabled=timeInputDisabled}}
{{/if}} diff --git a/app/assets/javascripts/discourse/app/templates/components/generated-invite-link.hbs b/app/assets/javascripts/discourse/app/templates/components/generated-invite-link.hbs index 9e108229721..cd83a56e3ef 100644 --- a/app/assets/javascripts/discourse/app/templates/components/generated-invite-link.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/generated-invite-link.hbs @@ -1,5 +1,7 @@

{{i18n "user.invited.link_generated"}}

- +

-

{{i18n "user.invited.valid_for" email=email}}

+{{#if email}} +

{{i18n "user.invited.valid_for" email=email}}

+{{/if}} diff --git a/app/assets/javascripts/discourse/app/templates/components/invite-link-panel.hbs b/app/assets/javascripts/discourse/app/templates/components/invite-link-panel.hbs new file mode 100644 index 00000000000..cc1b30041c0 --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/components/invite-link-panel.hbs @@ -0,0 +1,59 @@ +{{#if inviteModel.error}} +
+ {{html-safe errorMessage}} +
+{{/if}} + +
+ {{#if inviteModel.finished}} + {{generated-invite-link link=inviteModel.inviteLink}} + {{else}} + +
+ + {{group-selector + fullWidthWrap=true + groupFinder=groupFinder + groupNames=inviteModel.groupNames + placeholderKey="topic.invite_private.group_name"}} +
+ +
+ + {{input type="number" value=maxRedemptionAllowed class="max-redemptions-allowed-input" min="2" max=siteSettings.invite_link_max_redemptions_limit}} +
+ + + + {{/if}} +
+ + diff --git a/app/assets/javascripts/discourse/app/templates/invites/show.hbs b/app/assets/javascripts/discourse/app/templates/invites/show.hbs index 96804f5f827..8739fc39246 100644 --- a/app/assets/javascripts/discourse/app/templates/invites/show.hbs +++ b/app/assets/javascripts/discourse/app/templates/invites/show.hbs @@ -16,18 +16,30 @@

{{i18n "invites.invited_by"}}

{{user-info user=invitedBy}}

-

- {{html-safe yourEmailMessage}} - {{#if externalAuthsEnabled}} - {{i18n "invites.social_login_available"}} - {{/if}} -

+ {{#unless isInviteLink}} +

+ {{html-safe yourEmailMessage}} + {{#if externalAuthsEnabled}} + {{i18n "invites.social_login_available"}} + {{/if}} +

+ {{/unless}}
+ + {{#if isInviteLink}} + + {{/if}} +
{{input value=accountUsername id="new-account-username" name="username" maxlength=maxUsernameLength autocomplete="discourse"}} -  {{input-tip validation=usernameValidation id="username-validation"}} + {{input-tip validation=usernameValidation id="username-validation"}}
{{i18n "user.username.instructions"}}
@@ -42,7 +54,7 @@
{{password-field value=accountPassword type="password" id="new-account-password" capsLockOn=capsLockOn}} -  {{input-tip validation=passwordValidation}} + {{input-tip validation=passwordValidation}}
{{passwordInstructions}} {{i18n "invites.optional_description"}}
diff --git a/app/assets/javascripts/discourse/app/templates/user-invited-show.hbs b/app/assets/javascripts/discourse/app/templates/user-invited-show.hbs index 1def791972b..9396e7f90f5 100644 --- a/app/assets/javascripts/discourse/app/templates/user-invited-show.hbs +++ b/app/assets/javascripts/discourse/app/templates/user-invited-show.hbs @@ -5,11 +5,14 @@

{{i18n "user.invited.title"}}

{{#if model.can_see_invite_details}} -
+
@@ -17,7 +20,6 @@ {{d-button icon="plus" action=(route-action "showInvite") label="user.invited.create"}} {{#if canBulkInvite}} {{csv-uploader uploading=uploading}} - {{d-icon "question-circle"}} {{/if}} {{#if showBulkActionButtons}} {{#if rescindedAll}} @@ -54,15 +56,25 @@ {{i18n "user.invited.posts_read_count"}} {{i18n "user.invited.time_read"}} {{i18n "user.invited.days_visited"}} + {{#if canSendInviteLink}} + {{i18n "user.invited.source"}} + {{/if}} {{/if}} - {{else}} + {{else if invitePending}} {{i18n "user.invited.user"}} {{i18n "user.invited.sent"}} + {{else if inviteLinks}} + {{i18n "user.invited.link_url"}} + {{i18n "user.invited.link_created_at"}} + {{i18n "user.invited.link_redemption_stats"}} + {{i18n "user.invited.link_groups"}} + {{i18n "user.invited.link_expires_at"}} + {{/if}} {{#each model.invites as |invite|}} - {{#if invite.user}} + {{#if inviteRedeemed}} {{#link-to "user" invite.user}}{{avatar invite.user imageSize="tiny"}}{{/link-to}} {{#link-to "user" invite.user}}{{invite.user.username}}{{/link-to}} @@ -78,8 +90,11 @@ / {{html-safe invite.user.days_since_created}} + {{#if canSendInviteLink}} + {{html-safe invite.invite_source}} + {{/if}} {{/if}} - {{else}} + {{else if invitePending}} {{invite.email}} {{format-date invite.updated_at}} @@ -99,6 +114,19 @@ {{d-button icon="sync" action=(action "reinvite") actionParam=invite label="user.invited.reinvite"}} {{/if}} + {{else if inviteLinks}} + {{d-button icon="link" action=(route-action "editInvite" invite.invite_key) label="user.invited.copy_link"}} + {{format-date invite.created_at}} + {{number invite.redemption_count}} / {{number invite.max_redemptions_allowed}} + {{ invite.group_names }} + {{raw-date invite.expires_at leaveAgo="true"}} + + {{#if invite.rescinded}} + {{i18n "user.invited.rescinded"}} + {{else}} + {{d-button icon="times" action=(action "rescind") actionParam=invite label="user.invited.rescind"}} + {{/if}} + {{/if}} {{/each}} diff --git a/app/assets/stylesheets/common/base/login.scss b/app/assets/stylesheets/common/base/login.scss index 1e68921bec0..6f75cf30dca 100644 --- a/app/assets/stylesheets/common/base/login.scss +++ b/app/assets/stylesheets/common/base/login.scss @@ -145,6 +145,9 @@ font-weight: normal; line-height: $line-height-medium; } + .tip { + padding-left: 5px; + } } } diff --git a/app/assets/stylesheets/common/components/share-and-invite-modal.scss b/app/assets/stylesheets/common/components/share-and-invite-modal.scss index b964ea12b0d..f4c1422d9cf 100644 --- a/app/assets/stylesheets/common/components/share-and-invite-modal.scss +++ b/app/assets/stylesheets/common/components/share-and-invite-modal.scss @@ -62,7 +62,8 @@ } } -.share-and-invite.modal .invite.modal-panel { +.share-and-invite.modal .invite.modal-panel, +.invite-link.modal-panel { .error-message, .success-message { margin-bottom: 8px; @@ -83,12 +84,30 @@ width: 100%; } + .max-redemptions-allowed { + margin-bottom: 8px; + + .max-redemptions-allowed-input { + width: 20%; + min-width: 100px; + } + } + + .invite-link-expires-at .date-picker, + .time-input { + width: 150px; + } + .invite-user-input-wrapper { display: flex; div.ac-wrap { flex: 1; } } + + .invite-link-input { + width: 100%; + } } .footer { diff --git a/app/assets/stylesheets/desktop/user.scss b/app/assets/stylesheets/desktop/user.scss index d6b0342dfc4..f2e6445f0d9 100644 --- a/app/assets/stylesheets/desktop/user.scss +++ b/app/assets/stylesheets/desktop/user.scss @@ -80,6 +80,10 @@ margin-top: 10px; } +.invite-controls .btn { + margin-right: 0px; +} + .user-invite-list { width: 100%; margin-top: 15px; diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index a21b3af3460..ed5b6a01dcd 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -24,7 +24,8 @@ class InvitesController < ApplicationController store_preloaded("invite_info", MultiJson.dump( invited_by: UserNameSerializer.new(invite.invited_by, scope: guardian, root: false), email: invite.email, - username: UserNameSuggester.suggest(invite.email)) + username: UserNameSuggester.suggest(invite.email), + is_invite_link: invite.is_invite_link?) ) render layout: 'application' @@ -40,23 +41,30 @@ class InvitesController < ApplicationController def perform_accept_invitation params.require(:id) - params.permit(:username, :name, :password, :timezone, user_custom_fields: {}) + params.permit(:email, :username, :name, :password, :timezone, user_custom_fields: {}) invite = Invite.find_by(invite_key: params[:id]) if invite.present? begin - user = invite.redeem(username: params[:username], name: params[:name], password: params[:password], user_custom_fields: params[:user_custom_fields], ip_address: request.remote_ip) + user = if invite.is_invite_link? + invite.redeem_invite_link(email: params[:email], username: params[:username], name: params[:name], password: params[:password], user_custom_fields: params[:user_custom_fields], ip_address: request.remote_ip) + else + invite.redeem(username: params[:username], name: params[:name], password: params[:password], user_custom_fields: params[:user_custom_fields], ip_address: request.remote_ip) + end + if user.present? log_on_user(user) if user.active? user.update_timezone_if_missing(params[:timezone]) post_process_invite(user) + response = { success: true } + else + response = { success: false, message: I18n.t('invite.not_found_json') } end - response = { success: true } if user.present? && user.active? topic = invite.topics.first response[:redirect_to] = topic.present? ? path("#{topic.relative_url}") : path("/") - else + elsif user.present? response[:message] = I18n.t('invite.confirm_email') end @@ -67,6 +75,8 @@ class InvitesController < ApplicationController errors: e.record&.errors&.to_hash || {}, message: I18n.t('invite.error_message') } + rescue Invite::UserExists => e + render json: { success: false, message: [e.message] } end else render json: { success: false, message: I18n.t('invite.not_found_json') } @@ -101,27 +111,44 @@ class InvitesController < ApplicationController end def create_invite_link - params.require(:email) + params.permit(:email, :max_redemptions_allowed, :expires_at, :group_ids, :group_names, :topic_id) + + is_single_invite = params[:email].present? + unless is_single_invite + guardian.ensure_can_send_invite_links!(current_user) + end groups = Group.lookup_groups( group_ids: params[:group_ids], group_names: params[:group_names] ) guardian.ensure_can_invite_to_forum!(groups) - - topic = Topic.find_by(id: params[:topic_id]) - guardian.ensure_can_invite_to!(topic) if topic.present? - group_ids = groups.map(&:id) - invite_exists = Invite.exists?(email: params[:email], invited_by_id: current_user.id) - if invite_exists && !guardian.can_send_multiple_invites?(current_user) - return render json: failed_json, status: 422 + if is_single_invite + if params[:topic_id].present? + topic = Topic.find_by(id: params[:topic_id]) + guardian.ensure_can_invite_to!(topic) if topic.present? + end + + invite_exists = Invite.exists?(email: params[:email], invited_by_id: current_user.id) + if invite_exists && !guardian.can_send_multiple_invites?(current_user) + return render json: failed_json, status: 422 + end end begin - # generate invite link - if invite_link = Invite.generate_invite_link(params[:email], current_user, topic, group_ids) + invite_link = if is_single_invite + Invite.generate_single_use_invite_link(params[:email], current_user, topic, group_ids) + else + Invite.generate_multiple_use_invite_link( + invited_by: current_user, + max_redemptions_allowed: params[:max_redemptions_allowed], + expires_at: params[:expires_at], + group_ids: group_ids + ) + end + if invite_link.present? render_json_dump(invite_link) else render json: failed_json, status: 422 @@ -132,10 +159,10 @@ class InvitesController < ApplicationController end def destroy - params.require(:email) + params.require(:id) - invite = Invite.find_by(invited_by_id: current_user.id, email: params[:email]) - raise Discourse::InvalidParameters.new(:email) if invite.blank? + invite = Invite.find_by(invited_by_id: current_user.id, id: params[:id]) + raise Discourse::InvalidParameters.new(:id) if invite.blank? invite.trash!(current_user) render body: nil diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 8e63e97f669..57a2f7a400f 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -297,30 +297,44 @@ class UsersController < ApplicationController def invited guardian.ensure_can_invite_to_forum! - inviter = fetch_user_from_params(include_inactive: current_user.staff? || SiteSetting.show_inactive_accounts) offset = params[:offset].to_i || 0 - filter_by = params[:filter] + filter_by = params[:filter] || "redeemed" + inviter = fetch_user_from_params(include_inactive: current_user.staff? || SiteSetting.show_inactive_accounts) invites = if guardian.can_see_invite_details?(inviter) && filter_by == "pending" Invite.find_pending_invites_from(inviter, offset) - else + elsif filter_by == "redeemed" Invite.find_redeemed_invites_from(inviter, offset) + else + [] end show_emails = guardian.can_see_invite_emails?(inviter) - if params[:search].present? + if params[:search].present? && invites.present? filter_sql = '(LOWER(users.username) LIKE :filter)' filter_sql = '(LOWER(invites.email) LIKE :filter) or (LOWER(users.username) LIKE :filter)' if show_emails invites = invites.where(filter_sql, filter: "%#{params[:search].downcase}%") end render json: MultiJson.dump(InvitedSerializer.new( - OpenStruct.new(invite_list: invites.to_a, show_emails: show_emails, inviter: inviter), + OpenStruct.new(invite_list: invites.to_a, show_emails: show_emails, inviter: inviter, type: filter_by), scope: guardian, root: false )) end + def invite_links + guardian.ensure_can_invite_to_forum! + + inviter = fetch_user_from_params(include_inactive: current_user.try(:staff?) || (current_user && SiteSetting.show_inactive_accounts)) + guardian.ensure_can_see_invite_details!(inviter) + + offset = params[:offset].to_i || 0 + invites = Invite.find_links_invites_from(inviter, offset) + + render json: MultiJson.dump(invites: serialize_data(invites.to_a, InviteLinkSerializer), can_see_invite_details: guardian.can_see_invite_details?(inviter)) + end + def invited_count guardian.ensure_can_invite_to_forum! @@ -328,8 +342,9 @@ class UsersController < ApplicationController pending_count = Invite.find_pending_invites_count(inviter) redeemed_count = Invite.find_redeemed_invites_count(inviter) + links_count = Invite.find_links_invites_count(inviter) - render json: { counts: { pending: pending_count, redeemed: redeemed_count, + render json: { counts: { pending: pending_count, redeemed: redeemed_count, links: links_count, total: (pending_count.to_i + redeemed_count.to_i) } } end diff --git a/app/jobs/regular/anonymize_user.rb b/app/jobs/regular/anonymize_user.rb index 8dca0ca403e..1113476a81b 100644 --- a/app/jobs/regular/anonymize_user.rb +++ b/app/jobs/regular/anonymize_user.rb @@ -16,7 +16,7 @@ module Jobs def make_anonymous anonymize_ips(@anonymize_ip) if @anonymize_ip - Invite.with_deleted.where(user_id: @user_id).destroy_all + InvitedUser.where(user_id: @user_id).destroy_all EmailToken.where(user_id: @user_id).destroy_all EmailLog.where(user_id: @user_id).delete_all IncomingEmail.where("user_id = ? OR from_address = ?", @user_id, @prev_email).delete_all diff --git a/app/models/invite.rb b/app/models/invite.rb index 5fde9a45abe..80858c5dad2 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -5,6 +5,12 @@ class Invite < ActiveRecord::Base include RateLimiter::OnCreateRecord include Trashable + # TODO(2021-05-22): remove + self.ignored_columns = %w{ + user_id + redeemed_at + } + BULK_INVITE_EMAIL_LIMIT = 200 rate_limit :limit_invites_per_day @@ -13,6 +19,8 @@ class Invite < ActiveRecord::Base belongs_to :topic belongs_to :invited_by, class_name: 'User' + has_many :invited_users + has_many :users, through: :invited_users has_many :invited_groups has_many :groups, through: :invited_groups has_many :topic_invites @@ -22,15 +30,20 @@ class Invite < ActiveRecord::Base before_create do self.invite_key ||= SecureRandom.hex + self.expires_at ||= SiteSetting.invite_expiry_days.days.from_now end before_validation do self.email = Email.downcase(email) unless email.nil? end + validate :ensure_max_redemptions_allowed validate :user_doesnt_already_exist attr_accessor :email_already_exists + scope :single_use_invites, -> { where('invites.max_redemptions_allowed = 1') } + scope :multiple_use_invites, -> { where('invites.max_redemptions_allowed > 1') } + def self.emailed_status_types @emailed_status_types ||= Enum.new(not_required: 0, pending: 1, bulk_pending: 2, sending: 3, sent: 4) end @@ -40,18 +53,26 @@ class Invite < ActiveRecord::Base return if email.blank? user = Invite.find_user_by_email(email) - if user && user.id != self.user_id + if user && user.id != self.invited_users&.first&.user_id @email_already_exists = true errors.add(:email) end end + def is_invite_link? + max_redemptions_allowed > 1 + end + def redeemed? - redeemed_at.present? + if is_invite_link? + redemption_count >= max_redemptions_allowed + else + self.invited_users.count > 0 + end end def expired? - updated_at < SiteSetting.invite_expiry_days.days.ago + expires_at < Time.zone.now end # link_valid? indicates whether the invite link can be used to log in to the site @@ -61,7 +82,7 @@ class Invite < ActiveRecord::Base def redeem(username: nil, name: nil, password: nil, user_custom_fields: nil, ip_address: nil) if !expired? && !destroyed? && link_valid? - InviteRedeemer.new(self, username, name, password, user_custom_fields, ip_address).redeem + InviteRedeemer.new(invite: self, email: self.email, username: username, name: name, password: password, user_custom_fields: user_custom_fields, ip_address: ip_address).redeem end end @@ -74,8 +95,7 @@ class Invite < ActiveRecord::Base ) end - # generate invite link - def self.generate_invite_link(email, invited_by, topic = nil, group_ids = nil) + def self.generate_single_use_invite_link(email, invited_by, topic = nil, group_ids = nil) invite = create_invite_by_email(email, invited_by, topic: topic, group_ids: group_ids, @@ -123,6 +143,7 @@ class Invite < ActiveRecord::Base invite.update_columns( created_at: Time.zone.now, updated_at: Time.zone.now, + expires_at: SiteSetting.invite_expiry_days.days.from_now, emailed_status: emailed_status ) else @@ -160,6 +181,37 @@ class Invite < ActiveRecord::Base invite end + def self.generate_multiple_use_invite_link(invited_by:, max_redemptions_allowed: 5, expires_at: 1.month.from_now, group_ids: nil) + Invite.transaction do + create_args = { + invited_by: invited_by, + max_redemptions_allowed: max_redemptions_allowed.to_i, + expires_at: expires_at, + emailed_status: emailed_status_types[:not_required] + } + invite = Invite.create!(create_args) + + if group_ids.present? + now = Time.zone.now + invited_groups = group_ids.map { |group_id| { group_id: group_id, invite_id: invite.id, created_at: now, updated_at: now } } + InvitedGroup.insert_all(invited_groups) + end + + "#{Discourse.base_url}/invites/#{invite.invite_key}" + end + end + + # redeem multiple use invite link + def redeem_invite_link(email: nil, username: nil, name: nil, password: nil, user_custom_fields: nil, ip_address: nil) + DistributedMutex.synchronize("redeem_invite_link_#{self.id}") do + reload + if is_invite_link? && !expired? && !redeemed? && !destroyed? && link_valid? + raise UserExists.new I18n.t("invite_link.email_taken") if UserEmail.exists?(email: email) + InviteRedeemer.new(invite: self, email: email, username: username, name: name, password: password, user_custom_fields: user_custom_fields, ip_address: ip_address).redeem + end + end + end + def self.find_user_by_email(email) User.with_email(Email.downcase(email)).where(staged: false).first end @@ -176,30 +228,62 @@ class Invite < ActiveRecord::Base group_ids end - def self.find_all_invites_from(inviter, offset = 0, limit = SiteSetting.invites_per_page) - Invite.where(invited_by_id: inviter.id) + def self.find_all_pending_invites_from(inviter, offset = 0, limit = SiteSetting.invites_per_page) + Invite.single_use_invites + .joins("LEFT JOIN invited_users ON invites.id = invited_users.invite_id") + .joins("LEFT JOIN users ON invited_users.user_id = users.id") + .where('invited_users.user_id IS NULL') + .where(invited_by_id: inviter.id) .where('invites.email IS NOT NULL') - .includes(user: :user_stat) - .order("CASE WHEN invites.user_id IS NOT NULL THEN 0 ELSE 1 END, user_stats.time_read DESC, invites.redeemed_at DESC") + .order('invites.updated_at DESC') .limit(limit) .offset(offset) - .references('user_stats') end def self.find_pending_invites_from(inviter, offset = 0) - 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) - find_all_invites_from(inviter, offset).where('invites.user_id IS NOT NULL').order('invites.redeemed_at DESC') + find_all_pending_invites_from(inviter, offset) end def self.find_pending_invites_count(inviter) - find_all_invites_from(inviter, 0, nil).where('invites.user_id IS NULL').reorder(nil).count + find_all_pending_invites_from(inviter, 0, nil).reorder(nil).count + end + + def self.find_all_redeemed_invites_from(inviter, offset = 0, limit = SiteSetting.invites_per_page) + InvitedUser.includes(:invite) + .includes(user: :user_stat) + .where('invited_users.user_id IS NOT NULL') + .where('invites.invited_by_id = ?', inviter.id) + .order('user_stats.time_read DESC, invited_users.redeemed_at DESC') + .limit(limit) + .offset(offset) + .references('invite') + .references('user') + .references('user_stat') + end + + def self.find_redeemed_invites_from(inviter, offset = 0) + find_all_redeemed_invites_from(inviter, offset) end def self.find_redeemed_invites_count(inviter) - find_all_invites_from(inviter, 0, nil).where('invites.user_id IS NOT NULL').reorder(nil).count + find_all_redeemed_invites_from(inviter, 0, nil).reorder(nil).count + end + + def self.find_all_links_invites_from(inviter, offset = 0, limit = SiteSetting.invites_per_page) + Invite.multiple_use_invites + .includes(invited_groups: :group) + .where(invited_by_id: inviter.id) + .order('invites.updated_at DESC') + .limit(limit) + .offset(offset) + end + + def self.find_links_invites_from(inviter, offset = 0) + find_all_links_invites_from(inviter, offset) + end + + def self.find_links_invites_count(inviter) + find_all_links_invites_from(inviter, 0, nil).reorder(nil).count end def self.filter_by(email_or_username) @@ -223,25 +307,32 @@ class Invite < ActiveRecord::Base end def self.redeem_from_email(email) - invite = Invite.find_by(email: Email.downcase(email)) - InviteRedeemer.new(invite).redeem if invite + invite = Invite.single_use_invites.find_by(email: Email.downcase(email)) + InviteRedeemer.new(invite: invite, email: invite.email).redeem if invite invite end def resend_invite - self.update_columns(updated_at: Time.zone.now) + self.update_columns(updated_at: Time.zone.now, expires_at: SiteSetting.invite_expiry_days.days.from_now) Jobs.enqueue(:invite_email, invite_id: self.id) end def self.resend_all_invites_from(user_id) - Invite.where('invites.user_id IS NULL AND invites.email IS NOT NULL AND invited_by_id = ?', user_id).find_each do |invite| + Invite.single_use_invites + .joins(:invited_users) + .where('invited_users.user_id IS NULL AND invites.email IS NOT NULL AND invited_by_id = ?', user_id) + .find_each do |invite| invite.resend_invite end 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.updated_at < ?', - user.id, SiteSetting.invite_expiry_days.days.ago).find_each do |invite| + Invite.single_use_invites + .includes(:invited_users) + .where('invited_users.user_id IS NULL AND invites.email IS NOT NULL AND invited_by_id = ? AND invites.expires_at < ?', + user.id, Time.zone.now) + .references('invited_users') + .find_each do |invite| invite.trash!(user) end end @@ -253,26 +344,37 @@ class Invite < ActiveRecord::Base def self.base_directory File.join(Rails.root, "public", "uploads", "csv", RailsMultisite::ConnectionManagement.current_db) end + + def ensure_max_redemptions_allowed + if self.max_redemptions_allowed.nil? || self.max_redemptions_allowed == 1 + self.max_redemptions_allowed ||= 1 + else + if !self.max_redemptions_allowed.between?(2, SiteSetting.invite_link_max_redemptions_limit) + errors.add(:max_redemptions_allowed, I18n.t("invite_link.max_redemptions_limit", max_limit: SiteSetting.invite_link_max_redemptions_limit)) + end + end + end end # == Schema Information # # Table name: invites # -# id :integer not null, primary key -# invite_key :string(32) not null -# email :string -# invited_by_id :integer not null -# user_id :integer -# redeemed_at :datetime -# created_at :datetime not null -# updated_at :datetime not null -# deleted_at :datetime -# deleted_by_id :integer -# invalidated_at :datetime -# moderator :boolean default(FALSE), not null -# custom_message :text -# emailed_status :integer +# id :integer not null, primary key +# invite_key :string(32) not null +# email :string +# invited_by_id :integer not null +# created_at :datetime not null +# updated_at :datetime not null +# deleted_at :datetime +# deleted_by_id :integer +# invalidated_at :datetime +# moderator :boolean default(FALSE), not null +# custom_message :text +# emailed_status :integer +# max_redemptions_allowed :integer default(1), not null +# redemption_count :integer default(0), not null +# expires_at :datetime not null # # Indexes # diff --git a/app/models/invite_redeemer.rb b/app/models/invite_redeemer.rb index 9c716d2983f..ad49c18c6d2 100644 --- a/app/models/invite_redeemer.rb +++ b/app/models/invite_redeemer.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_fields, :ip_address) do +InviteRedeemer = Struct.new(:invite, :email, :username, :name, :password, :user_custom_fields, :ip_address, keyword_init: true) do def redeem Invite.transaction do @@ -14,8 +14,8 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_f end # extracted from User cause it is very specific to invites - def self.create_user_from_invite(invite, username, name, password = nil, user_custom_fields = nil, ip_address = nil) - user = User.where(staged: true).with_email(invite.email.strip.downcase).first + def self.create_user_from_invite(email:, invite:, username: nil, name: nil, password: nil, user_custom_fields: nil, ip_address: nil) + user = User.where(staged: true).with_email(email.strip.downcase).first user.unstage! if user user ||= User.new @@ -23,11 +23,11 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_f if username && UsernameValidator.new(username).valid_format? && User.username_available?(username) available_username = username else - available_username = UserNameSuggester.suggest(invite.email) + available_username = UserNameSuggester.suggest(email) end user.attributes = { - email: invite.email, + email: email, username: available_username, name: name || available_username, active: false, @@ -63,7 +63,7 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_f user.save! - if invite.emailed_status != Invite.emailed_status_types[:not_required] + if invite.emailed_status != Invite.emailed_status_types[:not_required] && email == invite.email user.email_tokens.create!(email: user.email) user.activate end @@ -86,35 +86,42 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_f end def invite_was_redeemed? - # Return true if a row was updated - mark_invite_redeemed == 1 + mark_invite_redeemed end def mark_invite_redeemed - 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') + if !invite.is_invite_link? && InvitedUser.exists?(invite_id: invite.id) + return false + end - if count == 1 + existing_user = get_existing_user + if existing_user.present? && InvitedUser.exists?(user_id: existing_user.id, invite_id: invite.id) + return false + end + + @invited_user_record = InvitedUser.create!(invite_id: invite.id, redeemed_at: Time.zone.now) + if invite.is_invite_link? && @invited_user_record.present? + Invite.increment_counter(:redemption_count, invite.id) + elsif @invited_user_record.present? delete_duplicate_invites end - count + @invited_user_record.present? end def get_invited_user result = get_existing_user - result ||= InviteRedeemer.create_user_from_invite(invite, username, name, password, user_custom_fields, ip_address) + result ||= InviteRedeemer.create_user_from_invite(invite: invite, email: email, username: username, name: name, password: password, user_custom_fields: user_custom_fields, ip_address: ip_address) result.send_welcome_message = false result end def get_existing_user - User.where(admin: false, staged: false).find_by_email(invite.email) + User.where(admin: false, staged: false).find_by_email(email) end def add_to_private_topics_if_invited - topic_ids = Topic.where(archetype: Archetype::private_message).includes(:invites).where(invites: { email: invite.email }).pluck(:id) + topic_ids = Topic.where(archetype: Archetype::private_message).includes(:invites).where(invites: { email: email }).pluck(:id) topic_ids.each do |id| TopicAllowedUser.create!(user_id: invited_user.id, topic_id: id) unless TopicAllowedUser.exists?(user_id: invited_user.id, topic_id: id) end @@ -129,9 +136,8 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_f end def send_welcome_message - if Invite.where('email = ?', invite.email).update_all(['user_id = ?', invited_user.id]) == 1 - invited_user.send_welcome_message = true - end + @invited_user_record.update!(user_id: invited_user.id) + invited_user.send_welcome_message = true end def approve_account_if_needed @@ -155,6 +161,10 @@ InviteRedeemer = Struct.new(:invite, :username, :name, :password, :user_custom_f end def delete_duplicate_invites - Invite.where('invites.email = ? AND redeemed_at IS NULL AND invites.id != ?', invite.email, invite.id).delete_all + Invite.single_use_invites + .joins("LEFT JOIN invited_users ON invites.id = invited_users.invite_id") + .where('invited_users.user_id IS NULL') + .where('invites.email = ? AND invites.id != ?', email, invite.id) + .delete_all end end diff --git a/app/models/invited_user.rb b/app/models/invited_user.rb new file mode 100644 index 00000000000..078878c7e77 --- /dev/null +++ b/app/models/invited_user.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class InvitedUser < ActiveRecord::Base + belongs_to :user + belongs_to :invite + + validates_presence_of :invite_id + validates_uniqueness_of :invite_id, scope: :user_id, conditions: -> { where.not(user_id: nil) } +end + +# == Schema Information +# +# Table name: invited_users +# +# id :bigint not null, primary key +# user_id :integer +# invite_id :integer not null +# redeemed_at :datetime +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# index_invited_users_on_invite_id (invite_id) +# index_invited_users_on_user_id_and_invite_id (user_id,invite_id) UNIQUE WHERE (user_id IS NOT NULL) +# diff --git a/app/models/user.rb b/app/models/user.rb index fdade36364c..ab30ea1b5ab 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -21,7 +21,6 @@ class User < ActiveRecord::Base has_many :user_archived_messages, dependent: :destroy has_many :email_change_requests, dependent: :destroy has_many :email_tokens, dependent: :destroy - has_many :invites, dependent: :destroy has_many :topic_links, dependent: :destroy has_many :user_uploads, dependent: :destroy has_many :user_emails, dependent: :destroy @@ -37,6 +36,7 @@ class User < ActiveRecord::Base has_many :acting_group_histories, dependent: :destroy, foreign_key: :acting_user_id, class_name: 'GroupHistory' has_many :targeted_group_histories, dependent: :destroy, foreign_key: :target_user_id, class_name: 'GroupHistory' has_many :reviewable_scores, dependent: :destroy + has_many :invites, foreign_key: :invited_by_id, dependent: :destroy has_one :user_option, dependent: :destroy has_one :user_avatar, dependent: :destroy @@ -47,6 +47,7 @@ class User < ActiveRecord::Base has_one :single_sign_on_record, dependent: :destroy has_one :anonymous_user_master, class_name: 'AnonymousUser', dependent: :destroy has_one :anonymous_user_shadow, ->(record) { where(active: true) }, foreign_key: :master_user_id, class_name: 'AnonymousUser', dependent: :destroy + has_one :invited_user, dependent: :destroy # delete all is faster but bypasses callbacks has_many :bookmarks, dependent: :delete_all @@ -425,7 +426,7 @@ class User < ActiveRecord::Base end def invited_by - used_invite = invites.where("redeemed_at is not null").includes(:invited_by).first + used_invite = Invite.joins(:invited_users).where("invited_users.user_id = ?", self.id).first used_invite.try(:invited_by) end diff --git a/app/serializers/invite_link_serializer.rb b/app/serializers/invite_link_serializer.rb new file mode 100644 index 00000000000..21a64509189 --- /dev/null +++ b/app/serializers/invite_link_serializer.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class InviteLinkSerializer < ApplicationSerializer + attributes :id, :invite_key, :created_at, :max_redemptions_allowed, :redemption_count, :expires_at, :group_names + + def group_names + object.groups.pluck(:name).join(", ") + end +end diff --git a/app/serializers/invite_serializer.rb b/app/serializers/invite_serializer.rb index 9c03c813fb1..f66f1c02c7c 100644 --- a/app/serializers/invite_serializer.rb +++ b/app/serializers/invite_serializer.rb @@ -1,8 +1,7 @@ # frozen_string_literal: true class InviteSerializer < ApplicationSerializer - - attributes :email, :updated_at, :redeemed_at, :expired, :user + attributes :id, :email, :updated_at, :expired def include_email? options[:show_emails] && !object.redeemed? @@ -11,11 +10,4 @@ class InviteSerializer < ApplicationSerializer def expired object.expired? end - - def user - ser = InvitedUserSerializer.new(object.user, scope: scope, root: false) - ser.invited_by = object.invited_by - ser.as_json - end - end diff --git a/app/serializers/invited_serializer.rb b/app/serializers/invited_serializer.rb index c0b07faa866..68ced8ba108 100644 --- a/app/serializers/invited_serializer.rb +++ b/app/serializers/invited_serializer.rb @@ -4,8 +4,15 @@ class InvitedSerializer < ApplicationSerializer attributes :invites, :can_see_invite_details def invites + serializer = if object.type == "pending" + InviteSerializer + else + InvitedUserSerializer + end + ActiveModel::ArraySerializer.new( object.invite_list, + each_serializer: serializer, scope: scope, root: false, show_emails: object.show_emails diff --git a/app/serializers/invited_user_record_serializer.rb b/app/serializers/invited_user_record_serializer.rb new file mode 100644 index 00000000000..d98f475bf33 --- /dev/null +++ b/app/serializers/invited_user_record_serializer.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +class InvitedUserRecordSerializer < BasicUserSerializer + + attributes :topics_entered, + :posts_read_count, + :last_seen_at, + :time_read, + :days_visited, + :days_since_created + + attr_accessor :invited_by + + def time_read + object.user_stat.time_read + end + + def include_time_read? + can_see_invite_details? + end + + def days_visited + object.user_stat.days_visited + end + + def include_days_visited? + can_see_invite_details? + end + + def topics_entered + object.user_stat.topics_entered + end + + def include_topics_entered? + can_see_invite_details? + end + + def posts_read_count + object.user_stat.posts_read_count + end + + def include_posts_read_count? + can_see_invite_details? + end + + def days_since_created + ((Time.now - object.created_at) / 60 / 60 / 24).ceil + end + + def include_days_since_created + can_see_invite_details? + end + + private + + def can_see_invite_details? + @can_see_invite_details ||= scope.can_see_invite_details?(invited_by) + end + +end diff --git a/app/serializers/invited_user_serializer.rb b/app/serializers/invited_user_serializer.rb index f793871f49b..d4db8da9537 100644 --- a/app/serializers/invited_user_serializer.rb +++ b/app/serializers/invited_user_serializer.rb @@ -1,54 +1,19 @@ # frozen_string_literal: true -class InvitedUserSerializer < BasicUserSerializer +class InvitedUserSerializer < ApplicationSerializer + attributes :id, :redeemed_at, :user, :invite_source - attributes :topics_entered, - :posts_read_count, - :last_seen_at, - :time_read, - :days_visited, - :days_since_created - - attr_accessor :invited_by - - def time_read - object.user_stat.time_read + def id + object.invite.id end - def include_time_read? - scope.can_see_invite_details?(invited_by) + def user + ser = InvitedUserRecordSerializer.new(object.user, scope: scope, root: false) + ser.invited_by = object.invite.invited_by + ser.as_json end - def days_visited - object.user_stat.days_visited + def invite_source + object.invite.is_invite_link? ? "link" : "email" end - - def include_days_visited? - scope.can_see_invite_details?(invited_by) - end - - def topics_entered - object.user_stat.topics_entered - end - - def include_topics_entered? - scope.can_see_invite_details?(invited_by) - end - - def posts_read_count - object.user_stat.posts_read_count - end - - def include_posts_read_count? - scope.can_see_invite_details?(invited_by) - end - - def days_since_created - ((Time.now - object.created_at) / 60 / 60 / 24).ceil - end - - def include_days_since_created - scope.can_see_invite_details?(invited_by) - end - end diff --git a/app/services/user_merger.rb b/app/services/user_merger.rb index b576eea8533..d6774b5080d 100644 --- a/app/services/user_merger.rb +++ b/app/services/user_merger.rb @@ -257,7 +257,7 @@ class UserMerger IncomingLink.where(user_id: @source_user.id).update_all(user_id: @target_user.id) IncomingLink.where(current_user_id: @source_user.id).update_all(current_user_id: @target_user.id) - Invite.with_deleted.where(user_id: @source_user.id).update_all(user_id: @target_user.id) + InvitedUser.where(user_id: @source_user.id).update_all(user_id: @target_user.id) Invite.with_deleted.where(invited_by_id: @source_user.id).update_all(invited_by_id: @target_user.id) Invite.with_deleted.where(deleted_by_id: @source_user.id).update_all(deleted_by_id: @target_user.id) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 308944b3b28..f43b88ef72d 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1288,7 +1288,7 @@ en: expired: "This invite has expired." rescind: "Remove" rescinded: "Invite removed" - rescind_all: "Remove all Expired Invites" + rescind_all: "Remove Expired Invites" rescinded_all: "All Expired Invites removed!" rescind_all_confirm: "Are you sure you want to remove all expired invites?" reinvite: "Resend Invite" @@ -1299,13 +1299,30 @@ en: time_read: "Read Time" days_visited: "Days Visited" account_age_days: "Account age in days" - create: "Send an Invite" + source: "Invited Via" + links_tab: "Links" + links_tab_with_count: "Links (%{count})" + link_url: "Link" + link_created_at: "Created" + link_redemption_stats: "Redemptions" + link_groups: Groups + link_expires_at: Expires + create: "Send Invite" + copy_link: "Copy Link" generate_link: "Copy Invite Link" link_generated: "Invite link generated successfully!" valid_for: "Invite link is only valid for this email address: %{email}" + single_user: "Single User" + multiple_user: "Multiple Users" + invite_link: + title: "Invite Link" + success: "Invite link generated successfully!" + error: "There was an error generating Invite link" + max_redemptions_allowed_label: "How many people are allowed to register using this link?" + expires_at: "When will this invite link expire?" bulk_invite: none: "You haven't invited anyone here yet. Send individual invites, or invite many people at once by uploading a CSV file." - text: "Bulk Invite from File" + text: "Bulk Invite" success: "File uploaded successfully, you will be notified via message when the process is complete." error: "Sorry, file should be CSV format." confirmation_message: "You’re about to email invites to everyone in the uploaded file." diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index a7a1a7c110f..eec321b8d81 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -235,6 +235,10 @@ en: max_rows: "First %{max_bulk_invites} invites has been sent. Try splitting the file in smaller parts." error: "There was an error uploading that file. Please try again later." + invite_link: + email_taken: "This email is already in use. If you already have an account please login or reset password." + max_redemptions_limit: "should be between 2 and %{max_limit}." + topic_invite: failed_to_invite: "The user cannot be invited into this topic without a group membership in either one of the following groups: %{group_names}." user_exists: "Sorry, that user has already been invited. You may only invite a user to a topic once." @@ -1714,6 +1718,8 @@ en: max_post_deletions_per_minute: "Maximum number of posts a user can delete per minute." max_post_deletions_per_day: "Maximum number of posts a user can delete per day." + invite_link_max_redemptions_limit: "Maximum redemptions allowed for invite links can't be more than this value." + alert_admins_if_errors_per_minute: "Number of errors per minute in order to trigger an admin alert. A value of 0 disables this feature. NOTE: requires restart." alert_admins_if_errors_per_hour: "Number of errors per hour in order to trigger an admin alert. A value of 0 disables this feature. NOTE: requires restart." diff --git a/config/routes.rb b/config/routes.rb index d837af541ba..96b81c8e260 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -468,6 +468,7 @@ Discourse::Application.routes.draw do get "#{root_path}/:username/invited" => "users#invited", constraints: { username: RouteFormat.username } get "#{root_path}/:username/invited_count" => "users#invited_count", constraints: { username: RouteFormat.username } get "#{root_path}/:username/invited/:filter" => "users#invited", constraints: { username: RouteFormat.username } + get "#{root_path}/:username/invite_links" => "users#invite_links", constraints: { username: RouteFormat.username } post "#{root_path}/action/send_activation_email" => "users#send_activation_email" get "#{root_path}/:username/summary" => "users#show", constraints: { username: RouteFormat.username } get "#{root_path}/:username/activity/topics.rss" => "list#user_topics_feed", format: :rss, constraints: { username: RouteFormat.username } diff --git a/config/site_settings.yml b/config/site_settings.yml index 11705c8e24b..e240fe35261 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1604,6 +1604,11 @@ rate_limits: max_post_deletions_per_day: min: 1 default: 10 + invite_link_max_redemptions_limit: + min: 2 + max: 1000000 + default: 5000 + client: true developer: force_hostname: diff --git a/db/migrate/20200428102014_add_bulk_invite_link_to_invites.rb b/db/migrate/20200428102014_add_bulk_invite_link_to_invites.rb new file mode 100644 index 00000000000..57636b6dcb4 --- /dev/null +++ b/db/migrate/20200428102014_add_bulk_invite_link_to_invites.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddBulkInviteLinkToInvites < ActiveRecord::Migration[6.0] + def change + add_column :invites, :max_redemptions_allowed, :integer, null: false, default: 1 + add_column :invites, :redemption_count, :integer, null: false, default: 0 + add_column :invites, :expires_at, :datetime, null: true + + invite_expiry_days = DB.query_single("SELECT value FROM site_settings WHERE name = 'invite_expiry_days'").first + invite_expiry_days = 30 if invite_expiry_days.blank? + execute <<~SQL + UPDATE invites SET expires_at = updated_at + INTERVAL '#{invite_expiry_days} days' + SQL + + change_column :invites, :expires_at, :datetime, null: false + end +end diff --git a/db/migrate/20200430072846_create_invited_users.rb b/db/migrate/20200430072846_create_invited_users.rb new file mode 100644 index 00000000000..fa3f843e607 --- /dev/null +++ b/db/migrate/20200430072846_create_invited_users.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class CreateInvitedUsers < ActiveRecord::Migration[6.0] + def change + create_table :invited_users do |t| + t.integer :user_id + t.integer :invite_id, null: false + t.datetime :redeemed_at + t.timestamps null: false + end + + add_index :invited_users, :invite_id + add_index :invited_users, [:user_id, :invite_id], unique: true, where: 'user_id IS NOT NULL' + end +end diff --git a/db/post_migrate/20200602153813_migrate_invite_redeemed_data_to_invited_users.rb b/db/post_migrate/20200602153813_migrate_invite_redeemed_data_to_invited_users.rb new file mode 100644 index 00000000000..29c29276735 --- /dev/null +++ b/db/post_migrate/20200602153813_migrate_invite_redeemed_data_to_invited_users.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class MigrateInviteRedeemedDataToInvitedUsers < ActiveRecord::Migration[6.0] + def up + %i{user_id redeemed_at}.each do |column| + Migration::ColumnDropper.mark_readonly(:invites, column) + end + + execute <<~SQL + INSERT INTO invited_users ( + user_id, + invite_id, + redeemed_at, + created_at, + updated_at + ) + SELECT user_id, id, redeemed_at, created_at, updated_at + FROM invites + WHERE user_id IS NOT NULL AND redeemed_at IS NOT NULL + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/badge_queries.rb b/lib/badge_queries.rb index 83480878731..a8cf8a91711 100644 --- a/lib/badge_queries.rb +++ b/lib/badge_queries.rb @@ -158,7 +158,8 @@ module BadgeQueries WHERE u.id IN ( SELECT invited_by_id FROM invites i - JOIN users u2 ON u2.id = i.user_id + JOIN invited_users iu ON iu.invite_id = i.id + JOIN users u2 ON u2.id = iu.user_id WHERE i.deleted_at IS NULL AND u2.active AND u2.trust_level >= #{trust_level.to_i} AND u2.silenced_till IS NULL GROUP BY invited_by_id HAVING COUNT(*) >= #{count.to_i} diff --git a/lib/guardian.rb b/lib/guardian.rb index da6d1bf2b79..5e6fe33282c 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -383,6 +383,10 @@ class Guardian user.admin? end + def can_send_invite_links?(user) + user.staff? + end + def can_send_multiple_invites?(user) user.staff? end diff --git a/plugins/discourse-narrative-bot/spec/jobs/send_default_welcome_message_spec.rb b/plugins/discourse-narrative-bot/spec/jobs/send_default_welcome_message_spec.rb index 58ae040cb65..da86df91b44 100644 --- a/plugins/discourse-narrative-bot/spec/jobs/send_default_welcome_message_spec.rb +++ b/plugins/discourse-narrative-bot/spec/jobs/send_default_welcome_message_spec.rb @@ -24,10 +24,11 @@ RSpec.describe Jobs::SendDefaultWelcomeMessage do end describe 'for an invited user' do - let(:invite) { Fabricate(:invite, user: user, redeemed_at: Time.zone.now) } + let(:invite) { Fabricate(:invite, email: 'foo@bar.com') } + let(:invited_user) { Fabricate(:invited_user, invite: invite, user: Fabricate(:user, email: 'foo@bar.com'), redeemed_at: Time.zone.now) } it 'should send the right welcome message' do - described_class.new.execute(user_id: invite.user_id) + described_class.new.execute(user_id: invited_user.user_id) topic = Topic.last @@ -38,7 +39,7 @@ RSpec.describe Jobs::SendDefaultWelcomeMessage do expect(topic.first_post.raw).to eq(I18n.t( "system_messages.welcome_invite.text_body_template", - SystemMessage.new(user).defaults + SystemMessage.new(invited_user.user).defaults ).chomp) expect(topic.closed).to eq(true) diff --git a/spec/fabricators/invited_user_fabricator.rb b/spec/fabricators/invited_user_fabricator.rb new file mode 100644 index 00000000000..05cb996a34a --- /dev/null +++ b/spec/fabricators/invited_user_fabricator.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +Fabricator(:invited_user) do + user + invite +end diff --git a/spec/models/email_token_spec.rb b/spec/models/email_token_spec.rb index 50df6b10041..46db9c98de0 100644 --- a/spec/models/email_token_spec.rb +++ b/spec/models/email_token_spec.rb @@ -122,7 +122,7 @@ describe EmailToken do Jobs.run_immediately! end - fab!(:invite) { Fabricate(:invite, email: 'test@example.com', user_id: nil) } + fab!(:invite) { Fabricate(:invite, email: 'test@example.com') } fab!(:invited_user) { Fabricate(:user, active: false, email: invite.email) } let(:user_email_token) { invited_user.email_tokens.first } let!(:confirmed_invited_user) { EmailToken.confirm(user_email_token.token) } diff --git a/spec/models/invite_redeemer_spec.rb b/spec/models/invite_redeemer_spec.rb index 814fe42124e..d29c9b6f57d 100644 --- a/spec/models/invite_redeemer_spec.rb +++ b/spec/models/invite_redeemer_spec.rb @@ -6,7 +6,8 @@ describe InviteRedeemer do describe '#create_user_from_invite' do it "should be created correctly" do - user = InviteRedeemer.create_user_from_invite(Fabricate(:invite, email: 'walter.white@email.com'), 'walter', 'Walter White') + invite = Fabricate(:invite, email: 'walter.white@email.com') + user = InviteRedeemer.create_user_from_invite(invite: invite, email: invite.email, username: 'walter', name: 'Walter White') expect(user.username).to eq('walter') expect(user.name).to eq('Walter White') expect(user).to be_active @@ -17,7 +18,8 @@ describe InviteRedeemer do it "can set the password and ip_address" do password = 's3cure5tpasSw0rD' ip_address = '192.168.1.1' - user = InviteRedeemer.create_user_from_invite(Fabricate(:invite, email: 'walter.white@email.com'), 'walter', 'Walter White', password, nil, ip_address) + invite = Fabricate(:invite, email: 'walter.white@email.com') + user = InviteRedeemer.create_user_from_invite(invite: invite, email: invite.email, username: 'walter', name: 'Walter White', password: password, ip_address: ip_address) expect(user).to have_password expect(user.confirm_password?(password)).to eq(true) expect(user.approved).to eq(true) @@ -27,8 +29,9 @@ describe InviteRedeemer do it "raises exception with record and errors" do error = nil + invite = Fabricate(:invite, email: 'walter.white@email.com') begin - InviteRedeemer.create_user_from_invite(Fabricate(:invite, email: 'walter.white@email.com'), 'walter', 'Walter White', 'aaa') + InviteRedeemer.create_user_from_invite(invite: invite, email: invite.email, username: 'walter', name: 'Walter White', password: 'aaa') rescue ActiveRecord::RecordInvalid => e error = e end @@ -38,7 +41,8 @@ describe InviteRedeemer do it "should unstage user" do staged_user = Fabricate(:staged, email: 'staged@account.com', active: true, username: 'staged1', name: 'Stage Name') - user = InviteRedeemer.create_user_from_invite(Fabricate(:invite, email: 'staged@account.com'), 'walter', 'Walter White') + invite = Fabricate(:invite, email: 'staged@account.com') + user = InviteRedeemer.create_user_from_invite(invite: invite, email: invite.email, username: 'walter', name: 'Walter White') expect(user.id).to eq(staged_user.id) expect(user.username).to eq('walter') @@ -49,7 +53,9 @@ describe InviteRedeemer do end it "should not activate user invited via links" do - user = InviteRedeemer.create_user_from_invite(Fabricate(:invite, email: 'walter.white@email.com', emailed_status: Invite.emailed_status_types[:not_required]), 'walter', 'Walter White') + invite = Fabricate(:invite, email: 'walter.white@email.com', emailed_status: Invite.emailed_status_types[:not_required]) + user = InviteRedeemer.create_user_from_invite(invite: invite, email: invite.email, username: 'walter', name: 'Walter White') + expect(user.username).to eq('walter') expect(user.name).to eq('Walter White') expect(user.email).to eq('walter.white@email.com') @@ -63,7 +69,7 @@ describe InviteRedeemer do let(:name) { 'john snow' } let(:username) { 'kingofthenorth' } let(:password) { 'know5nOthiNG' } - let(:invite_redeemer) { InviteRedeemer.new(invite, username, name) } + let(:invite_redeemer) { InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name) } it "should redeem the invite if invited by staff" do SiteSetting.must_approve_users = true @@ -112,19 +118,13 @@ describe InviteRedeemer do expect(user.approved).to eq(true) end - it "should not blow up if invited_by user has been removed" do + it "should delete invite if invited_by user has been removed" do invite.invited_by.destroy! - invite.reload - - user = invite_redeemer.redeem - - expect(user.name).to eq(name) - expect(user.username).to eq(username) - expect(user.invited_by).to eq(nil) + expect { invite.reload }.to raise_error(ActiveRecord::RecordNotFound) end it "can set password" do - user = InviteRedeemer.new(invite, username, name, password).redeem + user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password).redeem expect(user).to have_password expect(user.confirm_password?(password)).to eq(true) expect(user.approved).to eq(true) @@ -137,7 +137,7 @@ describe InviteRedeemer do required_field.id.to_s => 'value1', optional_field.id.to_s => 'value2' } - user = InviteRedeemer.new(invite, username, name, password, user_fields).redeem + user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password, user_custom_fields: user_fields).redeem expect(user).to be_present expect(user.custom_fields["user_field_#{required_field.id}"]).to eq('value1') @@ -147,7 +147,7 @@ describe InviteRedeemer do it "adds user to group" do group = Fabricate(:group, grant_trust_level: 2) InvitedGroup.create(group_id: group.id, invite_id: invite.id) - user = InviteRedeemer.new(invite, username, name, password).redeem + user = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name, password: password).redeem expect(user.group_users.count).to eq(4) expect(user.trust_level).to eq(2) @@ -160,7 +160,7 @@ describe InviteRedeemer do user.email = "john@example.com" user.save! - another_invite_redeemer = InviteRedeemer.new(invite, username, name) + another_invite_redeemer = InviteRedeemer.new(invite: invite, email: invite.email, username: username, name: name) another_user = another_invite_redeemer.redeem expect(another_user).to eq(nil) end @@ -176,7 +176,40 @@ describe InviteRedeemer do expect(user.invited_by).to eq(inviter) expect(inviter.notifications.count).to eq(1) - expect(invite.redeemed_at).not_to eq(nil) + expect(invite.invited_users.first).to be_present + end + + context 'invite_link' do + fab!(:invite_link) { Fabricate(:invite, max_redemptions_allowed: 5, expires_at: 1.month.from_now, emailed_status: Invite.emailed_status_types[:not_required]) } + let(:invite_redeemer) { InviteRedeemer.new(invite: invite_link, email: 'foo@example.com') } + + it 'works as expected' do + user = invite_redeemer.redeem + invite_link.reload + + expect(user.send_welcome_message).to eq(true) + expect(user.trust_level).to eq(SiteSetting.default_invitee_trust_level) + expect(user.active).to eq(false) + expect(invite_link.redemption_count).to eq(1) + end + + it "should not redeem the invite if InvitedUser record already exists for email" do + user = invite_redeemer.redeem + invite_link.reload + + another_invite_redeemer = InviteRedeemer.new(invite: invite_link, email: 'foo@example.com') + another_user = another_invite_redeemer.redeem + expect(another_user).to eq(nil) + end + + it "should redeem the invite if InvitedUser record does not exists for email" do + user = invite_redeemer.redeem + invite_link.reload + + another_invite_redeemer = InviteRedeemer.new(invite: invite_link, email: 'bar@example.com') + another_user = another_invite_redeemer.redeem + expect(another_user.is_a?(User)).to eq(true) + end end end diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index f66a8f84cd6..bd896e0777e 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -22,7 +22,6 @@ describe Invite do it "should not allow a user to invite themselves" do expect(invite.email_already_exists).to eq(true) end - end context 'email validators' do @@ -52,7 +51,6 @@ describe Invite do end context '#create' do - context 'saved' do subject { Fabricate(:invite) } @@ -81,7 +79,7 @@ describe Invite do context 'links' do it 'does not enqueue a job to email the invite' do expect do - Invite.generate_invite_link(iceking, inviter, topic) + Invite.generate_single_use_invite_link(iceking, inviter, topic) end.not_to change { Jobs::InviteEmail.jobs.size } end end @@ -142,7 +140,7 @@ describe Invite do it 'returns a new invite if the other has expired' do SiteSetting.invite_expiry_days = 1 - invite.update!(updated_at: 2.days.ago) + invite.update!(expires_at: 2.days.ago) new_invite = Invite.invite_by_email( 'iceking@adventuretime.ooo', inviter, topic @@ -166,7 +164,7 @@ describe Invite do it 'resets expiry of a resent invite' do SiteSetting.invite_expiry_days = 2 - invite.update!(updated_at: 10.days.ago) + invite.update!(expires_at: 10.days.ago) expect(invite).to be_expired invite.resend_invite @@ -183,17 +181,43 @@ describe Invite do it 'does not mark emailed_status as sending after generating invite link' do expect(invite.emailed_status).to eq(Invite.emailed_status_types[:sending]) - Invite.generate_invite_link(iceking, inviter, topic) + Invite.generate_single_use_invite_link(iceking, inviter, topic) expect(invite.reload.emailed_status).to eq(Invite.emailed_status_types[:not_required]) Invite.invite_by_email(iceking, inviter, topic) expect(invite.reload.emailed_status).to eq(Invite.emailed_status_types[:not_required]) - Invite.generate_invite_link(iceking, inviter, topic) + Invite.generate_single_use_invite_link(iceking, inviter, topic) expect(invite.reload.emailed_status).to eq(Invite.emailed_status_types[:not_required]) end end end + + context 'invite links' do + let(:inviter) { Fabricate(:user) } + + it "has sane defaults" do + Invite.generate_multiple_use_invite_link(invited_by: inviter) + invite_link = Invite.last + expect(invite_link.max_redemptions_allowed).to eq(5) + expect(invite_link.expires_at.to_date).to eq(1.month.from_now.to_date) + expect(invite_link.emailed_status).to eq(Invite.emailed_status_types[:not_required]) + expect(invite_link.is_invite_link?).to eq(true) + end + + it 'checks for max_redemptions_allowed range' do + SiteSetting.invite_link_max_redemptions_limit = 1000 + expect do + Invite.generate_multiple_use_invite_link(invited_by: inviter, max_redemptions_allowed: 1001) + end.to raise_error(ActiveRecord::RecordInvalid) + end + + it 'does not enqueue a job to email the invite' do + expect do + Invite.generate_multiple_use_invite_link(invited_by: inviter) + end.not_to change { Jobs::InviteEmail.jobs.size } + end + end end context 'an existing user' do @@ -205,7 +229,6 @@ describe Invite do Invite.invite_by_email(coding_horror.email, topic.user, topic) end.to raise_error(Invite::UserExists) end - end context 'a staged user' do @@ -228,7 +251,7 @@ describe Invite do it 'wont redeem an expired invite' do SiteSetting.invite_expiry_days = 10 - invite.update_column(:updated_at, 20.days.ago) + invite.update_column(:expires_at, 20.days.ago) expect(invite.redeem).to be_blank end @@ -253,12 +276,12 @@ describe Invite do end it 'does not delete already redeemed invite' do - redeemed_invite = Fabricate(:invite, email: invite.email, invited_by: another_user, redeemed_at: 1.day.ago) + redeemed_invite = Fabricate(:invite, email: invite.email, invited_by: another_user) + Fabricate(:invited_user, invite: invite, user: Fabricate(:user)) invite.redeem used_invite = Invite.find_by(id: redeemed_invite.id) expect(used_invite).not_to be_nil end - end context "as a moderator" do @@ -308,7 +331,6 @@ describe Invite do end context 'simple invite' do - let!(:user) { invite.redeem } it 'works correctly' do @@ -324,7 +346,7 @@ describe Invite do it 'works correctly' do # has set the user_id attribute - expect(invite.user).to eq(user) + expect(invite.invited_users.first.user).to eq(user) # returns true for redeemed expect(invite).to be_redeemed @@ -336,7 +358,6 @@ describe Invite do end end end - end context 'invited to topics' do @@ -379,15 +400,36 @@ describe Invite do end end end + + context 'invite_link' do + fab!(:invite_link) { Fabricate(:invite, email: nil, max_redemptions_allowed: 5, expires_at: 1.month.from_now, emailed_status: Invite.emailed_status_types[:not_required]) } + + it 'works correctly' do + user = invite_link.redeem_invite_link(email: 'foo@example.com') + expect(user.is_a?(User)).to eq(true) + expect(user.send_welcome_message).to eq(true) + expect(user.trust_level).to eq(SiteSetting.default_invitee_trust_level) + expect(user.active).to eq(false) + invite_link.reload + expect(invite_link.redemption_count).to eq(1) + end + + it 'returns error if user with that email already exists' do + user = Fabricate(:user) + expect do + invite_link.redeem_invite_link(email: user.email) + end.to raise_error(Invite::UserExists) + end + end end - describe '.find_all_invites_from' do + describe '.find_all_pending_invites_from' do context 'with user that has invited' do it 'returns invites' do inviter = Fabricate(:user) invite = Fabricate(:invite, invited_by: inviter) - invites = Invite.find_all_invites_from(inviter) + invites = Invite.find_all_pending_invites_from(inviter) expect(invites).to include invite end @@ -398,7 +440,7 @@ describe Invite do user = Fabricate(:user) Fabricate(:invite) - invites = Invite.find_all_invites_from(user) + invites = Invite.find_all_pending_invites_from(user) expect(invites).to be_empty end @@ -408,17 +450,16 @@ describe Invite do describe '.find_pending_invites_from' do it 'returns pending invites only' do inviter = Fabricate(:user) - Fabricate( + redeemed_invite = Fabricate( :invite, invited_by: inviter, - user_id: 123, email: 'redeemed@example.com' ) + Fabricate(:invited_user, invite: redeemed_invite, user: Fabricate(:user)) pending_invite = Fabricate( :invite, invited_by: inviter, - user_id: nil, email: 'pending@example.com' ) @@ -437,24 +478,70 @@ describe Invite do Fabricate( :invite, invited_by: inviter, - user_id: nil, email: 'pending@example.com' ) redeemed_invite = Fabricate( :invite, invited_by: inviter, - user_id: 123, email: 'redeemed@example.com' ) + Fabricate(:invited_user, invite: redeemed_invite, user: Fabricate(:user)) invites = Invite.find_redeemed_invites_from(inviter) expect(invites.length).to eq(1) - expect(invites.first).to eq redeemed_invite + expect(invites.first).to eq redeemed_invite.invited_users.first expect(Invite.find_redeemed_invites_count(inviter)).to eq(1) end + + it 'returns redeemed invites for invite links' do + inviter = Fabricate(:user) + invite_link = Fabricate( + :invite, + invited_by: inviter, + max_redemptions_allowed: 50 + ) + Fabricate(:invited_user, invite: invite_link, user: Fabricate(:user)) + Fabricate(:invited_user, invite: invite_link, user: Fabricate(:user)) + Fabricate(:invited_user, invite: invite_link, user: Fabricate(:user)) + + invites = Invite.find_redeemed_invites_from(inviter) + expect(invites.length).to eq(3) + expect(Invite.find_redeemed_invites_count(inviter)).to eq(3) + end + end + + describe '.find_links_invites_from' do + it 'returns invite links only' do + inviter = Fabricate(:user) + Fabricate( + :invite, + invited_by: inviter, + email: 'pending@example.com' + ) + + invite_link_1 = Fabricate( + :invite, + invited_by: inviter, + max_redemptions_allowed: 5 + ) + + invite_link_2 = Fabricate( + :invite, + invited_by: inviter, + max_redemptions_allowed: 50 + ) + + invites = Invite.find_links_invites_from(inviter) + + expect(invites.length).to eq(2) + expect(invites.first).to eq(invite_link_2) + expect(invites.first.max_redemptions_allowed).to eq(50) + + expect(Invite.find_links_invites_count(inviter)).to eq(2) + end end describe '.invalidate_for_email' do @@ -466,14 +553,14 @@ describe Invite do end it 'sets the matching invite to be invalid' do - invite = Fabricate(:invite, invited_by: Fabricate(:user), user_id: nil, email: email) + invite = Fabricate(:invite, invited_by: Fabricate(:user), email: email) expect(subject).to eq(invite) expect(subject.link_valid?).to eq(false) expect(subject).to be_valid end it 'sets the matching invite to be invalid without being case-sensitive' do - invite = Fabricate(:invite, invited_by: Fabricate(:user), user_id: nil, email: 'invite.me2@Example.COM') + invite = Fabricate(:invite, invited_by: Fabricate(:user), email: 'invite.me2@Example.COM') result = described_class.invalidate_for_email('invite.me2@EXAMPLE.com') expect(result).to eq(invite) expect(result.link_valid?).to eq(false) @@ -483,7 +570,7 @@ describe Invite do describe '.redeem_from_email' do fab!(:inviter) { Fabricate(:user) } - fab!(:invite) { Fabricate(:invite, invited_by: inviter, email: 'test@example.com', user_id: nil) } + fab!(:invite) { Fabricate(:invite, invited_by: inviter, email: 'test@example.com') } fab!(:user) { Fabricate(:user, email: invite.email) } it 'redeems the invite from email' do @@ -497,7 +584,6 @@ describe Invite do invite.reload expect(invite).not_to be_redeemed end - end describe '.rescind_all_expired_invites_from' do @@ -507,7 +593,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!(updated_at: 2.days.ago) + expired_invite.update!(expires_at: 2.days.ago) Invite.rescind_all_expired_invites_from(user) invite_1.reload invite_2.reload diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb index 5e2bf32aa36..3ba6d09af93 100644 --- a/spec/requests/invites_controller_spec.rb +++ b/spec/requests/invites_controller_spec.rb @@ -3,6 +3,8 @@ require 'rails_helper' describe InvitesController do + fab!(:admin) { Fabricate(:admin) } + context 'show' do fab!(:invite) { Fabricate(:invite) } fab!(:user) { Fabricate(:coding_horror) } @@ -28,7 +30,7 @@ describe InvitesController do end it "returns error if invite has already been redeemed" do - invite.update!(redeemed_at: 1.day.ago) + Fabricate(:invited_user, invite: invite, user: Fabricate(:user)) get "/invites/#{invite.invite_key}" expect(response.status).to eq(200) @@ -51,23 +53,23 @@ describe InvitesController do let!(:invite) { Fabricate(:invite, invited_by: user) } fab!(:another_invite) { Fabricate(:invite, email: 'anotheremail@address.com') } - it 'raises an error when the email is missing' do + it 'raises an error when the id is missing' do delete "/invites.json" expect(response.status).to eq(400) end - it "raises an error when the email cannot be found" do - delete "/invites.json", params: { email: 'finn@adventuretime.ooo' } + it "raises an error when the id cannot be found" do + delete "/invites.json", params: { id: 848 } expect(response.status).to eq(400) end it 'raises an error when the invite is not yours' do - delete "/invites.json", params: { email: another_invite.email } + delete "/invites.json", params: { id: another_invite.id } expect(response.status).to eq(400) end it "destroys the invite" do - delete "/invites.json", params: { email: invite.email } + delete "/invites.json", params: { id: invite.id } invite.reload expect(invite.trashed?).to be_truthy end @@ -100,7 +102,7 @@ describe InvitesController do it "allows admins to invite to groups" do group = Fabricate(:group) - sign_in(Fabricate(:admin)) + sign_in(admin) post "/invites.json", params: { email: email, group_names: group.name } expect(response.status).to eq(200) expect(Invite.find_by(email: email).invited_groups.count).to eq(1) @@ -119,14 +121,14 @@ describe InvitesController do end it "allows admin to send multiple invites to same email" do - user = sign_in(Fabricate(:admin)) + user = sign_in(admin) invite = Invite.invite_by_email("invite@example.com", user) post "/invites.json", params: { email: invite.email } expect(response.status).to eq(200) end it "responds with error message in case of validation failure" do - sign_in(Fabricate(:admin)) + sign_in(admin) post "/invites.json", params: { email: "test@mailinator.com" } expect(response.status).to eq(422) json = response.parsed_body @@ -135,70 +137,120 @@ describe InvitesController do end end - context '#create_invite_link' do - it 'requires you to be logged in' do - post "/invites/link.json", params: { - email: 'jake@adventuretime.ooo' - } - expect(response.status).to eq(403) + describe "#create_invite_link" do + describe 'single use invite link' do + it 'requires you to be logged in' do + post "/invites/link.json", params: { + email: 'jake@adventuretime.ooo' + } + expect(response.status).to eq(403) + end + + context 'while logged in' do + let(:email) { 'jake@adventuretime.ooo' } + + it "fails if you can't invite to the forum" do + sign_in(Fabricate(:user)) + post "/invites/link.json", params: { email: email } + expect(response).to be_forbidden + end + + it "fails for normal user if invite email already exists" do + user = sign_in(Fabricate(:trust_level_4)) + invite = Invite.invite_by_email("invite@example.com", user) + + post "/invites/link.json", params: { + email: invite.email + } + + expect(response.status).to eq(422) + end + + it "verifies that inviter is authorized to invite new user to a group-private topic" do + group = Fabricate(:group) + private_category = Fabricate(:private_category, group: group) + group_private_topic = Fabricate(:topic, category: private_category) + sign_in(Fabricate(:trust_level_4)) + + post "/invites/link.json", params: { + email: email, topic_id: group_private_topic.id + } + + expect(response).to be_forbidden + end + + it "allows admins to invite to groups" do + group = Fabricate(:group) + sign_in(admin) + + post "/invites/link.json", params: { + email: email, group_names: group.name + } + + expect(response.status).to eq(200) + expect(Invite.find_by(email: email).invited_groups.count).to eq(1) + end + + it "allows multiple group invite" do + Fabricate(:group, name: "security") + Fabricate(:group, name: "support") + sign_in(admin) + + post "/invites/link.json", params: { + email: email, group_names: "security,support" + } + + expect(response.status).to eq(200) + expect(Invite.find_by(email: email).invited_groups.count).to eq(2) + end + end end - context 'while logged in' do - let(:email) { 'jake@adventuretime.ooo' } - - it "fails if you can't invite to the forum" do - sign_in(Fabricate(:user)) - post "/invites/link.json", params: { email: email } + describe 'multiple use invite link' do + it 'requires you to be logged in' do + post "/invites/link.json", params: { + max_redemptions_allowed: 5 + } expect(response).to be_forbidden end - it "fails for normal user if invite email already exists" do - user = sign_in(Fabricate(:trust_level_4)) - invite = Invite.invite_by_email("invite@example.com", user) + context 'while logged in' do + it "fails for non-staff users" do + sign_in(Fabricate(:trust_level_4)) + post "/invites/link.json", params: { + max_redemptions_allowed: 5 + } + expect(response).to be_forbidden + end - post "/invites/link.json", params: { - email: invite.email - } + it "allows staff to invite to groups" do + moderator = Fabricate(:moderator) + sign_in(moderator) + group = Fabricate(:group) + group.add_owner(moderator) - expect(response.status).to eq(422) - end + post "/invites/link.json", params: { + max_redemptions_allowed: 5, + group_names: group.name + } - it "verifies that inviter is authorized to invite new user to a group-private topic" do - group = Fabricate(:group) - private_category = Fabricate(:private_category, group: group) - group_private_topic = Fabricate(:topic, category: private_category) - sign_in(Fabricate(:trust_level_4)) + expect(response.status).to eq(200) + expect(Invite.multiple_use_invites.last.invited_groups.count).to eq(1) + end - post "/invites/link.json", params: { - email: email, topic_id: group_private_topic.id - } + it "allows multiple group invite" do + Fabricate(:group, name: "security") + Fabricate(:group, name: "support") + sign_in(admin) - expect(response).to be_forbidden - end + post "/invites/link.json", params: { + max_redemptions_allowed: 5, + group_names: "security,support" + } - it "allows admins to invite to groups" do - group = Fabricate(:group) - sign_in(Fabricate(:admin)) - - post "/invites/link.json", params: { - email: email, group_names: group.name - } - - expect(response.status).to eq(200) - expect(Invite.find_by(email: email).invited_groups.count).to eq(1) - end - - it "allows multiple group invite" do - Fabricate(:group, name: "security") - Fabricate(:group, name: "support") - sign_in(Fabricate(:admin)) - - post "/invites/link.json", params: { - email: email, group_names: "security,support" - } - - expect(response.status).to eq(200) - expect(Invite.find_by(email: email).invited_groups.count).to eq(2) + expect(response.status).to eq(200) + expect(Invite.multiple_use_invites.last.invited_groups.count).to eq(2) + end end end end @@ -250,6 +302,20 @@ describe InvitesController do end end + context 'with an expired invite' do + fab!(:invite_link) { Fabricate(:invite, email: nil, max_redemptions_allowed: 5, expires_at: 1.day.ago, emailed_status: Invite.emailed_status_types[:not_required]) } + + it "response is not successful" do + put "/invites/show/#{invite_link.invite_key}.json" + + expect(response.status).to eq(200) + json = response.parsed_body + expect(json["success"]).to eq(false) + expect(json["message"]).to eq(I18n.t('invite.not_found_json')) + expect(session[:current_user_id]).to be_blank + end + end + context 'with a valid invite id' do fab!(:topic) { Fabricate(:topic) } let(:invite) do @@ -276,9 +342,9 @@ describe InvitesController do ) invite.reload expect(response.status).to eq(200) - expect(session[:current_user_id]).to eq(invite.user_id) + expect(session[:current_user_id]).to eq(invite.invited_users.first.user_id) expect(invite.redeemed?).to be_truthy - user = User.find(invite.user_id) + user = User.find(invite.invited_users.first.user_id) expect(user.ip_address).to be_present expect(user.registration_ip_address).to be_present end @@ -296,7 +362,7 @@ describe InvitesController do put "/invites/show/#{invite.invite_key}.json", params: { timezone: "Australia/Melbourne" } expect(response.status).to eq(200) invite.reload - user = User.find(invite.user_id) + user = User.find(invite.invited_users.first.user_id) expect(user.user_option.timezone).to eq("Australia/Melbourne") end end @@ -329,7 +395,7 @@ describe InvitesController do put "/invites/show/#{invite.invite_key}.json" expect(response.status).to eq(200) - expect(invite.reload.user.groups.pluck(:name)).to contain_exactly("moderators", "staff") + expect(invite.invited_users.first.user.groups.pluck(:name)).to contain_exactly("moderators", "staff") end context "without password" do @@ -409,15 +475,44 @@ describe InvitesController do expect(job_args["user_id"]).to eq(invited_user.id) expect(job_args["email_token"]).to eq(tokens.first) end - end - end - end end end + context 'with multiple use invite link' do + fab!(:invite_link) { Fabricate(:invite, email: nil, max_redemptions_allowed: 5, expires_at: 1.month.from_now, emailed_status: Invite.emailed_status_types[:not_required]) } + + it "sends an activation email and doesn't activate the user" do + expect do + put "/invites/show/#{invite_link.invite_key}.json", params: { email: "foobar@example.com", password: "verystrongpassword" } + end.not_to change { UserAuthToken.count } + + expect(response.status).to eq(200) + expect(response.parsed_body["success"]).to eq(true) + expect(response.parsed_body["message"]).to eq(I18n.t("invite.confirm_email")) + + invite_link.reload + expect(invite_link.redemption_count).to eq(1) + + invited_user = User.find_by_email("foobar@example.com") + expect(invited_user.active).to eq(false) + expect(invited_user.email_confirmed?).to eq(false) + + expect(Jobs::InvitePasswordInstructionsEmail.jobs.size).to eq(0) + expect(Jobs::CriticalUserEmail.jobs.size).to eq(1) + + tokens = EmailToken.where(user_id: invited_user.id, confirmed: false, expired: false).pluck(:token) + expect(tokens.size).to eq(1) + + job_args = Jobs::CriticalUserEmail.jobs.first["args"].first + expect(job_args["type"]).to eq("signup") + expect(job_args["user_id"]).to eq(invited_user.id) + expect(job_args["email_token"]).to eq(tokens.first) + end + end + context 'new registrations are disabled' do fab!(:topic) { Fabricate(:topic) } @@ -431,7 +526,7 @@ describe InvitesController do put "/invites/show/#{invite.invite_key}.json" expect(response.status).to eq(200) invite.reload - expect(invite.user_id).to be_blank + expect(invite.invited_users).to be_blank expect(invite.redeemed?).to be_falsey expect(response.body).to include(I18n.t("login.new_registrations_disabled")) end @@ -450,7 +545,7 @@ describe InvitesController do put "/invites/show/#{invite.invite_key}.json", params: { id: invite.invite_key } expect(response.status).to eq(200) invite.reload - expect(invite.user_id).to be_blank + expect(invite.invited_users).to be_blank expect(invite.redeemed?).to be_falsey expect(response.body).to include(I18n.t("login.already_logged_in", current_user: user.username)) end @@ -513,7 +608,7 @@ describe InvitesController do end it "allows admin to bulk invite" do - sign_in(Fabricate(:admin)) + sign_in(admin) post "/invites/upload_csv.json", params: { file: file, name: filename } expect(response.status).to eq(200) expect(Jobs::BulkInvite.jobs.size).to eq(1) @@ -521,7 +616,7 @@ describe InvitesController do it "sends limited invites at a time" do SiteSetting.max_bulk_invites = 3 - sign_in(Fabricate(:admin)) + sign_in(admin) post "/invites/upload_csv.json", params: { file: file, name: filename } expect(response.status).to eq(422) diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index d6edb84c063..fed5b3c726f 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -1505,7 +1505,8 @@ describe UsersController do inviter = Fabricate(:user, trust_level: 2) sign_in(inviter) invitee = Fabricate(:user) - _invite = Fabricate(:invite, invited_by: inviter, user: invitee) + _invite = Fabricate(:invite, invited_by: inviter) + Fabricate(:invited_user, invite: _invite, user: invitee) get "/u/#{user.username}/invited_count.json" expect(response.status).to eq(200) @@ -1535,61 +1536,66 @@ describe UsersController do inviter = Fabricate(:user, trust_level: 2) sign_in(inviter) + Fabricate(:invite, email: 'billybob@example.com', invited_by: inviter) + redeemed_invite = Fabricate(:invite, email: 'jimtom@example.com', invited_by: inviter) invitee = Fabricate(:user) - Fabricate(:invite, email: 'billybob@example.com', invited_by: inviter, user: invitee) - Fabricate(:invite, email: 'jimtom@example.com', invited_by: inviter, user: invitee) + Fabricate(:invited_user, invite: redeemed_invite, user: invitee) - get "/u/#{inviter.username}/invited.json", params: { search: 'billybob' } + get "/u/#{inviter.username}/invited.json", params: { filter: 'pending', search: 'billybob' } expect(response.status).to eq(200) invites = response.parsed_body['invites'] expect(invites.size).to eq(1) expect(invites.first).to include('email' => 'billybob@example.com') - get "/u/#{inviter.username}/invited.json", params: { search: invitee.username } + get "/u/#{inviter.username}/invited.json", params: { filter: 'redeemed', search: invitee.username } expect(response.status).to eq(200) invites = response.parsed_body['invites'] - expect(invites.size).to eq(2) - expect(invites[0]['email']).to be_present + expect(invites.size).to eq(1) + expect(invites[0]['user']).to be_present end it "doesn't filter by email if another regular user" do inviter = Fabricate(:user, trust_level: 2) sign_in(Fabricate(:user, trust_level: 2)) + Fabricate(:invite, email: 'billybob@example.com', invited_by: inviter) + redeemed_invite = Fabricate(:invite, email: 'jimtom@example.com', invited_by: inviter) invitee = Fabricate(:user) - Fabricate(:invite, email: 'billybob@example.com', invited_by: inviter, user: invitee) - Fabricate(:invite, email: 'jimtom@example.com', invited_by: inviter, user: invitee) + Fabricate(:invited_user, invite: redeemed_invite, user: invitee) - get "/u/#{inviter.username}/invited.json", params: { search: 'billybob' } + get "/u/#{inviter.username}/invited.json", params: { filter: 'pending', search: 'billybob' } expect(response.status).to eq(200) invites = response.parsed_body['invites'] expect(invites.size).to eq(0) - get "/u/#{inviter.username}/invited.json", params: { search: invitee.username } + get "/u/#{inviter.username}/invited.json", params: { filter: 'redeemed', search: invitee.username } expect(response.status).to eq(200) invites = response.parsed_body['invites'] - expect(invites.size).to eq(2) - expect(invites[0]['email']).to be_blank + expect(invites.size).to eq(1) + expect(invites[0]['user']).to be_present end it "filters by email if staff" do inviter = Fabricate(:user, trust_level: 2) sign_in(Fabricate(:moderator)) - invitee = Fabricate(:user) - Fabricate(:invite, email: 'billybob@example.com', invited_by: inviter, user: invitee) - Fabricate(:invite, email: 'jimtom@example.com', invited_by: inviter, user: invitee) + invite_1 = Fabricate(:invite, email: 'billybob@example.com', invited_by: inviter) + invitee_1 = Fabricate(:user) + Fabricate(:invited_user, invite: invite_1, user: invitee_1) + invite_2 = Fabricate(:invite, email: 'jimtom@example.com', invited_by: inviter) + invitee_2 = Fabricate(:user) + Fabricate(:invited_user, invite: invite_2, user: invitee_2) get "/u/#{inviter.username}/invited.json", params: { search: 'billybob' } expect(response.status).to eq(200) invites = response.parsed_body['invites'] expect(invites.size).to eq(1) - expect(invites[0]['email']).to be_present + expect(invites[0]['user']).to include('id' => invitee_1.id) end context 'with guest' do @@ -1604,18 +1610,19 @@ describe UsersController do end context 'with redeemed invites' do - it 'returns invites' do + it 'returns invited_users' do inviter = Fabricate(:user, trust_level: 2) sign_in(inviter) invitee = Fabricate(:user) - invite = Fabricate(:invite, invited_by: inviter, user: invitee) + invite = Fabricate(:invite, invited_by: inviter) + invited_user = Fabricate(:invited_user, invite: invite, user: invitee) get "/u/#{inviter.username}/invited.json" expect(response.status).to eq(200) invites = response.parsed_body['invites'] expect(invites.size).to eq(1) - expect(invites.first).to include('email' => invite.email) + expect(invites[0]).to include('id' => invite.id) end end end @@ -1641,7 +1648,6 @@ describe UsersController do it 'does not return invites' do user = sign_in(Fabricate(:user)) inviter = Fabricate(:user) - _invitee = Fabricate(:user) Fabricate(:invite, invited_by: inviter) stub_guardian(user) do |guardian| guardian.stubs(:can_see_invite_details?). @@ -1659,14 +1665,42 @@ describe UsersController do sign_in(Fabricate(:moderator)) inviter = Fabricate(:user) invitee = Fabricate(:user) - invite = Fabricate(:invite, invited_by: inviter, user: invitee) + invite = Fabricate(:invite, invited_by: inviter) + Fabricate(:invited_user, invite: invite, user: invitee) get "/u/#{inviter.username}/invited.json" expect(response.status).to eq(200) invites = response.parsed_body['invites'] expect(invites.size).to eq(1) - expect(invites.first).to include('email' => invite.email) + expect(invites[0]).to include('id' => invite.id) + end + end + + context 'with invite links' do + context 'with permission to see invite links' do + it 'returns invites' do + inviter = sign_in(Fabricate(:admin)) + invite = Fabricate(:invite, invited_by: inviter, email: nil, max_redemptions_allowed: 5, expires_at: 1.month.from_now, emailed_status: Invite.emailed_status_types[:not_required]) + + get "/u/#{inviter.username}/invite_links.json" + expect(response.status).to eq(200) + + invites = response.parsed_body['invites'] + expect(invites.size).to eq(1) + expect(invites.first).to include("id" => invite.id) + end + end + + context 'without permission to see invite links' do + it 'does not return invites' do + user = Fabricate(:user, trust_level: 2) + inviter = Fabricate(:admin) + Fabricate(:invite, invited_by: inviter, email: nil, max_redemptions_allowed: 5, expires_at: 1.month.from_now, emailed_status: Invite.emailed_status_types[:not_required]) + + get "/u/#{inviter.username}/invite_links.json" + expect(response.status).to eq(403) + end end end end diff --git a/spec/services/user_anonymizer_spec.rb b/spec/services/user_anonymizer_spec.rb index 5e9a64b7fca..da61af7b222 100644 --- a/spec/services/user_anonymizer_spec.rb +++ b/spec/services/user_anonymizer_spec.rb @@ -224,11 +224,11 @@ describe UserAnonymizer do end it "removes invites" do - Fabricate(:invite, user: user) - Fabricate(:invite, user: another_user) + Fabricate(:invited_user, invite: Fabricate(:invite), user: user) + Fabricate(:invited_user, invite: Fabricate(:invite), user: another_user) - expect { make_anonymous }.to change { Invite.count }.by(-1) - expect(Invite.where(user_id: user.id).count).to eq(0) + expect { make_anonymous }.to change { InvitedUser.count }.by(-1) + expect(InvitedUser.where(user_id: user.id).count).to eq(0) end it "removes email tokens" do diff --git a/spec/services/user_merger_spec.rb b/spec/services/user_merger_spec.rb index a239d2c2b62..939afd2ae9c 100644 --- a/spec/services/user_merger_spec.rb +++ b/spec/services/user_merger_spec.rb @@ -251,7 +251,8 @@ describe UserMerger do end it "updates invites" do - invite1 = Fabricate(:invite, invited_by: walter, user: source_user) + invite1 = Fabricate(:invite, invited_by: walter) + Fabricate(:invited_user, invite: invite1, user: source_user) invite2 = Fabricate(:invite, invited_by: source_user) invite3 = Fabricate(:invite, invited_by: source_user) invite3.trash!(source_user) @@ -260,7 +261,7 @@ describe UserMerger do [invite1, invite2, invite3].each { |x| x.reload } - expect(invite1.user).to eq(target_user) + expect(invite1.invited_users.first.user).to eq(target_user) expect(invite2.invited_by).to eq(target_user) expect(invite3.invited_by).to eq(target_user) expect(invite3.deleted_by).to eq(target_user) diff --git a/spec/services/username_changer_spec.rb b/spec/services/username_changer_spec.rb index 416c986ca9e..9b6a1a1873b 100644 --- a/spec/services/username_changer_spec.rb +++ b/spec/services/username_changer_spec.rb @@ -559,8 +559,8 @@ describe UsernameChanger do notified_user = Fabricate(:user) p1 = Fabricate(:post, post_number: 1, user: renamed_user) p2 = Fabricate(:post, post_number: 1, user: another_user) - Fabricate(:invite, invited_by: notified_user, user: renamed_user) - Fabricate(:invite, invited_by: notified_user, user: another_user) + Fabricate(:invited_user, invite: Fabricate(:invite, invited_by: notified_user), user: renamed_user) + Fabricate(:invited_user, invite: Fabricate(:invite, invited_by: notified_user), user: another_user) n01 = create_notification(:mentioned, notified_user, p1, original_and_display_username("alice")) n02 = create_notification(:mentioned, notified_user, p2, original_and_display_username("another_user")) diff --git a/test/javascripts/acceptance/invite-accept-test.js b/test/javascripts/acceptance/invite-accept-test.js index 27fd0233949..ee1dcf903fe 100644 --- a/test/javascripts/acceptance/invite-accept-test.js +++ b/test/javascripts/acceptance/invite-accept-test.js @@ -16,11 +16,13 @@ QUnit.test("Invite Acceptance Page", async assert => { name: "Neil Lalonde", title: "team" }, - email: "invited@asdf.com", - username: "invited" + email: null, + username: "invited", + is_invite_link: true }); await visit("/invites/myvalidinvitetoken"); + assert.ok(exists("#new-account-email"), "shows the email input"); assert.ok(exists("#new-account-username"), "shows the username input"); assert.equal( find("#new-account-username").val(), @@ -31,10 +33,16 @@ QUnit.test("Invite Acceptance Page", async assert => { assert.ok(exists("#new-account-password"), "shows the password input"); assert.ok( exists(".invites-show .btn-primary:disabled"), - "submit is disabled because name is not filled" + "submit is disabled because name and email is not filled" ); await fillIn("#new-account-name", "John Doe"); + assert.ok( + exists(".invites-show .btn-primary:disabled"), + "submit is disabled because email is not filled" + ); + + await fillIn("#new-account-email", "john.doe@example.com"); assert.not( exists(".invites-show .btn-primary:disabled"), "submit is enabled" @@ -54,10 +62,19 @@ QUnit.test("Invite Acceptance Page", async assert => { "submit is disabled" ); + await fillIn("#new-account-email", "john.doe@example"); + assert.ok(exists(".email-input .bad"), "email is not valid"); + assert.ok( + exists(".invites-show .btn-primary:disabled"), + "submit is disabled" + ); + await fillIn("#new-account-username", "validname"); await fillIn("#new-account-password", "secur3ty4Y0uAndMe"); + await fillIn("#new-account-email", "john.doe@example.com"); assert.ok(exists(".username-input .good"), "username is valid"); assert.ok(exists(".password-input .good"), "password is valid"); + assert.ok(exists(".email-input .good"), "email is valid"); assert.not( exists(".invites-show .btn-primary:disabled"), "submit is enabled" diff --git a/test/javascripts/acceptance/invite-show-user-fields-test.js b/test/javascripts/acceptance/invite-show-user-fields-test.js index e8ff05d8e03..5cfcb9606a9 100644 --- a/test/javascripts/acceptance/invite-show-user-fields-test.js +++ b/test/javascripts/acceptance/invite-show-user-fields-test.js @@ -1,4 +1,5 @@ import { acceptance } from "helpers/qunit-helpers"; +import PreloadStore from "discourse/lib/preload-store"; acceptance("Accept Invite - User Fields", { site: { @@ -26,6 +27,19 @@ acceptance("Accept Invite - User Fields", { }); QUnit.test("accept invite with user fields", async assert => { + PreloadStore.store("invite_info", { + invited_by: { + id: 123, + username: "neil", + avatar_template: "/user_avatar/localhost/neil/{size}/25_1.png", + name: "Neil Lalonde", + title: "team" + }, + email: "invited@asdf.com", + username: "invited", + is_invite_link: false + }); + await visit("/invites/myvalidinvitetoken"); assert.ok(exists(".invites-show"), "shows the accept invite page"); assert.ok(exists(".user-field"), "it has at least one user field");