Summary

Display deprecation notices on the API tokens config page and deprecation status in the resource.

Review Request #12631 — Created Sept. 26, 2022 and submitted Oct. 31, 2022, 2:38 p.m.

Information

Owner

maubin

Repository

Review Board

Branch

release-5.0.x

Bugs

Depends On

~~12627~~

Reviewers

Groups

reviewboard

People

Description

This change adds a deprecated field to the API token web API resource. This
is a bool showing whether the token was generated by a deprecated token
generator. We also now display a notice on the API tokens config page if a
token is deprecated. The notice encourages the user to delete the deprecated
token and generate a new one.

Testing Done


Made some GET requests to the /api-tokens/ list endpoint and saw that the

deprecated field was included in the token resources.
Created some deprecated tokens and made sure they behave correctly and

  appear with the notice in the API tokens page.
Ran JS unit tests.
Ran reviewboard/webapi/tests/test_api_token.py

Commits

Summary	ID
Handle token deprecation.	1e3856ec489ae198302d4477260a560313a60f6f

Files

Issues

Description	From	Last Updated
The summary's a bit too long. Should fit under 80 chars.	chipx86	Sept. 27, 2022, 9:49 p.m.
Summary of the change is still too long.	david	Oct. 7, 2022, 3:26 p.m.
I like it, but I'm also thinking that we should be consistent with the way we present both deprecation and …	chipx86	Sept. 28, 2022, 3:09 p.m.
Maybe a good time to just make this more manageable by having it a list with item per line.	chipx86	Sept. 27, 2022, 9:47 p.m.
How about "This token uses a deprecated format"?	david	Oct. 7, 2022, 3:33 p.m.
We try to only use colors stored in #rb-ns-ui.colors[@constant-name] these days, as that helps us keep the UI more consistent. …	chipx86	Oct. 27, 2022, 10:01 p.m.
Let's keep each item in the list in its own line, like: syncAttrs: [ '...', '...', ... '...', ] That …	chipx86	Oct. 27, 2022, 8:59 p.m.
For these new-style multi-line function definitions where we use typing, the final argument should always have a trailing comma.	chipx86	Oct. 27, 2022, 9:01 p.m.
Same here.	chipx86	Oct. 27, 2022, 9:01 p.m.
Same here.	chipx86	Oct. 27, 2022, 9:01 p.m.
Same here.	chipx86	Oct. 27, 2022, 9:01 p.m.
Same here.	chipx86	Oct. 27, 2022, 9:01 p.m.
Same here.	chipx86	Oct. 27, 2022, 9:01 p.m.

flake8 passed.

JSHint passed.

The issue has been resolved. Show all issues

The summary's a bit too long. Should fit under 80 chars.

Deprecation notice (Revision 1)

The issue has been dropped. Show all issues

I like it, but I'm also thinking that we should be consistent with the way we present both deprecation and expiration notices.

This has the advantage of being very visible, but has the same rough height as a token entry. Almost makes it look like it's replaced a token.

Maybe we should just make this a red inline like the expired notice, under the hash.

maubin Sept. 27, 2022, 2:57 p.m.

Ah yeah I see what you mean about it being the same size as a token entry. The inline sounds good, but I'm thinking of giving it a yellowish colour so that it has more of a warning vibe since deprecated tokens can still be used whereas invalidated/expired tokens can't.

reviewboard/static/rb/js/accountPrefsPage/views/apiTokensView.es6.js (Diff revision 1)

The issue has been resolved. Show all issues

Maybe a good time to just make this more manageable by having it a list with item per line.

Change Summary:

Updated the deprecation notice to a yellow inline text instead of a warning header.

Description:

		This change adds a `deprecated` field to the API token web API resource. This
		is a bool showing whether the token was generated by a deprecated token
~		generator. We also now display a notice on the API tokens config page if a token
~		is deprecated. The notice encourages the user to delete the deprecated token and
~		generate a new one.
	~	generator. We also now display a notice on the API tokens config page if a
	~	token is deprecated. The notice encourages the user to delete the deprecated
	~	token and generate a new one.

Commits:

	Summary	ID
	Handle token deprecation.	e2c89943ba759ebbd71bbb551f3c5376bc972b48
	Handle token deprecation.	d60f7f1503b2abf0c32829c6fa8dcc9696342f17

Diff:

Revision 2 (+142 -20)

Show changes

	reviewboard/accounts/forms/pages.py
	reviewboard/static/rb/css/pages/my-account.less
	reviewboard/static/rb/js/accountPrefsPage/views/apiTokensView.es6.js
	reviewboard/static/rb/js/resources/models/apiTokenModel.es6.js
	reviewboard/webapi/resources/api_token.py

