[feat] Add unsubscribe link to email notifications #307
Conversation
Implements and closes #132. --------- Co-authored-by: Federico Capoano <f.capoano@openwisp.io> Co-authored-by: Gagan Deep <pandafy.dev@gmail.com>
- Updated the `NotificationSettingSerializer` to also include `organization_name`. - New endpoint `/api/user-setting/organization/<uuid:organization_id>/` to allow changes toggling of email/web notification settings of a particular org with just a single API call.
…ail and web checkboxes
pandafy
force-pushed
the
notification-preferences
branch
from
bdadba4 to
0feb0d2
Compare
pandafy
force-pushed
the
notification-preferences
branch
from
2c7ffb4 to
8dea29f
Compare
…-notifications-unsubscribe
| <p id="status-message"> | ||
| {% if is_subscribed %} | ||
| {% trans 'You are currently subscribed to notifications.' %} | ||
| {% else %} | ||
| {% trans 'You are currently unsubscribed from notifications.' %} | ||
| {% endif %} | ||
| </p> |
There was a problem hiding this comment.
We are duplicating these text messages in JS. Maybe, we can create separate paragraph elements for each condition and hide one using CSS based on is_subscribed
| statusMessage.textContent = gettext( | ||
| "You are currently subscribed to notifications." | ||
| ); |
There was a problem hiding this comment.
Then, we can update this logic to hide/show the required message.
| toggleBtn.setAttribute("data-hasSubscribe", "false"); | ||
| } | ||
|
|
||
| confirmationMsg.textContent = gettext(data.message); |
There was a problem hiding this comment.
We are displaying output from the API/view here. This is not optimal. We can make the confirmation message independent of the API response. We can similar messages to statusMessage in the HTML and hide/show using CSS.
| confirmationMsg.textContent = gettext(data.message); | ||
| confirmationMsg.style.display = "block"; | ||
| } else { | ||
| window.alert(data.message); |
There was a problem hiding this comment.
This appears a bad UX. We can add an error message paragraph element instead of using alert. Also, we can instruct the user that they can login and update the notification preference from the notification preference page.
| } | ||
| }) | ||
| .catch((error) => { | ||
| window.console.error("Error:", error); |
There was a problem hiding this comment.
Same here, use should be notified of the error.
The email summary template adds additional style element. By default, send_email injects body_html inside the body tag. With this patch, the code extends the default template for email and fixes the semantics of HTML.
github-project-automation
bot
moved this to To do (general)
in OpenWISP Contributor's Board
pandafy
moved this from To do (general)
to Needs review
in OpenWISP Contributor's Board
There was a problem hiding this comment.
I don't see any documentation change relating to this new feature, but I think is relevant.
We could mention this feature here:
https://openwisp.io/docs/dev/notifications/user/web-email-notifications.html
I think we should update this screenshot to show the unsubscribe link in the footer:
https://raw.githubusercontent.com/openwisp/openwisp-notifications/docs/docs/images/email-template.png
(rather than updating, we should upload a new one, eg: docs/images/25/email-template.png).
Please also update this other screenshot as well:
https://github.com/openwisp/openwisp-notifications/blob/docs/docs/images/25/emails/batch-email.png
github-project-automation
bot
moved this from Needs review
to In progress
in OpenWISP Contributor's Board
|
|
||
| // Update button text and attribute | ||
| toggleBtn.textContent = gettext( | ||
| subscribe ? "Unsubscribe" : "Subscribe" |
| <p> | ||
| {% trans 'Manage other preferences?' %} | ||
| <a href="{% url 'notifications:notification_preference' %}"> | ||
| {% trans 'Click here' %} |
There was a problem hiding this comment.
For some reason I hadn't seen this.
For the 1000000th time, please don't do this, see Why You Should Never Write “Click Here”.
| @@ -0,0 +1,2 @@ | |||
| {% load i18n %} | |||
| <p style="margin-bottom: 0px; font-size: small">{% blocktrans %}To unsubscribe from these notifications, <a href="{{ unsubscribe_url }}">click here</a>.{% endblocktrans %}</p> | |||
There was a problem hiding this comment.
What do you think of centering this?
Why You Should Never Write “Click Here” applies here as well.
…enwisp/openwisp-notifications into feat/manage-notifications-unsubscribe
github-project-automation
bot
moved this from In progress
to Done
in OpenWISP Contributor's Board
Fixes: #256
FYR, Please note that this feature depends on PR #290 to manage global email notification preferences, which is not implemented in this PR. Currently, the user's first notification will be treated as their global email notification preference.
Regarding the email token generated: