Merge lp:~jcsackett/launchpad/form-macros-frighten-me into lp:launchpad
Status: | Merged |
---|---|
Approved by: | j.c.sackett |
Approved revision: | no longer in the source branch. |
Merged at revision: | 13485 |
Proposed branch: | lp:~jcsackett/launchpad/form-macros-frighten-me |
Merge into: | lp:launchpad |
Diff against target: |
175 lines (+25/-43) 7 files modified
lib/lp/answers/browser/tests/test_questiontarget.py (+2/-3) lib/lp/app/javascript/picker/tests/test_picker_patcher.js (+3/-1) lib/lp/app/widgets/popup.py (+7/-5) lib/lp/app/widgets/templates/form-picker-macros.pt (+3/-13) lib/lp/app/widgets/tests/test_popup.py (+6/-15) lib/lp/blueprints/browser/tests/test_specificationtarget.py (+2/-3) lib/lp/bugs/browser/tests/test_bugs.py (+2/-3) |
To merge this branch: | bzr merge lp:~jcsackett/launchpad/form-macros-frighten-me |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Benji York (community) | code | Approve | |
Review via email: mp+68277@code.launchpad.net |
Commit message
[r=benji][bug=799847] Cleans up form-picker-macros
Description of the change
Summary
=======
The form-picker-
This branch cleans this up to the extent possible.
Pre-implementation notes
=======
Benji York identified the problem in an earlier review; Aaron Bentley expanded on it in another pickers review, pointing out that the means of passing in all the various config data was likely more error prone since we were setting "booleans" in python code that were actually strings to be evaluated as javascript booleans.
Ian helped flesh out the final config ideas.
Proposed fix
============
The intial idea involved moving the make_picker function into the existing javascript files, and using the JSONRequestCache to get config data. It turns out that the popup.py widgets can't put values into JSONRequestCache (or if they can, it's sufficiently roundabout as to constitute a bad hack).
Implementation details
=======
The majority of the properties in the view are now assembled into a config property and a json_config propery, which uses simplejson to dump them to javascript. This approach was implemented simulataneously in this branch and in another that beat it to devel, so the majority of the work doesn't show up in the diff (being part of the tree). Please keep an eye open for bad conflict resolution artifacts on my part, since the collision was pretty massive. I believe I caught everything, and tests/UI all work as expected, but another set of eyes wouldn't hurt.
Because simplejson is now used to convert the variables into a javascript dictionary, the properties that were javascript style booleans can now be converted back into python booleans. They're unlikely to be used outside of the widget, but view code that does so will no longer be dangerous.
make_picker now exists in the picker files as initially planned.
Tests
=====
bin/test -vvct test_popup
bin/test -vvc --layer=YUI
QA
==
Checkout a personpicker instance (e.g. on +edit-people). Everything should work as before this branch.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
This branch looks good. I'm glad you're working on this.