Merge lp:~bryce/launchpad/api-docs-fixes-bug into lp:launchpad

[Bryce Harrington]

Flesh out docs for interfaces/bugs.py and fix various spelling errors.

Further improvements to the API documentation and spelling fixes.

Most notably includes fleshing out docs of interfaces/bugs.py, and fixing the spelling of 'visiblity' which appears throughout the codebase including in a user-visible string (reported as bug #455203), and clarifying that setCommentVisibility() is an admin-only function (#373508).

[Eleanor Berger (intellectronica)]

Bryce, all-in-all very nice improvements. There are a few potential problems though, resulting from the way interfaces are used in the UI. In Zope (and Launchpad), the same interfaces that are used to define objects can be used for publishing these objects. Many Launchpad forms, for example, use the Description attribute of fields to render the descriptive text next the field (the same goes for Title). The advantage of this approach is that you don't need to describe the same field twice. The disadvantage, is that the title and description you use must be appropriate both for use in the model and for use in the UI. Some of the titles and descriptions you've added might not be appropriate for the UI (the description for tags, for example, that appears on a bug's +edit page). For the same reason, some of the changes you've made are likely to require changes to pagetests, which I think you haven't adjusted yet in this branch. I think that the changes are good in most cases, but let's go once over the visible changes and make sure they work fine, and perhaps also submit this for UI-review.

b.t.w If you find places where you think the description should be different, all hope is not lost - it is still possible to customize the appearance of fields in the UI by changing them in the relevant classes in the the browser package.

review: Needs Information (code)

[Bryce Harrington (bryce)]

Hrm, that's distressing. Is there any way to differentiate between items that only show up on the API docs vs. ones that also show up in the web UI?

Personally, while it seems convenient and time-saving to reuse the web UI strings for the API docs, if the side-effect is making it harder to update the API docs, then it sort of seems like there is a risk that people will be reticent to improve the documentation for the API calls, lest it risk impacting the web UI.

Anyway, if I can find a way to discern which strings are used by the web UI, I can strip out those changes from this branch.

[Eleanor Berger (intellectronica)]

Bryce, with the docstrings you've removed from the diff you can land this branch. Thanks for improving the API documentation, I know for sure that it makes life much better for users.

For the future, I think it's still worth trying to come up with a way to update the documentation for only the API (without affecting the documentation of the interface itself). Perhaps by adding a new decorator, or passing an extra description to the `exported` decorator - that way you'll be able to provide even better documentation to the API which now you must avoid.

review: Approve (code)

=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py	2010-05-16 23:56:51 +0000
+++ lib/lp/bugs/browser/bug.py	2010-05-26 00:52:27 +0000
@@ -668,7 +668,7 @@
 
     @property
     def label(self):
-        return 'Bug #%i - Set visiblity and security' % self.context.bug.id
+        return 'Bug #%i - Set visibility and security' % self.context.bug.id
 
     page_title = label
 

=== modified file 'lib/lp/bugs/doc/bugmessage.txt'
--- lib/lp/bugs/doc/bugmessage.txt	2009-06-12 16:36:02 +0000
+++ lib/lp/bugs/doc/bugmessage.txt	2010-05-26 00:52:27 +0000
@@ -141,7 +141,7 @@
 == Last message date ==
 
 For each bug, we cache the date of the last message linked to it using
-the sttribute `date_last_message` in order to optimize searches the need
+the attribute `date_last_message` in order to optimize searches the need
 to compare this value for every bug in a large set.
 
     >>> test_message = bug_one.newMessage(

=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py	2010-04-29 17:49:19 +0000
+++ lib/lp/bugs/interfaces/bug.py	2010-05-26 00:52:27 +0000
@@ -206,7 +206,7 @@
             description=_("The person who set this bug private."),
             readonly=True))
     security_related = exported(
-        Bool(title=_("This bug is a security vulnerability"),
+        Bool(title=_("This bug is a security vulnerability."),
              required=False, default=False, readonly=True))
     displayname = TextLine(title=_("Text of the form 'Bug #X"),
         readonly=True)
@@ -216,7 +216,7 @@
     bugtasks = exported(
         CollectionField(
             title=_('BugTasks on this bug, sorted upstream, then '
-                    'ubuntu, then other distroseriess.'),
+                    'by ubuntu, then by other distroseries.'),
             value_type=Reference(schema=IBugTask),
             readonly=True),
         exported_as='bug_tasks')
@@ -239,12 +239,12 @@
     cve_links = Attribute('Links between this bug and CVE entries.')
     subscriptions = exported(
         doNotSnapshot(CollectionField(
-            title=_('Subscriptions.'),
+            title=_('Subscriptions'),
             value_type=Reference(schema=Interface),
             readonly=True)))
     duplicates = exported(
         CollectionField(
-            title=_('MultiJoin of the bugs which are dups of this one'),
+            title=_("MultiJoin of bugs which are dupes of this one."),
             value_type=BugField(), readonly=True))
     attachments = exported(
         CollectionField(
@@ -263,9 +263,10 @@
         List(title=_("Tags"), description=_("Separated by whitespace."),
              value_type=Tag(), required=False))
     is_complete = Bool(
+        title=_("Is Complete?"),
         description=_(
             "True or False depending on whether this bug is considered "
-            "completely addressed. A bug is Launchpad is completely "
+            "completely addressed. A bug in Launchpad is completely "
             "addressed when there are no tasks that are still open for "
             "the bug."),
         readonly=True)
@@ -278,12 +279,12 @@
         readonly=True)
     can_expire = exported(
         Bool(
-        title=_("Can the Incomplete bug expire if it becomes inactive? "
+            title=_("Can the Incomplete bug expire if it becomes inactive? "
                 "Expiration may happen when the bug permits expiration, "
                 "and a bugtask cannot be confirmed."),
-        readonly=True))
+            readonly=True))
     date_last_message = exported(
-        Datetime(title=_('Date of last bug message'),
+        Datetime(title=_("Date of last bug message"),
                  required=False, readonly=True))
     number_of_duplicates = exported(
         Int(title=_('The number of bugs marked as duplicates of this bug'),
@@ -305,7 +306,8 @@
         Int(title=_('The number of users unaffected by this bug'),
             required=True, readonly=True))
     users_affected = exported(doNotSnapshot(CollectionField(
-            title=_('Users affected (not including duplicates)'),
+            title=_('The number of users affected by this bug '
+                    '(not including duplicates)'),
             value_type=Reference(schema=IPerson),
             readonly=True)))
     users_unaffected = exported(doNotSnapshot(CollectionField(
@@ -713,6 +715,9 @@
 
             :security_related: True/False.
 
+        This may also cause the security contact to be subscribed
+        if one is registered and if the bug is not private.
+        
         Return True if a change is made, False otherwise.
         """
 
@@ -764,7 +769,10 @@
     @call_with(user=REQUEST_USER)
     @export_write_operation()
     def setCommentVisibility(user, comment_number, visible):
-        """Set the visible attribute on a bug comment."""
+        """Set the visible attribute on a bug comment.  This is restricted
+        to Launchpad admins, and will return a HTTP Error 401: Unauthorized
+        error for non-admin callers.
+        """
 
     def userCanView(user):
         """Return True if `user` can see this IBug, false otherwise."""
@@ -979,10 +987,10 @@
           * the reporter will be subscribed to the bug
 
           * distribution, product and package contacts (whichever ones are
-            applicable based on the bug report target) will bug subscribed to
+            applicable based on the bug report target) will be subscribed to
             all *public bugs only*
 
-          * for public upstreams bugs where there is no upstream bug contact,
+          * for public upstream bugs where there is no upstream bug contact,
             the product owner will be subscribed instead
 
           * if either product or distribution is specified, an appropiate

=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
--- lib/lp/bugs/interfaces/bugtask.py	2010-05-18 17:50:25 +0000
+++ lib/lp/bugs/interfaces/bugtask.py	2010-05-26 00:52:27 +0000
@@ -787,7 +787,7 @@
         required=False)
     importance = List(
         title=_('Importance'),
-        description=_('Show only bugs with the given importance'
+        description=_('Show only bugs with the given importance '
                       'or list of importances.'),
         value_type=IBugTask['importance'],
         required=False)

=== modified file 'lib/lp/bugs/interfaces/malone.py'
--- lib/lp/bugs/interfaces/malone.py	2009-06-25 00:40:31 +0000
+++ lib/lp/bugs/interfaces/malone.py	2010-05-26 00:52:27 +0000
@@ -69,7 +69,7 @@
           * the owner will be subscribed to the bug
 
           * distribution, product and package contacts (whichever ones are
-            applicable based on the bug report target) will bug subscribed to
+            applicable based on the bug report target) will be subscribed to
             all *public bugs only*
 
           * for public upstreams bugs where there is no upstream bug contact,

=== modified file 'lib/lp/code/doc/branch-visibility.txt'
--- lib/lp/code/doc/branch-visibility.txt	2010-01-12 14:52:41 +0000
+++ lib/lp/code/doc/branch-visibility.txt	2010-05-26 00:52:27 +0000
@@ -74,7 +74,7 @@
 
 == Visibility due to ownership ==
 
-The owners of the branch always have visiblity of their own branches.
+The owners of the branch always have visibility of their own branches.
 
     >>> branch.private = True
     >>> from canonical.launchpad.webapp.authorization import clear_cache

=== modified file 'lib/lp/code/model/tests/test_branchnamespace.py'
--- lib/lp/code/model/tests/test_branchnamespace.py	2009-07-29 02:16:43 +0000
+++ lib/lp/code/model/tests/test_branchnamespace.py	2010-05-26 00:52:27 +0000
@@ -1451,7 +1451,7 @@
 
 
 class PolicySimple(BranchVisibilityPolicyTestCase):
-    """Test the visiblity policy where the base visibility rule is PUBLIC with
+    """Test the visibility policy where the base visibility rule is PUBLIC with
     one team specified to have PRIVATE branches.
     """
 
@@ -1496,7 +1496,7 @@
 
 
 class PolicyPrivateOnly(BranchVisibilityPolicyTestCase):
-    """Test the visiblity policy where the base visibility rule is PUBLIC with
+    """Test the visibility policy where the base visibility rule is PUBLIC with
     one team specified to have the PRIVATE_ONLY rule.
 
     PRIVATE_ONLY only stops the user from changing the branch from private to
@@ -1545,7 +1545,7 @@
 
 
 class PolicyForbidden(BranchVisibilityPolicyTestCase):
-    """Test the visiblity policy where the base visibility rule is FORBIDDEN
+    """Test the visibility policy where the base visibility rule is FORBIDDEN
     with one team specified to have the PRIVATE branches and another team
     specified to have PUBLIC branches.
     """

=== modified file 'lib/lp/code/stories/branches/xx-branch-edit-privacy.txt'
--- lib/lp/code/stories/branches/xx-branch-edit-privacy.txt	2009-11-26 22:55:06 +0000
+++ lib/lp/code/stories/branches/xx-branch-edit-privacy.txt	2010-05-26 00:52:27 +0000
@@ -24,7 +24,7 @@
 Branches with no visibility policy
 ----------------------------------
 
-If there is no policy set, then the user is not able to change the visiblity
+If there is no policy set, then the user is not able to change the visibility
 of the branch.
 
     >>> browser = setupBrowser(auth='Basic [e-mail]:test')

=== modified file 'lib/lp/registry/browser/team.py'
--- lib/lp/registry/browser/team.py	2009-12-24 01:38:11 +0000
+++ lib/lp/registry/browser/team.py	2010-05-26 00:52:27 +0000
@@ -116,7 +116,7 @@
 
 
 class TeamFormMixin:
-    """Form to be used on forms which conditionally display team visiblity.
+    """Form to be used on forms which conditionally display team visibility.
 
     The visibility field should only be shown to users with
     launchpad.Commercial permission on the team.