FEATURE: Add buttons for unassigning from posts to the topic level assign menu #554
Conversation
AndrewPrigorshnev
force-pushed
the
feature/add-buttons-for-unassigning-from-posts-to-the-topic-level-assign-menu
branch
4 times, most recently
from
d2ab473
to
e0d3e6b
Compare
| @@ -15,6 +15,7 @@ import { iconHTML, iconNode } from "discourse-common/lib/icon-library"; | |||
| import discourseComputed from "discourse-common/utils/decorators"; | |||
| import I18n from "I18n"; | |||
| import BulkAssign from "../components/bulk-actions/assign-user"; | |||
| import TopicLevelAssignMenu from "../lib/topic-level-assign-menu"; | |||
There was a problem hiding this comment.
Maybe we could put this in /components? It feels more like a component than a lib.
Maybe it will cause an issue for now though, if we don't export a component, not sure.
There was a problem hiding this comment.
Yeah, this makes sense. We usually put such classes into the lib folder, but I agree that the components folder is a better place for it, even though technically speaking it's not an Ember component. I moved it into components/ and everything seem to work fine.
| @@ -917,7 +828,7 @@ export default { | |||
| } | |||
|
|
|||
| withPluginApi("0.13.0", (api) => { | |||
| includeIsAssignedOnTopic(api); | |||
| extendTopicModel(api); | |||
There was a problem hiding this comment.
I like the name change here 👍
assets/javascripts/discourse/initializers/extend-for-assigns.js
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,227 @@ | |||
| import { getOwner } from "@ember/application"; | |||
There was a problem hiding this comment.
Can you point me the new code here please?
There was a problem hiding this comment.
Yeah, sorry, it should be difficult to distinguish where is the old code and where the implementation of the new buttons. I extracted commits with refactoring and the new code. So now you can review it separately:
- REFACTOR: Extract topic level assign menu code into separate file
- REFACTOR: Extract methods (nothing new was added on this stage, I've just extracted some methods to make further changes easier)
- Implement
AndrewPrigorshnev
force-pushed
the
feature/add-buttons-for-unassigning-from-posts-to-the-topic-level-assign-menu
branch
3 times, most recently
from
781c7a6
to
1ca5dfa
Compare
| const map = new Map(); | ||
| this.assignees().forEach((user) => map.set(user.username, user)); | ||
| return [...map.values()]; |
There was a problem hiding this comment.
I think this could be a one-liner?
| const map = new Map(); | |
| this.assignees().forEach((user) => map.set(user.username, user)); | |
| return [...map.values()]; | |
| return [...new Set(this.assignees().map((u) => u.username))]; |
There was a problem hiding this comment.
Your version of code does another thing. We need to return an array of unique assignee objects here, while your one-liner returns an array of unique usernames.
AndrewPrigorshnev
force-pushed
the
feature/add-buttons-for-unassigning-from-posts-to-the-topic-level-assign-menu
branch
from
1ca5dfa
to
c4ec0b4
Compare
AndrewPrigorshnev
deleted the
feature/add-buttons-for-unassigning-from-posts-to-the-topic-level-assign-menu
branch
Before, we only had buttons for assigning and unassigning topic on this menu:
This PR adds dynamic buttons for unassigning posts to the menu: