From fecf3e20d9c91e7e357e77a92243a5378c805cc6 Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Tue, 9 Mar 2021 00:15:14 +0200 Subject: [PATCH] FEATURE: Various improvements to invite system (#12314) * FEATURE: Do not delete invite if link was copied * FIX: Show error to user if invite redeeming fails The error was only displayed to console. * UX: Better placement of bulk buttons Destroy all expired invites should be on the expired tab, not pending. * FIX: Ensure invited_groups is unique per invite and group * FIX: Do not refresh topic list if title unchanged * FIX: Do not close modal on enter This intereferes with the group and topic chooser. Wrapping everything in a form disables this behavior. * FIX: Move link and email options outside advanced section * FIX: Do not close modal if saving a link invite User may still want to copy the link. --- .../discourse/app/components/choose-topic.js | 5 + .../discourse/app/components/copy-button.js | 3 + .../app/controllers/create-invite.js | 34 ++++-- .../discourse/app/controllers/invites-show.js | 3 +- .../app/controllers/user-invited-show.js | 2 +- .../app/templates/modal/create-invite.hbs | 102 +++++++++--------- .../app/templates/user-invited-show.hbs | 21 ++-- app/models/invited_group.rb | 4 + ...5916_add_unique_index_to_invited_groups.rb | 15 +++ 9 files changed, 118 insertions(+), 71 deletions(-) create mode 100644 db/migrate/20210308195916_add_unique_index_to_invited_groups.rb diff --git a/app/assets/javascripts/discourse/app/components/choose-topic.js b/app/assets/javascripts/discourse/app/components/choose-topic.js index 4ff2faa283f..6786c941066 100644 --- a/app/assets/javascripts/discourse/app/components/choose-topic.js +++ b/app/assets/javascripts/discourse/app/components/choose-topic.js @@ -57,10 +57,15 @@ export default Component.extend({ @observes("topicTitle") topicTitleChanged() { + if (this.oldTopicTitle === this.topicTitle) { + return; + } + this.setProperties({ loading: true, noResults: true, selectedTopicId: null, + oldTopicTitle: this.topicTitle, }); this.search(this.topicTitle); diff --git a/app/assets/javascripts/discourse/app/components/copy-button.js b/app/assets/javascripts/discourse/app/components/copy-button.js index ca1409b1b8f..8eae4c3cdd6 100644 --- a/app/assets/javascripts/discourse/app/components/copy-button.js +++ b/app/assets/javascripts/discourse/app/components/copy-button.js @@ -11,6 +11,9 @@ export default Component.extend({ target.setSelectionRange(0, target.value.length); try { document.execCommand("copy"); + if (this.copied) { + this.copied(); + } } catch (err) {} }, }); diff --git a/app/assets/javascripts/discourse/app/controllers/create-invite.js b/app/assets/javascripts/discourse/app/controllers/create-invite.js index d01ed48acb6..1a5c137deb0 100644 --- a/app/assets/javascripts/discourse/app/controllers/create-invite.js +++ b/app/assets/javascripts/discourse/app/controllers/create-invite.js @@ -7,6 +7,7 @@ import { bufferedProperty } from "discourse/mixins/buffered-content"; import ModalFunctionality from "discourse/mixins/modal-functionality"; import Group from "discourse/models/group"; import Invite from "discourse/models/invite"; +import I18n from "I18n"; export default Controller.extend( ModalFunctionality, @@ -50,12 +51,15 @@ export default Controller.extend( }); }, + setAutogenerated(value) { + if ((this.autogenerated || !this.invite.id) && !value) { + this.invites.unshiftObject(this.invite); + } + + this.set("autogenerated", value); + }, + save(opts) { - const newRecord = - (this.autogenerated || !this.invite.id) && !opts.autogenerated; - - this.set("autogenerated", opts.autogenerated); - const data = { ...this.buffered.buffer }; if (data.groupIds !== undefined) { @@ -90,13 +94,16 @@ export default Controller.extend( .save(data) .then(() => { this.rollbackBuffer(); - - if (newRecord) { - this.invites.unshiftObject(this.invite); - } - + this.setAutogenerated(opts.autogenerated); if (!this.autogenerated) { - this.send("closeModal"); + if (this.type === "email" && opts.sendEmail) { + this.send("closeModal"); + } else { + this.appEvents.trigger("modal-body:flash", { + text: I18n.t("user.invited.invite.invite_saved"), + messageClass: "success", + }); + } } }) .catch((e) => @@ -138,6 +145,11 @@ export default Controller.extend( return hasBufferedChanges || (inviteEmail ? "email" : "link") !== type; }, + @action + mutAutogenerated(value) { + this.setAutogenerated(value); + }, + @action saveInvite(sendEmail) { this.appEvents.trigger("modal-body:clearFlash"); diff --git a/app/assets/javascripts/discourse/app/controllers/invites-show.js b/app/assets/javascripts/discourse/app/controllers/invites-show.js index 811c74f960f..1b5eb23e0b4 100644 --- a/app/assets/javascripts/discourse/app/controllers/invites-show.js +++ b/app/assets/javascripts/discourse/app/controllers/invites-show.js @@ -8,6 +8,7 @@ import PasswordValidation from "discourse/mixins/password-validation"; import UserFieldsValidation from "discourse/mixins/user-fields-validation"; import UsernameValidation from "discourse/mixins/username-validation"; import { ajax } from "discourse/lib/ajax"; +import { extractError } from "discourse/lib/ajax-error"; import discourseComputed from "discourse-common/utils/decorators"; import { emailValid } from "discourse/lib/utilities"; import { findAll as findLoginMethods } from "discourse/models/login-method"; @@ -154,7 +155,7 @@ export default Controller.extend( } }) .catch((error) => { - throw new Error(error); + this.set("errorMessage", extractError(error)); }); }, }, 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 3cffa2cbacd..3028bd2ce27 100644 --- a/app/assets/javascripts/discourse/app/controllers/user-invited-show.js +++ b/app/assets/javascripts/discourse/app/controllers/user-invited-show.js @@ -49,7 +49,7 @@ export default Controller.extend({ showBulkActionButtons(filter) { return ( filter === "pending" && - this.model.invites.length > 1 && + this.model.invites.length > 0 && this.currentUser.staff ); }, diff --git a/app/assets/javascripts/discourse/app/templates/modal/create-invite.hbs b/app/assets/javascripts/discourse/app/templates/modal/create-invite.hbs index abb68054348..0c708b83887 100644 --- a/app/assets/javascripts/discourse/app/templates/modal/create-invite.hbs +++ b/app/assets/javascripts/discourse/app/templates/modal/create-invite.hbs @@ -1,24 +1,16 @@ {{#d-modal-body title=(if invite.id "user.invited.invite.edit_title" "user.invited.invite.new_title")}} -
- - {{input name="invite_link" - class="invite-link" - value=invite.link - readonly=true}} - {{copy-button selector="input.invite-link"}} -
+
+
+ + {{input name="invite_link" + class="invite-link" + value=invite.link + readonly=true}} + {{copy-button selector="input.invite-link" copied=(action "mutAutogenerated" false)}} +
-

{{i18n "user.invited.invite.expires_at_time" time=expiresAtRelative}}

+

{{i18n "user.invited.invite.expires_at_time" time=expiresAtRelative}}

-

- {{#if showAdvanced}} - {{d-icon "caret-down"}} {{i18n "user.invited.invite.hide_advanced"}} - {{else}} - {{d-icon "caret-right"}} {{i18n "user.invited.invite.show_advanced"}} - {{/if}} -

- - {{#if showAdvanced}}
{{radio-button id="invite-type-link" name="invite-type" value="link" selection=type}} @@ -56,43 +48,53 @@ {{/if}} {{#if currentUser.staff}} -
- - {{group-chooser - content=allGroups - value=buffered.groupIds - labelProperty="name" - onChange=(action (mut buffered.groupIds)) - }} -
+

+ {{#if showAdvanced}} + {{d-icon "caret-down"}} {{i18n "user.invited.invite.hide_advanced"}} + {{else}} + {{d-icon "caret-right"}} {{i18n "user.invited.invite.show_advanced"}} + {{/if}} +

-
- {{choose-topic - selectedTopicId=buffered.topicId - topicTitle=buffered.topicTitle - additionalFilters="status:public" - label="user.invited.invite.invite_to_topic" - }} -
- -
- {{future-date-input - displayLabel=(i18n "user.invited.invite.expires_at") - includeDateTime=true - includeMidFuture=true - clearable=true - onChangeInput=(action (mut buffered.expires_at)) - }} -
- - {{#if isEmail}} + {{#if showAdvanced}}
- - {{textarea id="invite-message" value=buffered.custom_message}} + + {{group-chooser + content=allGroups + value=buffered.groupIds + labelProperty="name" + onChange=(action (mut buffered.groupIds)) + }}
+ +
+ {{choose-topic + selectedTopicId=buffered.topicId + topicTitle=buffered.topicTitle + additionalFilters="status:public" + label="user.invited.invite.invite_to_topic" + }} +
+ +
+ {{future-date-input + displayLabel=(i18n "user.invited.invite.expires_at") + includeDateTime=true + includeMidFuture=true + clearable=true + onChangeInput=(action (mut buffered.expires_at)) + }} +
+ + {{#if isEmail}} +
+ + {{textarea id="invite-message" value=buffered.custom_message}} +
+ {{/if}} {{/if}} {{/if}} - {{/if}} + {{/d-modal-body}}
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 8dbbbe4899d..4d4faf57d97 100644 --- a/app/assets/javascripts/discourse/app/templates/user-invited-show.hbs +++ b/app/assets/javascripts/discourse/app/templates/user-invited-show.hbs @@ -18,15 +18,20 @@ {{d-button icon="upload" action=(action "createInviteCsv") label="user.invited.bulk_invite.text"}} {{/if}} {{#if showBulkActionButtons}} - {{#if removedAll}} - {{i18n "user.invited.removed_all"}} - {{else}} - {{d-button icon="times" action=(action "destroyAllExpired") label="user.invited.remove_all"}} + {{#if inviteExpired}} + {{#if removedAll}} + {{i18n "user.invited.removed_all"}} + {{else}} + {{d-button icon="times" action=(action "destroyAllExpired") label="user.invited.remove_all"}} + {{/if}} {{/if}} - {{#if reinvitedAll}} - {{i18n "user.invited.reinvited_all"}} - {{else}} - {{d-button icon="sync" action=(action "reinviteAll") label="user.invited.reinvite_all"}} + + {{#if invitePending}} + {{#if reinvitedAll}} + {{i18n "user.invited.reinvited_all"}} + {{else}} + {{d-button icon="sync" action=(action "reinviteAll") label="user.invited.reinvite_all"}} + {{/if}} {{/if}} {{/if}}
diff --git a/app/models/invited_group.rb b/app/models/invited_group.rb index 063dbf4e209..d00595786de 100644 --- a/app/models/invited_group.rb +++ b/app/models/invited_group.rb @@ -15,3 +15,7 @@ end # created_at :datetime not null # updated_at :datetime not null # +# Indexes +# +# index_invited_groups_on_group_id_and_invite_id (group_id,invite_id) UNIQUE +# diff --git a/db/migrate/20210308195916_add_unique_index_to_invited_groups.rb b/db/migrate/20210308195916_add_unique_index_to_invited_groups.rb new file mode 100644 index 00000000000..c3f8b607694 --- /dev/null +++ b/db/migrate/20210308195916_add_unique_index_to_invited_groups.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddUniqueIndexToInvitedGroups < ActiveRecord::Migration[6.0] + def change + execute <<~SQL + DELETE FROM invited_groups a + USING invited_groups b + WHERE a.id < b.id + AND a.invite_id = b.invite_id + AND a.group_id = b.group_id + SQL + + add_index :invited_groups, [:group_id, :invite_id], unique: true + end +end