Summary

Adding sandboxing to action_hooks

Review Request #5710 — Created April 13, 2014 and submitted April 30, 2014, 1:03 a.m.

Information

Owner

edwinzg

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard, students

People

Description

Added sandboxing to action_hooks

Added unit tests to test suites at reviewboard/extensions/tests.py

Testing Done

Unit testing done - tested all 5 different action hooks (DiffViewerActionHook, HeaderActionHook, HeaderDropdownActionHook, ReivewRequestActionHook, ReviewRequestDropdownActionHook) with and without the sandboxing

Ran on all tests in extensions/tests.py

Issues

Description	From	Last Updated
Pleane remove the space at the end of this string.	david	April 18, 2014, 1:16 a.m.
This could be rewrapped a bit. In particular, it would be nice to have "%s.get_actions" together.	david	April 26, 2014, 8:49 p.m.
Do you need to modify this test to instantiate ReviewRequestApprovalTestHook now that you've removed that from SandboxExtension?	david	April 29, 2014, 6:06 p.m.
I think you should remove this and instead instantiate the ReviewRequestApprovalTestHook inside the test_is_approved_sandbox method (like you do for all …	david	April 29, 2014, 8:10 p.m.

reviewboard/extensions/templatetags/rb_extensions.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Pleane remove the space at the end of this string.
```

Summary:

[WIP] Adding sandboxing to action_hooks

Adding sandboxing to action_hooks

Description:

		Added sandboxing to action_hooks
	+
	+	Added unit tests to test suites at reviewboard/extensions/tests.py

Testing Done:

Diff:

Revision 2 (+131 -8)

Show changes

	reviewboard/extensions/tests.py
	reviewboard/extensions/templatetags/rb_extensions.py

This patch no longer applies cleanly. Can you rebase this onto the latest release-2.0.x and update?

reviewboard/extensions/templatetags/rb_extensions.py (Diff revision 2)
The issue has been resolved. Show all issues
```
This could be rewrapped a bit. In particular, it would be nice to have "%s.get_actions" together.
```

Diff:

Revision 3 (+104 -9)

Show changes

	reviewboard/extensions/tests.py
	reviewboard/extensions/templatetags/rb_extensions.py

reviewboard/extensions/tests.py (Diff revision 3)

The issue has been resolved. Show all issues

Do you need to modify this test to instantiate ReviewRequestApprovalTestHook now that you've removed that from SandboxExtension?

Can you run all the unit tests, not just the newly-added ones?

Testing Done:: Unit testing done - tested all 5 different action hooks (DiffViewerActionHook, HeaderActionHook, HeaderDropdownActionHook, ReivewRequestActionHook, ReviewRequestDropdownActionHook) with and without the sandboxing
+
+
Ran on all tests in extensions/tests.py

Diff:

Revision 4 (+104 -8)

Show changes

	reviewboard/extensions/tests.py
	reviewboard/extensions/templatetags/rb_extensions.py

reviewboard/extensions/tests.py (Diff revision 4)

The issue has been resolved. Show all issues

I think you should remove this and instead instantiate the ReviewRequestApprovalTestHook inside the test_is_approved_sandbox method (like you do for all of these new test hooks that you've introduced).

Diff:

Revision 5 (+107 -10)

Show changes

	reviewboard/extensions/tests.py
	reviewboard/extensions/templatetags/rb_extensions.py

Ship it!

```
Ship It!
```

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