diff --git a/app/controllers/discourse_assign/assign_controller.rb b/app/controllers/discourse_assign/assign_controller.rb index faf9fa8..9e9521c 100644 --- a/app/controllers/discourse_assign/assign_controller.rb +++ b/app/controllers/discourse_assign/assign_controller.rb @@ -167,6 +167,10 @@ module DiscourseAssign { error: I18n.t('discourse_assign.already_assigned', username: user.username) } when :forbidden_assign_to { error: I18n.t('discourse_assign.forbidden_assign_to', username: user.username) } + when :forbidden_assignee_not_pm_participant + { error: I18n.t('discourse_assign.forbidden_assignee_not_pm_participant', username: user.username) } + when :forbidden_assignee_cant_see_topic + { error: I18n.t('discourse_assign.forbidden_assignee_cant_see_topic', username: user.username) } else max = SiteSetting.max_assigned_topics { error: I18n.t('discourse_assign.too_many_assigns', username: user.username, max: max) } diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index beec70c..170acec 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -18,18 +18,20 @@ en: assigned_to: "Topic assigned to @%{username}" unassigned: "Topic was unassigned" already_claimed: "That topic has already been claimed." - already_assigned: 'Topic is already assigned to @%{username}' + already_assigned: "Topic is already assigned to @%{username}" too_many_assigns: "@%{username} has already reached the maximum number of assigned topics (%{max})." 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." flag_assigned: "Sorry, that flag's topic is assigned to another user" flag_unclaimed: "You must claim that topic before acting on the flag" topic_assigned_excerpt: "assigned you the topic '%{title}'" reminders_frequency: - never: 'never' - daily: 'daily' - weekly: 'weekly' - monthly: 'monthly' - quarterly: 'quarterly' + never: "never" + daily: "daily" + weekly: "weekly" + monthly: "monthly" + quarterly: "quarterly" assign_mailer: title: "Assign Mailer" subject_template: "[%{email_prefix}] %{assignee_name} assigned you to '%{topic_title}'!" @@ -48,7 +50,7 @@ en: %{newest_assignments} %{oldest_assignments} - + This reminder will be sent %{frequency} if you have more than one assigned topic. newest: | ### Newest diff --git a/lib/topic_assigner.rb b/lib/topic_assigner.rb index 4183403..6d9fb05 100644 --- a/lib/topic_assigner.rb +++ b/lib/topic_assigner.rb @@ -135,11 +135,18 @@ class ::TopicAssigner end def can_be_assigned?(assign_to) - return false unless allowed_user_ids.include?(assign_to.id) - Guardian.new(assign_to).can_see_topic?(@topic) + allowed_user_ids.include?(assign_to.id) + end + + def can_assignee_see_topic?(assignee) + Guardian.new(assignee).can_see_topic?(@topic) end def assign(assign_to, silent: false) + if !can_assignee_see_topic?(assign_to) + reason = @topic.private_message? ? :forbidden_assignee_not_pm_participant : :forbidden_assignee_cant_see_topic + return { success: false, reason: reason } + end return { success: false, reason: :forbidden_assign_to } unless can_be_assigned?(assign_to) return { success: false, reason: :already_assigned } if @topic.custom_fields && @topic.custom_fields[ASSIGNED_TO_ID] == assign_to.id.to_s return { success: false, reason: :too_many_assigns } unless can_assign_to?(assign_to) diff --git a/spec/lib/topic_assigner_spec.rb b/spec/lib/topic_assigner_spec.rb index bf72dd1..638f59e 100644 --- a/spec/lib/topic_assigner_spec.rb +++ b/spec/lib/topic_assigner_spec.rb @@ -195,14 +195,14 @@ RSpec.describe TopicAssigner do assign = TopicAssigner.new(pm, admin).assign(moderator) expect(assign[:success]).to eq(false) - expect(assign[:reason]).to eq(:forbidden_assign_to) + expect(assign[:reason]).to eq(:forbidden_assignee_not_pm_participant) end it 'fails to assign when the assigned user cannot view the topic' do assign = TopicAssigner.new(secure_topic, admin).assign(moderator) expect(assign[:success]).to eq(false) - expect(assign[:reason]).to eq(:forbidden_assign_to) + expect(assign[:reason]).to eq(:forbidden_assignee_cant_see_topic) end it "assigns the PM to the moderator when it's included in the list of allowed users" do diff --git a/spec/requests/assign_controller_spec.rb b/spec/requests/assign_controller_spec.rb index b040bc6..62bf77b 100644 --- a/spec/requests/assign_controller_spec.rb +++ b/spec/requests/assign_controller_spec.rb @@ -143,6 +143,32 @@ RSpec.describe DiscourseAssign::AssignController do I18n.t('discourse_assign.too_many_assigns', username: another_user.username, max: max_assigns) ) end + + it 'fails with a specific error message if the topic is a PM and the assignee can not see it' do + pm = Fabricate(:private_message_post, user: user).topic + another_user = Fabricate(:user) + add_to_assign_allowed_group(another_user) + put '/assign/assign.json', params: { + topic_id: pm.id, username: another_user.username + } + + expect(response.parsed_body['error']).to eq( + I18n.t('discourse_assign.forbidden_assignee_not_pm_participant', username: another_user.username) + ) + end + + it 'fails with a specific error message if the topic is not a PM and the assignee can not see it' do + topic = Fabricate(:topic, category: Fabricate(:private_category, group: Fabricate(:group))) + another_user = Fabricate(:user) + add_to_assign_allowed_group(another_user) + put '/assign/assign.json', params: { + topic_id: topic.id, username: another_user.username + } + + expect(response.parsed_body['error']).to eq( + I18n.t('discourse_assign.forbidden_assignee_cant_see_topic', username: another_user.username) + ) + end end context '#assigned' do