Merge lp:~julian-edwards/launchpad/first-ppa-name-bug-439305 into lp:launchpad

[Julian Edwards]

[Julian Edwards (julian-edwards)]

= Summary =
Allow first PPA created to have a name other than 'ppa'

== Proposed fix ==
The first PPA created always has a name of 'ppa' which is now unnecessary. It
was originally done this way so that we could migrate unnamed PPAs when adding
the multiple-PPAs feature and keep the old upload path.

== Pre-implementation notes ==
Chatted to Michael N, and I've refactored some of the Archive-specific queries
in model/person.py into IArchiveSet. The main isssue was that the
IPerson.archive property relied on the fact that the first PPA was created
with the 'ppa' name. It's now taking the first PPA it finds, ordered by ID.

== Implementation details ==
 * Added IArchiveSet method to query PPAs for a person and changed IPerson
code to use that.
 * Changed the first activation form to have the name field on it.
 * Drive-by fixes on the heading for the activation page.

== Tests ==
bin/test -cvvt person.txt -t xx-ppa-workflow.txt

== Demo and Q/A ==
See http://launchpad.dev/~name12/+activate-ppa

Admins can activate someone else's PPA so if you're logged in as Foo Bar then
you're good :)

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/soyuz/stories/ppa/xx-ppa-workflow.txt
  lib/lp/soyuz/templates/archive-activate.pt
  lib/lp/soyuz/doc/archive.txt
  lib/lp/soyuz/interfaces/archive.py
  lib/lp/soyuz/model/archive.py
  lib/lp/registry/model/person.py
  lib/lp/soyuz/browser/archive.py

== Pylint notices ==

lib/lp/soyuz/interfaces/archive.py
    40: [F0401] Unable to import 'lazr.enum' (No module named enum)
    53: [F0401] Unable to import 'lazr.restful.declarations' (No module named
restful)
    59: [F0401] Unable to import 'lazr.restful.fields' (No module named
restful)

lib/lp/soyuz/model/archive.py
    14: [F0401] Unable to import 'lazr.lifecycle.event' (No module named
lifecycle)

[Francis J. Lacoste (flacoste)]

review: Approve (release-critical)

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2009-09-30 01:20:39 +0000
+++ lib/lp/registry/model/person.py	2009-10-13 10:25:26 +0000
@@ -77,7 +77,7 @@
 from canonical.launchpad.interfaces.account import (
     AccountCreationRationale, AccountStatus, IAccount, IAccountSet,
     INACTIVE_ACCOUNT_STATUSES)
-from lp.soyuz.interfaces.archive import ArchivePurpose, NoSuchPPA
+from lp.soyuz.interfaces.archive import ArchivePurpose, IArchiveSet
 from lp.soyuz.interfaces.archivepermission import (
     IArchivePermissionSet)
 from canonical.launchpad.interfaces.authtoken import LoginTokenType
@@ -2286,8 +2286,7 @@
     @property
     def archive(self):
         """See `IPerson`."""
-        return Archive.selectOneBy(
-            owner=self, purpose=ArchivePurpose.PPA, name='ppa')
+        return getUtility(IArchiveSet).getPPAOwnedByPerson(self)
 
     @property
     def ppas(self):
@@ -2297,11 +2296,7 @@
 
     def getPPAByName(self, name):
         """See `IPerson`."""
-        ppa = Archive.selectOneBy(
-            owner=self, purpose=ArchivePurpose.PPA, name=name)
-        if ppa is None:
-            raise NoSuchPPA(name)
-        return ppa
+        return getUtility(IArchiveSet).getPPAOwnedByPerson(self, name)
 
     def isBugContributor(self, user=None):
         """See `IPerson`."""

=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2009-09-18 21:00:18 +0000
+++ lib/lp/soyuz/browser/archive.py	2009-10-13 10:25:26 +0000
@@ -1624,17 +1624,27 @@
 
     schema = IPPAActivateForm
     custom_widget('description', TextAreaWidget, height=3)
+    label = "Personal Package Archive Activation"
 
     @property
     def ubuntu(self):
         return getUtility(ILaunchpadCelebrities).ubuntu
 
+    @property
+    def initial_values(self):
+        """Set up default values for form fields."""
+        # Suggest a default value of "ppa" for the name for the
+        # first PPA activation.
+        if self.context.archive is None:
+            return {'name': 'ppa'}
+        return {}
+
     def setUpFields(self):
         """Override `LaunchpadFormView`.
 
         Reorder the fields in a way the make more sense to users and also
-        omit 'name' and present a checkbox for acknowledging the PPA-ToS
-        if the user is creating his first PPA.
+        present a checkbox for acknowledging the PPA-ToS if the user is
+        creating his first PPA.
         """
         LaunchpadFormView.setUpFields(self)
 
