Merge lp:~jpds/launchpad/fix_518385 into lp:launchpad

[Jonathan Davies]

Mark mirrors which are not official_candidates or official as private on the web UI.

[Jonathan Davies (jpds)]

= Summary =

Sometimes we have mirror admins who do not tick the "Apply to be an official mirror of this distribution" check box.

Mirrors which are not official_candidates or official should be marked as such with an IPrivacy layout on the web UI.

A UI example can be found at:

- http://people.canonical.com/~jpds/fix_518385.png

This branch builds on the new distributionmirror interface set up and thus needs lp:~jpds/launchpad/fix_518232 merged first.

= UI Testing =

Login as karl@c.c and visit these URLs:

 * https://launchpad.dev/ubuntu/+mirror/invalid-mirror
 * https://launchpad.dev/ubuntu/+mirror/archive-404-mirror - change details and unmark "Apply to be an official mirror of this distribution".

= 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/registry/browser/tests/distributionmirror-views.txt
  lib/lp/registry/doc/distribution-mirror.txt
  lib/lp/registry/interfaces/distributionmirror.py
  lib/lp/registry/model/distributionmirror.py
  lib/lp/registry/templates/distributionmirror-index.pt

[Aaron Bentley (abentley)]

I'm not comfortable reviewing this, because there's no pre-implementation call. It would also if the bug had been triaged or confirmed by someone from registry.

[Brad Crittenden (bac)]

Thank you Aaron, that is exactly the right response.

Jonathan I'm available to do a better-late-than-never pre-imp call/chat with you.

[Graham Binns (gmb)]

Hi Jonathan, Brad;

Has there been a pre/post-imp call / chat for this yet? If not I'm happy to make myself available for one.

[Graham Binns (gmb)]

Hi Jonathan,

Has there been any movement on this? You need to have a pre/post imp discussion about this branch before we can land it. I'm willing to make myself available this afternoon for one.

If you don't have time to have a discussion about this branch at the moment then I'll mark the merge proposal as rejected and you can resubmit it once you've had time to talk it over with someone (this is no reflection on you, but it's sitting at the top of the active reviews queue in an irritating fashion).

[Graham Binns (gmb)]

In fact, I'm going to mark this was WIP pending the pre-imp discussion.

Unmerged revisions

10301. By Jonathan Davies on 2010-02-07

Display the fact that a mirror registration is private but not why.

10300. By Jonathan Davies on 2010-02-07

Fixed indent for private docstring.

10299. By Jonathan Davies on 2010-02-07

Updated tests for new private information portlets.

10298. By Jonathan Davies on 2010-02-07

Give the privacy boxes different IDs for tests.

10297. By Jonathan Davies on 2010-02-07

Ensure that newly registered mirrors are private.

10296. By Jonathan Davies on 2010-02-07

Moved all_probe_records and last_probe_record down to IDistributionMirrorPublic
as IPrivacy seems to be making it upset.

10295. By Jonathan Davies on 2010-02-07

Docstring for private()

10294. By Jonathan Davies on 2010-02-07

Finalize private portlet conditions.

10293. By Jonathan Davies on 2010-02-07

Display private portlet's on mirror pages explaining why the mirror in question
has been marked as private.

10292. By Jonathan Davies on 2010-02-07

Implemented an IPrivacy layer to DistributionMirror.

=== modified file 'lib/lp/registry/browser/tests/distributionmirror-views.txt'
--- lib/lp/registry/browser/tests/distributionmirror-views.txt	2009-12-14 15:22:53 +0000
+++ lib/lp/registry/browser/tests/distributionmirror-views.txt	2010-02-07 23:03:20 +0000
@@ -149,6 +149,8 @@
 
     >>> print mirror.status.name
     PENDING_REVIEW
+    >>> print mirror.private
+    True
     >>> print mirror.date_reviewed, mirror.reviewer
     None None
 
@@ -195,6 +197,8 @@
 
     >>> print mirror.status.name
     OFFICIAL
+    >>> print mirror.private
+    False
     >>> print mirror.reviewer.name
     karl
     >>> print mirror.whiteboard
@@ -413,23 +417,60 @@
     The Hoary Hedgehog Release  Ubuntu, Edubuntu
     The Warty Warthog Release   Ubuntu, Kubuntu
 
-Mirror admins can also see a whiteboard
+Mirror admins can also see a whiteboard:
 
     >>> login('[e-mail]')
-    >>> user = getUtility(ILaunchBag).user
+    >>> karl_user = getUtility(ILaunchBag).user
     >>> cd_mirror.whiteboard = 'This is a good mirror.'
     >>> view = create_initialized_view(
-    ...     cd_mirror, '+index', principal=user)
+    ...     cd_mirror, '+index', principal=karl_user)
     >>> whiteboard = find_tag_by_id(view.render(), 'whiteboard')
     >>> print extract_text(whiteboard.find('dd'))
     This is a good mirror.
 
     >>> login('[e-mail]')
-    >>> user = getUtility(ILaunchBag).user
+    >>> no_priv_user = getUtility(ILaunchBag).user
     >>> view = create_initialized_view(
-    ...     cd_mirror, '+index', principal=user)
+    ...     cd_mirror, '+index', principal=no_priv_user)
     >>> print find_tag_by_id(view.render(), 'whiteboard')
     None
