Summary

[1.7] Allow Review UIs to decide whether they're usable on a case-by-case basis.

Review Request #6007 — Created June 17, 2014 and submitted June 17, 2014, 6:04 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-1.7.x

Bugs

Depends On

Commit

ade64f4...

Reviewers

Groups

reviewboard

People

Description

ReviewUI subclaseses can now define a is_enabled_for() function, which
returns whether it can be enabled for the given user, review request,
and/or file attachment. This allows, for instance, limiting a review UI
based on extra_data on an object, or based on the given user or
Local Site, or anything else.

Testing Done

Tested this with a file attachment returning True, and then returning False.

Implemented a custom version and tested by file attachment name.

Issues

Description	From	Last Updated
Same question here about kwargs vs args.	david	June 17, 2014, 5:57 p.m.
Does it require using kwargs for these? The variable names seem pretty self explanatory (is_enabled_for(request.user, review_request. file_attachment))	david	June 17, 2014, 5:57 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/ui/base.py
    reviewboard/reviews/templatetags/reviewtags.py

Ignored Files:
    reviewboard/templates/reviews/review_request_box.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/ui/base.py
    reviewboard/reviews/templatetags/reviewtags.py

Ignored Files:
    reviewboard/templates/reviews/review_request_box.html

reviewboard/reviews/templatetags/reviewtags.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Same question here about kwargs vs args.
```

reviewboard/reviews/views.py (Diff revision 1)

The issue has been resolved. Show all issues

Does it require using kwargs for these? The variable names seem pretty self explanatory (is_enabled_for(request.user, review_request. file_attachment))

chipx86 June 17, 2014, 5:47 p.m.

I wanted to basically allow this to be somewhat expansive for the future, and to allow implementations to just define:

def is_enabled_for(self, file_attachment, **kwargs):
    ...


Most won't need all the objects, so it didn't seem worth them all listing all of them in the right order.

david June 17, 2014, 5:50 p.m.

In that case, I think that ReviewUI.is_enabled_for() should list them as kwargs (using =None) and probably mention your subclass intent.

Change Summary:


Updated the is_enabled_for docs to describe the expectations when being called.
Added default arguments for all keyword arguments, indicating they should be used as keyword arguments and not positional.

Commit:

33718878fe60a8e926f4dd291d70ca991ab213f4

ade64f4cfd13c1d61324e7ceda7a255dda637e2a

Diff:

Revision 2 (+33 -4)

Show changes

	reviewboard/reviews/views.py
	reviewboard/reviews/templatetags/reviewtags.py
	reviewboard/reviews/ui/base.py
	reviewboard/templates/reviews/review_request_box.html

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/ui/base.py
    reviewboard/reviews/templatetags/reviewtags.py

Ignored Files:
    reviewboard/templates/reviews/review_request_box.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/ui/base.py
    reviewboard/reviews/templatetags/reviewtags.py

Ignored Files:
    reviewboard/templates/reviews/review_request_box.html

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-1.7.x (ba025c7)

Status:: Completed
Change Summary:: Pushed to release-2.0.x (ba025c7)

Status:: Completed
Change Summary:: Pushed to master (ba025c7)