Summary

Add decorator and mixin for requiring consent decisions

Review Request #9948 — Created May 15, 2018 and submitted May 24, 2018, 12:44 a.m.

Information

Owner

brennie

Repository

Djblets

Branch

release-1.0.x

Bugs

Depends On

~~9945~~

Blocks

9976

Commit

eba94b3...

Reviewers

Groups

djblets

People

Description

We now provide a view decorator (@check_pending_consent) and a view
mixin (CheckPendingConsentMixin) for requiring all consent decisions
to be made. If said decisions are not all made, the user will be
redirected to the URL specified in the
DJBLETS_PRIVACY_PENDING_CONSENT_REDIRECT_URL setting.

Testing Done

Ran unit tests.

Issues

Description	From	Last Updated
F401 'djblets.cache.backend.make_cache_key' imported but unused	reviewbot	May 16, 2018, 3:12 p.m.
Too many blank lines.	david	May 21, 2018, 5:10 p.m.
Doc comment and superclass say returns but this doesn't have a return statement.	david	May 21, 2018, 5:11 p.m.
n before r	david	May 21, 2018, 5:12 p.m.
t before v	david	May 21, 2018, 5:12 p.m.
t before v	david	May 21, 2018, 5:13 p.m.
This example is a little confusing. We have a class called SimpleForm which is actually a view, and then we …	david	May 21, 2018, 5:13 p.m.
Alphabetical order.	chipx86	May 23, 2018, 1:45 p.m.
:setting: isn't a thing. There's no reference or role for settings. For now, just do settings.BLAH.	chipx86	May 23, 2018, 1:45 p.m.
This should prefix settings. to add some context.	chipx86	May 23, 2018, 1:45 p.m.
We should allow this to be a function, since the URL may very well depend on the request.	chipx86	May 23, 2018, 1:46 p.m.
Too many blank lines. Same comment about :setting:.	chipx86	May 23, 2018, 1:46 p.m.
This should be one line. Why are we even taking request? Seems unnecessary. The decorator will still get it.	chipx86	May 23, 2018, 1:47 p.m.
This should just use record_consent_data_list(), and then there's no need to pull request.user out into user (we'll only access it …	chipx86	May 23, 2018, 1:52 p.m.
Should be: ``settings.DJBLETS_PRIVACY_CONSENT_TRACKER``	chipx86	May 23, 2018, 1:48 p.m.
Let's add an anchor here.	chipx86	May 23, 2018, 1:48 p.m.
Same.	chipx86	May 23, 2018, 1:48 p.m.
Feels like this is too complex an example. It looks like a form is required here. Let's keep it as …	chipx86	May 23, 2018, 1:50 p.m.
These aren't valid responses for a view.	chipx86	May 23, 2018, 1:50 p.m.
The extra blank line can go away.	chipx86	May 23, 2018, 1:50 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	May 23, 2018, 2:26 p.m.
Doesn't this redirect to settings.DJBLETS_PRIVACY_PENDING_CONSENT_REDIRECT_URL? That said, I'd actually be OK calling it PENDING_CONSENT_REDIRECT_URL everywhere :)	david	May 23, 2018, 7:26 p.m.
This should no longer be in "Args".	chipx86	May 24, 2018, 12:10 a.m.
No blank line here.	chipx86	May 24, 2018, 12:10 a.m.
This should take **kwargs.	chipx86	May 24, 2018, 12:10 a.m.
No trailing blank line.	chipx86	May 24, 2018, 12:10 a.m.

flake8 passed.

JSHint passed.

Commit:

657c8766854b8976df6bfdd2d754fda761aaff17

8e68c0f72028a7a01516d94966a9a28938b58835

Diff:

Revision 2 (+336 -1)

Show changes

	djblets/privacy/consent/views.py
	djblets/privacy/tests/test_consent_views.py
	docs/djblets/guides/privacy/consent.rst

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

djblets/privacy/tests/test_consent_views.py (Diff revision 2)
The issue has been resolved. Show all issues
```
F401 'djblets.cache.backend.make_cache_key' imported but unused
```

Commit:

8e68c0f72028a7a01516d94966a9a28938b58835

280412ec2d73e3368b12ebd4da40c21da66e5f1c

Diff:

Revision 3 (+335 -1)

Show changes

	djblets/privacy/consent/views.py
	djblets/privacy/tests/test_consent_views.py
	docs/djblets/guides/privacy/consent.rst

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Check for correct setting, cleanup.

Commit:

280412ec2d73e3368b12ebd4da40c21da66e5f1c

cc82924d8eea9d84628ff1ccf0e2cdf02baa6fab

Diff:

Revision 4 (+339 -1)

Show changes

	djblets/privacy/consent/views.py
	djblets/privacy/tests/test_consent_views.py
	docs/djblets/guides/privacy/consent.rst

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

