Edgewall Software
Modify

Opened 5 years ago

Last modified 3 months ago

#11871 new enhancement

Notification subscriber for @name mentions

Reported by: Peter Suter Owned by:
Priority: normal Milestone: next-dev-1.5.x
Component: notification Version:
Severity: normal Keywords:
Cc: leho@… Branch:
Release Notes:

Allow to subscribe to notifications when @username is mentioned.

API Changes:

Description

Many websites allow users to mention @name (e.g. @psuter) in comments to notify that user of the comment.

We could add an optional INotificationSubscriber implementing this for ticket comments.

Attachments (2)

T11871_name_mentions.diff (3.0 KB ) - added by Peter Suter 5 years ago.
Screen Shot 2017-03-01 at 22.34.37.png (30.1 KB ) - added by Ryan J Ollos 3 years ago.

Download all attachments as: .zip

Change History (28)

by Peter Suter, 5 years ago

Attachment: T11871_name_mentions.diff added

comment:1 by lkraav <leho@…>, 5 years ago

Cc: leho@… added

with the autocomplete plugin, this would be so tasty.

these guys have already taken a shot at @mentions https://github.com/trac-hacks/trac-mentions looks like they're using some library called jquery.mentionsInput.js for the @mention autocomplete.

Last edited 4 years ago by Ryan J Ollos (previous) (diff)

comment:3 by Peter Suter, 5 years ago

Owner: set to Peter Suter
Status: newassigned

log:psuter.hg@T11870_optional_subscribers (changeset:371e563351e0/psuter.hg) has a slightly updated version (uses config section from #11875).

comment:4 by Peter Suter, 5 years ago

Milestone: 1.1.31.1.4

comment:5 by Peter Suter, 5 years ago

Milestone: 1.1.4next-dev-1.1.x

This will have to wait for #11875.

comment:6 by spot@…, 5 years ago

Just wanted to mention that we moved the @ mentions plug-in to the trac hacks org and started testing it: https://github.com/trac-hacks/trac-mentions/issues

comment:7 by Peter Suter, 5 years ago

Milestone: next-dev-1.1.x1.1.5
Release Notes: modified (diff)

comment:8 by Jun Omae, 5 years ago

  • If @username in code block or `...@username...`, notification is sent to the users. I think we shouldn't send the mention.
  • No way to escape like !@username.
  • I think it would be good to highlight the mentions using IWikiSyntaxProvider.
  • I usually allow to use - and . characters in username. However, it cannot to use in the mention.

comment:9 by Peter Suter, 5 years ago

Thanks for the review. All good points.

I tried to address them in log:psuter.hg@T11870_optional_subscribers_v2 (changeset:b73a36add5d1/psuter.hg).

  • Remove code blocks before matching mentions.
    • Oops, I forgot ...@username....
  • Don't match !@username.
  • Decorate mentions with IWikiSyntaxProvider.
    • Suggestions for improved CSS are welcome.
    • Also decorates mentions outside of tickets, that will not lead to notifications.
  • Allow - and . characters in username.

in reply to:  9 comment:10 by Peter Suter, 5 years ago

Replying to psuter:

  • Oops, I forgot ...@username....

And mentions between mutliple codeblocks. Both fixed in changeset:0f452ca4ffbb/psuter.hg.

comment:11 by Jun Omae, 5 years ago

More improvements:

Another things:

  • Name mentions work only for ticket's comment.
    • Not working mentions in ticket's description when ticket is created.
    • Not working mentions in attachment's description when attachment is added.
  • NameMentionsSubscriber component requires NameMentionsSubscriber configured in [notification-subscriber] section.
    • I think we should make work without [notification-subscriber] configurations.

in reply to:  11 ; comment:12 by Peter Suter, 5 years ago

Replying to jomae:

Unit tests look great!

On Windows there seems to be a \n vs \r\n problem:

======================================================================
FAIL: test_wiki_syntax (__main__.NameMentionsSubscriberTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\Code\Trac\trunk\tracopt\notification\tests\name_mentions.py", line 79, in test_wiki_syntax
    unicode(format_to_html(self.env, context, comment)))
AssertionError: '<p>\nTo <span class="trac-name-mention" title="User blah">@blah</span>: @john joe@example\n</p>\n' != u'<p>\r\nTo <span class="trac-name-mentio
n" title="User blah">@blah</span>: @john joe@example\r\n</p>\r\n'

----------------------------------------------------------------------

Using a custom Formatter is an interesting idea. I have not had time to review this in depth yet.

The Mock context seems a bit scary though. (Do we need AbstractRequest / OfflineRequest?)

Another things:

  • Name mentions work only for ticket's comment.
    • Not working mentions in ticket's description when ticket is created.
    • Not working mentions in attachment's description when attachment is added.

Good point. I assume we should pass TicketChangeEvent(..., comment=...):

  • trac/ticket/notification.py

    diff -r bd2eaed2b6dd trac/ticket/notification.py
    a b  
    910910            return
    911911        ticket = Ticket(self.env, resource.id)
    912912        event = TicketChangeEvent(category, ticket, time, ticket['reporter'],
     913                                  attachment.description,
    913914                                  attachment=attachment)
    914915        try:
    915916            NotificationSystem(self.env).notify(event)
  • trac/ticket/web_ui.py

    diff -r bd2eaed2b6dd trac/ticket/web_ui.py
    a b  
    13871387
    13881388        # Notify
    13891389        event = TicketChangeEvent('created', ticket, ticket['time'],
    1390                                   ticket['reporter'])
     1390                                  ticket['reporter'], ticket['description'])
    13911391        try:
    13921392            NotificationSystem(self.env).notify(event)
    13931393        except Exception as e:
  • NameMentionsSubscriber component requires NameMentionsSubscriber configured in [notification-subscriber] section.
    • I think we should make work without [notification-subscriber] configurations.