+    
+And extra private information portlets:
+
+    >>> login('[e-mail]')
+    >>> karl_user = getUtility(ILaunchBag).user
+
+    >>> releases_mirror = ubuntu.getMirrorByName('random-releases-mirror')
+    >>> form = dict(form)
+    >>> form['field.official_candidate'] = 'off'
+    >>> form['field.actions.save'] = 'Save'
+    >>> view = create_initialized_view(
+    ...     releases_mirror, '+edit', form=form, principal=karl_user)
+    >>> view.errors
+    []
+
+    >>> index_view = create_initialized_view(
+    ...     releases_mirror, '+index', principal=karl_user)
+    >>> print extract_text(find_tag_by_id(index_view.render(),
+    ...     'privacy'))
+    Mirror registration viewing restricted to adminstration
+    and not publicly indexed.
+    
+    >>> form = dict(form)
+    >>> form['field.official_candidate'] = 'on'
+    >>> form['field.status'] = 'UNOFFICIAL'
+    >>> form['field.actions.save'] = 'Save'
+    >>> view = create_initialized_view(
+    ...     releases_mirror, '+review', form=form, principal=karl_user)
+    >>> view.errors
+    []
+
+    >>> index_view = create_initialized_view(
+    ...     releases_mirror, '+index', principal=karl_user)
+    >>> print extract_text(find_tag_by_id(index_view.render(),
+    ...     'privacy'))
+    Mirror registration viewing restricted to adminstration
+    and not publicly indexed.
 
 
 Distribution mirror RSS

=== modified file 'lib/lp/registry/doc/distribution-mirror.txt'
--- lib/lp/registry/doc/distribution-mirror.txt	2009-12-09 10:03:20 +0000
+++ lib/lp/registry/doc/distribution-mirror.txt	2010-02-07 23:03:20 +0000
@@ -34,6 +34,8 @@
 
     >>> new_mirror.isOfficial()
     False
+    >>> new_mirror.private
+    True
     >>> new_mirror.status
     <DBItem MirrorStatus.PENDING_REVIEW...
 

=== modified file 'lib/lp/registry/interfaces/distributionmirror.py'
--- lib/lp/registry/interfaces/distributionmirror.py	2010-02-07 23:03:19 +0000
+++ lib/lp/registry/interfaces/distributionmirror.py	2010-02-07 23:03:20 +0000
@@ -37,6 +37,7 @@
 from canonical.launchpad import _
 from canonical.launchpad.fields import (
     ContentNameField, PublicPersonChoice, URIField, Whiteboard)
+from canonical.launchpad.interfaces.launchpad import IPrivacy
 from canonical.launchpad.validators.name import name_validator
 from canonical.launchpad.validators import LaunchpadValidationError
 from canonical.launchpad.webapp.menu import structured
@@ -302,16 +303,13 @@
     official_candidate = exported(Bool(
         title=_('Apply to be an official mirror of this distribution'),
         required=False, readonly=False, default=True))
-    last_probe_record = Attribute(
-        'The last MirrorProbeRecord for this mirror.')
-    all_probe_records = Attribute('All MirrorProbeRecords for this mirror.')
     whiteboard = exported(Whiteboard(
         title=_('Whiteboard'), required=False, readonly=False,
         description=_("Notes on the current status of the mirror (only "
                       "visible to admins and the mirror's registrant).")))
 
 
-class IDistributionMirrorPublic(Interface):
+class IDistributionMirrorPublic(Interface, IPrivacy):
     """Public IDistributionMirror properties."""
 
     id = Int(title=_('The unique id'), required=True, readonly=True)
@@ -377,6 +375,9 @@
     source_series = Attribute(
         'All MirrorDistroSeriesSources of this mirror')
     arch_series = Attribute('All MirrorDistroArchSeries of this mirror')
+    last_probe_record = Attribute(
+        'The last MirrorProbeRecord for this mirror.')
+    all_probe_records = Attribute('All MirrorProbeRecords for this mirror.')
     has_ftp_or_rsync_base_url = Bool(
         title=_('Does this mirror have a FTP or Rsync base URL?'))
     base_url = Attribute('The HTTP or FTP base URL of this mirror')
@@ -570,6 +571,9 @@
     def getByRsyncUrl(url):
         """Return the mirror with the given Rsync URL or None."""
 
+    def private():
+        """Return True is the mirror is not an official candidate."""
+
 
 class IMirrorDistroArchSeries(Interface):
     """The mirror of the packages of a given Distro Arch Series"""

=== modified file 'lib/lp/registry/model/distributionmirror.py'
--- lib/lp/registry/model/distributionmirror.py	2009-10-26 18:40:04 +0000
+++ lib/lp/registry/model/distributionmirror.py	2010-02-07 23:03:20 +0000
@@ -392,6 +392,12 @@
                     paths.append((series, pocket, component, path))
         return paths
 
+    @property
+    def private(self):
+        """See IDistributionMirror."""
+        return (not self.official_candidate
+            or self.status != MirrorStatus.OFFICIAL)
+
 
 class DistributionMirrorSet:
     """See IDistributionMirrorSet"""

=== modified file 'lib/lp/registry/templates/distributionmirror-index.pt'
--- lib/lp/registry/templates/distributionmirror-index.pt	2009-12-08 15:51:34 +0000
+++ lib/lp/registry/templates/distributionmirror-index.pt	2010-02-07 23:03:20 +0000
@@ -148,6 +148,13 @@
   </tal:main>
 
   <tal:side metal:fill-slot="side" condition="context/required:launchpad.Edit">
+    <div id="privacy" class="portlet private"
+      tal:condition="context/private">
+      <span class="sprite private">
+        Mirror registration viewing <strong>restricted</strong> to adminstration
+        and not publicly indexed.
+      </span>
+    </div>
     <tal:menu replace="structure context/@@+global-actions" />
   </tal:side>
 </body>