Summary

Send email notifications when a token becomes expired.

Review Request #12591 — Created Sept. 12, 2022 and submitted Sept. 19, 2022, 8:45 p.m.

Information

Owner

maubin

Repository

Review Board

Branch

release-5.0.x

Bugs

Depends On

~~12589~~

Reviewers

Groups

reviewboard

People

Description

Currently, users do not get notified about expired tokens. With this change, an
email is sent the first time an expired token is used. The email describes when
the token expired and how to create new tokens. If the email was successfully
sent, any subsequent uses of the token do not trigger another email.

This change also fixes up all of the API token email templates by adding date
formatting and fixing some missing URLs. The tests for the templates were also
fixed up.

Testing Done


Created unit tests for the expired token emails and ran unit tests in

reviewboard/notifications/tests/test_email_sending.py
Manually tested authenticating with expired and non expired tokens, confirmed

  that appropriate emails were sent.

Commits

Summary	ID
Send email notifications when a token becomes expired.	c776280c3483ba6eb35f95e8ca56cc0b603e4f3b

Issues

Description	From	Last Updated
'djblets.webapi.signals.webapi_token_deleted' imported but unused Column: 1 Error code: F401	reviewbot	Sept. 12, 2022, 1:48 p.m.
continuation line under-indented for visual indent Column: 25 Error code: E128	reviewbot	Sept. 13, 2022, 4:47 p.m.
continuation line under-indented for visual indent Column: 25 Error code: E128	reviewbot	Sept. 13, 2022, 4:47 p.m.
I think this is meant to say "deleted"?	chipx86	Sept. 14, 2022, 11:24 a.m.
You can leave off the , None, since that's the default.	chipx86	Sept. 14, 2022, 11:25 a.m.
This would be better as a boolean, I think.	chipx86	Sept. 14, 2022, 11:25 a.m.
Can you add some additional checks to verify the display of the timestamps in the content? I noticed none of …	chipx86	Sept. 14, 2022, 1:49 p.m.
line too long (80 > 79 characters) Column: 80 Error code: E501	reviewbot	Sept. 14, 2022, 1:59 p.m.
line too long (80 > 79 characters) Column: 80 Error code: E501	reviewbot	Sept. 14, 2022, 1:59 p.m.
line too long (80 > 79 characters) Column: 80 Error code: E501	reviewbot	Sept. 14, 2022, 1:59 p.m.
line too long (80 > 79 characters) Column: 80 Error code: E501	reviewbot	Sept. 14, 2022, 1:59 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/notifications/email/__init__.py (Diff revision 1)

Show all issues

'djblets.webapi.signals.webapi_token_deleted' imported but unused

Column: 1
Error code: F401

Summary:

-	[WIP] Send email notifications when a token becomes expired.
+	Send email notifications when a token becomes expired.

Description:

~		Send email notifications when a token becomes expired.
	~	Currently, users do not get notified about expired tokens. With this change, an
	+	email is sent the first time an expired token is used. The email describes when
	+	the token expired and how to create new tokens. If the email was successfully
	+	sent, any subsequent uses of the token do not trigger another email.

Testing Done:

	+	Created unit tests for the expired token emails and ran unit tests in `reviewboard/notifications/tests/test_email_sending.py`
	+	Manually tested authenticating with expired and non expired tokens, confirmed that appropriate emails were sent.

Commits:

	Summary	ID
	Send email notifications when a token becomes expired.	c00ad41587f6c99ff4b7ed3b22dbdd0962e5be59
	Send email notifications when a token becomes expired.	9b8288b530a283b467b7763f4867576b4ca1e2a2

Depends On:

+	~~12589 - Create a signal for expired API tokens.~~

Diff:

Revision 2 (+296 -32)

Show changes

	reviewboard/notifications/email/__init__.py
	reviewboard/notifications/email/message.py
	reviewboard/notifications/email/signal_handlers.py
	reviewboard/notifications/tests/test_email_sending.py
	reviewboard/templates/notifications/api_token_expired.txt
	reviewboard/templates/notifications/api_token_expired.html

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Checks whether an expired notification has already been sent.

Commits:

	Summary	ID
	Send email notifications when a token becomes expired.	9b8288b530a283b467b7763f4867576b4ca1e2a2
	Send email notifications when a token becomes expired.	02beaf6960bc85338a1c4ca41bcdce943b513a73

Diff:

Revision 3 (+298 -32)

Show changes

	reviewboard/notifications/email/__init__.py
	reviewboard/notifications/email/message.py
	reviewboard/notifications/email/signal_handlers.py
	reviewboard/notifications/tests/test_email_sending.py
	reviewboard/templates/notifications/api_token_expired.txt
	reviewboard/templates/notifications/api_token_expired.html

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/notifications/email/signal_handlers.py (Diff revision 3)

