diff --git a/app/controllers/discourse_assign/assign_controller.rb b/app/controllers/discourse_assign/assign_controller.rb index c41cf55..8dbcd7c 100644 --- a/app/controllers/discourse_assign/assign_controller.rb +++ b/app/controllers/discourse_assign/assign_controller.rb @@ -133,24 +133,24 @@ module DiscourseAssign SQL end - group_assignment_count = Topic - .joins("JOIN assignments a ON a.target_id = topics.id AND a.target_type = 'Topic'") + group_assignments = Topic + .joins("JOIN assignments a ON a.topic_id = topics.id") .where(<<~SQL, group_id: group.id) a.assigned_to_id = :group_id AND a.assigned_to_type = 'Group' SQL - .count + .pluck(:topic_id) - assignment_count = Topic - .joins("JOIN assignments a ON a.target_id = topics.id AND a.target_type = 'Topic'") + assignments = Topic + .joins("JOIN assignments a ON a.topic_id = topics.id") .joins("JOIN group_users ON group_users.user_id = a.assigned_to_id ") .where("group_users.group_id = ?", group.id) .where("a.assigned_to_type = 'User'") - .count + .pluck(:topic_id) render json: { members: serialize_data(members, GroupUserAssignedSerializer), - assignment_count: assignment_count + group_assignment_count, - group_assignment_count: group_assignment_count + assignment_count: (assignments | group_assignments).count, + group_assignment_count: group_assignments.count } end @@ -174,6 +174,8 @@ module DiscourseAssign { error: I18n.t('discourse_assign.forbidden_group_assignee_not_pm_participant', group: assign_to.name) } when :forbidden_group_assignee_cant_see_topic { error: I18n.t('discourse_assign.forbidden_group_assignee_cant_see_topic', group: assign_to.name) } + when :too_many_assigns_for_topic + { error: I18n.t('discourse_assign.too_many_assigns_for_topic', limit: Assigner::ASSIGNMENTS_PER_TOPIC_LIMIT) } else max = SiteSetting.max_assigned_topics { error: I18n.t('discourse_assign.too_many_assigns', username: assign_to.username, max: max) } diff --git a/app/models/assignment.rb b/app/models/assignment.rb index ac67f17..f687235 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Assignment < ActiveRecord::Base - VALID_TYPES = %w(topic).freeze + VALID_TYPES = %w(topic post).freeze belongs_to :topic belongs_to :assigned_to, polymorphic: true diff --git a/assets/javascripts/discourse-assign/controllers/assign-user.js.es6 b/assets/javascripts/discourse-assign/controllers/assign-user.js.es6 index f291f94..d89520b 100644 --- a/assets/javascripts/discourse-assign/controllers/assign-user.js.es6 +++ b/assets/javascripts/discourse-assign/controllers/assign-user.js.es6 @@ -5,6 +5,7 @@ import { popupAjaxError } from "discourse/lib/ajax-error"; import { not, or } from "@ember/object/computed"; import { isEmpty } from "@ember/utils"; import { action } from "@ember/object"; +import discourseComputed from "discourse-common/utils/decorators"; export default Controller.extend({ topicBulkActions: controller(), @@ -38,6 +39,16 @@ export default Controller.extend({ }); }, + @discourseComputed("model.targetType") + i18nSuffix(targetType) { + switch (targetType) { + case "Post": + return "_post_modal"; + case "Topic": + return "_modal"; + } + }, + @action assignUser(name) { if (this.isBulkAction) { @@ -73,11 +84,11 @@ export default Controller.extend({ let path = "/assign/assign"; if (isEmpty(this.get("model.username"))) { - this.model.topic.set("assigned_to_user", null); + this.model.target.set("assigned_to_user", null); } if (isEmpty(this.get("model.group_name"))) { - this.model.topic.set("assigned_to_group", null); + this.model.target.set("assigned_to_group", null); } if ( @@ -94,8 +105,8 @@ export default Controller.extend({ data: { username: this.get("model.username"), group_name: this.get("model.group_name"), - target_id: this.get("model.topic.id"), - target_type: "Topic", + target_id: this.get("model.target.id"), + target_type: this.get("model.targetType"), }, }) .then(() => { diff --git a/assets/javascripts/discourse-assign/controllers/group-assigned-show.js.es6 b/assets/javascripts/discourse-assign/controllers/group-assigned-show.js.es6 index 93d19b5..d6640e2 100644 --- a/assets/javascripts/discourse-assign/controllers/group-assigned-show.js.es6 +++ b/assets/javascripts/discourse-assign/controllers/group-assigned-show.js.es6 @@ -48,9 +48,9 @@ export default UserTopicsList.extend({ }, @action - unassign(topic) { + unassign(targetId, targetType = "Topic") { this.taskActions - .unassign(topic.get("id")) + .unassign(targetId, targetType) .then(() => this.send("changeAssigned")); }, diff --git a/assets/javascripts/discourse-assign/controllers/user-activity-assigned.js.es6 b/assets/javascripts/discourse-assign/controllers/user-activity-assigned.js.es6 index d32fc27..a5172ae 100644 --- a/assets/javascripts/discourse-assign/controllers/user-activity-assigned.js.es6 +++ b/assets/javascripts/discourse-assign/controllers/user-activity-assigned.js.es6 @@ -58,9 +58,9 @@ export default UserTopicsList.extend({ }, @action - unassign(topic) { + unassign(targetId, targetType = "Topic") { this.taskActions - .unassign(topic.get("id")) + .unassign(targetId, targetType) .then(() => this.send("changeAssigned")); }, diff --git a/assets/javascripts/discourse-assign/initializers/extend-for-assigns.js.es6 b/assets/javascripts/discourse-assign/initializers/extend-for-assigns.js.es6 index ae00c4d..f88ec09 100644 --- a/assets/javascripts/discourse-assign/initializers/extend-for-assigns.js.es6 +++ b/assets/javascripts/discourse-assign/initializers/extend-for-assigns.js.es6 @@ -95,7 +95,7 @@ function registerTopicFooterButtons(api) { if (topic.assigned_to_user || topic.assigned_to_group) { this.set("topic.assigned_to_user", null); this.set("topic.assigned_to_group", null); - taskActions.unassign(topic.id).then(() => { + taskActions.unassign(topic.id, "Topic").then(() => { this.appEvents.trigger("post-stream:refresh", { id: topic.postStream.firstPostId, }); @@ -151,6 +151,38 @@ function initialize(api) { }, before: "top", }); + if (api.getCurrentUser() && api.getCurrentUser().can_assign) { + api.addPostMenuButton("assign", (post) => { + if (post.firstPost) { + return; + } + if (post.assigned_to_user || post.assigned_to_group) { + return { + action: "unassignPost", + icon: "user-times", + className: "unassign-post", + title: "discourse_assign.unassign_post.title", + position: "second-last-hidden", + }; + } else { + return { + action: "assignPost", + icon: "user-plus", + className: "assign-post", + title: "discourse_assign.assign_post.title", + position: "second-last-hidden", + }; + } + }); + api.attachWidgetAction("post", "assignPost", function () { + const taskActions = getOwner(this).lookup("service:task-actions"); + taskActions.assign(this.model, "Post"); + }); + api.attachWidgetAction("post", "unassignPost", function () { + const taskActions = getOwner(this).lookup("service:task-actions"); + taskActions.unassign(this.model.id, "Post"); + }); + } } api.addAdvancedSearchOptions( @@ -170,52 +202,55 @@ function initialize(api) { : {} ); - api.modifyClass("model:topic", { - pluginId: PLUGIN_ID, + function assignedToUserPath(assignedToUser) { + return getURL( + siteSettings.assigns_user_url_path.replace( + "{username}", + assignedToUser.username + ) + ); + } - @discourseComputed("assigned_to_user") - assignedToUserPath(assignedToUser) { - return getURL( - siteSettings.assigns_user_url_path.replace( - "{username}", - assignedToUser.username - ) - ); - }, - @discourseComputed("assigned_to_group") - assignedToGroupPath(assignedToGroup) { - return getURL(`/g/${assignedToGroup.name}/assigned/everyone`); - }, - }); + function assignedToGroupPath(assignedToGroup) { + return getURL(`/g/${assignedToGroup.name}/assigned/everyone`); + } api.modifyClass("model:bookmark", { pluginId: PLUGIN_ID, @discourseComputed("assigned_to_user") assignedToUserPath(assignedToUser) { - return getURL( - this.siteSettings.assigns_user_url_path.replace( - "{username}", - assignedToUser.username - ) - ); + return assignedToUserPath(assignedToUser); }, @discourseComputed("assigned_to_group") assignedToGroupPath(assignedToGroup) { - return getURL(`/g/${assignedToGroup.name}/assigned/everyone`); + return assignedToGroupPath(assignedToGroup); }, }); api.addPostSmallActionIcon("assigned", "user-plus"); + api.addPostSmallActionIcon("assigned_to_post", "user-plus"); api.addPostSmallActionIcon("assigned_group", "group-plus"); + api.addPostSmallActionIcon("assigned_group_to_post", "group-plus"); api.addPostSmallActionIcon("unassigned", "user-times"); api.addPostSmallActionIcon("unassigned_group", "group-times"); + api.addPostSmallActionIcon("unassigned_from_post", "user-times"); + api.addPostSmallActionIcon("unassigned_group_from_post", "group-times"); + + api.includePostAttributes("assigned_to_user", "assigned_to_group"); api.addPostTransformCallback((transformed) => { if ( - ["assigned", "unassigned", "assigned_group", "unassigned_group"].includes( - transformed.actionCode - ) + [ + "assigned", + "unassigned", + "assigned_group", + "unassigned_group", + "assigned_to_post", + "assigned_group_to_post", + "unassigned_from_post", + "unassigned_group_from_post", + ].includes(transformed.actionCode) ) { transformed.isSmallAction = true; transformed.canEdit = false; @@ -225,22 +260,45 @@ function initialize(api) { api.addDiscoveryQueryParam("assigned", { replace: true, refreshModel: true }); api.addTagsHtmlCallback((topic, params = {}) => { - const assignedToUser = topic.get("assigned_to_user.username"); - const assignedToGroup = topic.get("assigned_to_group.name"); - if (assignedToUser || assignedToGroup) { - const assignedPath = assignedToUser - ? topic.assignedToUserPath - : topic.assignedToGroupPath; - const tagName = params.tagName || "a"; - const icon = assignedToUser - ? iconHTML("user-plus") - : iconHTML("group-plus"); - const href = - tagName === "a" ? `href="${assignedPath}" data-auto-route="true"` : ""; - return `<${tagName} class="assigned-to discourse-tag simple" ${href}> + const [ + assignedToUser, + assignedToGroup, + assignedToIndirectly, + ] = Object.values( + topic.getProperties( + "assigned_to_user", + "assigned_to_group", + "indirectly_assigned_to" + ) + ); + + const assignedTo = [] + .concat( + assignedToUser, + assignedToGroup, + assignedToIndirectly ? Object.values(assignedToIndirectly) : [] + ) + .filter((element) => element) + .flat() + .uniqBy((assignee) => assignee.assign_path); + + if (assignedTo) { + return assignedTo + .map((assignee) => { + const assignedPath = getURL(assignee.assign_path); + const icon = iconHTML(assignee.assign_icon); + const name = assignee.username || assignee.name; + const tagName = params.tagName || "a"; + const href = + tagName === "a" + ? `href="${assignedPath}" data-auto-route="true"` + : ""; + return `<${tagName} class="assigned-to discourse-tag simple" ${href}> ${icon} - ${assignedToUser || assignedToGroup} + ${name} `; + }) + .join(""); } }); @@ -299,19 +357,31 @@ function initialize(api) { const topicId = topic.id; if (data.topic_id === topicId) { + let post; + if (data.post_id) { + post = topic.postStream.posts.find((p) => p.id === data.post_id); + } + const target = post || topic; if (data.assigned_type === "User") { - topic.set( + target.set( "assigned_to_user_id", data.type === "assigned" ? data.assigned_to.id : null ); - topic.set("assigned_to_user", data.assigned_to); + target.set("assigned_to_user", data.assigned_to); } if (data.assigned_type === "Group") { - topic.set( + target.set( "assigned_to_group_id", data.type === "assigned" ? data.assigned_to.id : null ); - topic.set("assigned_to_group", data.assigned_to); + target.set("assigned_to_group", data.assigned_to); + } + + if (data.post_id) { + if (data.type === "unassigned") { + delete topic.indirectly_assigned_to[data.post_id]; + } + this.appEvents.trigger("post-stream:refresh", { id: data.post_id }); } } this.appEvents.trigger("header:update-topic", topic); @@ -330,20 +400,32 @@ function initialize(api) { }); api.decorateWidget("post-contents:after-cooked", (dec) => { - if (dec.attrs.post_number === 1) { - const postModel = dec.getModel(); - if (postModel) { - const assignedToUser = get(postModel, "topic.assigned_to_user"); - const assignedToGroup = get(postModel, "topic.assigned_to_group"); - if (assignedToUser || assignedToGroup) { - return dec.widget.attach("assigned-to", { - assignedToUser, - assignedToGroup, - href: assignedToUser - ? get(postModel, "topic.assignedToUserPath") - : get(postModel, "topic.assignedToGroupPath"), - }); + const postModel = dec.getModel(); + if (postModel) { + let assignedToUser, assignedToGroup, postAssignment, href; + if (dec.attrs.post_number === 1) { + assignedToUser = get(postModel, "topic.assigned_to_user"); + assignedToGroup = get(postModel, "topic.assigned_to_group"); + } else { + postAssignment = postModel.topic.indirectly_assigned_to?.[postModel.id]; + if (postAssignment?.username) { + assignedToUser = postAssignment; } + if (postAssignment?.name) { + assignedToGroup = postAssignment; + } + } + if (assignedToUser || assignedToGroup) { + href = assignedToUser + ? assignedToUserPath(assignedToUser) + : assignedToGroupPath(assignedToGroup); + } + if (href) { + return dec.widget.attach("assigned-to", { + assignedToUser, + assignedToGroup, + href, + }); } } }); @@ -441,6 +523,10 @@ export default { withPluginApi("0.12.2", (api) => { api.addGroupPostSmallActionCode("assigned_group"); api.addGroupPostSmallActionCode("unassigned_group"); + api.addGroupPostSmallActionCode("assigned_to_post"); + api.addGroupPostSmallActionCode("assigned_group_to_post"); + api.addGroupPostSmallActionCode("unassigned_from_post"); + api.addGroupPostSmallActionCode("unassigned_group_from_post"); }); withPluginApi("0.12.3", (api) => { api.addUserSearchOption("assignableGroups"); diff --git a/assets/javascripts/discourse/components/assign-actions-dropdown.js.es6 b/assets/javascripts/discourse/components/assign-actions-dropdown.js.es6 index a1c6544..128ff4e 100644 --- a/assets/javascripts/discourse/components/assign-actions-dropdown.js.es6 +++ b/assets/javascripts/discourse/components/assign-actions-dropdown.js.es6 @@ -10,33 +10,55 @@ export default DropdownSelectBoxComponent.extend({ showFullTitle: true, computeContent() { - return [ - { - id: "unassign", - icon: this.group ? "group-times" : "user-times", - name: I18n.t("discourse_assign.unassign.title"), - description: I18n.t("discourse_assign.unassign.help", { - username: this.assignee, - }), - }, - { - id: "reassign", - icon: "users", - name: I18n.t("discourse_assign.reassign.title"), - description: I18n.t("discourse_assign.reassign.help"), - }, - ]; + let options = []; + if (this.assignee) { + options = options.concat([ + { + id: "unassign", + icon: this.group ? "group-times" : "user-times", + name: I18n.t("discourse_assign.unassign.title"), + description: I18n.t("discourse_assign.unassign.help", { + username: this.assignee, + }), + }, + { + id: "reassign", + icon: "users", + name: I18n.t("discourse_assign.reassign.title"), + description: I18n.t("discourse_assign.reassign.help"), + }, + ]); + } + + if (this.topic.indirectly_assigned_to) { + Object.entries(this.topic.indirectly_assigned_to).forEach((entry) => { + const [postId, assignee] = entry; + options = options.concat({ + id: `unassign_post_${postId}`, + icon: assignee.username ? "user-times" : "group-times", + name: I18n.t("discourse_assign.unassign_post.title"), + description: I18n.t("discourse_assign.unassign_post.help", { + username: assignee.username || assignee.name, + }), + }); + }); + } + return options; }, @action onSelect(id) { switch (id) { case "unassign": - this.unassign(this.topic, this.assignee); + this.unassign(this.topic.id); break; case "reassign": this.reassign(this.topic, this.assignee); break; } + const postId = id.match(/unassign_post_(\d+)/)?.[1]; + if (postId) { + this.unassign(postId, "Post"); + } }, }); diff --git a/assets/javascripts/discourse/services/task-actions.js.es6 b/assets/javascripts/discourse/services/task-actions.js.es6 index 853865a..90d1e57 100644 --- a/assets/javascripts/discourse/services/task-actions.js.es6 +++ b/assets/javascripts/discourse/services/task-actions.js.es6 @@ -3,22 +3,23 @@ import { ajax } from "discourse/lib/ajax"; import showModal from "discourse/lib/show-modal"; export default Service.extend({ - unassign(topicId) { + unassign(targetId, targetType = "Topic") { return ajax("/assign/unassign", { type: "PUT", data: { - target_id: topicId, - target_type: "Topic", + target_id: targetId, + target_type: targetType, }, }); }, - assign(topic) { + assign(target, targetType = "Topic") { return showModal("assign-user", { model: { - username: topic.get("assigned_to_user.username"), - group_name: topic.get("assigned_to_group.name"), - topic, + username: target.get("assigned_to_user.username"), + group_name: target.get("assigned_to_group.name"), + target, + targetType, }, }); }, diff --git a/assets/javascripts/discourse/templates/components/assigned-topic-list-item.hbs b/assets/javascripts/discourse/templates/components/assigned-topic-list-item.hbs index 5c31ab3..b7fd0fa 100644 --- a/assets/javascripts/discourse/templates/components/assigned-topic-list-item.hbs +++ b/assets/javascripts/discourse/templates/components/assigned-topic-list-item.hbs @@ -54,7 +54,7 @@ unassign=unassign reassign=reassign }} - {{else}} + {{else if topic.assigned_to_group}} {{assign-actions-dropdown topic=topic assignee=topic.assigned_to_group.name @@ -62,5 +62,10 @@ unassign=unassign reassign=reassign }} + {{else}} + {{assign-actions-dropdown + topic=topic + unassign=unassign + }} {{/if}} diff --git a/assets/javascripts/discourse/templates/mobile/components/assigned-topic-list-item.hbs b/assets/javascripts/discourse/templates/mobile/components/assigned-topic-list-item.hbs index 5d075ab..11cb685 100644 --- a/assets/javascripts/discourse/templates/mobile/components/assigned-topic-list-item.hbs +++ b/assets/javascripts/discourse/templates/mobile/components/assigned-topic-list-item.hbs @@ -28,13 +28,19 @@ unassign=unassign reassign=reassign }} - {{else}} + {{else if topic.assigned_to_group}} {{assign-actions-dropdown topic=topic assignee=topic.assigned_to_group.name + group=true unassign=unassign reassign=reassign }} + {{else}} + {{assign-actions-dropdown + topic=topic + unassign=unassign + }} {{/if}}
diff --git a/assets/javascripts/discourse/templates/modal/assign-user.hbs b/assets/javascripts/discourse/templates/modal/assign-user.hbs index 4a89eca..3891b40 100644 --- a/assets/javascripts/discourse/templates/modal/assign-user.hbs +++ b/assets/javascripts/discourse/templates/modal/assign-user.hbs @@ -1,6 +1,6 @@ -{{#d-modal-body title="discourse_assign.assign_modal.title" class="assign"}} +{{#d-modal-body title=(concat "discourse_assign.assign" i18nSuffix ".title") class="assign"}}
- {{i18n "discourse_assign.assign_modal.description"}} +

{{i18n (concat "discourse_assign.assign" i18nSuffix ".description")}}

{{email-group-user-chooser autocomplete="off" value=assigneeName diff --git a/assets/stylesheets/assigns.scss b/assets/stylesheets/assigns.scss index b48ee54..caf5a1c 100644 --- a/assets/stylesheets/assigns.scss +++ b/assets/stylesheets/assigns.scss @@ -69,6 +69,10 @@ margin-top: 5px; } +.assign-user-modal p { + margin-top: 0; +} + .assign-suggestions { margin-top: 15px; img { diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index cc0751d..3903361 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -7,8 +7,12 @@ en: action_codes: assigned: "assigned %{who} %{when}" assigned_group: "assigned %{who} %{when}" + assigned_to_post: "assigned %{who} to post %{when}" + assigned_group_to_post: "assigned %{who} to post %{when}" unassigned: "unassigned %{who} %{when}" unassigned_group: "unassigned %{who} %{when}" + unassigned_from_post: "unassigned %{who} from post %{when}" + unassigned_group_from_post: "unassigned %{who} from post %{when}" discourse_assign: add_unassigned_filter: "Add 'unassigned' filter to category" cant_act: "You cannot act on flags that have been assigned to other users" @@ -26,6 +30,11 @@ en: assign: title: "Assign" help: "Assign Topic to User" + assign_post: + title: "Assign Post" + unassign_post: + title: "Unassign from Post" + help: "Unassign %{username} from Post" reassign: title: "Re-assign" help: "Re-assign Topic to a different user" @@ -33,6 +42,9 @@ en: title: "Assign Topic" description: "Enter the name of the user you'd like to assign this topic" assign: "Assign" + assign_post_modal: + title: "Assign Post" + description: "Enter the name of the user you'd like to assign this post" claim: title: "claim" help: "Assign topic to yourself" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index a659727..1529ce3 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -20,6 +20,7 @@ en: already_claimed: "That topic has already been claimed." already_assigned: "Topic is already assigned to @%{username}" too_many_assigns: "@%{username} has already reached the maximum number of assigned topics (%{max})." + too_many_assigns_for_topic: "Limit of %{limit} assignments per topic has been reached." forbidden_assign_to: "@%{username} can't be assigned since they don't belong to assigned allowed groups." forbidden_assignee_not_pm_participant: "@%{username} can't be assigned because they don't have access to this PM. You can grant @%{username} access by inviting them to this PM." forbidden_assignee_cant_see_topic: "@%{username} can't be assigned because they don't have access to this topic." diff --git a/db/migrate/20211014043735_remove_topic_id_index.rb b/db/migrate/20211014043735_remove_topic_id_index.rb new file mode 100644 index 0000000..f29bf8f --- /dev/null +++ b/db/migrate/20211014043735_remove_topic_id_index.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class RemoveTopicIdIndex < ActiveRecord::Migration[6.1] + def change + remove_index :assignments, :topic_id + end +end diff --git a/jobs/regular/assign_notification.rb b/jobs/regular/assign_notification.rb index f88798f..e26ccc7 100644 --- a/jobs/regular/assign_notification.rb +++ b/jobs/regular/assign_notification.rb @@ -4,14 +4,15 @@ module Jobs class AssignNotification < ::Jobs::Base def execute(args) raise Discourse::InvalidParameters.new(:topic_id) if args[:topic_id].nil? + raise Discourse::InvalidParameters.new(:post_id) if args[:post_id].nil? raise Discourse::InvalidParameters.new(:assigned_to_id) if args[:assigned_to_id].nil? raise Discourse::InvalidParameters.new(:assigned_to_type) if args[:assigned_to_type].nil? raise Discourse::InvalidParameters.new(:assigned_by_id) if args[:assigned_by_id].nil? raise Discourse::InvalidParameters.new(:silent) if args[:silent].nil? topic = Topic.find(args[:topic_id]) + post = Post.find(args[:post_id]) assigned_by = User.find(args[:assigned_by_id]) - first_post = topic.posts.find_by(post_number: 1) assigned_to = args[:assigned_to_type] == "User" ? User.find(args[:assigned_to_id]) : Group.find(args[:assigned_to_id]) assigned_to_users = args[:assigned_to_type] == "User" ? [assigned_to] : assigned_to.users @@ -20,9 +21,9 @@ module Jobs next if assigned_by == user - PostAlerter.new(first_post).create_notification_alert( + PostAlerter.new(post).create_notification_alert( user: user, - post: first_post, + post: post, username: assigned_by.username, notification_type: Notification.types[:custom], excerpt: I18n.t( @@ -37,7 +38,7 @@ module Jobs notification_type: Notification.types[:custom], user_id: user.id, topic_id: topic.id, - post_number: 1, + post_number: post.post_number, high_priority: true, data: { message: args[:assigned_to_type] == "User" ? 'discourse_assign.assign_notification' : 'discourse_assign.assign_group_notification', diff --git a/jobs/regular/unassign_notification.rb b/jobs/regular/unassign_notification.rb index 00fb15c..bae8936 100644 --- a/jobs/regular/unassign_notification.rb +++ b/jobs/regular/unassign_notification.rb @@ -17,7 +17,6 @@ module Jobs notification_type: Notification.types[:custom], user_id: user.id, topic_id: topic.id, - post_number: 1 ).where("data like '%discourse_assign.assign_notification%' OR data like '%discourse_assign.assign_group_notification%'").destroy_all end end diff --git a/lib/assigner.rb b/lib/assigner.rb index 4a711c9..45b1300 100644 --- a/lib/assigner.rb +++ b/lib/assigner.rb @@ -4,6 +4,8 @@ require 'email/sender' require 'nokogiri' class ::Assigner + ASSIGNMENTS_PER_TOPIC_LIMIT = 5 + def self.backfill_auto_assign staff_mention = User .assign_allowed @@ -107,7 +109,6 @@ class ::Assigner end def self.publish_topic_tracking_state(topic, user_id) - # TODO decide later if we want general or separate method to publish about post tracking if topic.private_message? MessageBus.publish( "/private-messages/assigned", @@ -190,6 +191,8 @@ class ::Assigner assign_to.is_a?(User) ? :forbidden_assign_to : :forbidden_group_assign_to when topic.assignment&.assigned_to_id == assign_to.id assign_to.is_a?(User) ? :already_assigned : :group_already_assigned + when Assignment.where(topic: topic).count >= ASSIGNMENTS_PER_TOPIC_LIMIT + :too_many_assigns_for_topic when !can_assign_to?(assign_to) :too_many_assigns end @@ -205,20 +208,20 @@ class ::Assigner serializer = assignment.assigned_to_user? ? BasicUserSerializer : BasicGroupSerializer - #TODO assign notification for post Jobs.enqueue(:assign_notification, topic_id: topic.id, + post_id: topic_target? ? first_post.id : @target.id, assigned_to_id: assign_to.id, assigned_to_type: type, assigned_by_id: @assigned_by.id, silent: silent) - #TODO message bus for post assignment MessageBus.publish( "/staff/topic-assignment", { type: "assigned", topic_id: topic.id, + post_id: post_target? && @target.id, assigned_type: type, assigned_to: serializer.new(assign_to, scope: Guardian.new, root: false).as_json }, @@ -247,16 +250,14 @@ class ::Assigner end end if !silent - #TODO moderator post when assigned to post topic.add_moderator_post( @assigned_by, nil, bump: false, post_type: SiteSetting.assigns_public ? Post.types[:small_action] : Post.types[:whisper], - action_code: assignment.assigned_to_user? ? "assigned" : "assigned_group", + action_code: moderator_post_assign_action_code(assignment), custom_fields: { "action_code_who" => assign_to.is_a?(User) ? assign_to.username : assign_to.name } ) - end # Create a webhook event @@ -296,7 +297,6 @@ class ::Assigner first_post.publish_change_to_clients!(:revised, reload_topic: true) - #TODO unassign notification for post Jobs.enqueue(:unassign_notification, topic_id: topic.id, assigned_to_id: assignment.assigned_to.id, @@ -321,7 +321,6 @@ class ::Assigner assigned_to = assignment.assigned_to - # TODO unassign post for post assignment if SiteSetting.unassign_creates_tracking_post && !silent post_type = SiteSetting.assigns_public ? Post.types[:small_action] : Post.types[:whisper] topic.add_moderator_post( @@ -329,7 +328,7 @@ class ::Assigner bump: false, post_type: post_type, custom_fields: { "action_code_who" => assigned_to.is_a?(User) ? assigned_to.username : assigned_to.name }, - action_code: assignment.assigned_to_user? ? "unassigned" : "unassigned_group", + action_code: moderator_post_unassign_action_code(assignment), ) end @@ -362,9 +361,35 @@ class ::Assigner { type: 'unassigned', topic_id: topic.id, + post_id: post_target? && @target.id, + assigned_type: assignment.assigned_to.is_a?(User) ? "User" : "Group" }, user_ids: allowed_user_ids ) end end + + private + + def moderator_post_assign_action_code(assignment) + suffix = + if assignment.target.is_a?(Post) + "_to_post" + elsif assignment.target.is_a?(Topic) + "" + end + return "assigned#{suffix}" if assignment.assigned_to_user? + return "assigned_group#{suffix}" if assignment.assigned_to_group? + end + + def moderator_post_unassign_action_code(assignment) + suffix = + if assignment.target.is_a?(Post) + "_from_post" + elsif assignment.target.is_a?(Topic) + "" + end + return "unassigned#{suffix}" if assignment.assigned_to_user? + return "unassigned_group#{suffix}" if assignment.assigned_to_group? + end end diff --git a/lib/discourse_assign/helpers.rb b/lib/discourse_assign/helpers.rb index 831ed05..bb3dc8a 100644 --- a/lib/discourse_assign/helpers.rb +++ b/lib/discourse_assign/helpers.rb @@ -4,12 +4,12 @@ module DiscourseAssign module Helpers def self.build_assigned_to_user(user, topic) return if !user - { username: user.username, name: user.name, avatar_template: user.avatar_template, - assigned_at: Assignment.where(target: topic).pluck_first(:created_at) + assign_icon: 'user-plus', + assign_path: SiteSetting.assigns_user_url_path.gsub("{username}", user.username), } end @@ -22,8 +22,19 @@ module DiscourseAssign flair_color: group.flair_color, flair_icon: group.flair_icon, flair_upload_id: group.flair_upload_id, - assigned_at: Assignment.where(target: topic).pluck_first(:created_at) + assign_icon: 'group-plus', + assign_path: "/g/#{group.name}/assigned/everyone", } end + + def self.build_indirectly_assigned_to(post_assignments, topic) + post_assignments.map do |post_id, assigned_to| + if (assigned_to.is_a?(User)) + [post_id, build_assigned_to_user(assigned_to, topic)] + elsif assigned_to.is_a?(Group) + [post_id, build_assigned_to_group(assigned_to, topic)] + end + end.to_h + end end end diff --git a/plugin.rb b/plugin.rb index 536cdb3..1d581c0 100644 --- a/plugin.rb +++ b/plugin.rb @@ -52,6 +52,10 @@ after_initialize do has_one :assignment, as: :target, dependent: :destroy end + class ::Post + has_one :assignment, as: :target, dependent: :destroy + end + class ::Group scope :assignable, ->(user) { where("assignable_level in (:levels) OR @@ -195,6 +199,10 @@ after_initialize do end end + TopicView.on_preload do |topic_view| + topic_view.instance_variable_set(:@posts, topic_view.posts.includes(:assignment)) + end + TopicList.on_preload do |topics, topic_list| if SiteSetting.assign_enabled? can_assign = topic_list.current_user && topic_list.current_user.can_assign? @@ -202,7 +210,7 @@ after_initialize do if allowed_access && topics.length > 0 assignments = Assignment.strict_loading.where(topic: topics) - assignments_map = assignments.index_by(&:topic_id) + assignments_map = assignments.group_by(&:topic_id) user_ids = assignments.filter { |assignment| assignment.assigned_to_user? }.map(&:assigned_to_id) users_map = User.where(id: user_ids).select(UserLookup.lookup_columns).index_by(&:id) @@ -211,10 +219,23 @@ after_initialize do groups_map = Group.where(id: group_ids).index_by(&:id) topics.each do |topic| - assignment = assignments_map[topic.id] - assigned_to = users_map[assignment.assigned_to_id] if assignment&.assigned_to_user? - assigned_to = groups_map[assignment.assigned_to_id] if assignment&.assigned_to_group? + assignments = assignments_map[topic.id] + direct_assignment = assignments&.find { |assignment| assignment.target_type == "Topic" && assignment.target_id == topic.id } + indirectly_assigned_to = {} + assignments&.each do |assignment| + next if assignment.target_type == "Topic" + next indirectly_assigned_to[assignment.target_id] = users_map[assignment.assigned_to_id] if assignment&.assigned_to_user? + next indirectly_assigned_to[assignment.target_id] = groups_map[assignment.assigned_to_id] if assignment&.assigned_to_group? + end&.compact&.uniq + + assigned_to = + if direct_assignment&.assigned_to_user? + users_map[direct_assignment.assigned_to_id] + elsif direct_assignment&.assigned_to_group? + groups_map[direct_assignment.assigned_to_id] + end topic.preload_assigned_to(assigned_to) + topic.preload_indirectly_assigned_to(indirectly_assigned_to) end end end @@ -389,10 +410,22 @@ after_initialize do @assigned_to = assignment&.assigned_to end + add_to_class(:topic, :indirectly_assigned_to) do + return @indirectly_assigned_to if defined?(@indirectly_assigned_to) + @indirectly_assigned_to = Assignment.where(topic_id: id, target_type: "Post").inject({}) do |acc, assignment| + acc[assignment.target_id] = assignment.assigned_to + acc + end + end + add_to_class(:topic, :preload_assigned_to) do |assigned_to| @assigned_to = assigned_to end + add_to_class(:topic, :preload_indirectly_assigned_to) do |indirectly_assigned_to| + @indirectly_assigned_to = indirectly_assigned_to + end + # TopicList serializer add_to_serializer(:topic_list, :assigned_messages_count) do TopicQuery.new(object.current_user, guardian: scope, limit: false) @@ -426,6 +459,14 @@ after_initialize do (SiteSetting.assigns_public || scope.can_assign?) && object.topic.assigned_to&.is_a?(Group) end + add_to_serializer(:topic_view, :indirectly_assigned_to) do + DiscourseAssign::Helpers.build_indirectly_assigned_to(object.topic.indirectly_assigned_to, object.topic) + end + + add_to_serializer(:topic_view, :include_indirectly_assigned_to?) do + (SiteSetting.assigns_public || scope.can_assign?) && object.topic.indirectly_assigned_to.present? + end + # SuggestedTopic serializer add_to_serializer(:suggested_topic, :assigned_to_user, false) do DiscourseAssign::Helpers.build_assigned_to_user(object.assigned_to, object) @@ -443,7 +484,23 @@ after_initialize do (SiteSetting.assigns_public || scope.can_assign?) && object.assigned_to&.is_a?(Group) end + add_to_serializer(:suggested_topic, :indirectly_assigned_to) do + DiscourseAssign::Helpers.build_indirectly_assigned_to(object.indirectly_assigned_to, object) + end + + add_to_serializer(:suggested_topic, :include_indirectly_assigned_to?) do + (SiteSetting.assigns_public || scope.can_assign?) && object.indirectly_assigned_to.present? + end + # TopicListItem serializer + add_to_serializer(:topic_list_item, :indirectly_assigned_to) do + DiscourseAssign::Helpers.build_indirectly_assigned_to(object.indirectly_assigned_to, object) + end + + add_to_serializer(:topic_list_item, :include_indirectly_assigned_to?) do + (SiteSetting.assigns_public || scope.can_assign?) && object.indirectly_assigned_to.present? + end + add_to_serializer(:topic_list_item, :assigned_to_user) do BasicUserSerializer.new(object.assigned_to, scope: scope, root: false).as_json end @@ -504,6 +561,22 @@ after_initialize do topic.assigned_to end + add_to_serializer(:basic_user, :assign_icon) do + 'user-plus' + end + add_to_serializer(:basic_user, :assign_path) do + return if !object.is_a?(User) + SiteSetting.assigns_user_url_path.gsub("{username}", object.username) + end + + add_to_serializer(:basic_group, :assign_icon) do + 'group-plus' + end + + add_to_serializer(:basic_group, :assign_path) do + "/g/#{object.name}/assigned/everyone" + end + add_to_serializer(:user_bookmark, 'include_assigned_to_user?') do (SiteSetting.assigns_public || scope.can_assign?) && topic.assigned_to&.is_a?(User) end @@ -516,6 +589,23 @@ after_initialize do (SiteSetting.assigns_public || scope.can_assign?) && topic.assigned_to&.is_a?(Group) end + # PostSerializer + add_to_serializer(:post, :assigned_to_user) do + object.assignment&.assigned_to + end + + add_to_serializer(:post, 'include_assigned_to_user?') do + (SiteSetting.assigns_public || scope.can_assign?) && object.assignment&.assigned_to&.is_a?(User) + end + + add_to_serializer(:post, :assigned_to_group, false) do + object.assignment&.assigned_to + end + + add_to_serializer(:post, 'include_assigned_to_group?') do + (SiteSetting.assigns_public || scope.can_assign?) && object.assignment&.assigned_to&.is_a?(Group) + end + # CurrentUser serializer add_to_serializer(:current_user, :can_assign) do object.can_assign? diff --git a/spec/jobs/regular/assign_notification_spec.rb b/spec/jobs/regular/assign_notification_spec.rb index 21cd370..aef13c0 100644 --- a/spec/jobs/regular/assign_notification_spec.rb +++ b/spec/jobs/regular/assign_notification_spec.rb @@ -29,7 +29,7 @@ RSpec.describe Jobs::AssignNotification do it 'sends notification alert' do messages = MessageBus.track_publish("/notification-alert/#{user2.id}") do - described_class.new.execute({ topic_id: topic.id, assigned_to_id: user2.id, assigned_to_type: 'User', assigned_by_id: user1.id, silent: false }) + described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: user2.id, assigned_to_type: 'User', assigned_by_id: user1.id, silent: false }) end expect(messages.length).to eq(1) @@ -41,7 +41,7 @@ RSpec.describe Jobs::AssignNotification do assign_allowed_group.add(user) assert_publish_topic_state(pm, user) do - described_class.new.execute({ topic_id: pm.id, assigned_to_id: pm.allowed_users.first.id, assigned_to_type: 'User', assigned_by_id: user1.id, silent: false }) + described_class.new.execute({ topic_id: pm.id, post_id: pm_post.id, assigned_to_id: pm.allowed_users.first.id, assigned_to_type: 'User', assigned_by_id: user1.id, silent: false }) end end @@ -58,7 +58,7 @@ RSpec.describe Jobs::AssignNotification do topic_title: topic.title }.to_json ) - described_class.new.execute({ topic_id: topic.id, assigned_to_id: user2.id, assigned_to_type: 'User', assigned_by_id: user1.id, silent: false }) + described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: user2.id, assigned_to_type: 'User', assigned_by_id: user1.id, silent: false }) end end @@ -76,19 +76,19 @@ RSpec.describe Jobs::AssignNotification do it 'sends notification alert to all group members' do messages = MessageBus.track_publish("/notification-alert/#{user2.id}") do - described_class.new.execute({ topic_id: topic.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, silent: false }) + described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, silent: false }) end expect(messages.length).to eq(1) expect(messages.first.data[:excerpt]).to eq("assigned you the topic '#{topic.title}'") messages = MessageBus.track_publish("/notification-alert/#{user3.id}") do - described_class.new.execute({ topic_id: topic.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, silent: false }) + described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, silent: false }) end expect(messages.length).to eq(1) expect(messages.first.data[:excerpt]).to eq("assigned you the topic '#{topic.title}'") messages = MessageBus.track_publish("/notification-alert/#{user4.id}") do - described_class.new.execute({ topic_id: topic.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, silent: false }) + described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, silent: false }) end expect(messages.length).to eq(0) end @@ -109,7 +109,7 @@ RSpec.describe Jobs::AssignNotification do ) end - described_class.new.execute({ topic_id: topic.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, silent: false }) + described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, silent: false }) end end end diff --git a/spec/jobs/regular/unassign_notification_spec.rb b/spec/jobs/regular/unassign_notification_spec.rb index cd39d6b..ae087f3 100644 --- a/spec/jobs/regular/unassign_notification_spec.rb +++ b/spec/jobs/regular/unassign_notification_spec.rb @@ -27,10 +27,10 @@ RSpec.describe Jobs::UnassignNotification do context 'User' do it 'deletes notifications' do - Jobs::AssignNotification.new.execute({ topic_id: topic.id, assigned_to_id: user2.id, assigned_to_type: 'User', assigned_by_id: user1.id, silent: false }) + Jobs::AssignNotification.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: user2.id, assigned_to_type: 'User', assigned_by_id: user1.id, silent: false }) expect { - described_class.new.execute({ topic_id: topic.id, assigned_to_id: user2.id, assigned_to_type: 'User' }) + described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: user2.id, assigned_to_type: 'User' }) }.to change { user2.notifications.count }.by(-1) end @@ -39,7 +39,7 @@ RSpec.describe Jobs::UnassignNotification do assign_allowed_group.add(user) assert_publish_topic_state(pm, user) do - described_class.new.execute({ topic_id: pm.id, assigned_to_id: pm.allowed_users.first.id, assigned_to_type: 'User' }) + described_class.new.execute({ topic_id: pm.id, post_id: pm_post.id, assigned_to_id: pm.allowed_users.first.id, assigned_to_type: 'User' }) end end end @@ -55,10 +55,10 @@ RSpec.describe Jobs::UnassignNotification do end it 'deletes notifications' do - Jobs::AssignNotification.new.execute({ topic_id: topic.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, silent: false }) + Jobs::AssignNotification.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: group.id, assigned_to_type: 'Group', assigned_by_id: user1.id, silent: false }) expect { - described_class.new.execute({ topic_id: topic.id, assigned_to_id: group.id, assigned_to_type: 'Group' }) + described_class.new.execute({ topic_id: topic.id, post_id: post.id, assigned_to_id: group.id, assigned_to_type: 'Group' }) }.to change { Notification.count }.by(-2) end end diff --git a/spec/serializers/flagged_topic_serializer_spec.rb b/spec/serializers/flagged_topic_serializer_spec.rb index 16ee23d..8fde18b 100644 --- a/spec/serializers/flagged_topic_serializer_spec.rb +++ b/spec/serializers/flagged_topic_serializer_spec.rb @@ -45,8 +45,9 @@ describe FlaggedTopicSerializer do expect(json[:flagged_topic][:assigned_to_user]).to match({ username: user.username, name: user.name, + assign_icon: "user-plus", avatar_template: /letter_avatar_proxy.*/, - assigned_at: Assignment.last.created_at, + assign_path: "/u/#{user.username}/activity/assigned", }) expect(json[:flagged_topic]).to_not have_key(:assigned_to_group) end @@ -75,7 +76,8 @@ describe FlaggedTopicSerializer do flair_color: assign_allowed_group.flair_color, flair_icon: assign_allowed_group.flair_icon, flair_upload_id: assign_allowed_group.flair_upload_id, - assigned_at: Assignment.last.created_at, + assign_icon: "group-plus", + assign_path: "/g/#{assign_allowed_group.name}/assigned/everyone", }) expect(json[:flagged_topic]).to_not have_key(:assigned_to_user) end diff --git a/test/javascripts/acceptance/assign-enabled-test.js.es6 b/test/javascripts/acceptance/assign-enabled-test.js.es6 index aaa8161..dcbefbb 100644 --- a/test/javascripts/acceptance/assign-enabled-test.js.es6 +++ b/test/javascripts/acceptance/assign-enabled-test.js.es6 @@ -80,6 +80,22 @@ acceptance("Discourse Assign | Assign desktop", function (needs) { return helper.response({ success: true }); }); }); + test("Post contains hidden assign button", async (assert) => { + updateCurrentUser({ can_assign: true }); + await visit("/t/internationalization-localization/280"); + + assert.ok( + !exists("#post_2 .extra-buttons .d-icon-user-plus"), + "assign to post button is hidden" + ); + await click("#post_2 button.show-more-actions"); + assert.ok( + exists("#post_2 .extra-buttons .d-icon-user-plus"), + "assign to post button exists" + ); + await click("#post_2 .extra-buttons .d-icon-user-plus"); + assert.ok(exists(".assign.modal-body"), "assign modal opens"); + }); test("Footer dropdown contains button", async (assert) => { updateCurrentUser({ can_assign: true }); diff --git a/test/javascripts/acceptance/assigned-topic-test.js.es6 b/test/javascripts/acceptance/assigned-topic-test.js.es6 index 6dcadcf..2877035 100644 --- a/test/javascripts/acceptance/assigned-topic-test.js.es6 +++ b/test/javascripts/acceptance/assigned-topic-test.js.es6 @@ -25,7 +25,6 @@ acceptance("Discourse Assign | Assigned topic", function (needs) { name: "Robin Ward", avatar_template: "/letter_avatar/eviltrout/{size}/3_f9720745f5ce6dfc2b5641fca999d934.png", - assigned_at: "2021-06-13T16:33:14.189Z", }; return helper.response(topic); }); @@ -34,7 +33,6 @@ acceptance("Discourse Assign | Assigned topic", function (needs) { let topic = cloneJSON(topicFixtures["/t/28830/1.json"]); topic["assigned_to_group"] = { name: "Developers", - assigned_at: "2021-06-13T16:33:14.189Z", }; return helper.response(topic); });