# Inconsistent code / requirements for PM tagging

**URL:** https://meta.discourse.org/t/inconsistent-code-requirements-for-pm-tagging/362931
**Category:** Bug
**Tags:** tags, personal-messages
**Created:** [April 22, 2025, 8:14am UTC](https://meta.discourse.org/t/inconsistent-code-requirements-for-pm-tagging/362931 "2025-04-22T08:14:18Z")
**Posts on this page:** 1
**Page:** 1

<div class="post-metadata">

### Author: ![RGJ](https://sea3.discourse-cdn.com/meta/user_avatar/meta.discourse.org/rgj/32/523185_2.png) [@RGJ](https://meta.discourse.org/u/RGJ)
#### Post date: [April 22, 2025, 8:14am UTC](https://meta.discourse.org/t/inconsistent-code-requirements-for-pm-tagging/362931/1 "2025-04-22T08:14:18Z")

</div>

### The code for handling tagging of personal messages is inconsistent.

In some parts of the code base, being in `pm_tags_allowed_for_groups` is sufficient for being able to tag a personal message.

In other parts of the code base, one needs to be in both `pm_tags_allowed_for_groups` and `tag_topic_allowed_groups`.

This is especially restrictive when there is a need for users to be able to tag their own personal messages while you don’t want them to tag public topics (e.g. this [Private template permissions to override allowed PM tagging groups](https://meta.discourse.org/t/private-template-permissions-to-override-allowed-pm-tagging-groups/277513) ), i.e. when `pm_tags_allowed_for_groups` is not a proper subset of `tag_topic_allowed_groups`.

### Analysis

In [`TagGuardian`](https://github.com/discourse/discourse/blob/b0ab1b23214ec8377dbac5defa118e6a2427edf4/lib/guardian/tag_guardian.rb#L22) these are distinct settings. You only need to be in `pm_tags_allowed_for_groups` in order to be able to tag a PM.

```rb
  def can_tag_topics?
    SiteSetting.tagging_enabled && @user.in_any_groups?(SiteSetting.tag_topic_allowed_groups_map)
  end
  
  def can_tag_pms?
    return false if !SiteSetting.tagging_enabled
    return false if @user.blank?
    return true if @user == Discourse.system_user

    group_ids = SiteSetting.pm_tags_allowed_for_groups_map
    group_ids.include?(Group::AUTO_GROUPS[:everyone]) ||
      @user.group_users.exists?(group_id: group_ids)
  end

```

The same goes for [`Guardian.can_tag?`](https://github.com/discourse/discourse/blob/b2ce3746508b0fec369a42b0525699afe56596f7/lib/guardian.rb#L231) The settings are distinct.

```rb
  def can_tag?(topic)
    return false if topic.blank?

    topic.private_message? ? can_tag_pms? : can_tag_topics?
  end

```

[`ListController`](https://github.com/discourse/discourse/blob/b2ce3746508b0fec369a42b0525699afe56596f7/app/controllers/list_controller.rb#L205) is happy with only `can_tag_pms?` as well

```rb
    when :private_messages_tag
      raise Discourse::NotFound if target_user.id != current_user.id
      raise Discourse::NotFound if !guardian.can_tag_pms? 

```

However, [`TopicGuardian`](https://github.com/discourse/discourse/blob/b2ce3746508b0fec369a42b0525699afe56596f7/lib/guardian/topic_guardian.rb#L331) requires a user to be in both `tag_topic_allowed_groups` and `pm_tags_allowed_for_groups` if they want to be able to tag a PM.

```rb
  def can_edit_tags?(topic)
    return false unless can_tag_topics?
    return false if topic.private_message? && !can_tag_pms?
    return true if can_edit_topic?(topic)

    if topic&.first_post&.wiki &&
         @user.in_any_groups?(SiteSetting.edit_wiki_post_allowed_groups_map)
      return can_create_post?(topic)
    end

    false
  end

```

Client side, the same restriction is in [`Composer`](https://github.com/discourse/discourse/blob/b2ce3746508b0fec369a42b0525699afe56596f7/app/assets/javascripts/discourse/app/services/composer.js#L305)

```js
  @discourseComputed("model.canEditTitle", "model.creatingPrivateMessage")
  canEditTags(canEditTitle, creatingPrivateMessage) {
    const isPrivateMessage =
      creatingPrivateMessage || this.get("model.topic.isPrivateMessage");
    return (
      canEditTitle &&
      this.site.can_tag_topics &&
      (!isPrivateMessage || this.site.can_tag_pms)
    );
  }

```

and [`TopicController`](https://github.com/discourse/discourse/blob/b2ce3746508b0fec369a42b0525699afe56596f7/app/assets/javascripts/discourse/app/controllers/topic.js#L235)

```js
  @discourseComputed("model.isPrivateMessage")
  canEditTags(isPrivateMessage) {
    return (
      this.site.get("can_tag_topics") &&
      (!isPrivateMessage || this.site.get("can_tag_pms"))
    );
  }

```

But the [`Move to Topic`](https://github.com/discourse/discourse/blob/b2ce3746508b0fec369a42b0525699afe56596f7/app/assets/javascripts/discourse/app/components/modal/move-to-topic.gjs#L101) modal does not have this restriction:

```js
  get canTagMessages() {
    return this.site.can_tag_pms;
  }

```

and neither does [`ChatToTopicSelector`](https://github.com/discourse/discourse/blob/8367819b1b52e5b141ea8ced73216d798f7b243a/plugins/chat/assets/javascripts/discourse/components/chat-to-topic-selector.gjs#L28)

```js
@alias("site.can_tag_pms") canTagMessages;

```

and neither does [`UserPrivateMessagesController`](https://github.com/discourse/discourse/blob/b2ce3746508b0fec369a42b0525699afe56596f7/app/assets/javascripts/discourse/app/controllers/user-private-messages.js#L90)

```js
@readOnly("site.can_tag_pms") pmTaggingEnabled;

    if (this.pmTaggingEnabled) {
      content.push({
        id: this.router.urlFor("userPrivateMessages.tags", usernameLower),
        name: i18n("user.messages.tags"),
        icon: "tags",
      });
    }

```

### Summary

| Class | C/S | Behavior |
| --- | --- | --- |
| TagGuardian | server | pm\_tags\_allowed\_for\_groups only |
| Guardian | server | pm\_tags\_allowed\_for\_groups only |
| ListController | server | pm\_tags\_allowed\_for\_groups only |
| TopicGuardian | server | both pm\_tags\_allowed\_for\_groups and tag\_topic\_allowed\_groups |
| Composer | client | both pm\_tags\_allowed\_for\_groups and tag\_topic\_allowed\_groups |
| TopicController | client | both pm\_tags\_allowed\_for\_groups and tag\_topic\_allowed\_groups |
| Move to Topic modal | client | pm\_tags\_allowed\_for\_groups only |
| ChatToTopicSelector | client | pm\_tags\_allowed\_for\_groups only |
| | | |

### Additional issues

I found two more additional impediments:

[`PostRevisor`](https://github.com/discourse/discourse/blob/main/lib/post_revisor.rb#L109C1-L110C35) uses `tc.guardian.can_tag_topics?` instead of `tc.guardian.can_tag?(tc.topic)`

and in `DiscourseTagging`

`tag_topic_by_names` does the right thing

```rb
  def self.tag_topic_by_names(topic, guardian, tag_names_arg, append: false)
    if guardian.can_tag?(topic)    

```

but then [`tags_for_saving`](https://github.com/discourse/discourse/blob/main/lib/discourse_tagging.rb#L733) does not

```rb

   def self.tags_for_saving(tags_arg, guardian, opts = {})
    return [] unless guardian.can_tag_topics? && tags_arg.present?

```

This last one is especially nasty since the function is not aware of whether it is working on a PM.