launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03754
[Merge] lp:~sinzui/launchpad/project-picker-0 into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/project-picker-0 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/project-picker-0/+merge/62576
Restore the choose link to the bugs, blueprints, and answers scope widget.
Launchpad bug: https://bugs.launchpad.net/bugs/787856
Pre-implementation: no one
The ProjectScopeWidget is being setup correctly, but the templates do not ever
use __call__() to render the widget and the supporting js. Neither the widget
or the three broken templates have changed in years. I conclude the the
supporting js that was assumed to be loaded was removed. This is a very old
widget that predates YUI and maybe MochiKit. Refactoring to remove old
scripts, or change how we included scripts into the page broke these three
pages.
The fix is trivial now that I understand what is wrong. The templates avoid
__call__ because they are contriving a layout. The layout can be achieved be
changing the table and using the widget is the standard fashion.
--------------------------------------------------------------------
RULES
* Update the templates to call target_widget, or maybe the parent
widget's __call__ if the layout is not broken.
QA
* Visit https://bugs.qastaging.launchpad.net/
* Verify the search field is focused.
* Verify there is an ajax choose link after the One project field.
* Visit https://blueprints.qastaging.launchpad.net/
* Verify the search field is focused.
* Verify there is an ajax choose link after the One project field.
* Visit https://answers.qastaging.launchpad.net/
* Verify the search field is focused.
* Verify there is an ajax choose link after the One project field.
ADDENDUM
Bug 541979 may have been fixed by switching to standard widget.
* Visit https://answers.qastaging.launchpad.net/
* Search for "ubuntu" using the Choose link.
* Verify Ubuntu it found.
LINT
lib/lp/answers/browser/tests/test_questiontarget.py
lib/lp/answers/templates/questions-index.pt
lib/lp/blueprints/browser/tests/test_specificationtarget.py
lib/lp/blueprints/templates/specifications-index.pt
lib/lp/bugs/browser/tests/test_bugs.py
lib/lp/bugs/templates/malone-index.pt
^ there is lint in test_specificationtarget.py that I can fix before I land.
TEST
./bin/test -vv \
-t TestMaloneView, -t SpecificationSetViewTestCase
-t QuestionSetViewTestCase
IMPLEMENTATION
The diff is very large for such small trivial changes. The minimum
change I needed to make the for work was to update
<td tal:content="structure target_widget/chooseLink" />
to
<td tal:content="structure target_widget" />
so that the link and the script is rendered. All three forms were subtlety
different, and all manually rendered the scope widgets parts, which was
not necessary. The form focus script was missing from two of the forms too.
I decided to make the three forms more alike. They are no identical though,
and the three tests I added are very similar, but not identical. The forms
are tested elsewhere, as is the picker widget. The break was not discovered
for weeks/months because there was no test that verify the pages manually
rendered all the parts required for the form to operate with ajax.
lib/lp/answers/browser/tests/test_questiontarget.py
lib/lp/answers/templates/questions-index.pt
lib/lp/blueprints/browser/tests/test_specificationtarget.py
lib/lp/blueprints/templates/specifications-index.pt
lib/lp/bugs/browser/tests/test_bugs.py
lib/lp/bugs/templates/malone-index.pt
--
https://code.launchpad.net/~sinzui/launchpad/project-picker-0/+merge/62576
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/project-picker-0 into lp:launchpad.
=== modified file 'lib/lp/answers/browser/tests/test_questiontarget.py'
--- lib/lp/answers/browser/tests/test_questiontarget.py 2011-05-03 14:02:40 +0000
+++ lib/lp/answers/browser/tests/test_questiontarget.py 2011-05-26 22:32:32 +0000
@@ -12,6 +12,7 @@
from zope.component import getUtility
from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
+from canonical.launchpad.testing.pages import find_tag_by_id
from canonical.testing.layers import DatabaseFunctionalLayer
from lp.answers.interfaces.questioncollection import IQuestionSet
from lp.app.enums import ServiceUsage
@@ -193,3 +194,33 @@
self.assertCommonPageElements(content)
self.assertTrue(
content.find(True, id='configure-support') is not None)
+
+
+class QuestionSetViewTestCase(TestCaseWithFactory):
+ """Test the answers application root view."""
+
+ layer = DatabaseFunctionalLayer
+
+ def test_search_questions_form_rendering(self):
+ # The view's template directly renders the form widgets.
+ question_set = getUtility(IQuestionSet)
+ view = create_initialized_view(question_set, '+index')
+ content = find_tag_by_id(view.render(), 'search-all-questions')
+ self.assertEqual('form', content.name)
+ self.assertTrue(
+ content.find(True, id='text') is not None)
+ self.assertTrue(
+ content.find(True, id='field.actions.search') is not None)
+ self.assertTrue(
+ content.find(True, id='field.scope.option.all') is not None)
+ self.assertTrue(
+ content.find(True, id='field.scope.option.project') is not None)
+ target_widget = view.widgets['scope'].target_widget
+ self.assertTrue(
+ content.find(True, id=target_widget.show_widget_id) is not None)
+ text = str(content)
+ picker_script = (
+ "Y.lp.app.picker.create('DistributionOrProductOrProjectGroup'")
+ self.assertTrue(picker_script in text)
+ focus_script = "setFocusByName('field.search_text')"
+ self.assertTrue(focus_script in text)
=== modified file 'lib/lp/answers/templates/questions-index.pt'
--- lib/lp/answers/templates/questions-index.pt 2011-02-23 17:39:56 +0000
+++ lib/lp/answers/templates/questions-index.pt 2011-05-26 22:32:32 +0000
@@ -28,7 +28,7 @@
</div>
<div class="yui-g">
- <form class="central" action="" method="get"
+ <form id="search-all-questions" class="central" action="" method="get"
tal:attributes="action request/URL">
<table>
<tbody>
@@ -49,27 +49,21 @@
/>
</td>
</tr>
- <tr tal:define="target_widget nocall:view/widgets/scope/target_widget">
- <td tal:attributes="class view/scope_css_class"
- style="text-align: right;">
-
- <tal:scope
- replace="structure view/widgets/scope/renderScopeOptions" />
-
- <tal:input replace="structure target_widget/inputField" />
-
- <tal:sort
- replace="structure view/widgets/sort/hidden" />
- <tal:status
- replace="structure view/widgets/status/hidden" />
- <div class="message"
- tal:condition="view/scope_error"
- tal:content="structure view/scope_error">
- Error message
- </div>
- </td>
- <td tal:content="structure target_widget/chooseLink" />
- </tr>
+ <tr>
+ <td colspan="2"
+ tal:attributes="class view/scope_css_class">
+ <input tal:replace="structure view/widgets/scope" />
+ <div class="message"
+ tal:condition="view/widgets/scope/error"
+ tal:content="view/widgets/scope/error">
+ Error message
+ </div>
+ <script type="text/javascript"
+ tal:define="script view/focusedElementScript"
+ tal:condition="script"
+ tal:content="structure script" ></script>
+ </td>
+ </tr>
</tbody>
</table>
</form>
=== modified file 'lib/lp/blueprints/browser/tests/test_specificationtarget.py'
--- lib/lp/blueprints/browser/tests/test_specificationtarget.py 2010-09-23 15:15:02 +0000
+++ lib/lp/blueprints/browser/tests/test_specificationtarget.py 2011-05-26 22:32:32 +0000
@@ -6,6 +6,7 @@
from BeautifulSoup import BeautifulSoup
+from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
from canonical.launchpad.testing.pages import find_tag_by_id
@@ -16,6 +17,7 @@
)
from lp.app.enums import ServiceUsage
from lp.blueprints.browser.specificationtarget import HasSpecificationsView
+from lp.blueprints.interfaces.specification import ISpecificationSet
from lp.blueprints.publisher import BlueprintsLayer
from lp.testing import (
login_person,
@@ -274,3 +276,33 @@
def test_LAUNCHPAD_does_not_block_robots(self):
self._verify_robots_not_blocked(ServiceUsage.LAUNCHPAD)
+
+
+class SpecificationSetViewTestCase(TestCaseWithFactory):
+ """Test the specification application root view."""
+
+ layer = DatabaseFunctionalLayer
+
+ def test_search_specifications_form_rendering(self):
+ # The view's template directly renders the form widgets.
+ specification_set = getUtility(ISpecificationSet)
+ view = create_initialized_view(specification_set, '+index')
+ content = find_tag_by_id(view.render(), 'search-all-specifications')
+ self.assertEqual('form', content.name)
+ self.assertTrue(
+ content.find(True, id='text') is not None)
+ self.assertTrue(
+ content.find(True, id='field.actions.search') is not None)
+ self.assertTrue(
+ content.find(True, id='field.scope.option.all') is not None)
+ self.assertTrue(
+ content.find(True, id='field.scope.option.project') is not None)
+ target_widget = view.widgets['scope'].target_widget
+ self.assertTrue(
+ content.find(True, id=target_widget.show_widget_id) is not None)
+ text = str(content)
+ picker_script = (
+ "Y.lp.app.picker.create('DistributionOrProductOrProjectGroup'")
+ self.assertTrue(picker_script in text)
+ focus_script = "setFocusByName('field.search_text')"
+ self.assertTrue(focus_script in text)
=== modified file 'lib/lp/blueprints/templates/specifications-index.pt'
--- lib/lp/blueprints/templates/specifications-index.pt 2010-12-20 20:25:58 +0000
+++ lib/lp/blueprints/templates/specifications-index.pt 2011-05-26 22:32:32 +0000
@@ -28,7 +28,7 @@
</li>
</ul>
- <form class="central" action="" method="get"
+ <form id="search-all-specifications" class="central" action="" method="get"
tal:attributes="action request/URL">
<table>
<tbody>
@@ -43,30 +43,23 @@
/>
</td>
<td>
- <input
- type="submit"
- name="field.actions.search"
- value="Find Blueprints"
- />
+ <input tal:replace="structure view/search_action/render"/>
</td>
</tr>
- <tr tal:define="
- target_widget nocall:view/widgets/scope/target_widget">
- <td tal:attributes="class view/scope_css_class"
- style="text-align: right;">
-
- <tal:scope
- replace="structure view/widgets/scope/renderScopeOptions" />
-
- <tal:input replace="structure target_widget/inputField" />
-
+ <tr>
+ <td colspan="2"
+ tal:attributes="class view/scope_css_class">
+ <input tal:replace="structure view/widgets/scope" />
<div class="message"
- tal:condition="view/scope_error"
- tal:content="structure view/scope_error">
+ tal:condition="view/widgets/scope/error"
+ tal:content="view/widgets/scope/error">
Error message
</div>
+ <script type="text/javascript"
+ tal:define="script view/focusedElementScript"
+ tal:condition="script"
+ tal:content="structure script" ></script>
</td>
- <td tal:content="structure target_widget/chooseLink" />
</tr>
</tbody>
</table>
=== modified file 'lib/lp/bugs/browser/tests/test_bugs.py'
--- lib/lp/bugs/browser/tests/test_bugs.py 2011-03-02 21:44:33 +0000
+++ lib/lp/bugs/browser/tests/test_bugs.py 2011-05-26 22:32:32 +0000
@@ -8,6 +8,7 @@
from zope.component import getUtility
from canonical.launchpad.webapp.publisher import canonical_url
+from canonical.launchpad.testing.pages import find_tag_by_id
from canonical.testing.layers import DatabaseFunctionalLayer
from lp.bugs.interfaces.malone import IMaloneApplication
from lp.bugs.publisher import BugsLayer
@@ -67,3 +68,26 @@
self.assertEqual(
"Bug ['fnord', 'pting'] is not registered.", view.error_message)
self.assertEqual(None, view.request.response.getHeader('Location'))
+
+ def test_search_bugs_form_rendering(self):
+ # The view's template directly renders the form widgets.
+ view = create_initialized_view(self.application, '+index')
+ content = find_tag_by_id(view.render(), 'search-all-bugs')
+ self.assertEqual('form', content.name)
+ self.assertTrue(
+ content.find(True, id='field.searchtext') is not None)
+ self.assertTrue(
+ content.find(True, id='field.actions.search') is not None)
+ self.assertTrue(
+ content.find(True, id='field.scope.option.all') is not None)
+ self.assertTrue(
+ content.find(True, id='field.scope.option.project') is not None)
+ target_widget = view.widgets['scope'].target_widget
+ self.assertTrue(
+ content.find(True, id=target_widget.show_widget_id) is not None)
+ text = str(content)
+ picker_script = (
+ "Y.lp.app.picker.create('DistributionOrProductOrProjectGroup'")
+ self.assertTrue(picker_script in text)
+ focus_script = "setFocusByName('field.searchtext')"
+ self.assertTrue(focus_script in text)
=== modified file 'lib/lp/bugs/templates/malone-index.pt'
--- lib/lp/bugs/templates/malone-index.pt 2011-02-23 18:08:18 +0000
+++ lib/lp/bugs/templates/malone-index.pt 2011-05-26 22:32:32 +0000
@@ -16,7 +16,7 @@
tal:content="view/error_message">
Error message.
</p>
- <form class="central" action="/bugs/+bugs" method="get">
+ <form id="search-all-bugs" class="central" action="/bugs/+bugs" method="get">
<table>
<tbody>
<tr>
@@ -24,38 +24,27 @@
<input tal:replace="structure view/widgets/searchtext" />
</td>
<td>
- <input type="submit" name="search" value="Search Bug Reports" />
+ <input id="field.actions.search" type="submit"
+ name="search" value="Search Bug Reports" />
</td>
</tr>
- <tr tal:define="options nocall:view/widgets/scope/options;
- target_widget nocall:view/widgets/scope/target_widget">
- <td tal:attributes="class view/target_css_class"
- style="text-align: right;">
- <div>
- <label>
- <input tal:replace="structure options/all" />
- All projects
- </label>
- <label>
- <input tal:replace="structure options/project" />
- One project:
- <tal:input replace="structure target_widget/inputField" />
- </label>
- </div>
- <div class="message"
- tal:condition="view/target_error"
- tal:content="structure view/target_error">
+ <tr>
+ <td colspan="2"
+ tal:attributes="class view/target_css_class">
+ <input tal:replace="structure view/widgets/scope" />
+ <div class="message"
+ tal:condition="view/widgets/scope/error"
+ tal:content="view/widgets/scope/error">
Error message
- </div>
+ </div>
</td>
- <td tal:content="structure target_widget/chooseLink" />
</tr>
</tbody>
</table>
<script type="text/javascript"
tal:define="script view/focusedElementScript"
tal:condition="script"
- tal:content="structure script" />
+ tal:content="structure script" ></script>
</form>
<p id="application-summary">
Launchpad’s bug tracker helps software teams to
