flake8
-
reviewboard/accounts/tests.py (Diff revision 1)
Review Request #9796 — Created March 16, 2018 and submitted
When syntax highlighting was disabled globally, we were deleting the
form field from theAccountSettingsForm
. However, later on we were
trying to read from this field, causing an exception and the my account
page to not load.Now when syntax highlighting is disabled, we mark the field as disabled
and refuse updates to it. It will also always render as if the user had
it disabled so it does not appear in a disabled but always on state for
the user.
Ran unit tests.
The my account page no longer raises an exception.
Description | From | Last Updated |
---|---|---|
This no longer has the right saving logic. |
chipx86 | |
F821 undefined name 'new_profile' |
reviewbot | |
Missing Returns: |
david | |
Alphabetical order. |
chipx86 | |
Docstring? |
david | |
Docstring? |
david | |
Maybe a bit cleaner if we pass user into self._request and have it assign it. |
david | |
We already pull this in above. We shouldn't need to re-fetch. |
chipx86 | |
"Disabled" globally? |
chipx86 | |
So I was going to say we should combine the if statements, but really, we can just do: if not … |
chipx86 | |
Can you call this something like _build_request? Also, private methods go last. |
chipx86 | |
Typos: "through", "middleware" |
chipx86 | |
Just realized, there's no reason to do this in clean() anymore. Instead, the setting should only be conditionally saved in … |
chipx86 |
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+84 -3) |
reviewboard/accounts/tests.py (Diff revision 2) |
---|
Maybe a bit cleaner if we pass
user
intoself._request
and have it assign it.
Addressed David's issues.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+106 -3) |
reviewboard/accounts/forms/pages.py (Diff revision 3) |
---|
We already pull this in above. We shouldn't need to re-fetch.
reviewboard/accounts/forms/pages.py (Diff revision 3) |
---|
So I was going to say we should combine the
if
statements, but really, we can just do:if not siteconfig.get(...): self.cleaned_data.pop('syntax_highlighting', None)
reviewboard/accounts/tests.py (Diff revision 3) |
---|
Can you call this something like
_build_request
?Also, private methods go last.
address issues, rebase
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 4 (+105 -3) |
reviewboard/accounts/forms/pages.py (Diff revision 4) |
---|
Just realized, there's no reason to do this in
clean()
anymore. Instead, the setting should only be conditionally saved insave()
ifdiffviewer_syntax_highlighting
is set.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 5 (+87 -6) |
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 6 (+90 -7) |
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 7 (+89 -7) |