I don't understand. Please explain?

Thanks!

in reply to:  12 ; comment:13 by Jun Omae, 5 years ago

Replying to psuter:

On Windows there seems to be a \n vs \r\n problem:

Oh, I didn't test on Windows. Fixed in [38a8ffc6e8ed/jomae.hg].

Using a custom Formatter is an interesting idea. I have not had time to review this in depth yet.

The Mock context seems a bit scary though. (Do we need AbstractRequest / OfflineRequest?)

I noticed we could use RenderingContext instead of Mock. Modified in [231791a3fe43/jomae.hg].

  • Name mentions work only for ticket's comment.
    • Not working mentions in ticket's description when ticket is created.
    • Not working mentions in attachment's description when attachment is added.

Good point. I assume we should pass TicketChangeEvent(..., comment=...):

Thanks. The patch is committed with unit tests in [4445153f8783/jomae.hg].

  • NameMentionsSubscriber component requires NameMentionsSubscriber configured in [notification-subscriber] section.
    • I think we should make work without [notification-subscriber] configurations.

I don't understand. Please explain?

In jomae.hg/tracopt/notification/tests/name_mentions.py@T11870_optional_subscribers_v2:41#L36, [notification-subscriber] ... = NameMentionsSubscriber is added. If the line is removed, name-mentions feature doesn't work and NameMentionsSubscriber.default_subscriptions() returns an empty list. As the result, many tests would fail.

I misunderstood that to enable user mention feature is configuring only tracopt.notification.name_mentions.* = enabled in [components] section. I missed that it needs to configure also [notification-subscriber] section. [notification-subscriber] is flexible however it feels subtle to configure.

comment:14 by Ryan J Ollos, 5 years ago

Milestone: 1.1.51.1.6

comment:15 by Ryan J Ollos, 4 years ago

Milestone: 1.1.61.1.7

comment:16 by Ryan J Ollos, 4 years ago

Milestone: 1.1.71.2

Milestone renamed

comment:17 by Ryan J Ollos, 4 years ago

Milestone: 1.2next-dev-1.3.x

Moving tickets out of milestone:1.2. Please move back if you intend to complete the work within the next several weeks.

in reply to:  13 comment:18 by Peter Suter, 4 years ago

Owner: Peter Suter removed
Status: assignednew

Replying to jomae:

Sorry, I haven't managed to reply earlier. I looks to me like you have everything ready. Feel free to assign this ticket to yourself and / or commit your latest changes. Thanks!

