DEV: Remove notify user topic from share modal (#16085)
This feature was rarely used, could be used for spamming users and was impossible to add a context to why the user was notified of a topic. A simple private messages that includes the link and personalized message can be used instead.
This commit is contained in:
parent
b21bf840cb
commit
e3b4998efc
|
@ -2,8 +2,6 @@ import Controller from "@ember/controller";
|
||||||
import { action } from "@ember/object";
|
import { action } from "@ember/object";
|
||||||
import { getAbsoluteURL } from "discourse-common/lib/get-url";
|
import { getAbsoluteURL } from "discourse-common/lib/get-url";
|
||||||
import discourseComputed from "discourse-common/utils/decorators";
|
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 Sharing from "discourse/lib/sharing";
|
||||||
import showModal from "discourse/lib/show-modal";
|
import showModal from "discourse/lib/show-modal";
|
||||||
import { bufferedProperty } from "discourse/mixins/buffered-content";
|
import { bufferedProperty } from "discourse/mixins/buffered-content";
|
||||||
|
@ -19,7 +17,6 @@ export default Controller.extend(
|
||||||
topic: null,
|
topic: null,
|
||||||
post: null,
|
post: null,
|
||||||
allowInvites: false,
|
allowInvites: false,
|
||||||
showNotifyUsers: false,
|
|
||||||
restrictedGroups: null,
|
restrictedGroups: null,
|
||||||
|
|
||||||
onShow() {
|
onShow() {
|
||||||
|
@ -27,7 +24,6 @@ export default Controller.extend(
|
||||||
topic: null,
|
topic: null,
|
||||||
post: null,
|
post: null,
|
||||||
allowInvites: false,
|
allowInvites: false,
|
||||||
showNotifyUsers: false,
|
|
||||||
});
|
});
|
||||||
|
|
||||||
if (this.model && this.model.read_restricted) {
|
if (this.model && this.model.read_restricted) {
|
||||||
|
@ -71,66 +67,16 @@ export default Controller.extend(
|
||||||
);
|
);
|
||||||
},
|
},
|
||||||
|
|
||||||
@action
|
|
||||||
onChangeUsers(usernames) {
|
|
||||||
this.set("users", usernames.uniq());
|
|
||||||
},
|
|
||||||
|
|
||||||
@action
|
@action
|
||||||
share(source) {
|
share(source) {
|
||||||
this.set("showNotifyUsers", false);
|
|
||||||
Sharing.shareSource(source, {
|
Sharing.shareSource(source, {
|
||||||
title: this.topic.title,
|
title: this.topic.title,
|
||||||
url: this.url,
|
url: this.url,
|
||||||
});
|
});
|
||||||
},
|
},
|
||||||
|
|
||||||
@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,
|
|
||||||
post_number: this.post ? this.post.post_number : undefined,
|
|
||||||
},
|
|
||||||
})
|
|
||||||
.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
|
@action
|
||||||
inviteUsers() {
|
inviteUsers() {
|
||||||
this.set("showNotifyUsers", false);
|
|
||||||
const controller = showModal("create-invite");
|
const controller = showModal("create-invite");
|
||||||
controller.setProperties({
|
controller.setProperties({
|
||||||
inviteToTopic: true,
|
inviteToTopic: true,
|
||||||
|
|
|
@ -21,13 +21,6 @@
|
||||||
{{share-source source=s title=topic.title action=(action "share")}}
|
{{share-source source=s title=topic.title action=(action "share")}}
|
||||||
{{/each}}
|
{{/each}}
|
||||||
|
|
||||||
{{d-button
|
|
||||||
class="btn-default notify"
|
|
||||||
label="topic.share.notify_users.title"
|
|
||||||
icon="hand-point-right"
|
|
||||||
action=(action "toggleNotifyUsers")
|
|
||||||
}}
|
|
||||||
|
|
||||||
{{#if allowInvites}}
|
{{#if allowInvites}}
|
||||||
{{d-button
|
{{d-button
|
||||||
class="btn-default invite"
|
class="btn-default invite"
|
||||||
|
@ -37,29 +30,6 @@
|
||||||
}}
|
}}
|
||||||
{{/if}}
|
{{/if}}
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
{{#if showNotifyUsers}}
|
|
||||||
<div class="input-group invite-users">
|
|
||||||
<label for="invite-users">{{i18n "topic.share.notify_users.instructions"}}</label>
|
|
||||||
<div class="notify-user-input">
|
|
||||||
{{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=(if users false true)
|
|
||||||
action=(action "notifyUsers")
|
|
||||||
}}
|
|
||||||
</div>
|
|
||||||
</div>
|
|
||||||
{{/if}}
|
|
||||||
</div>
|
</div>
|
||||||
</form>
|
</form>
|
||||||
{{/d-modal-body}}
|
{{/d-modal-body}}
|
||||||
|
|
|
@ -1,7 +1,6 @@
|
||||||
import { click, visit } from "@ember/test-helpers";
|
import { click, visit } from "@ember/test-helpers";
|
||||||
import {
|
import {
|
||||||
acceptance,
|
acceptance,
|
||||||
count,
|
|
||||||
exists,
|
exists,
|
||||||
queryAll,
|
queryAll,
|
||||||
} from "discourse/tests/helpers/qunit-helpers";
|
} from "discourse/tests/helpers/qunit-helpers";
|
||||||
|
@ -30,12 +29,6 @@ acceptance("Share and Invite modal", function (needs) {
|
||||||
"it shows the topic sharing url"
|
"it shows the topic sharing url"
|
||||||
);
|
);
|
||||||
|
|
||||||
assert.ok(count("button[class*='share-']") > 1, "it shows social sources");
|
|
||||||
assert.ok(
|
|
||||||
exists(".link-share-actions .notify"),
|
|
||||||
"it shows the notify button"
|
|
||||||
);
|
|
||||||
|
|
||||||
assert.ok(
|
assert.ok(
|
||||||
exists(".link-share-actions .invite"),
|
exists(".link-share-actions .invite"),
|
||||||
"it shows the invite button"
|
"it shows the invite button"
|
||||||
|
|
|
@ -684,48 +684,6 @@ class TopicsController < ApplicationController
|
||||||
end
|
end
|
||||||
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
|
|
||||||
|
|
||||||
post_number = 1
|
|
||||||
post_number = params[:post_number].to_i if params[:post_number].present?
|
|
||||||
raise Discourse::InvalidParameters.new(:post_number) if post_number < 1 || post_number > topic.highest_post_number
|
|
||||||
|
|
||||||
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: post_number)
|
|
||||||
.where('created_at > ?', 1.hour.ago)
|
|
||||||
|
|
||||||
if !last_notification.exists?
|
|
||||||
topic.create_invite_notification!(
|
|
||||||
user,
|
|
||||||
Notification.types[:invited_to_topic],
|
|
||||||
current_user.username,
|
|
||||||
post_number: post_number
|
|
||||||
)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
render json: success_json
|
|
||||||
end
|
|
||||||
|
|
||||||
def invite_group
|
def invite_group
|
||||||
group = Group.find_by(name: params[:group])
|
group = Group.find_by(name: params[:group])
|
||||||
raise Discourse::NotFound unless group
|
raise Discourse::NotFound unless group
|
||||||
|
|
|
@ -2851,12 +2851,6 @@ en:
|
||||||
restricted_groups:
|
restricted_groups:
|
||||||
one: "Only visible to members of group: %{groupNames}"
|
one: "Only visible to members of group: %{groupNames}"
|
||||||
other: "Only visible to members of groups: %{groupNames}"
|
other: "Only visible to members of groups: %{groupNames}"
|
||||||
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"
|
invite_users: "Invite"
|
||||||
|
|
||||||
print:
|
print:
|
||||||
|
|
|
@ -848,7 +848,6 @@ Discourse::Application.routes.draw do
|
||||||
post "t/:topic_id/timings" => "topics#timings", constraints: { topic_id: /\d+/ }
|
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" => "topics#invite", constraints: { topic_id: /\d+/ }
|
||||||
post "t/:topic_id/invite-group" => "topics#invite_group", 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/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/merge-topic" => "topics#merge_topic", constraints: { topic_id: /\d+/ }
|
||||||
post "t/:topic_id/change-owner" => "topics#change_post_owners", constraints: { topic_id: /\d+/ }
|
post "t/:topic_id/change-owner" => "topics#change_post_owners", constraints: { topic_id: /\d+/ }
|
||||||
|
|
|
@ -2873,46 +2873,6 @@ RSpec.describe TopicsController do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#invite_notify' do
|
|
||||||
before do
|
|
||||||
topic.update!(highest_post_number: 1)
|
|
||||||
end
|
|
||||||
|
|
||||||
it 'does not notify same user multiple times' do
|
|
||||||
sign_in(user)
|
|
||||||
|
|
||||||
expect { post "/t/#{topic.id}/invite-notify.json", params: { usernames: [user_2.username] } }
|
|
||||||
.to change { Notification.count }.by(1)
|
|
||||||
expect(response.status).to eq(200)
|
|
||||||
|
|
||||||
expect { post "/t/#{topic.id}/invite-notify.json", params: { usernames: [user_2.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: [user_2.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, user_2.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, user_2.username] } }
|
|
||||||
.to change { Notification.count }.by(2)
|
|
||||||
expect(response.status).to eq(200)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
describe '#invite_group' do
|
describe '#invite_group' do
|
||||||
let!(:admins) { Group[:admins] }
|
let!(:admins) { Group[:admins] }
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue