Merge lp:~jcsackett/launchpad/api-wants-questionset into lp:launchpad

[j.c.sackett]

[r=sinzui][bug=761877] Adds questionset toplevel to launchpad API.

Summary
=======
This exposes question set on the api, so https://api.launchpad.net/devel/questions/1 works, and the API doesn't require url hacking to get at a question.

This is a resubmit of a previous MP, specifically to address problems that were revealed when attempting to land this branch.

Preimp
======
Spoke with Curtis Hovey about exposing the top level elements of an application.

Implementation
==============
lib/lp/answers/browser/question.py
----------------------------------
Change the traverse method for the questionset. It previously returned a redirect to the canonical_url of the question directly, and was request unaware, so it would redirect outside of api. Since canonical_url is called on returned objects higher up the stack (and *is* request aware), we can just rely on that.

lib/lp/answers/interfaces/questioncollection.py
lib/lp/answers/interfaces/webservice.py
---------------------------------------
Exposed the needed elements of the QuestionSet interface on the API.
Some patching to deal with circular imports.

lib/lp/answers/interfaces/question.py
-------------------------------------------------
Exported IQuestion to all versions of the API, so the entry_schema is available for IQuestionSet (which cannot be restricted to a particular API version).

Tests
=====
bin/test -m lp.answers
bin/test -t test_wadl

QA
==
Since this changes traversal for questions, first make sure a the usual question links are working (e.g. /questions/$question_id and /$product/+question/$question_id).

Then, attempt to load a question via the api over https://api.launchpad.net/devel/questions/$question_id and https://api.launchpad.net/devel/$product/+question/$question_id. They should both show the same data.

Lint
====
make lint output:

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/answers/browser/question.py
  lib/lp/answers/interfaces/question.py
  lib/lp/answers/interfaces/questioncollection.py
  lib/lp/answers/interfaces/webservice.py

./lib/lp/answers/interfaces/questioncollection.py
      94: E302 expected 2 blank lines, found 0
./lib/lp/answers/interfaces/webservice.py
      18: 'IQuestionSet' imported but unused
      17: 'IQuestion' imported but unused

The two unused imports are needed for the webservice zcml.

[Benji York (benji)]

This looks good.

I tried to find a way to silence the "imported but unused" lint, but couldn't find a good way to do it.

review: Approve (code)

[Jonathan Lange (jml)]

On Mon, Apr 11, 2011 at 5:01 PM, Benji York <email address hidden> wrote:
> Review: Approve code
> This looks good.
>
> I tried to find a way to silence the "imported but unused" lint, but couldn't find a good way to do it.

One idiom in other pyflakes-using projects is:

"""
from foo import bar

# Shut up pyflakes. 'bar' imported for side-effect X
bar
"""

[Curtis Hovey (sinzui)]

Hi Jonathan.

I think these changes are good, but I would be more certain if there were tests verifying that IQuestionSet is exported and the methods operate with the exported params. I expect an update to lib/lp/answers/tests/test_question_webservice.py that exercises the object properties and methods. Or maybe a test like /lib/lp/registry/stories/webservice/xx-people.txt that would document how I might write scripts that use this API.

Please add the following to lib/lp/answers/interfaces/webservice.py because I would delete those unused imports when lint told me they were unused:

# The two unused imports are needed for the webservice zcml
IQuestionSet
IQuestion

I image I would QA this with something like:

from launchpadlib.launchpad import Launchpad
lp = Launchpad.login_anonymously(
    'test', 'https://api.launchpad.net/', version='devel')
question = lp.questions['119712']
# expected: Request manual merge of my account
print question.title

review: Needs Fixing (code)

[j.c.sackett (jcsackett)]

> Please add the following to lib/lp/answers/interfaces/webservice.py because I
> would delete those unused imports when lint told me they were unused:
>
> # The two unused imports are needed for the webservice zcml
> IQuestionSet
> IQuestion

Those actually are used now. I failed to update the lint when I resubmitted.

jc@jabberwocky:~/Code/launchpad/lp-branches/api-wants-questionset$ make lint
= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/answers/browser/question.py
  lib/lp/answers/interfaces/question.py
  lib/lp/answers/interfaces/questioncollection.py
  lib/lp/answers/interfaces/webservice.py

./lib/lp/answers/interfaces/question.py
     496: E302 expected 2 blank lines, found 1

[j.c.sackett (jcsackett)]

I've pushed up changes that introduce a test of the API being hooked up to IQuestionSet.

http://paste.ubuntu.com/594190/

[Curtis Hovey (sinzui)]

Thank you very much. I think this is good to land.

review: Approve (code)

=== modified file 'lib/lp/answers/browser/question.py'
--- lib/lp/answers/browser/question.py	2011-02-02 15:43:31 +0000
+++ lib/lp/answers/browser/question.py	2011-04-18 16:21:50 +0000
@@ -237,7 +237,7 @@
             question = None
         if question is None:
             raise NotFoundError(name)
-        return redirection(canonical_url(question), status=301)
+        return redirection(canonical_url(question, self.request), status=301)
 
 
 class QuestionBreadcrumb(Breadcrumb):