djblets/privacy/consent/views.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Too many blank lines.
```

djblets/privacy/consent/views.py (Diff revision 4)

The issue has been resolved. Show all issues

Doc comment and superclass say returns but this doesn't have a return statement.

djblets/privacy/tests/test_consent_views.py (Diff revision 4)
The issue has been resolved. Show all issues
```
n before r
```
djblets/privacy/tests/test_consent_views.py (Diff revision 4)
The issue has been resolved. Show all issues
```
t before v
```
djblets/privacy/tests/test_consent_views.py (Diff revision 4)
The issue has been resolved. Show all issues
```
t before v
```

docs/djblets/guides/privacy/consent.rst (Diff revision 4)

The issue has been resolved. Show all issues

This example is a little confusing. We have a class called SimpleForm which is actually a view, and then we instantiate a SimpleForm that's a form. Let's import a SimpleForm from some nonexistant place, and then call the class itself SimpleView.

Change Summary:

Addressed David's feedback.

Commit:

cc82924d8eea9d84628ff1ccf0e2cdf02baa6fab

b8eeb8086a7ea2aca7c7d02c78b9a4ee09b25eaa

Diff:

Revision 5 (+342 -1)

Show changes

	djblets/privacy/consent/views.py
	djblets/privacy/tests/test_consent_views.py
	docs/djblets/guides/privacy/consent.rst

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

djblets/privacy/consent/views.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Alphabetical order.
```

djblets/privacy/consent/views.py (Diff revision 5)

The issue has been resolved. Show all issues

:setting: isn't a thing. There's no reference or role for settings. For now, just do settings.BLAH.

djblets/privacy/consent/views.py (Diff revision 5)
The issue has been resolved. Show all issues
```
This should prefix settings. to add some context.
```

djblets/privacy/consent/views.py (Diff revision 5)

The issue has been resolved. Show all issues

We should allow this to be a function, since the URL may very well depend on the request.

djblets/privacy/consent/views.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Too many blank lines.
Same comment about :setting:.
```

djblets/privacy/consent/views.py (Diff revision 5)

The issue has been resolved. Show all issues

This should be one line.

Why are we even taking request? Seems unnecessary. The decorator will still get it.

djblets/privacy/tests/test_consent_views.py (Diff revision 5)

The issue has been resolved. Show all issues

This should just use record_consent_data_list(), and then there's no need to pull request.user out into user (we'll only access it once).
We can also get rid of the tracker = get_consent_tracker(), just doing get_consent_tracker().record_consent_data_list(...).
Same comments apply to other tests.

docs/djblets/guides/privacy/consent.rst (Diff revision 5)

The issue has been resolved. Show all issues

Should be:

``settings.DJBLETS_PRIVACY_CONSENT_TRACKER``

docs/djblets/guides/privacy/consent.rst (Diff revision 5)
The issue has been resolved. Show all issues
```
Let's add an anchor here.
```
docs/djblets/guides/privacy/consent.rst (Diff revision 5)
The issue has been resolved. Show all issues
```
Same.
```

docs/djblets/guides/privacy/consent.rst (Diff revision 5)

The issue has been resolved. Show all issues

Feels like this is too complex an example. It looks like a form is required here. Let's keep it as simple as the plain view.

docs/djblets/guides/privacy/consent.rst (Diff revision 5)
The issue has been resolved. Show all issues
```
These aren't valid responses for a view.
```
docs/djblets/guides/privacy/consent.rst (Diff revision 5)
The issue has been resolved. Show all issues
```
The extra blank line can go away.
```

Change Summary:

Addressed feedback.

Commit:

b8eeb8086a7ea2aca7c7d02c78b9a4ee09b25eaa

4e6b0c0c84b98a5f5e9153968575ed95854ec3a5

Diff:

Revision 6 (+364)

Show changes

	djblets/privacy/consent/views.py
	djblets/privacy/tests/test_consent_views.py
	docs/djblets/guides/privacy/consent.rst

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

djblets/privacy/consent/views.py (Diff revision 6)
The issue has been resolved. Show all issues
```
E126 continuation line over-indented for hanging indent
```

Change Summary:

Flake8 fixups

Commit:

4e6b0c0c84b98a5f5e9153968575ed95854ec3a5

71edf3db91cf4da078ce8bfd7d9f9ef7c07a6622

Diff:

Revision 7 (+364)

Show changes

	djblets/privacy/consent/views.py
	djblets/privacy/tests/test_consent_views.py
	docs/djblets/guides/privacy/consent.rst

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

djblets/privacy/consent/views.py (Diff revision 7)

The issue has been resolved. Show all issues

Doesn't this redirect to settings.DJBLETS_PRIVACY_PENDING_CONSENT_REDIRECT_URL?

That said, I'd actually be OK calling it PENDING_CONSENT_REDIRECT_URL everywhere :)

Change Summary:

Addressed feedback.

Commit:

71edf3db91cf4da078ce8bfd7d9f9ef7c07a6622

a59e02b199f601e1e738214fb4cfc309d0d74528

Diff:

Revision 8 (+364)

Show changes

	djblets/privacy/consent/views.py
	djblets/privacy/tests/test_consent_views.py
	docs/djblets/guides/privacy/consent.rst

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

djblets/privacy/consent/views.py (Diff revision 8)
The issue has been resolved. Show all issues
```
This should no longer be in "Args".
```
docs/djblets/guides/privacy/consent.rst (Diff revision 8)
The issue has been resolved. Show all issues
```
No blank line here.
```
docs/djblets/guides/privacy/consent.rst (Diff revision 8)
The issue has been resolved. Show all issues
```
This should take **kwargs.
```
docs/djblets/guides/privacy/consent.rst (Diff revision 8)
The issue has been resolved. Show all issues
```
No trailing blank line.
```

Change Summary:

Addressed feedback.

Commit:

a59e02b199f601e1e738214fb4cfc309d0d74528

eba94b34d0d262e038a443c46e985ae5dcd487ae

Diff:

Revision 9 (+359)

Show changes

	djblets/privacy/consent/views.py
	djblets/privacy/tests/test_consent_views.py
	docs/djblets/guides/privacy/consent.rst

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-1.0.x (0b169b8)