Added Files:

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

The issue has been resolved. Show all issues
```
Summary of the change is still too long.
```
reviewboard/static/rb/js/accountPrefsPage/views/apiTokensView.es6.js (Diff revision 2)
The issue has been resolved. Show all issues
```
How about "This token uses a deprecated format"?
```

Change Summary:

Changed the deprecation notice message.

Summary:

Add a deprecated field to the API tokens resource and display deprecation notices on the API tokens config page.

Display deprecation notices on the API tokens config page and deprecation status in the resource.

Commits:

	Summary	ID
	Handle token deprecation.	d60f7f1503b2abf0c32829c6fa8dcc9696342f17
	Handle token deprecation.	e22576ec684acb5772c1c82527cd5ef7ec73d214

Diff:

Revision 3 (+142 -20)

Show changes

	reviewboard/accounts/forms/pages.py
	reviewboard/static/rb/css/pages/my-account.less
	reviewboard/static/rb/js/accountPrefsPage/views/apiTokensView.es6.js
	reviewboard/static/rb/js/resources/models/apiTokenModel.es6.js
	reviewboard/webapi/resources/api_token.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Change Summary:

Changed Version Added:'s to 5.0.1 and added type hints to some functions.

Commits:

	Summary	ID
	Handle token deprecation.	e22576ec684acb5772c1c82527cd5ef7ec73d214
	Handle token deprecation.	289a81d41f7decc43267d3cead82b950527dbbe1

Diff:

Revision 4 (+232 -38)

Show changes

	reviewboard/accounts/forms/pages.py
	reviewboard/static/rb/css/pages/my-account.less
	reviewboard/static/rb/js/accountPrefsPage/views/apiTokensView.es6.js
	reviewboard/static/rb/js/resources/models/apiTokenModel.es6.js
	reviewboard/webapi/resources/api_token.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/static/rb/css/pages/my-account.less (Diff revision 4)

The issue has been resolved. Show all issues

We try to only use colors stored in #rb-ns-ui.colors[@constant-name] these days, as that helps us keep the UI more consistent. However, there isn't one for this, and we may not be able to really have one without using brown (which doesn't convey the same thing).

The reason is that orange colors contrast poorly with white backgrounds, and in fact will almost surely fail WCAG (Web Content Accessibility Guidelines). Browsers can help check for this, but this tool is also useful:

https://webaim.org/resources/contrastchecker/

If you plug the hex code for this RGB (#D1AE00) into the tool, you'll see it fails validation.

Rather than go with a text color here, I think a better approach is to use black, but prefix with a warning icon. So, same general idea as the alert you had before, but without all the chrome of the alert. Just the icon and text.

You can do that with the same approach that the alert styles do, with a :before {} for this rule that uses content: @fa-var-warning.

reviewboard/static/rb/js/accountPrefsPage/views/apiTokensView.es6.js (Diff revision 4)

The issue has been resolved. Show all issues

Let's keep each item in the list in its own line, like:

syncAttrs: [
    '...',
    '...',
    ...
    '...',
]

That keeps this maintainable as we add to it, and ensures we don't have to change the syncAttrs line if adding something before deprecated (which could cascade into changes for a couple of lines in the diff viewer) and that we won't have to change the valid line if adding something after it (since it'll end in a trailing comma like all others, letting us easily append at will).

reviewboard/webapi/resources/api_token.py (Diff revision 4)

The issue has been resolved. Show all issues

For these new-style multi-line function definitions where we use typing, the final argument should always have a trailing comma.

reviewboard/webapi/resources/api_token.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Same here.
```
reviewboard/webapi/resources/api_token.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Same here.
```
reviewboard/webapi/resources/api_token.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Same here.
```
reviewboard/webapi/resources/api_token.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Same here.
```
reviewboard/webapi/resources/api_token.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Same here.
```

Change Summary:


Some formatting fixes.
Removed the yellow text colour for the deprecation notice and uses a warning icon instead.

Commits:

	Summary	ID
	Handle token deprecation.	289a81d41f7decc43267d3cead82b950527dbbe1
	Handle token deprecation.	1e3856ec489ae198302d4477260a560313a60f6f

Diff:

Revision 5 (+246 -38)

Show changes

	reviewboard/accounts/forms/pages.py
	reviewboard/static/rb/css/pages/my-account.less
	reviewboard/static/rb/js/accountPrefsPage/views/apiTokensView.es6.js
	reviewboard/static/rb/js/resources/models/apiTokenModel.es6.js
	reviewboard/webapi/resources/api_token.py

Added Files:

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-5.0.x (b39d4a9)