Add support for specifying a field's widget for tool plugins.
Review Request #6240 — Created Aug. 18, 2014 and submitted
| Information | |
|---|---|
| anselina | |
| ReviewBot | |
| release-0.2.x | |
| 6241 | |
| c50f6cd... | |
| Reviewers | |
| reviewbot | |
Previously, we could only use the default widget for each field in a tool
plugin's options. This change allows us to specify which widget we want to use
on a field, along with any widget-specific attributes.I also added a check to make sure that the field class we get is an actual
Field class.
- Made fields without widgets specified.
- Made a field, specifying the widget but no attributes, and saw the widget's
default attributes get used. - Made a field, specifying the widget and its attributes.
I saw that the above fields were displayed correctly on the "Change Review Bot
Tool" page (see the attached screenshot), and was able to modify them.
- Tried to make an invalid field, and saw the appropriate exception.
- Tried to make an invalid widget, and saw the appropriate exception.
I also checked that all existing tool plugin forms display properly.
| Description | From | Last Updated |
|---|---|---|
|
lets move the field_options = option.get('field_options', {}) above the widget code, and then instead of passing widget separately to field_class(widget=widget, … |
SM smacleod | |
|
Lets use a TypeError instead. Also, I think we should use class_str instead of field_class in the exception. |
SM smacleod | |
|
TypeError and widget_str |
SM smacleod | |
|
We should either a) catch ImportError and AttributeError here and then do something with the exception or b) not bother … |
SM smacleod |
SM
-
As discussed on IRC, we're going to take a different approach to
specifying the widget.Instead of adding it as part of the module path for the field, lets have
a new key on the field schema "widget" which will act almost like the
form field itself. This "widget" will be a dictionary specifying the
widget's module path, and options it takes.
Change Summary:
The widget is now specified as a dictionary, instead of being a part of field_type.
Testing Done: |
|
|||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
|||||||||||||||||||||||||||||||||
Diff: |
Revision 2 (+50 -15) |
|||||||||||||||||||||||||||||||||
Removed Files: |
||||||||||||||||||||||||||||||||||
Added Files: |
-
Tool: PEP8 Style Checker Processed Files: extension/reviewbotext/forms.py Tool: Pyflakes Processed Files: extension/reviewbotext/forms.py
SM
-
This is great and something Review Bot has needed for a long time. :D
-
-
extension/reviewbotext/forms.py (Diff revision 2) lets move the
field_options = option.get('field_options', {})
above the widget code, and then instead of passing widget separately to
field_class(widget=widget, **field_options)
we can just set
field_options['widget'] = widget_class(attrs=widget_attrs)
inside of the conditional. -
extension/reviewbotext/forms.py (Diff revision 2) Lets use a
TypeErrorinstead. Also, I think we should useclass_strinstead offield_classin the exception. -
-
extension/reviewbotext/forms.py (Diff revision 2) We should either
a) catchImportErrorandAttributeErrorhere and then do something with the exception or
b) not bother catching them if we don't know what to do with them.
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 3 (+51 -15) |
Tool: PEP8 Style Checker Processed Files: extension/reviewbotext/forms.py Tool: Pyflakes Processed Files: extension/reviewbotext/forms.py