Clarification around tag subscriptions


(Dave McClure) #1

I started looking into implementing this feature and found some oddities related to how the combination of Slack subscriptions for categories and tags work. I submitted a PR for one issue, but can see some additional ones that we may need to tackle.

Currently, a Slack plugin subscription in the UI makes it look like you can choose a category and a set of tags for a single filter. But in my tests, it looks like any time you add a tag, it sets the category to “all” when you save the filter.

I think this is OK, since nowhere else in the application do we allow you to combine a category and set of tags in a single filter (eg, user notification preferences).

Can I proceed with the assumption that we don’t actually want to allow a user to choose a category other than “all” in a filter that includes tags?


(Vinoth Kannan) #2

No. It’s a bug. And I created the PR below. Thanks for your report :thumbsup:

Update: It is merged now.

As you mentioned we already allowing users to choose both category and tags combination in admin UI. It means we are allowing users to choose the below combinations

Notification will be sent on all new topic creations when

  1. All Categories + No Tags => User can see the topic
  2. All Categories + TAG1 => Topic with tag TAG1 + User can see
  3. CATEGORY1 + No tags => Topic in category CATEGORY1 + User can see
  4. CATEGORY1 + TAG1 => Topic in category CATEGORY1 with tag TAG1 + User can see

(Dave McClure) #3

Ok, I will make sure to preserve that behavior in this case.

But still wondering if this adds enough value to maintain in the long run:

I can provide some examples where is it further complicates the UX.

For instance, let’s say I have a filter that is: (cat1, [tag1], following). Then I had a filter that is (cat1, [tag2], watching). Are these one filter or two at this point? Let’s assume 2.

Then I change the 2nd filter from watching to following. Should it then be combined and leave me with [cat1, [tag1, tag2], following)?

These problems are solvable, but I’m just not sure it’s worth it in the long run.


(Vinoth Kannan) #4

It is @tgxworld’s recommended way and I like it. Because it gives all the possibilities to the filter rules.

Yes it is little bit complicates. While testing your example I found a bug again now. But I hope it is worth it and useful to other plugin users @codinghorror @tgxworld?


(Dave McClure) #5

More concretely, I think it may make sense to work towards a design where a subscription is analogous to a user’s notification preferences. But instead of the user_id being the key, the Slack channel would be the key.

Each Slack channel would have at most 1 subscription.

So a given filter would be something like:

{
  key: '#channel'
  val: {
    watching: {categories: [], tags: []},
    following: {categories: [], tags: []},
    muted: {categories: [], tags: []},
  }
}

Updating a channel’s subscription would just mutate one record, and the code would enforce that a given category or tag can only belong to one of the respective lists. Notifications would match topics to a subscription filter based on whether the category OR any tags match, and continue to use the existing logic for precedence of muted over watching and following.


(David Taylor) #6

Hi @mcwumbly. I’m working on the new discourse-chat-integration plugin, so have spent quite a lot of time over the last couple of weeks re-implementing the filtering system from the slack plugin.

In doing so, I’ve ironed out (hopefully) all of the “oddities” you described at the start of this topic. I only just found this topic, hence why I didn’t post before starting work!

While I like the idea of 1 record for each channel, I can’t see how it work work for the case:

Watching category #bug for posts with tag #pr-welcome
Watch category #support for posts with tag #unsupported-install

With the data model you described above, you cannot have different tag preferences for different categories.

To make it work the model would have to be more like

{
  key: '#channel'
  val: {
    watching: {[
                 {category: 'bug', tags: ['pr-welcome']},
                 {category: 'support', tags: ['unsupported-install']},
              ]},
    following: {[]},
    muted: {[]},
  }
}

And at that point it just simplifies to the existing table structure of

channel filter category tags
#channel watch bug pr-welcome
#channel watch support unsupported-install

Am I missing something?


(Dave McClure) #7

If we are keeping the feature to combine categories and tags in a single filler, then doing something closer to what you’ve described makes sense. I think this may introduce more complexities in the UX, particularly when it comes to updating them via Slack commands, though, and I personally question whether it’s worth it.


(David Taylor) #8

Sure, that makes sense.

If we are keeping the ability to have categories & tags in one rule, then this is the UX I’ve been working on within slack. It’s based on the existing slack plugin, with some adaptations which I hope make it better :thinking:

/discourse status

/discourse watch some-category
would just add the rule to the list, sorted in order of precedence (topmost matching rule wins).

You can also have multiple tags in the same command
/discourse watch some-category tag:food tag:new

And completely miss out the category if you like (‘all categories’ is assumed)
/discourse watch tag:test

Then deleting the second rule in the list is as simple as
/discourse remove 2

I think having simple add/remove rules options is a simpler UX than the existing interface (which tries to work out how to augment the existing rules in order to match your command). It also matches the admin UI exactly which should help. There would still be some simple logic to avoid duplicates.

What do you think? Does that help with the UX complexities you describe?


(Dave McClure) #9

Hey @David_Taylor, sorry for the delayed response.

If you see value in supporting the ability to create a filter with multiple tags in one command, I might suggest this syntax instead though, which more closely matches how things are displayed:

/discourse watch some-category tags:foo,bar,baz

One way to think about the current implementation is that a given tuple (category, tag) can only have one status associated with it from the following set: (watching, following, muted, nil). And a user can only modify a single tuple at a time. (This is where the “it tries to work out how to augment the existing rules” comes in. But I don’t think it’s actually too difficult to reason about from the user’s perspective).

To make things simpler, the Slack command UX currently only allows you to create or update filters where category or tag is nil. But thinking about it more now, I don’t think it would be too difficult or add too much additional complexity to support the combination of category/tag filters in the current design.

That said, I’m not attached to it if you find that the design you’re currently exploring is the way things should go.

I would recommend exploring this a bit more since I agree that seems like the biggest change in your new design. See if you can think of likely ways that people may end up with filters that have overlapping matches and what, if anything, the plugin should do to try to help there.

You might also want to take a peek at the code for the plugin as it exists in my current fork as I think the implementation for the rule augmentation has been simplified quite a bit, but this pull request is still pending.


(David Taylor) #10

That does sound better - I’ll go with that.

Will do - overlapping matches definitely need to be reduced as much as possible, as that can be quite confusing from a UX point of view.

:+1: Will have a look at this over the next week.

Thanks for the feedback - I’m hoping by the end of this week to have something in a vaguely usable state that people can experiment with :slight_smile:


(Alan Tan) #11