Launchpad itself

Merge lp:~thumper/launchpad/kill-bad-form-preloads into lp:launchpad

  1. kill-bad-form-preloads
  2. Merge into devel
Proposed by Tim Penhey
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 11770
Proposed branch: lp:~thumper/launchpad/kill-bad-form-preloads
Merge into: lp:launchpad
Diff against target: 60 lines (+17/-10)
3 files modified
lib/lp/code/templates/branch-index.pt (+11/-8)
lib/lp/code/templates/branch-information.pt (+1/-1)
lib/lp/code/templates/branch-related-bugs-specs.pt (+5/-1)
To merge this branch: bzr merge lp:~thumper/launchpad/kill-bad-form-preloads
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) code js Approve
Review via email: mp+38800@code.launchpad.net

Commit message

Only load the link bug and subscription ++form++ when the user is logged in.

Description of the change

Several simple changes to stop lots of 404s.

Only do the javascript binding and loading of the forms if the user is logged in.

Windmill tests should be sufficient.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

I think this is an improvement for now, but wouldn't it be nice to
have a login triggered if someone tries something, rather than having
the form appear less complete? (Perhaps I misunderstand what happens).

Revision history for this message
Tim Penhey (thumper) wrote :

We can't easily trigger the login from the ajax calls. By not hooking up the link bug, we are leaving the link there, but not connected to ajax. That way they do get sent to the login page, even though they then go to an icky non-js version for linking a bug.

The edit button just shouldn't be there if they are not able to edit.

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

We discussed on IRC. It's annoying that this isn't tested, but a windmill test would be overkill. Alternatives won't help enough to carry their weight.

Rockstar came up with a good suggestion for the longer term: the login check should happen inside the widget.

review: Approve (js)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/templates/branch-index.pt'
2--- lib/lp/code/templates/branch-index.pt 2010-05-28 16:14:15 +0000
3+++ lib/lp/code/templates/branch-index.pt 2010-10-19 03:11:44 +0000
4@@ -36,15 +36,18 @@
5 'lp.code.branch.subscription', function(Y) {
6
7 Y.on('load', function(e) {
8- var subscription_portlet = new Y.lp.code.branch.subscription.SubscriptionWidget({
9- contentBox: '#portlet-subscribers'
10- });
11- subscription_portlet.render();
12-
13- Y.lp.code.branch.status.connect_status(${view/status_config});
14+ var logged_in = LP.client.links['me'] !== undefined;
15+
16+ if (logged_in) {
17+ var subscription_portlet = new Y.lp.code.branch.subscription.SubscriptionWidget({
18+ contentBox: '#portlet-subscribers'
19+ });
20+ subscription_portlet.render();
21+
22+ Y.lp.code.branch.status.connect_status(${view/status_config});
23+ }
24 Y.lp.code.branchmergeproposal.diff.connect_diff_links();
25- },
26- window);
27+ }, window);
28 });
29 "/>
30 </metal:block>
31
32=== modified file 'lib/lp/code/templates/branch-information.pt'
33--- lib/lp/code/templates/branch-information.pt 2009-09-28 21:46:01 +0000
34+++ lib/lp/code/templates/branch-information.pt 2010-10-19 03:11:44 +0000
35@@ -26,7 +26,7 @@
36 <span id="branch-details-status-value">
37 <span tal:attributes="class string:value branchstatus${context/lifecycle_status/name}"
38 tal:content="structure context/lifecycle_status/title" />&nbsp;
39- <a href="+edit-status">
40+ <a href="+edit-status" tal:condition="context/required:launchpad.Edit">
41 <img class="editicon" src="/@@/edit"/>
42 </a>
43 </span>
44
45=== modified file 'lib/lp/code/templates/branch-related-bugs-specs.pt'
46--- lib/lp/code/templates/branch-related-bugs-specs.pt 2010-06-10 07:54:59 +0000
47+++ lib/lp/code/templates/branch-related-bugs-specs.pt 2010-10-19 03:11:44 +0000
48@@ -81,7 +81,11 @@
49 }
50
51 Y.on('domready', function() {
52- Y.lp.code.branch.bugspeclinks.connect_branchlinks();
53+ var logged_in = LP.client.links['me'] !== undefined;
54+
55+ if (logged_in) {
56+ Y.lp.code.branch.bugspeclinks.connect_branchlinks();
57+ }
58 });
59
60 });