Skip to content
This repository was archived by the owner on Jul 14, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions app/controllers/discourse_assign/assign_controller.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
# frozen_string_literal: true

module DiscourseAssign
class AssignController < Admin::AdminController
before_action :ensure_logged_in
class AssignController < ApplicationController
requires_login
before_action :ensure_logged_in, :ensure_assign_allowed

def suggestions
users = [current_user]
users += User
.where('admin OR moderator')
.where('users.id <> ?', current_user.id)
.joins(<<~SQL
JOIN(
Expand All @@ -18,7 +18,9 @@ def suggestions
HAVING COUNT(*) < #{SiteSetting.max_assigned_topics}
) as X ON X.user_id = users.id
SQL
).order('X.last_assigned DESC')
)
.assign_allowed
.order('X.last_assigned DESC')
.limit(6)

render json: ActiveModel::ArraySerializer.new(users,
Expand Down Expand Up @@ -88,5 +90,9 @@ def translate_failure(reason, user)
{ error: I18n.t('discourse_assign.too_many_assigns', username: user.username, max: max) }
end
end

def ensure_assign_allowed
raise Discourse::InvalidAccess.new unless current_user.can_assign?
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { getOwner } from "discourse-common/lib/get-owner";
export default {
shouldRender(args, component) {
const needsButton =
component.currentUser && component.currentUser.get("staff");
component.currentUser && component.currentUser.get("can_assign");
return (
needsButton &&
(!component.get("site.mobileView") || args.topic.get("isPrivateMessage"))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{{#if currentUser.staff}}
{{#if currentUser.can_assign}}
{{#link-to 'userActivity.assigned'}}
{{d-icon "user-plus"}} {{i18n 'discourse_assign.assigned'}}
{{/link-to}}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export function shouldShowAssigned(args, component) {
const needsButton =
component.currentUser && component.currentUser.get("staff");
component.currentUser && component.currentUser.get("can_assign");
return (
needsButton &&
(!component.get("site.mobileView") || args.model.get("isPrivateMessage"))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export default {
shouldRender(args, component) {
return component.currentUser && component.currentUser.get("staff");
return component.currentUser && component.currentUser.get("can_assign");
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { ajax } from "discourse/lib/ajax";
import { popupAjaxError } from "discourse/lib/ajax-error";

export default Ember.Controller.extend({
taskActions: Ember.inject.service(),
assignSuggestions: function() {
ajax("/assign/suggestions").then(users => {
this.set("assignSuggestions", users);
Expand All @@ -23,6 +24,7 @@ export default Ember.Controller.extend({
actions: {
assignUser(user) {
this.set("model.username", user.username);
this.set("model.allowedGroups", this.get("taskActions").allowedGroups);
this.send("assign");
},

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ function modifySelectKit(api) {
api
.modifySelectKit("topic-footer-mobile-dropdown")
.modifyContent((context, existingContent) => {
if (context.get("currentUser.staff")) {
if (context.get("currentUser.can_assign")) {
const hasAssignement = context.get("topic.assigned_to_user");
const button = {
id: ACTION_ID,
Expand All @@ -31,7 +31,7 @@ function modifySelectKit(api) {
return existingContent;
})
.onSelect((context, value) => {
if (!context.get("currentUser.staff") || value !== ACTION_ID) {
if (!context.get("currentUser.can_assign") || value !== ACTION_ID) {
return;
}

Expand Down Expand Up @@ -157,7 +157,7 @@ function initialize(api) {
});

api.addUserMenuGlyph(widget => {
if (widget.currentUser && widget.currentUser.get("staff")) {
if (widget.currentUser && widget.currentUser.get("can_assign")) {
return {
label: "discourse_assign.assigned",
className: "assigned",
Expand Down
12 changes: 11 additions & 1 deletion assets/javascripts/discourse/services/task-actions.js.es6
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
import { ajax } from "discourse/lib/ajax";
import showModal from "discourse/lib/show-modal";
import { getOwner } from "discourse-common/lib/get-owner";

export default Ember.Service.extend({
init() {
this._super(...arguments);

this.allowedGroups = getOwner(this)
.lookup("site-settings:main")
.assign_allowed_on_groups.split("|");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this.siteSettings not work here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, that's why I had to use this 😨

},

unassign(topicId) {
return ajax("/assign/unassign", {
type: "PUT",
Expand All @@ -13,7 +22,8 @@ export default Ember.Service.extend({
return showModal("assign-user", {
model: {
topic: topic,
username: topic.get("assigned_to_user.username")
username: topic.get("assigned_to_user.username"),
allowedGroups: this.allowedGroups
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
single=true
allowAny=false
group="staff"
groupMembersOf=model.allowedGroups
excludeCurrentUser=false
includeMentionableGroups=false
hasGroups=false
Expand Down
1 change: 1 addition & 0 deletions config/locales/server.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ en:
remind_assigns: "Remind users about pending assigns."
remind_assigns_frequency: "Frequency for reminding users about assigned topics."
max_assigned_topics: "Maximum number of topics that can be assigned to a user."
assign_allowed_on_groups: "Groups that are allowed to assign topics."
discourse_assign:
assigned_to: "Topic assigned to @%{username}"
unassigned: "Topic was unassigned"
Expand Down
7 changes: 7 additions & 0 deletions config/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,10 @@ plugins:
client: true
default: 10
min: 1
assign_allowed_on_groups:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this as a site setting and I feel like this should be a group option stored in a group custom fields. The trouble with a site setting is that when a group is renamed or deleted, the site settings becomes inaccurate. Having to handle that in a nice manner isn't going to be easy.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the site setting makes it easier to manage multiple allowed groups. Maybe we can track when a group gets renamed/deleted and make sure it remains synced?

Copy link
Contributor

@tgxworld tgxworld May 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm maybe but I still think that having to maintain this sync is quite tricky. Maybe have a look how type: category for site settings is handled? I'm not sure myself but worth exploring.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ended up using type: group_list instead.

client: true
type: group_list
list_type: compact
default: 'staff'
allow_any: false
refresh: true
9 changes: 7 additions & 2 deletions jobs/scheduled/enqueue_reminders.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ def skip_enqueue?
SiteSetting.remind_assigns_frequency.nil? || !SiteSetting.assign_enabled?
end

def allowed_group_ids
allowed_groups = SiteSetting.assign_allowed_on_groups.split('|')
Group.where(name: allowed_groups).pluck(:id).join(',')
end

def user_ids
global_frequency = SiteSetting.remind_assigns_frequency
frequency = ActiveRecord::Base.sanitize_sql("COALESCE(user_frequency.value, '#{global_frequency}')::INT")
Expand All @@ -31,10 +36,10 @@ def user_ids
ON topic_custom_fields.value::INT = user_frequency.user_id
AND user_frequency.name = '#{PendingAssignsReminder::REMINDERS_FREQUENCY}'

INNER JOIN users ON topic_custom_fields.value::INT = users.id
INNER JOIN group_users ON topic_custom_fields.value::INT = group_users.user_id
INNER JOIN topics ON topics.id = topic_custom_fields.topic_id AND (topics.deleted_at IS NULL)

WHERE (users.moderator OR users.admin)
WHERE group_users.group_id IN (#{allowed_group_ids})
AND #{frequency} > 0
AND (
last_reminder.value IS NULL OR
Expand Down
51 changes: 25 additions & 26 deletions lib/topic_assigner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,20 @@ class ::TopicAssigner
ASSIGNED_BY_ID = 'assigned_by_id'

def self.backfill_auto_assign
staff_mention = User.where('moderator OR admin')
staff_mention = User.assign_allowed
.pluck('username')
.map { |name| "p.cooked ILIKE '%mention%@#{name}%'" }
.map! { |name| "p.cooked ILIKE '%mention%@#{name}%'" }
.join(' OR ')

sql = <<SQL
SELECT p.topic_id, MAX(post_number) post_number
FROM posts p
JOIN topics t ON t.id = p.topic_id
LEFT JOIN topic_custom_fields tc ON tc.name = '#{ASSIGNED_TO_ID}' AND tc.topic_id = p.topic_id
WHERE p.user_id IN (SELECT id FROM users WHERE moderator OR admin) AND
( #{staff_mention} ) AND tc.value IS NULL AND NOT t.closed AND t.deleted_at IS NULL
GROUP BY p.topic_id
SQL
sql = <<~SQL
SELECT p.topic_id, MAX(post_number) post_number
FROM posts p
JOIN topics t ON t.id = p.topic_id
LEFT JOIN topic_custom_fields tc ON tc.name = '#{ASSIGNED_TO_ID}' AND tc.topic_id = p.topic_id
WHERE p.user_id IN (SELECT id FROM users WHERE moderator OR admin) AND
( #{staff_mention} ) AND tc.value IS NULL AND NOT t.closed AND t.deleted_at IS NULL
GROUP BY p.topic_id
SQL

assigned = 0
puts
Expand Down Expand Up @@ -54,7 +54,7 @@ def self.assign_other_passes?(post)
def self.auto_assign(post, force: false)
return unless SiteSetting.assigns_by_staff_mention

if post.user && post.topic && post.user.staff?
if post.user && post.topic && post.user.can_assign?
can_assign = force || post.topic.custom_fields[ASSIGNED_TO_ID].nil?

assign_other = assign_other_passes?(post) && mentioned_staff(post)
Expand All @@ -72,32 +72,31 @@ def self.auto_assign(post, force: false)
end

def self.is_last_staff_post?(post)
allowed_user_ids = User.assign_allowed.pluck(:id).join(',')

sql = <<~SQL
SELECT 1 FROM posts p
JOIN users u ON u.id = p.user_id AND (moderator OR admin)
JOIN users u ON u.id = p.user_id
WHERE p.deleted_at IS NULL AND p.topic_id = :topic_id
AND u.id IN (#{allowed_user_ids})
having max(post_number) = :post_number

SQL

args = {
topic_id: post.topic_id,
post_number: post.post_number
}

# TODO post 2.1 release remove
if defined?(DB)
DB.exec(sql, args) == 1
else
Post.exec_sql(sql, args).to_a.length == 1
end
DB.exec(sql, args) == 1
end

def self.mentioned_staff(post)
mentions = post.raw_mentions
if mentions.present?
User.where('moderator OR admin')
.human_users
allowed_groups = SiteSetting.assign_allowed_on_groups.split('|')

User.human_users
.joins(:groups).where(groups: { name: allowed_groups })
.where('username_lower IN (?)', mentions.map(&:downcase))
.first
end
Expand All @@ -108,8 +107,8 @@ def initialize(topic, user)
@topic = topic
end

def staff_ids
User.real.staff.pluck(:id)
def allowed_user_ids
User.assign_allowed.pluck(:id)
end

def can_assign_to?(user)
Expand Down Expand Up @@ -143,7 +142,7 @@ def assign(assign_to, silent: false)
topic_id: @topic.id,
assigned_to: BasicUserSerializer.new(assign_to, root: false).as_json
},
user_ids: staff_ids
user_ids: allowed_user_ids
)

publish_topic_tracking_state(@topic, assign_to.id)
Expand Down Expand Up @@ -274,7 +273,7 @@ def unassign(silent: false)
type: 'unassigned',
topic_id: @topic.id,
},
user_ids: staff_ids
user_ids: allowed_user_ids
)

publish_topic_tracking_state(@topic, assigned_user.id)
Expand Down
Loading