Summary

Allow Features to disable web API resources

Review Request #8470 — Created Oct. 17, 2016 and submitted Oct. 24, 2016, 4:28 p.m.

Information

Owner

brennie

Repository

Review Board

Branch

release-3.0.x

Bugs

Depends On

~~8472~~

Blocks

8471, 8477

Commit

bb2dddd...

Reviewers

Groups

reviewboard

People

Description

Review Board web API resources now have a required_features attribute

which indicates which features must be enabled for the feature to be

available. If any of the listed features are disabled, the resource will

return a 403 Forbidden error for any of the resource's methods. When all

features are enabled, the resource will behave as normal.

Testing Done

Ran unit tests.

Issues

Description	From	Last Updated
Col: 1 W391 blank line at end of file	reviewbot	Oct. 17, 2016, 5:52 p.m.
'Feature' imported but unused	reviewbot	Oct. 17, 2016, 8:17 p.m.
Col: 21 E126 continuation line over-indented for hanging indent	reviewbot	Oct. 17, 2016, 8:17 p.m.
redefinition of unused 'test_disabled_feature_get_list' from line 125	reviewbot	Oct. 17, 2016, 8:17 p.m.
redefinition of unused 'test_disabled_feature_get' from line 131	reviewbot	Oct. 17, 2016, 8:17 p.m.
Col: 23 E251 unexpected spaces around keyword / parameter equals	reviewbot	Oct. 17, 2016, 8:17 p.m.
Col: 25 E251 unexpected spaces around keyword / parameter equals	reviewbot	Oct. 17, 2016, 8:17 p.m.
This should also state that this is optional and what the default behavior is.	chipx86	Oct. 19, 2016, 3:06 p.m.
The docs make this seem like the function is all about features. I think the summary and description should start …	chipx86	Oct. 19, 2016, 3:09 p.m.
Blank line after the docstring.	david	Oct. 19, 2016, 3:10 p.m.
Blank line after the docstring.	david	Oct. 19, 2016, 3:10 p.m.
Blank line here.	chipx86	Oct. 19, 2016, 3:09 p.m.
Private methods go last.	chipx86	Oct. 19, 2016, 3:10 p.m.
The suspense is killing me.	david	Oct. 19, 2016, 3:11 p.m.
No blank line here.	chipx86	Oct. 19, 2016, 3:10 p.m.
Typo: specied.	david	Oct. 24, 2016, 4:04 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/base.py
    reviewboard/webapi/tests/test_base.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/base.py
    reviewboard/webapi/tests/test_base.py

reviewboard/webapi/tests/test_base.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 W391 blank line at end of file
```

Diff:

Revision 2 (+204)

Show changes

	reviewboard/webapi/base.py
	reviewboard/webapi/tests/test_base.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/base.py
    reviewboard/webapi/tests/test_base.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/base.py
    reviewboard/webapi/tests/test_base.py

Depends On:: ~~8472 - Add a LocalSite-enabled feature checker~~

Diff:

Revision 3 (+311)

Show changes

	reviewboard/webapi/base.py
	reviewboard/webapi/tests/test_base.py

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/base.py
    reviewboard/webapi/tests/test_base.py



Tool: Pyflakes
Processed Files:
    reviewboard/webapi/base.py
    reviewboard/webapi/tests/test_base.py

reviewboard/webapi/base.py (Diff revision 3)
The issue has been resolved. Show all issues
```
 'Feature' imported but unused
```
reviewboard/webapi/tests/test_base.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Col: 21
 E126 continuation line over-indented for hanging indent
```
reviewboard/webapi/tests/test_base.py (Diff revision 3)
The issue has been resolved. Show all issues
```
 redefinition of unused 'test_disabled_feature_get_list' from line 125
```
reviewboard/webapi/tests/test_base.py (Diff revision 3)
The issue has been resolved. Show all issues
```
 redefinition of unused 'test_disabled_feature_get' from line 131
```
reviewboard/webapi/tests/test_base.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Col: 23
 E251 unexpected spaces around keyword / parameter equals
```
reviewboard/webapi/tests/test_base.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Col: 25
 E251 unexpected spaces around keyword / parameter equals
```

Diff:

Revision 4 (+310)

Show changes

	reviewboard/webapi/base.py
	reviewboard/webapi/tests/test_base.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/base.py
    reviewboard/webapi/tests/test_base.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/base.py
    reviewboard/webapi/tests/test_base.py

Mind making a change to use this for the status updates resource?

brennie Oct. 19, 2016, 3:10 p.m.
```
Sure!
```

reviewboard/webapi/tests/test_base.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Blank line after the docstring.
```
reviewboard/webapi/tests/test_base.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Blank line after the docstring.
```
reviewboard/webapi/tests/test_base.py (Diff revision 4)
The issue has been resolved. Show all issues
```
The suspense is killing me.
```
1. brennie Oct. 19, 2016, 3:11 p.m.
  Looks fine to me :)

reviewboard/webapi/base.py (Diff revision 4)

The issue has been resolved. Show all issues

This should also state that this is optional and what the default behavior is.

reviewboard/webapi/base.py (Diff revision 4)

The issue has been resolved. Show all issues

The docs make this seem like the function is all about features. I think the summary and description should start off by just saying what call_method_view does, generally. Then it can modify that by talking about features, and about API policies (because this method ultimately takes that into account as well, even though the code in this block of code doesn't itself address it).

reviewboard/webapi/tests/test_base.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Blank line here.
```
reviewboard/webapi/tests/test_base.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Private methods go last.
```
reviewboard/webapi/tests/test_base.py (Diff revision 4)
The issue has been resolved. Show all issues
```
No blank line here.
```

Change Summary:

Address David & Christian's issues

Diff:

Revision 5 (+325)

Show changes

	reviewboard/webapi/base.py
	reviewboard/webapi/tests/test_base.py

Tool: Pyflakes
Processed Files:
    reviewboard/webapi/base.py
    reviewboard/webapi/tests/test_base.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/webapi/base.py
    reviewboard/webapi/tests/test_base.py

Ship it!

reviewboard/webapi/base.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Typo: specied.
```

Status:: Completed
Change Summary:: Pushed to release-3.0.x (d239388)