From cfee2728ce4b8f6de95f48d61008e436fe8db17d Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Fri, 23 Apr 2021 19:18:23 +0300 Subject: [PATCH] FEATURE: New share topic modal (#12804) The old share modal used to host both share and invite functionality, under two tabs. The new "Share Topic" modal can be used only for sharing, but has a link to the invite modal. Among the sharing methods, there is also "Notify" which points out that existing users will simply be notified (this was not clear before). Staff members can notify as many users as they want, but regular users are restricted to one at a time, no more than max_topic_invitations_per_day. The user will not receive another notification if they have been notified of the same topic in past hour. The "Create Invite" modal also suffered some changes: the two radio boxes for selecting the type (invite or email) have been replaced by a single checkbox (is email?) and then the two labels about emails have been replaced by a single one, some fields were reordered and the advanced options toggle was moved to the bottom right of the modal. --- .../app/controllers/create-invite.js | 20 ++- .../discourse/app/controllers/share-topic.js | 116 ++++++++++++++++++ .../app/initializers/topic-footer-buttons.js | 37 +----- .../app/templates/modal/create-invite.hbs | 93 +++++++------- .../app/templates/modal/share-topic.hbs | 61 +++++++++ .../acceptance/create-invite-modal-test.js | 13 +- .../share-and-invite-desktop-test.js | 96 --------------- ...ite-mobile-test.js => share-topic-test.js} | 105 ++++++++-------- app/assets/stylesheets/common/base/modal.scss | 42 ++++++- app/assets/stylesheets/desktop/modal.scss | 3 +- app/controllers/topics_controller.rb | 33 +++++ app/models/topic.rb | 46 ++++--- config/locales/client.en.yml | 13 +- config/locales/server.en.yml | 1 + config/routes.rb | 1 + spec/requests/topics_controller_spec.rb | 38 ++++++ 16 files changed, 444 insertions(+), 274 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/controllers/share-topic.js create mode 100644 app/assets/javascripts/discourse/app/templates/modal/share-topic.hbs delete mode 100644 app/assets/javascripts/discourse/tests/acceptance/share-and-invite-desktop-test.js rename app/assets/javascripts/discourse/tests/acceptance/{share-and-invite-mobile-test.js => share-topic-test.js} (52%) diff --git a/app/assets/javascripts/discourse/app/controllers/create-invite.js b/app/assets/javascripts/discourse/app/controllers/create-invite.js index 04310faeaa4..d0c5b2b27a3 100644 --- a/app/assets/javascripts/discourse/app/controllers/create-invite.js +++ b/app/assets/javascripts/discourse/app/controllers/create-invite.js @@ -30,6 +30,8 @@ export default Controller.extend( }); this.setProperties({ + invite: null, + invites: null, autogenerated: false, showAdvanced: false, }); @@ -41,7 +43,7 @@ export default Controller.extend( if (this.autogenerated) { this.invite .destroy() - .then(() => this.invites.removeObject(this.invite)); + .then(() => this.invites && this.invites.removeObject(this.invite)); } }, @@ -53,7 +55,7 @@ export default Controller.extend( }, setAutogenerated(value) { - if ((this.autogenerated || !this.invite.id) && !value) { + if (this.invites && (this.autogenerated || !this.invite.id) && !value) { this.invites.unshiftObject(this.invite); } @@ -168,6 +170,15 @@ export default Controller.extend( this.save({ sendEmail: false, copy: true }); }, + @action + toggleLimitToEmail() { + const limitToEmail = !this.limitToEmail; + this.setProperties({ + limitToEmail, + type: limitToEmail ? "email" : "link", + }); + }, + @action saveInvite(sendEmail) { this.appEvents.trigger("modal-body:clearFlash"); @@ -181,5 +192,10 @@ export default Controller.extend( this.set("buffered.email", result[0].email[0]); }); }, + + @action + toggleAdvanced() { + this.toggleProperty("showAdvanced"); + }, } ); diff --git a/app/assets/javascripts/discourse/app/controllers/share-topic.js b/app/assets/javascripts/discourse/app/controllers/share-topic.js new file mode 100644 index 00000000000..82f3045c876 --- /dev/null +++ b/app/assets/javascripts/discourse/app/controllers/share-topic.js @@ -0,0 +1,116 @@ +import Controller from "@ember/controller"; +import { action } from "@ember/object"; +import { getAbsoluteURL } from "discourse-common/lib/get-url"; +import discourseComputed from "discourse-common/utils/decorators"; +import { ajax } from "discourse/lib/ajax"; +import { extractError } from "discourse/lib/ajax-error"; +import Sharing from "discourse/lib/sharing"; +import showModal from "discourse/lib/show-modal"; +import { bufferedProperty } from "discourse/mixins/buffered-content"; +import ModalFunctionality from "discourse/mixins/modal-functionality"; +import I18n from "I18n"; + +export default Controller.extend( + ModalFunctionality, + bufferedProperty("invite"), + { + onShow() { + this.set("showNotifyUsers", false); + }, + + @discourseComputed("topic.shareUrl") + topicUrl(url) { + return url ? getAbsoluteURL(url) : null; + }, + + @discourseComputed( + "topic.{isPrivateMessage,invisible,category.read_restricted}" + ) + sources(topic) { + const privateContext = + this.siteSettings.login_required || + (topic && topic.isPrivateMessage) || + (topic && topic.invisible) || + topic.category.read_restricted; + + return Sharing.activeSources( + this.siteSettings.share_links, + privateContext + ); + }, + + @action + copied() { + return this.appEvents.trigger("modal-body:flash", { + text: I18n.t("topic.share.copied"), + messageClass: "success", + }); + }, + + @action + onChangeUsers(usernames) { + this.set("users", usernames.uniq()); + }, + + @action + share(source) { + this.set("showNotifyUsers", false); + Sharing.shareSource(source, { + title: this.topic.title, + url: this.topicUrl, + }); + }, + + @action + toggleNotifyUsers() { + if (this.showNotifyUsers) { + this.set("showNotifyUsers", false); + } else { + this.setProperties({ + showNotifyUsers: true, + users: [], + }); + } + }, + + @action + notifyUsers() { + if (this.users.length === 0) { + return; + } + + ajax(`/t/${this.topic.id}/invite-notify`, { + type: "POST", + data: { usernames: this.users }, + }) + .then(() => { + this.setProperties({ showNotifyUsers: false }); + this.appEvents.trigger("modal-body:flash", { + text: I18n.t("topic.share.notify_users.success", { + count: this.users.length, + username: this.users[0], + }), + messageClass: "success", + }); + }) + .catch((error) => { + this.appEvents.trigger("modal-body:flash", { + text: extractError(error), + messageClass: "error", + }); + }); + }, + + @action + inviteUsers() { + this.set("showNotifyUsers", false); + const controller = showModal("create-invite"); + controller.set("showAdvanced", true); + controller.buffered.setProperties({ + topicId: this.topic.id, + topicTitle: this.topic.title, + }); + controller.save({ autogenerated: true }); + }, + } +); diff --git a/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js b/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js index 812e1b83124..6cbc34c5462 100644 --- a/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js +++ b/app/assets/javascripts/discourse/app/initializers/topic-footer-buttons.js @@ -25,39 +25,10 @@ export default { }, title: "topic.share.help", action() { - const panels = [ - { - id: "share", - title: "topic.share.extended_title", - model: { - topic: this.topic, - }, - }, - ]; - - if (this.canInviteTo && !this.inviteDisabled) { - let invitePanelTitle; - - if (this.isPM) { - invitePanelTitle = "topic.invite_private.title"; - } else if (this.invitingToTopic) { - invitePanelTitle = "topic.invite_reply.title"; - } else { - invitePanelTitle = "user.invited.create"; - } - - panels.push({ - id: "invite", - title: invitePanelTitle, - model: { - inviteModel: this.topic, - }, - }); - } - - showModal("share-and-invite", { - modalClass: "share-and-invite", - panels, + const controller = showModal("share-topic"); + controller.setProperties({ + allowInvites: this.canInviteTo && !this.inviteDisabled, + topic: this.topic, }); }, dropdown() { 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 0991bc70acc..1bb823f2537 100644 --- a/app/assets/javascripts/discourse/app/templates/modal/create-invite.hbs +++ b/app/assets/javascripts/discourse/app/templates/modal/create-invite.hbs @@ -16,15 +16,25 @@

{{expiresAtLabel}}

-
- {{radio-button id="invite-type-link" name="invite-type" value="link" selection=type}} - -
+ {{input type="checkbox" id="invite-type" checked=limitToEmail click=(action "toggleLimitToEmail")}} + -
- {{radio-button id="invite-type-email" name="invite-type" value="email" selection=type}} - -
+ {{#if isEmail}} +
+ {{input + id="invite-email" + value=buffered.email + placeholderKey="topic.invite_reply.email_placeholder" + }} + {{#if capabilities.hasContactPicker}} + {{d-button + icon="address-book" + action=(action "searchContact") + class="btn-primary open-contact-picker" + }} + {{/if}} +
+ {{/if}}
{{#if isLink}} @@ -41,34 +51,28 @@ {{/if}} {{#if isEmail}} -
- -
- {{input - id="invite-email" - value=buffered.email - placeholderKey="topic.invite_reply.email_placeholder" - }} - {{#if capabilities.hasContactPicker}} - {{d-button - icon="address-book" - action=(action "searchContact") - class="btn-primary open-contact-picker" - }} - {{/if}} + {{#if showAdvanced}} +
+ + {{textarea id="invite-message" value=buffered.custom_message}}
-
+ {{/if}} {{/if}} {{#if currentUser.staff}} -

- {{#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}} +
+ {{choose-topic + selectedTopicId=buffered.topicId + topicTitle=buffered.topicTitle + additionalFilters="status:public" + label="user.invited.invite.invite_to_topic" + }} +
+ {{/if}} + {{/if}} + {{#if currentUser.staff}} {{#if showAdvanced}}
@@ -79,16 +83,11 @@ onChange=(action (mut buffered.groupIds)) }}
+ {{/if}} + {{/if}} -
- {{choose-topic - selectedTopicId=buffered.topicId - topicTitle=buffered.topicTitle - additionalFilters="status:public" - label="user.invited.invite.invite_to_topic" - }} -
- + {{#if currentUser.staff}} + {{#if showAdvanced}}
{{future-date-input displayLabel=(i18n "user.invited.invite.expires_at") @@ -98,13 +97,6 @@ onChangeInput=(action (mut buffered.expires_at)) }}
- - {{#if isEmail}} -
- - {{textarea id="invite-message" value=buffered.custom_message}} -
- {{/if}} {{/if}} {{/if}} @@ -130,8 +122,9 @@ {{/if}} {{d-button - label="cancel" - class="btn-flat" - action=(route-action "closeModal") + action=(action "toggleAdvanced") + class="show-advanced" + icon="cog" + title=(if showAdvanced "user.invited.invite.hide_advanced" "user.invited.invite.show_advanced") }}
diff --git a/app/assets/javascripts/discourse/app/templates/modal/share-topic.hbs b/app/assets/javascripts/discourse/app/templates/modal/share-topic.hbs new file mode 100644 index 00000000000..cf9591c80e9 --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/modal/share-topic.hbs @@ -0,0 +1,61 @@ +{{#d-modal-body title="topic.share.title"}} +
+
+ +
+ {{input + name="invite-link" + class="invite-link" + value=topicUrl + readonly=true + }} + {{copy-button selector="input.invite-link" copied=(action "copied")}} +
+
+ +
+ {{#each sources as |s|}} + {{share-source source=s title=topic.title action=(action "share")}} + {{/each}} + + {{d-button + class="btn-primary" + label="topic.share.notify_users.title" + icon="users" + action=(action "toggleNotifyUsers") + }} + + {{#if allowInvites}} + {{d-button + class="btn-primary" + label="topic.share.invite_users" + icon="user-plus" + action=(action "inviteUsers") + }} + {{/if}} +
+ + {{#if showNotifyUsers}} +
+ +
+ {{user-chooser + value=users + onChange=(action "onChangeUsers") + options=(hash + topicId=topic.id + maximum=(unless currentUser.staff 1) + excludeCurrentUser=true + ) + }} + {{d-button + icon="check" + class="btn-primary" + disabled=(not users) + action=(action "notifyUsers") + }} +
+
+ {{/if}} +
+{{/d-modal-body}} diff --git a/app/assets/javascripts/discourse/tests/acceptance/create-invite-modal-test.js b/app/assets/javascripts/discourse/tests/acceptance/create-invite-modal-test.js index da6d3d34206..1aed75b4ad7 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/create-invite-modal-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/create-invite-modal-test.js @@ -43,7 +43,7 @@ acceptance("Invites - Create & Edit Invite Modal", function (needs) { "shows an invite link when modal is opened" ); - await click("#invite-show-advanced a"); + await click(".modal-footer .show-advanced"); await assert.ok( find(".invite-to-groups").length > 0, "shows advanced options" @@ -57,7 +57,7 @@ acceptance("Invites - Create & Edit Invite Modal", function (needs) { "shows advanced options" ); - await click(".modal-footer .btn:last-child"); + await click(".modal-close"); assert.ok(deleted, "deletes the invite if not saved"); }); @@ -77,7 +77,7 @@ acceptance("Invites - Create & Edit Invite Modal", function (needs) { "adds invite to list after saving" ); - await click(".modal-footer .btn:last-child"); + await click(".modal-close"); assert.notOk(deleted, "does not delete invite on close"); }); @@ -87,7 +87,7 @@ acceptance("Invites - Create & Edit Invite Modal", function (needs) { await click(".invite-link .btn"); - await click(".modal-footer .btn:last-child"); + await click(".modal-close"); assert.notOk(deleted, "does not delete invite on close"); }); @@ -95,7 +95,7 @@ acceptance("Invites - Create & Edit Invite Modal", function (needs) { await visit("/u/eviltrout/invited/pending"); await click(".invite-controls .btn:first-child"); - await click("#invite-type-email"); + await click("#invite-type"); await click(".invite-link .btn"); assert.equal( find("#modal-alert").text(), @@ -130,7 +130,6 @@ acceptance("Invites - Link Invites", function (needs) { await visit("/u/eviltrout/invited/pending"); await click(".invite-controls .btn:first-child"); - await click("#invite-type-link"); assert.ok( find("#invite-max-redemptions").length, "shows max redemptions field" @@ -173,7 +172,7 @@ acceptance("Invites - Email Invites", function (needs) { await visit("/u/eviltrout/invited/pending"); await click(".invite-controls .btn:first-child"); - await click("#invite-type-email"); + await click("#invite-type"); assert.ok(find("#invite-email").length, "shows email field"); diff --git a/app/assets/javascripts/discourse/tests/acceptance/share-and-invite-desktop-test.js b/app/assets/javascripts/discourse/tests/acceptance/share-and-invite-desktop-test.js deleted file mode 100644 index 39cc6dfac52..00000000000 --- a/app/assets/javascripts/discourse/tests/acceptance/share-and-invite-desktop-test.js +++ /dev/null @@ -1,96 +0,0 @@ -import { - acceptance, - exists, - queryAll, -} from "discourse/tests/helpers/qunit-helpers"; -import { click, visit } from "@ember/test-helpers"; -import { test } from "qunit"; - -acceptance("Share and Invite modal - desktop", function (needs) { - needs.user(); - - test("Topic footer button", async function (assert) { - await visit("/t/internationalization-localization/280"); - - assert.ok( - exists("#topic-footer-button-share-and-invite"), - "the button exists" - ); - - await click("#topic-footer-button-share-and-invite"); - - assert.ok(exists(".share-and-invite.modal"), "it shows the modal"); - - assert.ok( - exists(".share-and-invite.modal .modal-tab.share"), - "it shows the share tab" - ); - - assert.ok( - exists(".share-and-invite.modal .modal-tab.share.is-active"), - "it activates the share tab by default" - ); - - assert.ok( - exists(".share-and-invite.modal .modal-tab.invite"), - "it shows the invite tab" - ); - - assert.equal( - queryAll(".share-and-invite.modal .modal-panel.share .title").text(), - "Topic: Internationalization / localization", - "it shows the topic title" - ); - - assert.ok( - queryAll(".share-and-invite.modal .modal-panel.share .topic-share-url") - .val() - .includes("/t/internationalization-localization/280?u=eviltrout"), - "it shows the topic sharing url" - ); - - assert.ok( - queryAll(".share-and-invite.modal .social-link").length > 1, - "it shows social sources" - ); - - await click(".share-and-invite.modal .modal-tab.invite"); - - assert.ok( - exists( - ".share-and-invite.modal .modal-panel.invite .send-invite:disabled" - ), - "send invite button is disabled" - ); - - assert.ok( - exists( - ".share-and-invite.modal .modal-panel.invite .generate-invite-link:disabled" - ), - "generate invite button is disabled" - ); - }); - - test("Post date link", async function (assert) { - await visit("/t/internationalization-localization/280"); - await click("#post_2 .post-info.post-date a"); - - assert.ok(exists("#share-link"), "it shows the share modal"); - }); -}); - -acceptance("Share url with badges disabled - desktop", function (needs) { - needs.user(); - needs.settings({ enable_badges: false }); - test("topic footer button - badges disabled - desktop", async function (assert) { - await visit("/t/internationalization-localization/280"); - await click("#topic-footer-button-share-and-invite"); - - assert.notOk( - queryAll(".share-and-invite.modal .modal-panel.share .topic-share-url") - .val() - .includes("?u=eviltrout"), - "it doesn't add the username param when badges are disabled" - ); - }); -}); diff --git a/app/assets/javascripts/discourse/tests/acceptance/share-and-invite-mobile-test.js b/app/assets/javascripts/discourse/tests/acceptance/share-topic-test.js similarity index 52% rename from app/assets/javascripts/discourse/tests/acceptance/share-and-invite-mobile-test.js rename to app/assets/javascripts/discourse/tests/acceptance/share-topic-test.js index 3ec3650a042..ed50a785c45 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/share-and-invite-mobile-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/share-topic-test.js @@ -1,12 +1,55 @@ +import { click, visit } from "@ember/test-helpers"; import { acceptance, exists, queryAll, } from "discourse/tests/helpers/qunit-helpers"; -import { click, visit } from "@ember/test-helpers"; import selectKit from "discourse/tests/helpers/select-kit-helper"; import { test } from "qunit"; +acceptance("Share and Invite modal", function (needs) { + needs.user(); + + test("Topic footer button", async function (assert) { + await visit("/t/internationalization-localization/280"); + + assert.ok( + exists("#topic-footer-button-share-and-invite"), + "the button exists" + ); + + await click("#topic-footer-button-share-and-invite"); + + assert.ok(exists(".share-topic-modal"), "it shows the modal"); + + assert.ok( + queryAll("input.invite-link") + .val() + .includes("/t/internationalization-localization/280?u=eviltrout"), + "it shows the topic sharing url" + ); + + assert.ok(queryAll(".social-link").length > 1, "it shows social sources"); + + assert.ok( + exists(".btn-primary[aria-label='Notify']"), + "it shows the notify button" + ); + + assert.ok( + exists(".btn-primary[aria-label='Invite']"), + "it shows the invite button" + ); + }); + + test("Post date link", async function (assert) { + await visit("/t/internationalization-localization/280"); + await click("#post_2 .post-info.post-date a"); + + assert.ok(exists("#share-link"), "it shows the share modal"); + }); +}); + acceptance("Share and Invite modal - mobile", function (needs) { needs.user(); needs.mobileView(); @@ -23,67 +66,19 @@ acceptance("Share and Invite modal - mobile", function (needs) { await subject.expand(); await subject.selectRowByValue("share-and-invite"); - assert.ok(exists(".share-and-invite.modal"), "it shows the modal"); - - assert.ok( - exists(".share-and-invite.modal .modal-tab.share"), - "it shows the share tab" - ); - - assert.ok( - exists(".share-and-invite.modal .modal-tab.share.is-active"), - "it activates the share tab by default" - ); - - assert.ok( - exists(".share-and-invite.modal .modal-tab.invite"), - "it shows the invite tab" - ); - - assert.equal( - queryAll(".share-and-invite.modal .modal-panel.share .title").text(), - "Topic: Internationalization / localization", - "it shows the topic title" - ); - - assert.ok( - queryAll(".share-and-invite.modal .modal-panel.share .topic-share-url") - .val() - .includes("/t/internationalization-localization/280?u=eviltrout"), - "it shows the topic sharing url" - ); - - assert.ok( - queryAll(".share-and-invite.modal .social-link").length > 1, - "it shows social sources" - ); - }); - - test("Post date link", async function (assert) { - await visit("/t/internationalization-localization/280"); - await click("#post_2 .post-info.post-date a"); - - assert.ok(exists("#share-link"), "it shows the share modal"); + assert.ok(exists(".share-topic-modal"), "it shows the modal"); }); }); -acceptance("Share url with badges disabled - mobile", function (needs) { +acceptance("Share url with badges disabled - desktop", function (needs) { needs.user(); - needs.mobileView(); - needs.settings({ - enable_badges: false, - }); - test("topic footer button - badges disabled - mobile", async function (assert) { + needs.settings({ enable_badges: false }); + test("topic footer button - badges disabled - desktop", async function (assert) { await visit("/t/internationalization-localization/280"); - - const subject = selectKit(".topic-footer-mobile-dropdown"); - await subject.expand(); - await subject.selectRowByValue("share-and-invite"); + await click("#topic-footer-button-share-and-invite"); assert.notOk( - queryAll(".share-and-invite.modal .modal-panel.share .topic-share-url") - .val() - .includes("?u=eviltrout"), + queryAll("input.invite-link").val().includes("?u=eviltrout"), "it doesn't add the username param when badges are disabled" ); }); diff --git a/app/assets/stylesheets/common/base/modal.scss b/app/assets/stylesheets/common/base/modal.scss index 5e13f274fa8..ec88aa71bcb 100644 --- a/app/assets/stylesheets/common/base/modal.scss +++ b/app/assets/stylesheets/common/base/modal.scss @@ -829,7 +829,8 @@ } } -.create-invite-modal { +.create-invite-modal, +.share-topic-modal { .input-group { margin-bottom: 1em; @@ -842,8 +843,8 @@ } } - .radio-group { - input[type="radio"] { + .invite-type { + input[type="checkbox"] { display: inline; vertical-align: middle; margin-top: -1px; @@ -855,12 +856,14 @@ } .group-chooser, + .user-chooser, .future-date-input-selector { width: 100%; } .input-group input[type="text"], .input-group .btn, + .user-chooser .select-kit-header, .future-date-input .select-kit-header { height: 34px; } @@ -906,4 +909,37 @@ width: 80px; } } + + .show-advanced { + margin-left: auto; + margin-right: 0; + } +} + +.share-topic-modal { + .sources { + align-items: center; + display: flex; + flex-wrap: wrap; + flex-direction: row; + margin-bottom: 1em; + + .social-link { + font-size: $font-up-6; + margin-right: 8px; + } + + .btn-primary { + border-radius: 4px; + height: calc(#{$font-up-6} - 4px); + margin-bottom: 2px; + margin-right: 8px; + padding-left: 8px; + padding-right: 8px; + + .d-icon { + font-size: $font-up-3; + } + } + } } diff --git a/app/assets/stylesheets/desktop/modal.scss b/app/assets/stylesheets/desktop/modal.scss index 0f6cb8603a9..6356b420f8f 100644 --- a/app/assets/stylesheets/desktop/modal.scss +++ b/app/assets/stylesheets/desktop/modal.scss @@ -109,7 +109,8 @@ } .create-invite-modal, -.create-invite-bulk-modal { +.create-invite-bulk-modal, +.share-topic-modal { .modal-inner-container { width: 600px; } diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 171d246432b..343178d9a94 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -635,6 +635,39 @@ class TopicsController < ApplicationController end end + def invite_notify + topic = Topic.find_by(id: params[:topic_id]) + guardian.ensure_can_see!(topic) + + usernames = params[:usernames] + raise Discourse::InvalidParameters.new(:usernames) if !usernames.kind_of?(Array) || (!current_user.staff? && usernames.size > 1) + + users = User.where(username_lower: usernames.map(&:downcase)) + raise Discourse::InvalidParameters.new(:usernames) if usernames.size != users.size + + topic.rate_limit_topic_invitation(current_user) + + users.find_each do |user| + if !user.guardian.can_see_topic?(topic) + return render json: failed_json.merge(error: I18n.t('topic_invite.user_cannot_see_topic', username: user.username)), status: 422 + end + end + + users.find_each do |user| + last_notification = user.notifications + .where(notification_type: Notification.types[:invited_to_topic]) + .where(topic_id: topic.id) + .where(post_number: 1) + .where('created_at > ?', 1.hour.ago) + + if !last_notification.exists? + topic.create_invite_notification!(user, Notification.types[:invited_to_topic], current_user.username) + end + end + + render json: success_json + end + def invite_group group = Group.find_by(name: params[:group]) raise Discourse::NotFound unless group diff --git a/app/models/topic.rb b/app/models/topic.rb index a08e31af58a..2e48bd8677b 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -1683,6 +1683,27 @@ class Topic < ActiveRecord::Base email_addresses.to_a end + def create_invite_notification!(target_user, notification_type, username) + target_user.notifications.create!( + notification_type: notification_type, + topic_id: self.id, + post_number: 1, + data: { + topic_title: self.title, + display_username: username + }.to_json + ) + end + + def rate_limit_topic_invitation(invited_by) + RateLimiter.new( + invited_by, + "topic-invitations-per-day", + SiteSetting.max_topic_invitations_per_day, + 1.day.to_i + ).performed! + end + private def invite_to_private_message(invited_by, target_user, guardian) @@ -1711,7 +1732,7 @@ class Topic < ActiveRecord::Base Topic.transaction do rate_limit_topic_invitation(invited_by) - if group_ids + if group_ids.present? ( self.category.groups.where(id: group_ids).where(automatic: false) - target_user.groups.where(automatic: false) @@ -1743,29 +1764,6 @@ class Topic < ActiveRecord::Base def apply_per_day_rate_limit_for(key, method_name) RateLimiter.new(user, "#{key}-per-day", SiteSetting.get(method_name), 1.day.to_i) end - - def create_invite_notification!(target_user, notification_type, username) - target_user.notifications.create!( - notification_type: notification_type, - topic_id: self.id, - post_number: 1, - data: { - topic_title: self.title, - display_username: username - }.to_json - ) - end - - def rate_limit_topic_invitation(invited_by) - RateLimiter.new( - invited_by, - "topic-invitations-per-day", - SiteSetting.max_topic_invitations_per_day, - 1.day.to_i - ).performed! - - true - end end # == Schema Information diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index d24a66485f8..7ed29431221 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1505,10 +1505,8 @@ en: show_advanced: "Show Advanced Options" hide_advanced: "Hide Advanced Options" - type_link: "Invite one or more people with a link" - type_email: "Invite just one email address" + restrict_email: "Restrict the invite to one email address" - email: "Limit to email address:" max_redemptions_allowed: "Max number of uses:" add_to_groups: "Add to groups:" @@ -2664,6 +2662,15 @@ en: title: "Share" extended_title: "Share a link" help: "share a link to this topic" + instructions: "Share a link to this topic:" + copied: "Topic link copied." + notify_users: + title: "Notify" + instructions: "Notify the following users about this topic:" + success: + one: "Successfully notified %{username} about this topic." + other: "Successfully notified all users about this topic." + invite_users: "Invite" print: title: "Print" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index adc1d8694fd..a05451336ba 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -264,6 +264,7 @@ en: muted_topic: "Sorry, that user muted this topic." receiver_does_not_allow_pm: "Sorry, that user does not allow you to send them private messages." sender_does_not_allow_pm: "Sorry, you do not allow that user to send you private messages." + user_cannot_see_topic: "%{username} cannot see the topic." backup: operation_already_running: "An operation is currently running. Can't start a new job right now." diff --git a/config/routes.rb b/config/routes.rb index a56923845db..e294d167f7a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -808,6 +808,7 @@ Discourse::Application.routes.draw do post "t/:topic_id/timings" => "topics#timings", constraints: { topic_id: /\d+/ } post "t/:topic_id/invite" => "topics#invite", constraints: { topic_id: /\d+/ } post "t/:topic_id/invite-group" => "topics#invite_group", constraints: { topic_id: /\d+/ } + post "t/:topic_id/invite-notify" => "topics#invite_notify", constraints: { topic_id: /\d+/ } post "t/:topic_id/move-posts" => "topics#move_posts", constraints: { topic_id: /\d+/ } post "t/:topic_id/merge-topic" => "topics#merge_topic", constraints: { topic_id: /\d+/ } post "t/:topic_id/change-owner" => "topics#change_post_owners", constraints: { topic_id: /\d+/ } diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index ae921e092fc..d42354c59f6 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -2542,6 +2542,44 @@ RSpec.describe TopicsController do end end + describe '#invite_notify' do + let(:user2) { Fabricate(:user) } + + it 'does not notify same user multiple times' do + sign_in(user) + + expect { post "/t/#{topic.id}/invite-notify.json", params: { usernames: [user2.username] } } + .to change { Notification.count }.by(1) + expect(response.status).to eq(200) + + expect { post "/t/#{topic.id}/invite-notify.json", params: { usernames: [user2.username] } } + .to change { Notification.count }.by(0) + expect(response.status).to eq(200) + + freeze_time 1.day.from_now + + expect { post "/t/#{topic.id}/invite-notify.json", params: { usernames: [user2.username] } } + .to change { Notification.count }.by(1) + expect(response.status).to eq(200) + end + + it 'does not let regular users to notify multiple users' do + sign_in(user) + + expect { post "/t/#{topic.id}/invite-notify.json", params: { usernames: [admin.username, user2.username] } } + .to change { Notification.count }.by(0) + expect(response.status).to eq(400) + end + + it 'lets staff to notify multiple users' do + sign_in(admin) + + expect { post "/t/#{topic.id}/invite-notify.json", params: { usernames: [user.username, user2.username] } } + .to change { Notification.count }.by(2) + expect(response.status).to eq(200) + end + end + describe '#invite_group' do let!(:admins) { Group[:admins] }