Opened 10 years ago
Last modified 2 years ago
#11871 new enhancement
Notification subscriber for @name mentions
Reported by: | Peter Suter | Owned by: | |
---|---|---|---|
Priority: | normal | Milestone: | next-dev-1.7.x |
Component: | notification | Version: | |
Severity: | normal | Keywords: | |
Cc: | leho@…, s.skutnik@…, aikido.guy@… | Branch: | |
Release Notes: |
Allow to subscribe to notifications when |
||
API Changes: | |||
Internal 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)
Change History (31)
by , 10 years ago
Attachment: | T11871_name_mentions.diff added |
---|
comment:3 by , 10 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
log:psuter.hg@T11870_optional_subscribers (changeset:371e563351e0/psuter.hg) has a slightly updated version (uses config section from #11875).
comment:4 by , 10 years ago
Milestone: | 1.1.3 → 1.1.4 |
---|
comment:6 by , 10 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 , 10 years ago
Milestone: | next-dev-1.1.x → 1.1.5 |
---|---|
Release Notes: | modified (diff) |
comment:8 by , 10 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.
follow-up: 10 comment:9 by , 10 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...
.
- Oops, I forgot
- 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.
comment:10 by , 10 years ago
Replying to psuter:
- Oops, I forgot
...@username...
.
And mentions between mutliple codeblocks. Both fixed in changeset:0f452ca4ffbb/psuter.hg.
follow-up: 12 comment:11 by , 10 years ago
More improvements:
- Added unit tests in [b35059119b63/jomae.hg]
- Fixed not working with nested monospaces and code blocks in [bd2eaed2b6dd/jomae.hg]
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 requiresNameMentionsSubscriber
configured in[notification-subscriber]
section.- I think we should make work without
[notification-subscriber]
configurations.
- I think we should make work without
follow-up: 13 comment:12 by , 10 years ago
Replying to jomae:
- Added unit tests in [b35059119b63/jomae.hg]
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' ----------------------------------------------------------------------
- Fixed not working with nested monospaces and code blocks in [bd2eaed2b6dd/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?)
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 910 910 return 911 911 ticket = Ticket(self.env, resource.id) 912 912 event = TicketChangeEvent(category, ticket, time, ticket['reporter'], 913 attachment.description, 913 914 attachment=attachment) 914 915 try: 915 916 NotificationSystem(self.env).notify(event) -
trac/ticket/web_ui.py
diff -r bd2eaed2b6dd trac/ticket/web_ui.py
a b 1387 1387 1388 1388 # Notify 1389 1389 event = TicketChangeEvent('created', ticket, ticket['time'], 1390 ticket['reporter'] )1390 ticket['reporter'], ticket['description']) 1391 1391 try: 1392 1392 NotificationSystem(self.env).notify(event) 1393 1393 except Exception as e:
NameMentionsSubscriber
component requiresNameMentionsSubscriber
configured in[notification-subscriber]
section.
- I think we should make work without
[notification-subscriber]
configurations.
I don't understand. Please explain?
Thanks!
follow-up: 18 comment:13 by , 10 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].
- Fixed not working with nested monospaces and code blocks in [bd2eaed2b6dd/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 requiresNameMentionsSubscriber
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 , 9 years ago
Milestone: | 1.1.5 → 1.1.6 |
---|
comment:15 by , 9 years ago
Milestone: | 1.1.6 → 1.1.7 |
---|
comment:17 by , 9 years ago
Milestone: | 1.2 → next-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.
comment:18 by , 9 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
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!
comment:20 by , 8 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 betracopt/notification/subscriber/name_mention.py
rather thantracopt/notification/name_mention.py
.- Currently, 4 subscribers are implemented —
all_tickets.py
,component_owner.py
,name_mentions.py
,watch_components.py
.
- Currently, 4 subscribers are implemented —
by , 8 years ago
Attachment: | Screen Shot 2017-03-01 at 22.34.37.png added |
---|
follow-up: 22 comment:21 by , 8 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.
- 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
relatedassociated 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.
comment:22 by , 8 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
relatedassociated 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 , 8 years ago
Milestone: | next-dev-1.3.x → next-stable-1.2.x |
---|---|
Owner: | set to |
Status: | new → assigned |
comment:24 by , 8 years ago
Changes look good. Layout of Watch Ticket Components panel is much better.
comment:25 by , 8 years ago
TODO Update API doc for INotificationSubscriber when changes are committed.
comment:26 by , 5 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:27 by , 5 years ago
Milestone: | next-stable-1.2.x → next-dev-1.5.x |
---|
comment:28 by , 5 years ago
Cc: | added |
---|
comment:29 by , 4 years ago
Cc: | 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.