=== modified file 'lib/lp/answers/interfaces/question.py'
--- lib/lp/answers/interfaces/question.py	2011-03-27 21:40:32 +0000
+++ lib/lp/answers/interfaces/question.py	2011-04-18 16:21:50 +0000
@@ -25,10 +25,6 @@
     operation_parameters,
     REQUEST_USER,
     )
-from lazr.restful.fields import (
-    CollectionField,
-    Reference,
-    )
 from zope.interface import (
     Attribute,
     Interface,
@@ -67,7 +63,7 @@
 class IQuestion(IHasOwner):
     """A single question, often a support request."""
 
-    export_as_webservice_entry(as_of="devel")
+    export_as_webservice_entry(as_of='beta')
 
     id = exported(Int(
         title=_('Question Number'), required=True, readonly=True,
@@ -489,7 +485,7 @@
     @operation_for_version('devel')
     def setCommentVisibility(user, comment_number, visible):
         """Set the visible attribute on a question message.
-        
+
         This is restricted to Launchpad admins and registry members, and will
         return a HTTP Error 401: Unauthorized error for non-admin callers.
         """

=== modified file 'lib/lp/answers/interfaces/questioncollection.py'
--- lib/lp/answers/interfaces/questioncollection.py	2010-08-20 20:31:18 +0000
+++ lib/lp/answers/interfaces/questioncollection.py	2011-04-18 16:21:50 +0000
@@ -11,14 +11,25 @@
     'IQuestionCollection',
     'IQuestionSet',
     'ISearchableByQuestionOwner',
-    'QUESTION_STATUS_DEFAULT_SEARCH'
+    'QUESTION_STATUS_DEFAULT_SEARCH',
     ]
 
 from zope.interface import (
     Attribute,
     Interface,
     )
-
+from zope.schema import Int
+
+from lazr.restful.declarations import (
+    collection_default_content,
+    export_as_webservice_collection,
+    export_operation_as,
+    export_read_operation,
+    operation_for_version,
+    operation_parameters,
+    )
+
+from canonical.launchpad import _
 from lp.answers.interfaces.questionenums import QuestionStatus
 
 
@@ -47,7 +58,7 @@
         against the question's language. If None or an empty sequence,
         the language is not included as a filter criteria.
 
-        :sort:  An attribute of QuestionSort. If None, a default value is used.
+        :sort: An attribute of QuestionSort. If None, a default value is used.
         When there is a search_text value, the default is to sort by
         RELEVANCY, otherwise results are sorted NEWEST_FIRST.
         """
@@ -82,8 +93,17 @@
 class IQuestionSet(IQuestionCollection):
     """A utility that contain all the questions published in Launchpad."""
 
+    export_as_webservice_collection(Interface)
+
     title = Attribute('Title')
 
+    @operation_parameters(
+        question_id=Int(
+            title=_('The id of the question to get'),
+            required=True))
+    @export_read_operation()
+    @export_operation_as("getByID")
+    @operation_for_version('devel')
     def get(question_id, default=None):
         """Return the question with the given id.
 
@@ -98,6 +118,7 @@
         comments in the last <days_before_expiration> days.
         """
 
+    @collection_default_content(limit=5)
     def getMostActiveProjects(limit=5):
         """Return the list of projects that asked the most questions in
         the last 60 days.

=== modified file 'lib/lp/answers/interfaces/webservice.py'
--- lib/lp/answers/interfaces/webservice.py	2011-03-21 21:34:04 +0000
+++ lib/lp/answers/interfaces/webservice.py	2011-04-18 16:21:50 +0000
@@ -11,6 +11,15 @@
 
 __all__ = [
     'IQuestion',
+    'IQuestionSet',
     ]
 
+from lazr.restful.declarations import LAZR_WEBSERVICE_EXPORTED
+
 from lp.answers.interfaces.question import IQuestion
+from lp.answers.interfaces.questioncollection import IQuestionSet
+
+IQuestionSet.queryTaggedValue(
+    LAZR_WEBSERVICE_EXPORTED)['collection_entry_schema'] = IQuestion
+IQuestionSet['get'].queryTaggedValue(
+    LAZR_WEBSERVICE_EXPORTED)['return_type'] = IQuestion

=== modified file 'lib/lp/answers/tests/test_question_webservice.py'
--- lib/lp/answers/tests/test_question_webservice.py	2011-03-27 16:03:44 +0000
+++ lib/lp/answers/tests/test_question_webservice.py	2011-04-18 16:21:50 +0000
@@ -46,7 +46,16 @@
         dd = dt.findNextSibling('dd')
         return str(dd.contents.pop())
 
+    def test_top_level_question_get(self):
+        # The top level question set can be used via the api to get
+        # a question by id via redirect without url hacking.
+        response = self.webservice.get(
+            '/questions/%s' % self.question.id, 'application/xhtml+xml')
+        self.assertEqual(response.status, 301)
+
+
     def test_GET_xhtml_representation(self):
+        # A question's xhtml representation is available on the api.
         response = self.webservice.get(
             '/%s/+question/%d' % (self.question.target.name,
                 self.question.id),
@@ -58,6 +67,7 @@
             "<p>This is a question</p>")
 
     def test_PATCH_xhtml_representation(self):
+        # You can update the question through the api with PATCH.
         new_title = "No, this is a question"
 
         question_json = self.webservice.get(