Make the expiration controls on the API tokens config page nicer.
Review Request #12670 — Created Oct. 5, 2022 and submitted
This change offers several improvements to the expiration controls on the
API tokens config page:
- Users can edit the expiration of expired tokens.
- Users can set the specific time of the expiration.
- Users can set the expiration to a past date/time.With this, the API tokens config page offers the same flexibility for editing
expiration dates as the API.This change also resets the
expired_notification_sent
flag in a token's
extra_data
field whenever a token's expiration is updated, so that users can
get notified again if they set a new expiration date on already expired tokens.
- Ran JS unit tests.
- Ran
./reviewboard/webapi/tests/test_api_token.py
- Manually tested the expiration controls with various combinations of
starting states, ending states, and with the other controls on the page.
Summary | ID |
---|---|
2856e1c74e0a25cd93dc0069a30a72d7da6123a9 |
Description | From | Last Updated |
---|---|---|
Can you update the description to talk about the changes to the browser compatibility list? |
|
|
Let's put an Oxford Comma after description and change that to "expiration date and time". |
|
|
For chaining, the preferred format would be: this._$datePicker = $(dedent` ... `) .prepend(this.options.descriptorText); Though if we're looking to build this … |
|
|
We probably don't need to worry about changes here. This is important for transpiling code, but the usage of datetime-local … |
|
|
I think what Christian meant was: this._$datePicker = $( '<span class="rb-c-date-inline-editor__picker">' ) We also don't need the closing </span> here. |
|
|
No need for dedent here, since there's only one line. You can simply have a string on the next line. |
|
|
This will be 5.0.2 now. |
|
|
Same here. |
|
|
No need for dedent here. |
|
|
For repeat index lookups, let's assign to a variable. Helps group together related assertions, and reduces visual noise. |
|
-
-
-
docs/manual/webapi/2.0/authenticating.rst (Diff revision 1) Let's put an Oxford Comma after
description
and change that to "expiration date and time". -
reviewboard/static/rb/js/ui/views/inlineEditorView.es6.js (Diff revision 1) For chaining, the preferred format would be:
this._$datePicker = $(dedent` ... `) .prepend(this.options.descriptorText);
Though if we're looking to build this and put something in it at the same time, we should probably just use a template, or build the DOM elements incrementally (since we have to find the
input
, we might as well do the latter).

Change Summary:
- Updated the description to mention the browser compatibility changes
- Updated the docs to mention that the expiration time can be changed
- Builds DOM elements incrementally for the
createField()
methods in the date and datetime inline editors.
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commits: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 2 (+657 -97) |
Checks run (2 succeeded)

Change Summary:
Rebased onto release 5.0.x
Commits: |
|
|||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+656 -96) |
Checks run (2 succeeded)

Change Summary:
Squashed the change into one commit when moving the dev branch over to my new laptop.
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 4 (+652 -92) |
Checks run (2 succeeded)
-
Looks really good! These changes are small.
-
.browserslistrc (Diff revision 4) We probably don't need to worry about changes here. This is important for transpiling code, but the usage of
datetime-local
doesn't really impact that. It's true thatdatetime-local
may not work on older browsers, but that's probably okay. People who need to be on older browsers that need to edit this can put in a date manually, if needed. -
reviewboard/static/rb/js/ui/views/inlineEditorView.es6.js (Diff revision 4) No need for
dedent
here, since there's only one line. You can simply have a string on the next line. -
-
reviewboard/static/rb/js/ui/views/inlineEditorView.es6.js (Diff revision 4) No need for
dedent
here. -
For repeat index lookups, let's assign to a variable. Helps group together related assertions, and reduces visual noise.

Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commits: |
|
||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 5 (+698 -106) |
Checks run (2 succeeded)
-
-
reviewboard/static/rb/js/ui/views/inlineEditorView.es6.js (Diff revisions 4 - 5) I think what Christian meant was:
this._$datePicker = $( '<span class="rb-c-date-inline-editor__picker">' )
We also don't need the closing
</span>
here. -

Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 6 (+698 -106) |