Merge lp:~rvb/launchpad/fix-missing-add-distro-link into lp:launchpad
Status: | Merged |
---|---|
Merged at revision: | 12561 |
Proposed branch: | lp:~rvb/launchpad/fix-missing-add-distro-link |
Merge into: | lp:launchpad |
Diff against target: |
57 lines (+24/-2) 3 files modified
lib/lp/registry/browser/distribution.py (+3/-1) lib/lp/registry/browser/menu.py (+5/-0) lib/lp/soyuz/stories/soyuz/xx-distribution-add.txt (+16/-1) |
To merge this branch: | bzr merge lp:~rvb/launchpad/fix-missing-add-distro-link |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
j.c.sackett (community) | Approve | ||
Review via email: mp+52104@code.launchpad.net |
Commit message
[r=jcsackett][bug=728553] Added a 'Register a distribution' link to the distros page.
Description of the change
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Added a link to create a new distribution to the distributions page
(shown only to admin users).
The permission check will probably need to be against another permission
than 'launchpad.Admin' once the permission model for creating
distributions is in place.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/
lib/lp/
lib/lp/
./lib/lp/
1: narrative uses a moin header.
3: narrative has trailing whitespace.
6: source has bad indentation.
14: source has bad indentation.
20: source has bad indentation.
27: source has bad indentation.
31: source has bad indentation.
35: source exceeds 78 characters.
39: source has bad indentation.
43: source has bad indentation.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://
iQIcBAEBAgAGBQJ
n5sP05zCUC/
2+6gIc35r7/
BMw19mZaMZMZpnO
tvp1mVIbWLvnssj
uCD1ap8ufLwUTE8
fcJzMECvkEDvodq
3zXQ9I6i85UcZAQ
pkBPEi9FhIsVDQt
BOEngVMMaJTMhA8
JGYaeEn6sDdcFOt
xRHdSkx+
=6flm
-----END PGP SIGNATURE-----
> === modified file 'lib/lp/ registry/ browser/ menu.py' registry/ browser/ menu.py 2010-08-20 20:31:18 +0000 registry/ browser/ menu.py 2011-03-03 17:49:43 +0000 people/ +newteam' , text, icon='add') with_permission ('launchpad. Admin') distribution( self): distros/ +add', text, icon='add') account( self):
> --- lib/lp/
> +++ lib/lp/
> @@ -47,6 +47,14 @@
> text = 'Register a team'
> return Link('/
>
> + # TODO: rvb 20110103 DISTRO.PERMISSIONS
> + # this should be checked against the right permission once
> + # the permission model for distributions is in place
> + @enabled_
> + def register_
> + text = 'Register a distribution'
> + return Link('/
> +
> def create_
> text = 'Create an account'
> # Only enable this link for anonymous users.
The TODO above should really be put in as an XXX, and probably have an associated bug. See https:/ /dev.launchpad. net/PolicyandPr ocess/XXXPolicy for more info on what I mean.
I'm unsure if a decorator or a zcml entry would be the best way to lock down permissions on this. Did you have a particular rationale for going with the decorator?
> === modified file 'lib/lp/ soyuz/stories/ soyuz/xx- distribution- add.txt' soyuz/stories/ soyuz/xx- distribution- add.txt 2009-08-13 15:12:16 +0000 soyuz/stories/ soyuz/xx- distribution- add.txt 2011-03-03 17:49:43 +0000 launchpad. dev/distros") getLink( "Register a distribution") open("http:// launchpad. dev/distros") getLink( "Register a distribution").url launchpad. dev/distros/ +add' launchpad. dev/distros/ +add")
> --- lib/lp/
> +++ lib/lp/
> @@ -1,6 +1,21 @@
> = Creating new distributions =
>
> -This can be done only by launchpad admins.
> +A non launchpad admin doesn't see the link to create a new distribution on
> +the distributions page:
> +
> + >>> user_browser.open("http://
> + >>> user_browser.
> + Traceback (most recent call last):
> + ...
> + LinkNotFoundError
> +
> +A launchpad admin sees the link to create a new distribution:
> +
> + >>> admin_browser.
> + >>> admin_browser.
> + 'http://
> +
> +A launchpad admin can create a new distribution:
>
> >>> user_browser.open("http://
> Traceback (most recent call last):
This story looks fine, but I would really prefer a unittest instead of or in addition to this. Historically we've over-relied on doctests and stories, which don't perfectly isolate things that may be failing.
I see that you had a unittest in a previous revision, that actually looks pretty good, though it can probably be added to lib/lp/ registry/ browser/ tests. What was the reason to convert it to a story?