@@ -1643,7 +1653,7 @@
                 'name', 'displayname', 'description')
         else:
             self.form_fields = self.form_fields.select(
-                'displayname', 'accepted', 'description')
+                'name', 'displayname', 'accepted', 'description')
 
     def validate(self, data):
         """Ensure user has checked the 'accepted' checkbox."""

=== modified file 'lib/lp/soyuz/doc/archive.txt'
--- lib/lp/soyuz/doc/archive.txt	2009-10-09 16:03:02 +0000
+++ lib/lp/soyuz/doc/archive.txt	2009-10-13 10:25:26 +0000
@@ -1603,7 +1603,7 @@
     PPA for Celso Providelo
     PPA for Launchpad Buildd Admins
 
-The same happens for specific upload rights granted on 3rd-part
+The same happens for specific upload rights granted on 3rd-party
 PPAs. When 'No Privileges' gets upload rights to Celso's PPA,
 it gets listed by `getPPAsForUser`.
 
@@ -1629,6 +1629,23 @@
     >>> jblack_ppas.count()
     0
 
+Another similar method, getPPAOwnedByPersonUser(), will return the named PPA
+owned by the person, or if the person is not supplied will default to the
+first PPA that the person created.
+
+    >>> print archive_set.getPPAOwnedByPerson(cprov).displayname
+    PPA for Celso Providelo
+
+    >>> print archive_set.getPPAOwnedByPerson(cprov, name="ppa").displayname
+    PPA for Celso Providelo
+
+If the named PPA does not exist, a NoSuchPPA exception is raised.
+
+    >>> print archive_set.getPPAOwnedByPerson(cprov, name="goat").displayname
+    Traceback (most recent call last):
+    ...
+    NoSuchPPA: No such ppa: 'goat'.
+
 The method getPrivatePPAs() will return a result set of all PPAs that are
 private.
 