Show all issues

continuation line under-indented for visual indent

Column: 25
Error code: E128

reviewboard/notifications/email/signal_handlers.py (Diff revision 3)

Show all issues

continuation line under-indented for visual indent

Column: 25
Error code: E128

Change Summary:

Fix under-indented lines.

Commits:

	Summary	ID
	Send email notifications when a token becomes expired.	02beaf6960bc85338a1c4ca41bcdce943b513a73
	Send email notifications when a token becomes expired.	ac5d088ae9646d3b353cc14e2dfca19a8de5daae

Diff:

Revision 4 (+298 -32)

Show changes

	reviewboard/notifications/email/__init__.py
	reviewboard/notifications/email/message.py
	reviewboard/notifications/email/signal_handlers.py
	reviewboard/notifications/tests/test_email_sending.py
	reviewboard/templates/notifications/api_token_expired.txt
	reviewboard/templates/notifications/api_token_expired.html

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/notifications/email/signal_handlers.py (Diff revision 4)
Show all issues
```
I think this is meant to say "deleted"?
```
reviewboard/notifications/email/signal_handlers.py (Diff revision 4)
Show all issues
```
You can leave off the , None, since that's the default.
```
reviewboard/notifications/email/signal_handlers.py (Diff revision 4)
Show all issues
```
This would be better as a boolean, I think.
```

reviewboard/notifications/tests/test_email_sending.py (Diff revision 4)

Show all issues

Can you add some additional checks to verify the display of the timestamps in the content?

I noticed none of our API token e-mails force a timestamp format, but our other templates all do. Inconsistent, which.. fine, it's been that way a while, but for some future-proofing, it'll be good to make sure we have some testing around how it actually ends up rendering.

In fact, these templates are so small, let's compare the full strings (assertHTMLEqual for the HTML one).

Change Summary:


Changed expired_notification_sent to a boolean
Spruced up the API token email templates by adding date formatting and fixing some missing URLs.
Spruced up the tests for the templates by checking the full strings of the text and html templates.

Description:

		Currently, users do not get notified about expired tokens. With this change, an
		email is sent the first time an expired token is used. The email describes when
		the token expired and how to create new tokens. If the email was successfully
		sent, any subsequent uses of the token do not trigger another email.
	+
	+	This change also fixes up all of the API token email templates by adding date
	+	formatting and fixing some missing URLs. The tests for the templates were also
	+	fixed up.

Commits:

	Summary	ID
	Send email notifications when a token becomes expired.	ac5d088ae9646d3b353cc14e2dfca19a8de5daae
	Send email notifications when a token becomes expired.	c67837f6476fa1c1d5feda35fbedc82aaffd57d8

Diff:

Revision 5 (+680 -142)

Show changes

	reviewboard/notifications/email/__init__.py
	reviewboard/notifications/email/message.py
	reviewboard/notifications/email/signal_handlers.py
	reviewboard/notifications/tests/test_email_sending.py
	reviewboard/templates/notifications/api_token_created.txt
	reviewboard/templates/notifications/api_token_created.html
	reviewboard/templates/notifications/api_token_deleted.txt
	reviewboard/templates/notifications/api_token_expired.txt
	3 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/notifications/tests/test_email_sending.py (Diff revision 5)
Show all issues
```
line too long (80 > 79 characters)

Column: 80
Error code: E501
```
reviewboard/notifications/tests/test_email_sending.py (Diff revision 5)
Show all issues
```
line too long (80 > 79 characters)

Column: 80
Error code: E501
```
reviewboard/notifications/tests/test_email_sending.py (Diff revision 5)
Show all issues
```
line too long (80 > 79 characters)

Column: 80
Error code: E501
```
reviewboard/notifications/tests/test_email_sending.py (Diff revision 5)
Show all issues
```
line too long (80 > 79 characters)

Column: 80
Error code: E501
```

Change Summary:

Shortened some lines that were too long.

Commits:

	Summary	ID
	Send email notifications when a token becomes expired.	c67837f6476fa1c1d5feda35fbedc82aaffd57d8
	Send email notifications when a token becomes expired.	c776280c3483ba6eb35f95e8ca56cc0b603e4f3b

Diff:

Revision 6 (+688 -142)

Show changes

	reviewboard/notifications/email/__init__.py
	reviewboard/notifications/email/message.py
	reviewboard/notifications/email/signal_handlers.py
	reviewboard/notifications/tests/test_email_sending.py
	reviewboard/templates/notifications/api_token_created.txt
	reviewboard/templates/notifications/api_token_created.html
	reviewboard/templates/notifications/api_token_deleted.txt
	reviewboard/templates/notifications/api_token_expired.txt
	3 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (27c2137)