Todo
This function is responsible for detecting user mentions and sending email notifications. For the detection markdown extenstion can be reused. For sending emails existing interfaces can be used; or any other better mechanism will be discussed before the implementation.
What is expected
When a user is mentioned (and the markdown is saved) the relevant user will be notified with an email
Hi.. @brondsem
We need to decide better approach to extract list of usernames to send notifications using Mailbox. Therefore I am thinking we can reuse
g.markdown.convert()
then we get some text like<div class="markdown_content"><p><a class="user-mention" href="/u/admin1/">@admin1</a> twr </p>
So we can extract user mentions without rewriting the logic using
BeautifulSoap
. Can we discuss some better ideasYea I think using the markdown conversion is good, so it matches exactly what is rendered as links.
Another option using markdown would be to have
UserMentionExtension
record the usernames as it processes them, instead of using BeautifulSoup later. But I am not sure where it would save the usernames, since it is a step in the middle of a whole series of markdown steps. I'm not able to think right now of a good place for that Extension to save or return a value. Maybe there is are internal Markdown variables or methods that could be used, but I'm not familiar with them, and we probably don't want to get into markdown internals too much (we are also behind on Markdown versions, and upgrading will probably change internals). Or maybe it could store the usernames into a global or class variable, but that's messy especially for multiple usages and to be threadsafe.So I think your idea sounds like the best one I can think currently.
Hi.. @brondsem
I pushed a comment to ss/8284 containing some initial work for email sending. Could you please check and advice
Looks like a good start to this ticket. Here's some thoughts:
In
get_usernames_from_md
, theusernames
variable should be a set instead of a list, so if a username is mentioned multiple times, they don't get multiple notifications.Our email templates are markdown (and jinja), so might be nice to name it
.md
like some other templates. And since its markdown, you can make the link be within the text, instead of just a URL on its own line. Something likeat [{{ project_name }} {{ mount_point }} {{ artifact.link_text() }} ]({{ h.absurl(artifact.url()) }})
(haven't tested that to see how it works or how it looks).It might be good to change
send_usermentions_notification
to be a background task, since sometimes markdown conversion can take a while if the text is huge. Then it could be inAllura/allura/tasks/notification_tasks.py
which would be nicer than on app_globals.py.We'll want to call
send_usermentions_notification(
in more places than just comments - like tickets created, wiki pages created/edited, etc. Maybe add one or two more examples for now, but at some point you'll want to put them in many places. (You can search forcreate_activity(
calls, which cover a lot of different artifact actions)Thinking about overall functionality which probably is more related to unsubscribe [#8285], but somebody might get notified about something via regular subscriptions, and then also get a notification because of their username mention. Maybe ok, maybe not? Also if some text is edited, we probably don't want to notify every time its edited. But maybe we do if their username was newly added? That might be hard to keep track of. Stuff to think about with regard to that next ticket and what data needs to be stored in new fields or new models.
Related
Tickets:
#8285Hi Dave,
an interesting thought. Why don't we use the subscription mechanism
directly?
A user mentioning a user could autosubscribe him on the artifact. So he
will get all updates on it, too. But he will be able to unsubscribe at
any time.
An additional requirement then, would be, that the user may wants to define
in his settings if the auto-subscription should be done or not.
If not, they just want to be notified once about the mentioning.
In conclusion I think people just want to be informed once about the
mentioning. Changes should be handled using subscriptions. And the
auto-subscribe-to-mentions-option, would be an optional feature.
What do you think?
Cheers,
Ingo
Dave Brondsema dave@brondsema.net schrieb am Mi., 10. Juli 2019, 22:31:
Related
Tickets:
#8284Tickets:
#8285Hi..@brondsem Thanks for the feedback
I will conitnue as above ideas accordingly. As you mentioned we can send notifications to the newly mentioned users list when an artifact is edited rather than sending to all. (we can do by using text from db and using text from input) We need to think about these situations
A - Actually in the artifact edit email, if the message is too long user may not read it. But if we send an email with subject something like "Your name was mentioned" it will the attention of user. so, do we need to send both emails?
B - Sometimes there may be a new ticket but user is not subscribed and also user is mentioned in somewhere in the ticket. So, better to send the user mention notification ?
Regarding
send_usermentions_notification
Can we focus on comment add/edit for now? since people usually use mentions in discussions mostly. We can add this method insidecreate_activity
but issue is when artifcat is modified there is an issue to get old text. So sounds we need to add it under each relevant artifact add/editCould you advise your ideas
Thanks
Last edit: Shalitha Suranga 2019-07-14
I am not sure what exactly I think would be best for notifications and subscriptions, there are a lot of options. I guess I would recommend we start simple and then as time allows add more towards the ideal best approach. If we try to implement something special for all the different situations it will be complex code, probably confusing for users, and more likely to have bugs. So for example, we can ignore edits for now, those won't be that common and adding a username during an edit is even more rare.
Also simple would be to keep username notifications separate from subscriptions. If that means someone gets 2 emails because they were subscribed and mentioned, then ok. They might want that. Or if they get too many they can turn off username notifications. (Might be nice to make that a per-project setting eventually, I think we were planning on just a single option for now).
Once there is simple but good enough implementation, we can think more about additional settings, or combining emails, etc.
For this ticket specifically, I hope my slow response didn't hold you up. (If you're waiting on me, feel free to go ahead with what you think best, or a different area of the project). Yes I agree
send_usermentions_notification
will have to be under each relevant controller to have access to the artifact. Focusing on comments is good for now, but maybe one other place like new tickets would be good too. That way you can make sure it'll work from multiple places (and add all the other places later).Hi.. @brondsem
Yes agree we can start simple and expand with other artifact types too. I have updated
ss/8284
by adding these improvemntsThanks
Good improvements. Was pretty easy to handle old_text for edits I see - nice.
I would also try to use
artifact.link_text()
in the email content, I think that will add a lot of value in showing the thread title, etc. Instead of only mount point name.And the artifact_link URL isn't always right, for example on ticket comments it ends up with a
_discuss
url which is an internal not-really-used thing.main_url()
could work, or tryurl_paginated()
Beyond that this seems pretty good. However I want to wait to merge this until subscribe/unsubscribe in next ticket is done (Don't want to have situations where people get notifications they can't unsubscribe from). So just branch off of this work for the next ticket, that should work fine I think.
Hi..
I have used
link_text
and removed project and mount point because those are already appearing in mail subject. Also, I have usedurl_paginated
if artifact type isPost
(comment)Thanks
Looks good!
Hi..
So as we discussed we can keep this open and look into [#8285]
Related
Tickets:
#8285Hi..
Since these works are already merged into ss/8285 can we close this ticket when we close [#8285] ?
Thanks
Related
Tickets:
#8285