launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03920
lp:~allenap/launchpad/localpackagediffs-latest-comment-bug-746379 into lp:launchpad
Gavin Panella has proposed merging lp:~allenap/launchpad/localpackagediffs-latest-comment-bug-746379 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #746379 in Launchpad itself: "On +localpackagediffs, when adding a new comment, it updates the expandable section but not the "latest comment" in the table."
https://bugs.launchpad.net/launchpad/+bug/746379
For more details, see:
https://code.launchpad.net/~allenap/launchpad/localpackagediffs-latest-comment-bug-746379/+merge/64334
In DistroSeries:+localpackagediffs, When adding a comment to a difference row, this branch ensures that the "Latest comment" column is also updated. It does this by requesting a +latest-comment-fragment view of the new comment from the server. This is a new view that the +localpackagediffs template has been updated to use also.
In common with most of the code on +localpackagediffs, this code is untested. I don't mean to offend anyone, but the code in that module (lp.registry.distroseriesdifferences_details) made me a little bit sad. The priority of this bug is not high enough to warrant the refactoring needed to make it sanely testable.
--
https://code.launchpad.net/~allenap/launchpad/localpackagediffs-latest-comment-bug-746379/+merge/64334
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/localpackagediffs-latest-comment-bug-746379 into lp:launchpad.
=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml 2011-06-02 18:44:46 +0000
+++ lib/lp/registry/browser/configure.zcml 2011-06-12 14:31:37 +0000
@@ -193,10 +193,16 @@
module="lp.registry.browser.distroseriesdifference"
classes="DistroSeriesDifferenceNavigation"/>
<browser:url
- for="lp.registry.interfaces.distroseriesdifferencecomment.IDistroSeriesDifferenceComment"
+ for="..interfaces.distroseriesdifferencecomment.IDistroSeriesDifferenceComment"
path_expression="string:comments/${id}"
attribute_to_parent="distro_series_difference"
rootsite="mainsite"/>
+ <browser:page
+ for="..interfaces.distroseriesdifferencecomment.IDistroSeriesDifferenceComment"
+ template="../templates/distroseriesdifferencecomment-latest-comment-fragment.pt"
+ class="canonical.launchpad.webapp.LaunchpadView"
+ permission="zope.Public"
+ name="+latest-comment-fragment" />
<adapter
factory="lp.registry.browser.distroseriesdifference.CommentXHTMLRepresentation"
name="lazr.restful.EntryResource" />
=== added file 'lib/lp/registry/browser/tests/test_distroseriesdifferencecomment.py'
--- lib/lp/registry/browser/tests/test_distroseriesdifferencecomment.py 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/browser/tests/test_distroseriesdifferencecomment.py 2011-06-12 14:31:37 +0000
@@ -0,0 +1,30 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for `lp.registry.browser.distroseriesdifferencecomment`."""
+
+__metaclass__ = type
+
+from lxml import html
+
+from canonical.testing.layers import LaunchpadFunctionalLayer
+from lp.testing import TestCaseWithFactory
+from lp.testing.views import create_initialized_view
+
+
+class TestDistroSeriesDifferenceCommentFragment(TestCaseWithFactory):
+ """`IDistroSeriesDifferenceComment` +latest-comment-fragment view."""
+
+ layer = LaunchpadFunctionalLayer
+
+ def test_render(self):
+ comment_text = "_123456789" * 10
+ comment = self.factory.makeDistroSeriesDifferenceComment(
+ comment=comment_text)
+ view = create_initialized_view(comment, '+latest-comment-fragment')
+ root = html.fromstring(view())
+ self.assertEqual("span", root.tag)
+ self.assertEqual("%s..." % comment_text[:47], root.text.strip())
+ self.assertEqual(
+ "/~%s" % comment.comment_author.name,
+ root.find("span").find("a").get("href"))
=== modified file 'lib/lp/registry/javascript/distroseriesdifferences_details.js'
--- lib/lp/registry/javascript/distroseriesdifferences_details.js 2011-06-10 19:03:53 +0000
+++ lib/lp/registry/javascript/distroseriesdifferences_details.js 2011-06-12 14:31:37 +0000
@@ -109,6 +109,31 @@
};
/**
+ * Update the latest comment on the difference row.
+ *
+ * @param comment_entry {lp.client.Entry} An object representing
+ * a DistroSeriesDifferenceComment.
+ */
+ var update_latest_comment = function(comment_entry, placeholder) {
+ var comment_latest_fragment_url =
+ comment_entry.get("web_link") + "/+latest-comment-fragment";
+ var config = {
+ on: {
+ success: function(comment_latest_fragment_html) {
+ placeholder.set(
+ "innerHTML", comment_latest_fragment_html);
+ Y.lazr.anim.green_flash({node: placeholder}).run();
+ },
+ failure: function(id, response) {
+ Y.lazr.anim.red_flash({node: placeholder}).run();
+ }
+ },
+ accept: Y.lp.client.XHTML
+ };
+ lp_client.get(comment_latest_fragment_url, config);
+ };
+
+ /**
* Handle the add comment event.
*
* This method adds a comment via the API and update the UI.
@@ -118,7 +143,8 @@
* @param api_uri {string} The uri for the distroseriesdifference to which
* the comment is to be added.
*/
- var add_comment_handler = function(comment_form, api_uri) {
+ var add_comment_handler = function(
+ comment_form, latest_comment_placeholder, api_uri) {
var comment_text = comment_form.one('textarea').get('value');
@@ -127,7 +153,7 @@
var success_handler = function(comment_entry) {
// Grab the XHTML representation of the comment
// and prepend it to the list of comments.
- config = {
+ var config = {
on: {
success: function(comment_html) {
comment_node = Y.Node.create(comment_html);
@@ -143,6 +169,7 @@
lp_client.get(comment_entry.get('self_link'), config);
comment_form.one('textarea').set('value', '');
toggle_comment_in_progress(comment_form);
+ update_latest_comment(comment_entry, latest_comment_placeholder);
};
var failure_handler = function(id, response) {
// Re-enable field with red flash.
@@ -176,7 +203,8 @@
* @param api_uri {string} The uri for the distroseriesdifference to which
* the comment is to be added.
*/
- var setup_add_comment = function(placeholder, api_uri) {
+ var setup_add_comment = function(
+ placeholder, latest_comment_placeholder, api_uri) {
placeholder.insert([
'<a class="widget-hd js-action sprite add" href="">',
' Add comment</a>',
@@ -203,7 +231,8 @@
placeholder.one('button').on('click', function(e) {
e.preventDefault();
- add_comment_handler(placeholder, api_uri);
+ add_comment_handler(
+ placeholder, latest_comment_placeholder, api_uri);
});
};
@@ -211,12 +240,14 @@
* Get the extra information for this diff to display.
*
* @param uri {string} The uri for the extra diff info.
+ * @param master_container {Node} The node that triggered the load
+ * of the extra info.
* @param container {Node} A node which must contain a div with the
* class 'diff-extra-container' into which the results
* are inserted.
*/
- var get_extra_diff_info = function(uri, container, source_name,
- parent_distro_name,
+ var get_extra_diff_info = function(uri, master_container, container,
+ source_name, parent_distro_name,
parent_series_name) {
var in_progress_message = Y.lp.soyuz.base.makeInProgressNode(
'Fetching difference details ...');
@@ -238,6 +269,7 @@
var blacklist_slot = args.container.one('div.blacklist-options');
if (blacklist_slot !== null) {
setup_blacklist_options(blacklist_slot, source_name, api_uri);
+<<<<<<< TREE
}
// The add comment slot is only available when the user has the
// right to add comments.
@@ -248,14 +280,23 @@
}
// Set-up diffs and the means to request them.
namespace.setup_packages_diff_states(args.container, api_uri);
+=======
+ setup_add_comment(
+ args.container.one('div.add-comment-placeholder'),
+ args.master_container.one('td.latest-comment-fragment'),
+ api_uri);
+ namespace.setup_packages_diff_states(args.container, api_uri);
+ }
+>>>>>>> MERGE-SOURCE
};
var failure_cb = function(transaction_id, response, args) {
var retry_handler = function(e) {
e.preventDefault();
get_extra_diff_info(
- args.uri, args.container, args.source_name,
- args.parent_distro_name, args.parent_series_name);
+ args.uri, args.master_container, args.container,
+ args.source_name, args.parent_distro_name,
+ args.parent_series_name);
};
var failure_message = Y.lp.soyuz.base.makeFailureNode(
'Failed to fetch difference details.', retry_handler);
@@ -273,7 +314,12 @@
'success': success_cb,
'failure': failure_cb
},
+<<<<<<< TREE
"arguments": {
+=======
+ 'arguments': {
+ 'master_container': master_container,
+>>>>>>> MERGE-SOURCE
'container': container,
'uri': uri,
'source_name': source_name
@@ -308,8 +354,8 @@
row.insert(details_row, 'after');
var uri = toggle.get('href');
get_extra_diff_info(
- uri, details_row.one('td'), source_name, parent_distro_name,
- parent_series_name);
+ uri, row, details_row.one('td'), source_name,
+ parent_distro_name, parent_series_name);
} else {
details_row = next_row;
}
=== modified file 'lib/lp/registry/templates/distroseries-localdifferences.pt'
--- lib/lp/registry/templates/distroseries-localdifferences.pt 2011-05-27 07:50:38 +0000
+++ lib/lp/registry/templates/distroseries-localdifferences.pt 2011-06-12 14:31:37 +0000
@@ -167,15 +167,11 @@
</tal:derived>
</td>
- <td>
- <tal:comment tal:define="comment difference/latest_comment"
- tal:condition="comment">
- <span tal:replace="comment/body_text/fmt:shorten/50">I'm on this.</span>
- <br /><span class="greyed-out greylink"><span
- tal:replace="comment/comment_date/fmt:approximatedate">2005-09-16</span>
- by <a tal:replace="structure
- comment/comment_author/fmt:link">joesmith</a></span>
- </tal:comment>
+ <td class="latest-comment-fragment">
+ <tal:latest-comment-fragment
+ define="comment difference/latest_comment"
+ content="structure comment/@@+latest-comment-fragment"
+ condition="comment" />
</td>
</tr>
=== added file 'lib/lp/registry/templates/distroseriesdifferencecomment-latest-comment-fragment.pt'
--- lib/lp/registry/templates/distroseriesdifferencecomment-latest-comment-fragment.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/templates/distroseriesdifferencecomment-latest-comment-fragment.pt 2011-06-12 14:31:37 +0000
@@ -0,0 +1,10 @@
+<span
+ xmlns="http://www.w3.org/1999/xhtml";
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+ i18n:domain="launchpad">
+ <span tal:replace="context/body_text/fmt:shorten/50">I'm on this.</span>
+ <br /><span class="greyed-out greylink"><span
+ tal:replace="context/comment_date/fmt:approximatedate">2005-09-16</span>
+ by <a tal:replace="structure context/comment_author/fmt:link" /></span>
+</span>
Follow ups
