From 5888025dec4fc9aeb18bb9db3e732cbf9735e584 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Fri, 8 Dec 2023 11:48:34 +0100 Subject: [PATCH] FIX: Various assignment status issues (#536) --- .../discourse/components/assign-user-form.hbs | 2 +- .../discourse/components/assign-user-form.js | 4 +- .../components/bulk-actions/assign-user.js | 4 +- .../components/modal/assign-user.hbs | 4 +- .../discourse/components/modal/assign-user.js | 9 ++- .../initializers/extend-for-assigns.js | 67 +++++-------------- assets/stylesheets/assigns.scss | 9 +-- lib/assigner.rb | 5 +- plugin.rb | 7 +- .../acceptance/assign-enabled-test.js | 41 +++++++++--- .../acceptance/bulk-actions-test.js | 42 +++++++----- 11 files changed, 99 insertions(+), 95 deletions(-) diff --git a/assets/javascripts/discourse/components/assign-user-form.hbs b/assets/javascripts/discourse/components/assign-user-form.hbs index 2aeb2a5b..c9ced8fa 100644 --- a/assets/javascripts/discourse/components/assign-user-form.hbs +++ b/assets/javascripts/discourse/components/assign-user-form.hbs @@ -36,7 +36,7 @@ @id="assign-status" @content={{this.availableStatuses}} @value={{this.status}} - @onChange={{action (mut @model.status)}} + @onChange={{fn (mut @model.status)}} /> {{/if}} diff --git a/assets/javascripts/discourse/components/assign-user-form.js b/assets/javascripts/discourse/components/assign-user-form.js index ce8fa9b2..0501018d 100644 --- a/assets/javascripts/discourse/components/assign-user-form.js +++ b/assets/javascripts/discourse/components/assign-user-form.js @@ -26,9 +26,7 @@ export default class AssignUserForm extends Component { get status() { return ( - this.args.model.status || - this.args.model.target.assignment_status || - this.siteSettings.assign_statuses.split("|")[0] + this.args.model.status || this.siteSettings.assign_statuses.split("|")[0] ); } diff --git a/assets/javascripts/discourse/components/bulk-actions/assign-user.js b/assets/javascripts/discourse/components/bulk-actions/assign-user.js index 3636fa36..66112122 100644 --- a/assets/javascripts/discourse/components/bulk-actions/assign-user.js +++ b/assets/javascripts/discourse/components/bulk-actions/assign-user.js @@ -1,8 +1,9 @@ import Component from "@glimmer/component"; import { action } from "@ember/object"; +import { TrackedObject } from "@ember-compat/tracked-built-ins"; export default class AssignUser extends Component { - model = {}; + model = new TrackedObject({}); // `submit` property will be mutated by the `AssignUserForm` component formApi = { @@ -14,6 +15,7 @@ export default class AssignUser extends Component { return this.args.performAndRefresh({ type: "assign", username: this.model.username, + status: this.model.status, note: this.model.note, }); } diff --git a/assets/javascripts/discourse/components/modal/assign-user.hbs b/assets/javascripts/discourse/components/modal/assign-user.hbs index bd730068..d543f90b 100644 --- a/assets/javascripts/discourse/components/modal/assign-user.hbs +++ b/assets/javascripts/discourse/components/modal/assign-user.hbs @@ -1,7 +1,7 @@ <:body> @@ -12,7 +12,7 @@ class="btn-primary" @action={{this.formApi.submit}} @label={{if - @model.reassign + this.model.reassign "discourse_assign.reassign.title" "discourse_assign.assign_modal.assign" }} diff --git a/assets/javascripts/discourse/components/modal/assign-user.js b/assets/javascripts/discourse/components/modal/assign-user.js index 938a39aa..fb68511d 100644 --- a/assets/javascripts/discourse/components/modal/assign-user.js +++ b/assets/javascripts/discourse/components/modal/assign-user.js @@ -1,11 +1,14 @@ import Component from "@glimmer/component"; import { action } from "@ember/object"; import { inject as service } from "@ember/service"; +import { TrackedObject } from "@ember-compat/tracked-built-ins"; import I18n from "I18n"; export default class AssignUser extends Component { @service taskActions; + model = new TrackedObject(this.args.model); + // `submit` property will be mutated by the `AssignUserForm` component formApi = { submit() {}, @@ -14,7 +17,7 @@ export default class AssignUser extends Component { get title() { let i18nSuffix; - switch (this.args.model.targetType) { + switch (this.model.targetType) { case "Post": i18nSuffix = "_post_modal"; break; @@ -25,7 +28,7 @@ export default class AssignUser extends Component { return I18n.t( `discourse_assign.assign${i18nSuffix}.${ - this.args.model.reassign ? "reassign_title" : "title" + this.model.reassign ? "reassign_title" : "title" }` ); } @@ -33,6 +36,6 @@ export default class AssignUser extends Component { @action async onSubmit() { this.args.closeModal(); - await this.taskActions.assign(this.args.model); + await this.taskActions.assign(this.model); } } diff --git a/assets/javascripts/discourse/initializers/extend-for-assigns.js b/assets/javascripts/discourse/initializers/extend-for-assigns.js index 76600a8d..e1e66879 100644 --- a/assets/javascripts/discourse/initializers/extend-for-assigns.js +++ b/assets/javascripts/discourse/initializers/extend-for-assigns.js @@ -616,7 +616,6 @@ function initialize(api) { topicAssignee, assignedToIndirectly.map((assigned) => ({ assignee: assigned.assigned_to, - status: assigned.assignment_status, note: assigned.assignment_note, })) ) @@ -928,57 +927,23 @@ export default { api.addUserSearchOption("assignableGroups"); - if (api.addBulkActionButton) { - api.addBulkActionButton({ - label: "topics.bulk.assign", - icon: "user-plus", - class: "btn-default assign-topics", - action({ setComponent }) { - setComponent(BulkAssign); - }, - }); - - api.addBulkActionButton({ - label: "topics.bulk.unassign", - icon: "user-times", - class: "btn-default unassign-topics", - action({ performAndRefresh }) { - performAndRefresh({ type: "unassign" }); - }, - }); - } else { - // TODO: Remove this path after core 3.1.0.beta7 is released - const { - default: TopicButtonAction, - addBulkButton, - } = require("discourse/controllers/topic-bulk-actions"); - - TopicButtonAction.reopen({ - actions: { - showReAssign() { - const controller = getOwner(this).lookup( - "controller:bulk-assign" - ); - controller.set("model", { username: "", note: "" }); - this.send("changeBulkTemplate", "modal/bulk-assign"); - }, - - unassignTopics() { - this.performAndRefresh({ type: "unassign" }); - }, - }, - }); - - addBulkButton("showReAssign", "assign", { - icon: "user-plus", - class: "btn-default assign-topics", - }); + api.addBulkActionButton({ + label: "topics.bulk.assign", + icon: "user-plus", + class: "btn-default assign-topics", + action({ setComponent }) { + setComponent(BulkAssign); + }, + }); - addBulkButton("unassignTopics", "unassign", { - icon: "user-times", - class: "btn-default unassign-topics", - }); - } + api.addBulkActionButton({ + label: "topics.bulk.unassign", + icon: "user-times", + class: "btn-default unassign-topics", + action({ performAndRefresh }) { + performAndRefresh({ type: "unassign" }); + }, + }); }); }, }; diff --git a/assets/stylesheets/assigns.scss b/assets/stylesheets/assigns.scss index 3a803205..bc9db180 100644 --- a/assets/stylesheets/assigns.scss +++ b/assets/stylesheets/assigns.scss @@ -66,13 +66,8 @@ margin-left: 5px; } -.modal.assign { - .modal-inner-container { - width: 400px; - } - - .d-modal__body, - .modal-body { +.d-modal.assign { + .d-modal__body { overflow-y: unset; } diff --git a/lib/assigner.rb b/lib/assigner.rb index f18b5867..65efca02 100644 --- a/lib/assigner.rb +++ b/lib/assigner.rb @@ -223,14 +223,15 @@ def forbidden_reasons(assign_to:, type:, note:, status:, allow_self_reassign:) def update_details(assign_to, note, status, skip_small_action_post: false) case - when @target.assignment.note != note && @target.assignment.status != status && status.present? + when note.present? && status.present? && @target.assignment.note != note && + @target.assignment.status != status small_action_text = <<~TEXT Status: #{@target.assignment.status} → #{status} #{note} TEXT change_type = "details" - when @target.assignment.note != note + when note.present? && @target.assignment.note != note small_action_text = note change_type = "note" when @target.assignment.status != status diff --git a/plugin.rb b/plugin.rb index fb4d9f22..cf4ea328 100644 --- a/plugin.rb +++ b/plugin.rb @@ -645,7 +645,11 @@ module ::DiscourseAssign if @user.can_assign? assign_user = User.find_by_username(@operation[:username]) topics.each do |topic| - Assigner.new(topic, @user).assign(assign_user, note: @operation[:note]) + Assigner.new(topic, @user).assign( + assign_user, + status: @operation[:status], + note: @operation[:note], + ) end end end @@ -657,6 +661,7 @@ module ::DiscourseAssign end register_permitted_bulk_action_parameter :username + register_permitted_bulk_action_parameter :status register_permitted_bulk_action_parameter :note add_to_class(:user_bookmark_base_serializer, :assigned_to) do diff --git a/test/javascripts/acceptance/assign-enabled-test.js b/test/javascripts/acceptance/assign-enabled-test.js index 05eb477b..9c0d211b 100644 --- a/test/javascripts/acceptance/assign-enabled-test.js +++ b/test/javascripts/acceptance/assign-enabled-test.js @@ -44,7 +44,7 @@ acceptance("Discourse Assign | Assign mobile", function (needs) { assert.true(menu.rowByValue("assign").exists()); await menu.selectRowByValue("assign"); - assert.dom(".assign.modal").exists("assign modal opens"); + assert.dom(".assign.d-modal").exists("assign modal opens"); }); }); @@ -86,12 +86,12 @@ acceptance("Discourse Assign | Assign desktop", function (needs) { .exists("assign to post button exists"); await click("#post_2 .extra-buttons .d-icon-user-plus"); - assert.dom(".assign.modal").exists("assign modal opens"); + assert.dom(".assign.d-modal").exists("assign modal opens"); - const menu = selectKit(".assign.modal .user-chooser"); + const menu = selectKit(".assign.d-modal .user-chooser"); assert.true(menu.isExpanded(), "user selector is expanded"); - await click(".assign.modal .btn-primary"); + await click(".assign.d-modal .btn-primary"); assert.dom(".error-label").includesText("Choose a user to assign"); await menu.expand(); @@ -108,16 +108,16 @@ acceptance("Discourse Assign | Assign desktop", function (needs) { }); await fillIn("#assign-modal-note", "a note!"); - await click(".assign.modal .btn-primary"); + await click(".assign.d-modal .btn-primary"); - assert.dom(".assign.modal").doesNotExist("assign modal closes"); + assert.dom(".assign.d-modal").doesNotExist("assign modal closes"); }); test("Footer dropdown contains button", async function (assert) { await visit("/t/internationalization-localization/280"); await click("#topic-footer-button-assign"); - assert.dom(".assign.modal").exists("assign modal opens"); + assert.dom(".assign.d-modal").exists("assign modal opens"); }); }); @@ -151,12 +151,35 @@ acceptance("Discourse Assign | Assign Status enabled", function (needs) { }); test("Modal contains status dropdown", async function (assert) { + pretender.put("/assign/assign", ({ requestBody }) => { + const body = parsePostData(requestBody); + assert.strictEqual(body.target_type, "Topic"); + assert.strictEqual(body.target_id, "280"); + assert.strictEqual(body.username, "eviltrout"); + assert.strictEqual(body.status, "In Progress"); + + return response({ success: true }); + }); + await visit("/t/internationalization-localization/280"); await click("#topic-footer-button-assign"); assert - .dom(".assign.modal #assign-status") + .dom(".assign.d-modal #assign-status") .exists("assign status dropdown exists"); + + const statusDropdown = selectKit("#assign-status"); + assert.strictEqual(statusDropdown.header().value(), "New"); + + await statusDropdown.expand(); + await statusDropdown.selectRowByValue("In Progress"); + assert.strictEqual(statusDropdown.header().value(), "In Progress"); + + const menu = selectKit(".assign.d-modal .user-chooser"); + await menu.expand(); + await menu.selectRowByIndex(0); + + await click(".assign.d-modal .btn-primary"); }); }); @@ -190,7 +213,7 @@ acceptance("Discourse Assign | Assign Status disabled", function (needs) { await click("#topic-footer-button-assign"); assert - .dom(".assign.modal #assign-status") + .dom(".assign.d-modal #assign-status") .doesNotExist("assign status dropdown doesn't exists"); }); }); diff --git a/test/javascripts/acceptance/bulk-actions-test.js b/test/javascripts/acceptance/bulk-actions-test.js index c27415f6..a79005d0 100644 --- a/test/javascripts/acceptance/bulk-actions-test.js +++ b/test/javascripts/acceptance/bulk-actions-test.js @@ -13,7 +13,10 @@ acceptance("Discourse Assign | Bulk actions", function (needs) { moderator: true, can_assign: true, }); - needs.settings({ assign_enabled: true }); + needs.settings({ + assign_enabled: true, + enable_assign_status: true, + }); needs.pretender((server, helper) => { server.get("/assign/suggestions", () => { @@ -35,6 +38,22 @@ acceptance("Discourse Assign | Bulk actions", function (needs) { }); test("Assigning users to topics", async function (assert) { + pretender.put("/topics/bulk", ({ requestBody }) => { + const body = parsePostData(requestBody); + assert.deepEqual(body.operation, { + type: "assign", + username: "eviltrout", + status: "In Progress", + note: "a note!", + }); + assert.deepEqual(body["topic_ids[]"], [ + topic1.dataset.topicId, + topic2.dataset.topicId, + ]); + + return response({ success: true }); + }); + await visit("/latest"); await click("button.bulk-select"); @@ -61,22 +80,15 @@ acceptance("Discourse Assign | Bulk actions", function (needs) { await menu.selectRowByIndex(0); assert.strictEqual(menu.header().value(), "eviltrout"); - pretender.put("/topics/bulk", ({ requestBody }) => { - const body = parsePostData(requestBody); - assert.deepEqual(body.operation, { - type: "assign", - username: "eviltrout", - note: "a note!", - }); - assert.deepEqual(body["topic_ids[]"], [ - topic1.dataset.topicId, - topic2.dataset.topicId, - ]); + await fillIn("#assign-modal-note", "a note!"); - return response({ success: true }); - }); + const statusDropdown = selectKit("#assign-status"); + assert.strictEqual(statusDropdown.header().value(), "New"); + + await statusDropdown.expand(); + await statusDropdown.selectRowByValue("In Progress"); + assert.strictEqual(statusDropdown.header().value(), "In Progress"); - await fillIn("#assign-modal-note", "a note!"); await click(".topic-bulk-actions-modal .btn-primary"); }); });