Summary:

Allow Admin Widget Integration through ReviewBoard Extensions

Review Request #6573 — Created Nov. 9, 2014 and submitted Nov. 22, 2014, 9:53 p.m.

Information
Owner:	justy777
Repository:	Review Board
Branch:	master
Bugs:
Depends On:
Blocks:	6597
Commit:	dc89704...
Reviewers
Groups:	reviewboard, students
People:

Description

There's now a AdminWidgetHook that extensions can use to register administartion widgets. This handles registering, and unregistering Admin widgets.

Testing Done

Unit tests have been written to make sure the AdminWidgetHook properly registers and unregisters widgets. A third unit test checks that the Widget subclass being registered ends up in primary_widgets list, when primary is set to True in the hook initalization.

Issues

Description	From	Last Updated
This test should verify that the widget is correctly registered (not just checking that there are no exceptions)	david	Nov. 21, 2014, 12:36 p.m.
If you want to test something that something doesn't throw an exception, wrap it in a try..except, like so: try: …	brennie	Nov. 19, 2014, 3:01 p.m.
This test should verify that the widget is correctly unregistered (not just checking that there are no exceptions)	david	Nov. 21, 2014, 12:36 p.m.
This test should verify that the widget is correctly registered (not just checking that there are no exceptions)	david	Nov. 21, 2014, 12:36 p.m.
There's really no reason to mark this string for translation.	david	Nov. 21, 2014, 3:05 p.m.
Can you swap the order of these two methods?	david	Nov. 21, 2014, 3:05 p.m.
No blank line here.	chipx86	Nov. 22, 2014, 2:03 p.m.
"adding a new widget ..."	chipx86	Nov. 22, 2014, 2:02 p.m.
This would be better using assertIn(TestWidget, secondary_widgets). Same below.	chipx86	Nov. 22, 2014, 2:08 p.m.
Should be a ValueError.	chipx86	Nov. 22, 2014, 2:50 p.m.
Col: 24 E128 continuation line under-indented for visual indent	reviewbot	Nov. 22, 2014, 2:53 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/admin/widgets.py
    reviewboard/extensions/hooks.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/admin/widgets.py
    reviewboard/extensions/hooks.py

Description:

~		There's now a SCMToolHook that extensions can use to register source code management tools. This handles registering, unregistering, and retriving the SCM tools.
	~	There's now a AdminWidget that extensions can use to register administartion widgets. This handles registering, and unregistering Admin widgets.

		TODO:
		Write unit tests.
		Make sure the hook is properly intergrated.

Summary:

-	[WIP] Allow Admin Widget Integration through ReviewBoard Extensions
+	Allow Admin Widget Integration through ReviewBoard Extensions

Description:

		There's now a AdminWidget that extensions can use to register administartion widgets. This handles registering, and unregistering Admin widgets.
-
-		TODO:
-		Write unit tests.
-		Make sure the hook is properly intergrated.

Testing Done:

Commit:

-	38aa8b31200697ac90f53b26d152a3cd77405795
+	f7fb7a39a798e297c1639e8b5cd6a8b3b0d451bd

Diff:

Revision 2 (+112 -18)

Show changes

	reviewboard/admin/widgets.py
	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/admin/widgets.py
    reviewboard/extensions/hooks.py
    reviewboard/extensions/tests.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/admin/widgets.py
    reviewboard/extensions/hooks.py
    reviewboard/extensions/tests.py

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

Show all issues

If you want to test something that something doesn't throw an exception, wrap it in a try..except, like so:

try:
    some_fn()
except SomeException:
    self.fail('some_fn raised SomeException')

Same for test_unregister and test_register_with_primary

justy777 Nov. 19, 2014, 3:01 p.m.

David advised me that it is not necessary. If an exception is thrown then it causes an error, and the test still fails.

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

Show all issues

This test should verify that the widget is correctly registered (not just checking that there are no exceptions)

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

Show all issues

This test should verify that the widget is correctly unregistered (not just checking that there are no exceptions)

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

Show all issues

This test should verify that the widget is correctly registered (not just checking that there are no exceptions)

Commit:

-	f7fb7a39a798e297c1639e8b5cd6a8b3b0d451bd
+	c855823040fdad84485ca7bf302f04583b6edb2c

Diff:

Revision 3 (+114 -18)

Show changes

	reviewboard/admin/widgets.py
	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/admin/widgets.py
    reviewboard/extensions/hooks.py
    reviewboard/extensions/tests.py



Tool: Pyflakes
Processed Files:
    reviewboard/admin/widgets.py
    reviewboard/extensions/hooks.py
    reviewboard/extensions/tests.py

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

Show all issues

There's really no reason to mark this string for translation.

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

Show all issues

Can you swap the order of these two methods?

Description:

~		There's now a AdminWidget that extensions can use to register administartion widgets. This handles registering, and unregistering Admin widgets.
	~	There's now a AdminWidgetHook that extensions can use to register administartion widgets. This handles registering, and unregistering Admin widgets.

Commit:

-	c855823040fdad84485ca7bf302f04583b6edb2c
+	8095d0a9c496490a8b9cbab58f7d891c332286a4

Diff:

Revision 4 (+113 -18)

Show changes

	reviewboard/admin/widgets.py
	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/admin/widgets.py
    reviewboard/extensions/hooks.py
    reviewboard/extensions/tests.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/admin/widgets.py
    reviewboard/extensions/hooks.py
    reviewboard/extensions/tests.py

This looks great! Just a couple small things left.

reviewboard/admin/widgets.py (Diff revision 4)
Show all issues
```
No blank line here.
```
reviewboard/extensions/hooks.py (Diff revision 4)
Show all issues
```
"adding a new widget ..."
```

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

Show all issues

This would be better using assertIn(TestWidget, secondary_widgets).
Same below.

Commit:

-	8095d0a9c496490a8b9cbab58f7d891c332286a4
+	98834ebf76e3d1163316f781e7fc4513a8384b7c

Diff:

Revision 5 (+111 -18)

Show changes

	reviewboard/admin/widgets.py
	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/admin/widgets.py
    reviewboard/extensions/hooks.py
    reviewboard/extensions/tests.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/admin/widgets.py
    reviewboard/extensions/hooks.py
    reviewboard/extensions/tests.py

Ship it!

reviewboard/admin/widgets.py (Diff revision 5)
Show all issues
```
Should be a ValueError.
```

Commit:

-	98834ebf76e3d1163316f781e7fc4513a8384b7c
+	d8aa9af1014df88b14af9b7b13167fc2e32bd518

Diff:

Revision 6 (+111 -18)

Show changes

	reviewboard/admin/widgets.py
	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/admin/widgets.py
    reviewboard/extensions/hooks.py
    reviewboard/extensions/tests.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/admin/widgets.py
    reviewboard/extensions/hooks.py
    reviewboard/extensions/tests.py

reviewboard/admin/widgets.py (Diff revision 6)

Show all issues

Col: 24
 E128 continuation line under-indented for visual indent

Commit:

-	d8aa9af1014df88b14af9b7b13167fc2e32bd518
+	dc89704761840e62db28043de9c83f1b5b716194

Diff:

Revision 7 (+111 -18)

Show changes

	reviewboard/admin/widgets.py
	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/admin/widgets.py
    reviewboard/extensions/hooks.py
    reviewboard/extensions/tests.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/admin/widgets.py
    reviewboard/extensions/hooks.py
    reviewboard/extensions/tests.py

Status: Closed (submitted)

Change Summary:

Pushed to master (71383b5)