=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2009-09-23 07:41:13 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2009-10-13 10:25:26 +0000
@@ -1075,11 +1075,14 @@
 
     name = TextLine(
         title=_("PPA name"), required=True, constraint=name_validator,
-        description=_("A unique name used to identify this PPA."))
+        description=_("A unique name used to identify this PPA. It will "
+                      "form part of the URL to the archive repository."))
 
     displayname = StrippedTextLine(
         title=_("Displayname"), required=True,
-        description=_("Displayname for this PPA."))
+        description=_("Displayname for this PPA. It will be used in "
+                      "the signing key's description if this is the "
+                      "first PPA for a person."))
 
     description = Text(
         title=_("PPA contents description"), required=False,
@@ -1170,6 +1173,18 @@
     def __iter__():
         """Iterates over existent archives, including the main_archives."""
 
+    def getPPAOwnedByPerson(person, name=None):
+        """Return the named PPA owned by person.
+
+        :param person: An `IPerson`
+        :param name: The PPA name required.
+
+        If the person is not supplied it will default to the
+        first PPA that the person created.
+
+        :raises NoSuchPPA: if the named PPA does not exist.
+        """
+
     def getPPAsForUser(user):
         """Return all PPAs the given user can participate.
 

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2009-10-09 16:03:02 +0000
+++ lib/lp/soyuz/model/archive.py	2009-10-13 10:25:26 +0000
@@ -56,7 +56,7 @@
     AlreadySubscribed, ArchiveDependencyError, ArchiveNotPrivate,
     ArchivePurpose, DistroSeriesNotFound, IArchive, IArchiveSet,
     IDistributionArchive, InvalidComponent, IPPA, MAIN_ARCHIVE_PURPOSES,
-    PocketNotFound, VersionRequiresName, default_name_by_purpose)
+    NoSuchPPA, PocketNotFound, VersionRequiresName, default_name_by_purpose)
 from lp.soyuz.interfaces.archiveauthtoken import IArchiveAuthTokenSet
 from lp.soyuz.interfaces.archivepermission import (
     ArchivePermissionType, IArchivePermissionSet)
@@ -1399,6 +1399,19 @@
             return 0
         return int(size)
 
+    def getPPAOwnedByPerson(self, person, name=None):
+        """See `IArchiveSet`."""
+        store = Store.of(person)
+        clause = [
+            Archive.purpose == ArchivePurpose.PPA,
+            Archive.owner == person]
+        if name is not None:
+            clause.append(Archive.name == name)
+        result = store.find(Archive, *clause).order_by(Archive.id).first()
+        if name is not None and result is None:
+            raise NoSuchPPA(name)
+        return result
+
     def getPPAsForUser(self, user):
         """See `IArchiveSet`."""
         # Avoiding circular imports.

=== modified file 'lib/lp/soyuz/stories/ppa/xx-copy-packages.txt'
--- lib/lp/soyuz/stories/ppa/xx-copy-packages.txt	2009-09-23 10:29:52 +0000
+++ lib/lp/soyuz/stories/ppa/xx-copy-packages.txt	2009-10-13 10:25:26 +0000
@@ -68,7 +68,7 @@
 
     >>> jblack_browser.getLink('Create a new PPA').click()
     >>> print jblack_browser.title
-    Activate Personal Package Archive...
+    Personal Package Archive Activation : James Blackwell
 
     >>> jblack_browser.getControl(
     ...     name="field.displayname").value = 'PPA for James Blackwell'

=== modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-private-teams.txt'
--- lib/lp/soyuz/stories/ppa/xx-ppa-private-teams.txt	2009-09-18 15:24:30 +0000
+++ lib/lp/soyuz/stories/ppa/xx-ppa-private-teams.txt	2009-10-13 10:25:26 +0000
@@ -33,8 +33,8 @@
 The form looks almost identical to that for a public team.
 
    >>> browser.getLink('Create a new PPA').click()
-   >>> browser.title
-   'Activate Personal Package Archive...
+   >>> print browser.title
+   Personal Package Archive Activation : ...Private Team...
 
 There is, however, an extra bit of information indicating the new PPA
 will be private.

=== modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-workflow.txt'
--- lib/lp/soyuz/stories/ppa/xx-ppa-workflow.txt	2009-09-18 15:24:30 +0000
+++ lib/lp/soyuz/stories/ppa/xx-ppa-workflow.txt	2009-10-13 10:25:26 +0000
@@ -36,7 +36,7 @@
     >>> sample_browser.getLink("Create a new PPA").click()
 
     >>> print sample_browser.title
-    Activate Personal Package Archive...
+    Personal Package Archive Activation : Sample Person
 
 This page presents a pointer to the current PPA-ToS (terms of service)
 with mandatory 'displayname' and checkbox ('accepted') fields. The
@@ -54,7 +54,11 @@
     Activating this PPA will block future renaming of Sample Person
     ...
 
-'Displayname' is required.
+'PPA name' and 'Displayname' are required fields.  For the first activated
+PPA, the name is pre-filled with a suggestion of "ppa":
+
+    >>> print sample_browser.getControl(name="field.name").value
+    ppa
 
     >>> sample_browser.getControl("Activate").click()
 
@@ -63,10 +67,12 @@
     There is 1 error.
     Required input is missing.
 
-By submitting the form without acknowledge the PPA-ToS results in a
+By submitting the form without acknowledging the PPA-ToS results in a
 error with a specific message.
 
     >>> sample_browser.getControl(
+    ...     name="field.name").value = 'sampleppa'
+    >>> sample_browser.getControl(
     ...     name="field.displayname").value = 'Sample PPA'
     >>> sample_browser.getControl("Activate").click()
 
@@ -116,7 +122,7 @@
     PPA description
     Howdy, cowboys !
 
-Empty 'description' fields are not be rendered.
+Empty 'description' fields are not rendered.
 
     >>> sample_browser.getLink("Change details").click()
     >>> sample_browser.getControl(name="field.description").value = ('')
@@ -143,7 +149,7 @@
     There is 1 error.
     Required input is missing.
 
-Once the PPA is activated, Sample user account cannot be renamed
+Once the PPA is activated, the Sample user account cannot be renamed
 anymore. Changing the account name affects the PPA repository paths
 and we don't have infrastructure in place to support that yet. See
 more information about the feature in bug #87326.
@@ -193,10 +199,12 @@
 
     >>> sample_browser.getLink('Create a new PPA').click()
 
-    >>> sample_browser.title
-    'Activate Personal Package Archive...
+    >>> print sample_browser.title
+    Personal Package Archive Activation : ...
 
     >>> sample_browser.getControl(
+    ...     name="field.name").value = 'develppa'
+    >>> sample_browser.getControl(
     ...     name="field.displayname").value = 'Devel PPA'
     >>> sample_browser.getControl(name="field.accepted").value = True
     >>> sample_browser.getControl(
@@ -285,6 +293,7 @@
     Create a new PPA
 
     >>> admin_browser.getLink("Create a new PPA").click()
+    >>> admin_browser.getControl(name="field.name").value = 'hackppa'
     >>> admin_browser.getControl(
     ...    name="field.displayname").value = 'Hack PPA'
     >>> admin_browser.getControl(name="field.accepted").value = True
@@ -330,7 +339,7 @@
 Trying to shortcut the URL as a non-privileged user does not work:
 
     >>> sample_browser.open(
-    ...     "http://launchpad.dev/~jblack/+archive/ppa/+admin")
+    ...     "http://launchpad.dev/~jblack/+archive/hackppa/+admin")
     Traceback (most recent call last):
     ...
     Unauthorized...
@@ -416,7 +425,8 @@
 
     >>> limit = 2 ** 31 - 1
 
-    >>> admin_browser.open("http://launchpad.dev/~jblack/+archive/ppa/+admin")
+    >>> admin_browser.open(
+    ...     "http://launchpad.dev/~jblack/+archive/hackppa/+admin")
     >>> admin_browser.getControl(
     ...     name="field.authorized_size").value = str(limit)
     >>> admin_browser.getControl("Save").click()
@@ -470,11 +480,13 @@
 Prepare the forms in both browsers to activate the default PPA for the
 user 'Foo Bar'.
 
+    >>> browser1.getControl(name="field.name").value = 'boomppa'
     >>> browser1.getControl(name="field.displayname").value = 'Boom PPA'
     >>> browser1.getControl(name="field.accepted").value = True
     >>> browser1.getControl(
     ...    name="field.description").value = 'PPA rocks!'
 
+    >>> browser2.getControl(name="field.name").value = 'boomppa'
     >>> browser2.getControl(name="field.displayname").value = 'Boom PPA'
     >>> browser2.getControl(name="field.accepted").value = True
     >>> browser2.getControl(
@@ -500,18 +512,16 @@
     >>> for error in get_feedback_messages(browser2.contents):
     ...     print error
     There is 1 error.
-    The default PPA is already activated.
-    Please specify a name for the new PPA and resubmit the form.
-
-    >>> browser2.getControl(name="field.name").value
-    ''
-
-
-== Activating a named PPA ==
-
-Users who already have a PPA may as well activate a second one.
-
-That's the case for Celso.
+    You already have a PPA named 'boomppa'.
+
+    >>> print browser2.getControl(name="field.name").value
+    boomppa
+
+
+== Activating an additional PPA ==
+
+Users who already have a PPA may activate a second one.  That's the case for
+Celso.
 
     >>> cprov_browser.open("http://launchpad.dev/~cprov")
 
@@ -521,15 +531,15 @@
     Create a new PPA
 
 Celso can simply click on 'Create a new PPA' and will be presented to
-the usual PPA activation form where the checkbox for acknowledge the
-PPA-ToS is replaced by a 'name' field and a list of 'Existing PPAs'.
-Launchpad requires an user to acknowledge the PPA-ToS only once for
+the usual PPA activation form where the checkbox for acknowledging the
+PPA-ToS is no longer present and a list of 'Existing PPAs' is presented.
+Launchpad requires a user to acknowledge the PPA-ToS only once for
 all his PPAs.
 
     >>> cprov_browser.getLink("Create a new PPA").click()
 
     >>> print cprov_browser.title
-    Activate Personal Package Archive...
+    Personal Package Archive Activation : Celso Providelo
 
     >>> print_tag_with_id(cprov_browser.contents, 'ppas')
     Existing PPAs
@@ -543,12 +553,14 @@
     >>> print extract_text(
     ...     first_tag_by_class(cprov_browser.contents, 'form'))
     PPA name:
-      A unique name used to identify this PPA.
+      A unique name used to identify this PPA. It will form part of the URL
+      to the archive repository.
     Displayname:
-      Displayname for this PPA.
+      Displayname for this PPA. It will be used in the signing key's
+      description if this is the first PPA for a person.
     PPA contents description: (Optional)
-      A short description of this PPA. URLs are allowed and will be
-      rendered as links.
+      A short description of this PPA. URLs are allowed and will be rendered
+      as links.
 
 Note that, differently than the time when the first PPA was activated,
 this time there is no warning about the fact that the context renaming
@@ -559,7 +571,11 @@
     ...     first_tag_by_class(cprov_browser.contents, 'actions'))
     or Cancel
 
-If Celso does not fill 'PPA name' an error is raised.
+The 'PPA name' field is not pre-filled and if Celso does not fill it then
+an error is raised.
+
+    >>> print cprov_browser.getControl(name="field.name").value
+    <BLANKLINE>
 
     >>> cprov_browser.getControl(
     ...     name="field.displayname").value = 'Edge PPA'

=== modified file 'lib/lp/soyuz/templates/archive-activate.pt'
--- lib/lp/soyuz/templates/archive-activate.pt	2009-08-15 11:59:38 +0000
+++ lib/lp/soyuz/templates/archive-activate.pt	2009-10-13 10:25:26 +0000
@@ -8,10 +8,6 @@
   >
 <body>
 
-<div metal:fill-slot="heading">
-  <h1>Personal Package Archive Activation</h1>
-</div>
-
 <div metal:fill-slot="main">
   <div class="top-portlet">
     A PPA is a place where you can build and publish your own packages.