Summary:

Send email notifications when a token becomes expired.

Review Request #12591 — Created Sept. 12, 2022 and submitted Sept. 19, 2022, 7: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
Send email notifications when a token becomes expired.

Screenshots

Issues

Description	From	Last Updated
'djblets.webapi.signals.webapi_token_deleted' imported but unused Column: 1 Error code: F401	reviewbot	Sept. 12, 2022, 12:48 p.m.
continuation line under-indented for visual indent Column: 25 Error code: E128	reviewbot	Sept. 13, 2022, 3:47 p.m.
continuation line under-indented for visual indent Column: 25 Error code: E128	reviewbot	Sept. 13, 2022, 3:47 p.m.
I think this is meant to say "deleted"?	chipx86	Sept. 14, 2022, 10:24 a.m.
You can leave off the , None, since that's the default.	chipx86	Sept. 14, 2022, 10:25 a.m.
This would be better as a boolean, I think.	chipx86	Sept. 14, 2022, 10: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, 12:49 p.m.
line too long (80 > 79 characters) Column: 80 Error code: E501	reviewbot	Sept. 14, 2022, 12:59 p.m.
line too long (80 > 79 characters) Column: 80 Error code: E501	reviewbot	Sept. 14, 2022, 12:59 p.m.
line too long (80 > 79 characters) Column: 80 Error code: E501	reviewbot	Sept. 14, 2022, 12:59 p.m.
line too long (80 > 79 characters) Column: 80 Error code: E501	reviewbot	Sept. 14, 2022, 12: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
-	Send email notifications when a token becomes expired.
+	Send email notifications when a token becomes expired.

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
-	Send email notifications when a token becomes expired.
+	Send email notifications when a token becomes expired.

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
-	Send email notifications when a token becomes expired.
+	Send email notifications when a token becomes expired.

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
-	Send email notifications when a token becomes expired.
+	Send email notifications when a token becomes expired.

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
-	Send email notifications when a token becomes expired.
+	Send email notifications when a token becomes expired.

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)