launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #12596
[Merge] lp:~wallyworld/launchpad/change-bug-info-type-refresh2-1057142 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/change-bug-info-type-refresh2-1057142 into lp:launchpad.
Commit message:
Update the "Affects Project/Package" links on the bugtask page so that they are shown/hidden as appropriate when the info type is changed.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1057142 in Launchpad itself: "The div.actions block is not refreshed after changing a bug's information type"
https://bugs.launchpad.net/launchpad/+bug/1057142
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/change-bug-info-type-refresh2-1057142/+merge/126845
== Implementation ==
Update the "Affects Project/Package" links on the bugtask page so that they are shown/hidden as appropriate when the info type is changed.
The BugSecrecyEditView already includes new subscriber data in the result of the ajax response. Add to this return data the new state of the affects links and modify the javascript to add/remove the required css class accordingly.
== Tests ==
Add yui tests. Also, tweak some banner code so that it doesn't blow up under tests due to animations.
Add extra test for the secrecy view also.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/app/javascript/information_type.js
lib/lp/app/javascript/banners/banner.js
lib/lp/app/javascript/tests/test_information_type.html
lib/lp/app/javascript/tests/test_information_type.js
lib/lp/bugs/browser/bug.py
lib/lp/bugs/browser/bugtask.py
lib/lp/bugs/browser/tests/test_bug_views.py
--
https://code.launchpad.net/~wallyworld/launchpad/change-bug-info-type-refresh2-1057142/+merge/126845
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/change-bug-info-type-refresh2-1057142 into lp:launchpad.
=== modified file 'lib/lp/app/javascript/banners/banner.js'
--- lib/lp/app/javascript/banners/banner.js 2012-09-25 18:47:19 +0000
+++ lib/lp/app/javascript/banners/banner.js 2012-09-28 02:37:22 +0000
@@ -53,9 +53,13 @@
duration: anim_times.slide_out,
easing: Y.Easing.easeOut
});
- fade_in.run();
- body_space.run();
- login_space.run();
+ if (anim_times.fade > 0) {
+ fade_in.run();
+ }
+ if (anim_times.slide_out > 0) {
+ body_space.run();
+ login_space.run();
+ }
},
_hideBanner: function () {
=== modified file 'lib/lp/app/javascript/information_type.js'
--- lib/lp/app/javascript/information_type.js 2012-09-27 03:17:02 +0000
+++ lib/lp/app/javascript/information_type.js 2012-09-28 02:37:22 +0000
@@ -159,21 +159,32 @@
};
ns.save_success = function(widget, context, value, subscribers_list,
- subscribers_data) {
+ result_data) {
context.information_type =
ns.get_cache_data_from_key(value, 'value', 'name');
ns.update_privacy_banner(value);
widget._showSucceeded();
- if (Y.Lang.isObject(subscribers_data)) {
- var subscribers = subscribers_data.subscription_data;
+ if (Y.Lang.isObject(result_data)) {
+ var subscribers = result_data.subscription_data;
subscribers_list._loadSubscribersFromList(subscribers);
- var cache_data = subscribers_data.cache_data;
+ var cache_data = result_data.cache_data;
var item;
for (item in cache_data) {
if (cache_data.hasOwnProperty(item)) {
LP.cache[item] = cache_data[item];
}
}
+ // Update the bugtask actions.
+ var project_span = Y.one('#also-affects-product');
+ if (Y.Lang.isValue(project_span)) {
+ project_span.toggleClass(
+ 'private-disallow', !result_data.can_add_project_task);
+ }
+ var package_span = Y.one('#also-affects-package');
+ if (Y.Lang.isValue(project_span)) {
+ package_span.toggleClass(
+ 'private-disallow', !result_data.can_add_package_task);
+ }
}
if (Y.Lang.isValue(subscribers_list)){
var subscription_ns = Y.lp.bugs.bugtask_index.portlets.subscription;
@@ -338,7 +349,8 @@
*/
ns.update_privacy_banner = function(value) {
var body = Y.one('body');
- var privacy_banner = Y.lp.app.banner.privacy.getPrivacyBanner();
+ var privacy_banner = Y.lp.app.banner.privacy.getPrivacyBanner(
+ undefined, skip_animation);
var private_type = LP.cache.information_type_data[value].is_private;
if (private_type) {
body.replaceClass('public', 'private');
=== modified file 'lib/lp/app/javascript/tests/test_information_type.html'
--- lib/lp/app/javascript/tests/test_information_type.html 2012-08-31 14:35:33 +0000
+++ lib/lp/app/javascript/tests/test_information_type.html 2012-09-28 02:37:22 +0000
@@ -103,6 +103,8 @@
<div id="information-type-description">Everyone can see this information.</div>
</div>
</div>
+ <span id="also-affects-product"></span>
+ <span id="also-affects-package" class="private-disallow"></span>
<div id="current_user_subscription">
<span></span>
<a href="#" text="" class="menu-link-mute_subscription unmute"></a>
=== modified file 'lib/lp/app/javascript/tests/test_information_type.js'
--- lib/lp/app/javascript/tests/test_information_type.js 2012-09-25 19:15:04 +0000
+++ lib/lp/app/javascript/tests/test_information_type.js 2012-09-28 02:37:22 +0000
@@ -112,12 +112,14 @@
var save_success_called = false;
ns.save_success = function(widget, context, value,
subscribers_list,
- subscribers_data) {
+ result_data) {
Y.Assert.areEqual('USERDATA', value);
Y.Assert.areEqual(
- 'subscribers', subscribers_data.subscription_data);
+ 'subscribers', result_data.subscription_data);
Y.Assert.areEqual(
- 'value', subscribers_data.cache_data.item);
+ 'value', result_data.cache_data.item);
+ Y.Assert.isBoolean(result_data.can_add_project_task);
+ Y.Assert.isBoolean(result_data.can_add_package_task);
save_success_called = true;
};
ns.save(
@@ -125,7 +127,9 @@
LP.cache.bug, null, true);
this.mockio.success({
responseText: '{"subscription_data": "subscribers",' +
- '"cache_data": {"item": "value"}}',
+ '"cache_data": {"item": "value"},' +
+ '"can_add_project_task": true,' +
+ '"can_add_package_task": true}',
responseHeaders: {'Content-Type': 'application/json'}});
Y.Assert.areEqual(
document.URL + '/+secrecy', this.mockio.last_request.url);
@@ -199,7 +203,7 @@
load_subscribers_called = true;
}
};
- var subscribers_data = {
+ var result_data = {
subscription_data: 'subscriber',
cache_data: {
item1: 'value1',
@@ -208,13 +212,33 @@
};
ns.save_success(
this.widget, LP.cache.bug, 'PUBLIC', subscribers_list,
- subscribers_data);
+ result_data);
Y.Assert.isTrue(load_subscribers_called);
Y.Assert.areEqual('value1', window.LP.cache.item1);
Y.Assert.areEqual('value2', window.LP.cache.item2);
this._unshim_privacy_banner(old_func);
},
+ // A successful save updates the task actions.
+ test_save_success_with_task_data: function() {
+ this.makeWidget();
+ var subscribers_list = {
+ _loadSubscribersFromList: function(subscription_data) {
+ }
+ };
+ var result_data = {
+ can_add_project_task: false,
+ can_add_package_task: true
+ };
+ ns.save_success(
+ this.widget, LP.cache.bug, 'PUBLIC', subscribers_list,
+ result_data);
+ Y.Assert.isTrue(
+ Y.one('#also-affects-product').hasClass('private-disallow'));
+ Y.Assert.isFalse(
+ Y.one('#also-affects-package').hasClass('private-disallow'));
+ },
+
// Select a new information type and respond with a validation error.
_assert_save_with_validation_error: function() {
this.makeWidget();
@@ -351,7 +375,7 @@
var public_ev = Y.on('information_type:is_public', function (ev) {
Y.Assert.areEqual('PUBLIC', ev.value);
called = true;
- });
+ });
// However is should not fire an is_private event.
var private_ev = Y.on('information_type:is_private', function (ev) {
called = false;
=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py 2012-09-20 13:59:25 +0000
+++ lib/lp/bugs/browser/bug.py 2012-09-28 02:37:22 +0000
@@ -912,8 +912,22 @@
bug, bug_before_modification, changed_fields,
user=self.user))
if self.request.is_ajax:
+ # Avoid circular imports
+ from lp.bugs.browser.bugtask import (
+ can_add_package_task_to_bug,
+ can_add_project_task_to_bug,
+ )
if changed:
- return self._getSubscriptionDetails()
+ result_data = self._getSubscriptionDetails()
+ result_data['can_add_project_task'] = (
+ can_add_project_task_to_bug(bug))
+ result_data['can_add_package_task'] = (
+ can_add_package_task_to_bug(bug))
+ self.request.response.setHeader(
+ 'content-type', 'application/json')
+ return dumps(
+ result_data, cls=ResourceJSONEncoder,
+ media_type=EntryResource.JSON_TYPE)
else:
return ''
@@ -946,10 +960,7 @@
result_data = dict(
cache_data=cache,
subscription_data=subscription_data)
- self.request.response.setHeader('content-type', 'application/json')
- return dumps(
- result_data, cls=ResourceJSONEncoder,
- media_type=EntryResource.JSON_TYPE)
+ return result_data
def _handlePrivacyChanged(self, user_will_be_subscribed):
"""Handle the case where the privacy of the bug has been changed.
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2012-09-20 13:59:25 +0000
+++ lib/lp/bugs/browser/bugtask.py 2012-09-28 02:37:22 +0000
@@ -31,6 +31,8 @@
'BugTaskTableRowView',
'BugTaskTextView',
'BugTaskView',
+ 'can_add_package_task_to_bug',
+ 'can_add_project_task_to_bug',
'get_buglisting_search_filter_url',
'get_comments_for_bugtask',
'get_sortorder_from_request',
@@ -3890,34 +3892,40 @@
return None
def canAddProjectTask(self):
- """Can a new bug task on a project be added to this bug?
-
- If a bug has any bug tasks already, were it to be Proprietary or
- Embargoed, it cannot be marked as also affecting any other
- project, so return False.
- """
- bug = self.context
- if bug.information_type not in PROPRIETARY_INFORMATION_TYPES:
- return True
- return len(bug.bugtasks) == 0
+ return can_add_project_task_to_bug(self.context)
def canAddPackageTask(self):
- """Can a new bug task on a src pkg be added to this bug?
-
- If a bug has any existing bug tasks on a project, were it to
- be Proprietary or Embargoed, then it cannot be marked as
- affecting a package, so return False.
-
- A task on a given package may still be illegal to add, but
- this will be caught when bug.addTask() is attempted.
- """
- bug = self.context
- if bug.information_type not in PROPRIETARY_INFORMATION_TYPES:
- return True
- for pillar in bug.affected_pillars:
- if IProduct.providedBy(pillar):
- return False
- return True
+ return can_add_package_task_to_bug(self.context)
+
+
+def can_add_project_task_to_bug(bug):
+ """Can a new bug task on a project be added to this bug?
+
+ If a bug has any bug tasks already, were it to be Proprietary or
+ Embargoed, it cannot be marked as also affecting any other
+ project, so return False.
+ """
+ if bug.information_type not in PROPRIETARY_INFORMATION_TYPES:
+ return True
+ return len(bug.bugtasks) == 0
+
+
+def can_add_package_task_to_bug(bug):
+ """Can a new bug task on a src pkg be added to this bug?
+
+ If a bug has any existing bug tasks on a project, were it to
+ be Proprietary or Embargoed, then it cannot be marked as
+ affecting a package, so return False.
+
+ A task on a given package may still be illegal to add, but
+ this will be caught when bug.addTask() is attempted.
+ """
+ if bug.information_type not in PROPRIETARY_INFORMATION_TYPES:
+ return True
+ for pillar in bug.affected_pillars:
+ if IProduct.providedBy(pillar):
+ return False
+ return True
class BugTaskTableRowView(LaunchpadView, BugTaskBugWatchMixin,
=== modified file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py 2012-09-18 19:44:35 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py 2012-09-28 02:37:22 +0000
@@ -418,6 +418,15 @@
subscriber.name, subscriber_data['subscriber']['name'])
self.assertEqual('Discussion', subscriber_data['subscription_level'])
+ def test_secrecy_view_ajax_can_add_tasks(self):
+ # The return data contains flags indicating whether project and package
+ # tasks can be added.
+ bug = self.factory.makeBug()
+ result_data = self._assert_secrecy_view_ajax_render(
+ bug, 'USERDATA', True)
+ self.assertTrue(result_data['can_add_project_task'])
+ self.assertTrue(result_data['can_add_package_task'])
+
def test_secrecy_view_ajax_render_no_check(self):
# An information type change request is processed as expected when the
# bug will become invisible but and no visibility check is performed.
Follow ups