Last edited 4 years ago by Peter Suter (previous) (diff)

comment:20 by Jun Omae, 3 years ago

Rebased on current 1.2-stable: jomae.hg@T11870_optional_subscribers_v3

  • We should add unit tests for optional subscribers. Only name_mentions.py has unit tests.
  • I consider all subscriber components should be contained in tracopt/notification/subscriber.py. Otherwise, it should be tracopt/notification/subscriber/name_mention.py rather than tracopt/notification/name_mention.py.
    • Currently, 4 subscribers are implemented — all_tickets.py, component_owner.py, name_mentions.py, watch_components.py.
Last edited 3 years ago by Jun Omae (previous) (diff)

by Ryan J Ollos, 3 years ago

comment:21 by Ryan J Ollos, 3 years ago

The namespace changes in comment:20 sound good to me. I noticed a few minor issues:

  • I noted several minor things on BitBucket changesets. Hopefully the comments are easy to spot
  • The CSS adds an @ when rendering @user1, so it renders as @@user1.
  • I don't see a way to save changes to Watch Ticket Components. The Save changes button above the section doesn't seem to be effective when components are checked.
  • Some of the is modified rules should be is created or modified, to be consistent with Ticket that I own is created or modified.
    • Ticket that I'm listed in the CC field is created or modified
    • Any ticket is created or modified
    • Ticket in a component that I own is created or modified
    • Ticket in a component that I'm watching is created or modified
  • I would change related to associated in the Watch Ticket Components hint: Components are defined by the site administrators. Watch components to receive notifications when related associated tickets are created or updated.
  • I think we'll eventually want to redesign the Watch Ticket Components section to handle the case of many components, but that might depend on having a multiselect listbox, or some other way to add multiple components in a single submit such as autocompleting component names in an input.

Note to self, TODO mark th:DefaultCcPlugin as deprecated when this ticket is closed.

Last edited 3 years ago by Ryan J Ollos (previous) (diff)

in reply to:  21 comment:22 by Jun Omae, 3 years ago

Replying to Ryan J Ollos:

The namespace changes in comment:20 sound good to me. I noticed a few minor issues:

Thanks for the reviewing and many suggestions!

  • I noted several minor things on BitBucket changesets. Hopefully the comments are easy to spot
  • The CSS adds an @ when rendering @user1, so it renders as @@user1.
  • I don't see a way to save changes to Watch Ticket Components. The Save changes button above the section doesn't seem to be effective when components are checked.
  • Some of the is modified rules should be is created or modified, to be consistent with Ticket that I own is created or modified.
    • Ticket that I'm listed in the CC field is created or modified
    • Any ticket is created or modified
    • Ticket in a component that I own is created or modified
    • Ticket in a component that I'm watching is created or modified
  • I would change related to associated in the Watch Ticket Components hint: Components are defined by the site administrators. Watch components to receive notifications when related associated tickets are created or updated.

Revised jomae.hg@T11870_optional_subscribers_v3. I'll add unit tests.

  • I think we'll eventually want to redesign the Watch Ticket Components section to handle the case of many components, but that might depend on having a multiselect listbox, or some other way to add multiple components in a single submit such as autocompleting component names in an input.

Agreed. Tentatively, I just modified watch components list like permissions list in permissions admin panel.

comment:23 by Jun Omae, 3 years ago

Milestone: next-dev-1.3.xnext-stable-1.2.x
Owner: set to Jun Omae
Status: newassigned

comment:24 by Ryan J Ollos, 3 years ago

Changes look good. Layout of Watch Ticket Components panel is much better.

comment:25 by Ryan J Ollos, 3 years ago

TODO Update API doc for INotificationSubscriber when changes are committed.

Last edited 3 years ago by Ryan J Ollos (previous) (diff)

comment:26 by Ryan J Ollos, 4 months ago

Owner: Jun Omae removed
Status: assignednew

comment:27 by Ryan J Ollos, 3 months ago

Milestone: next-stable-1.2.xnext-dev-1.5.x

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.
The ticket will be disowned. Next status will be 'new'.
as The resolution will be set. Next status will be 'closed'.
The owner will be changed from (none) to anonymous. Next status will be 'assigned'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.