Merge lp:~sinzui/launchpad/apply-commercial-subscription into lp:launchpad

Proposed by Curtis Hovey
Status: Merged
Approved by: Brad Crittenden
Approved revision: no longer in the source branch.
Merged at revision: 14781
Proposed branch: lp:~sinzui/launchpad/apply-commercial-subscription
Merge into: lp:launchpad
Diff against target: 0 lines
To merge this branch: bzr merge lp:~sinzui/launchpad/apply-commercial-subscription
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+92547@code.launchpad.net

Commit message

[r=bac][bug=929569] Allow anyone to apply a commercial subscription to any active project.

Description of the change

Allow anyone to apply a commercial subscription to any active project.

    Pre-implementation: wgrant, wallyworld

We want to permit commercial projects to configure sharing, bug
tracking, and code hosting. Commercial projects are identified by a
current commercial subscription. There are about 30 projects that have
commercial featured enabled but do not have a commercial subscription.
Lp will not let me setup a commercial subscription for them because the
projects do not have a proprietary license. The special features will
disappear from these project pages

--------------------------------------------------------------------

RULES

    * Remove the constraint that checks for a proprietary license.
      The constraint is the CommercialProjects projects vocabulary
      used by the view. Lp has tests that verify that calling
      redeemSubscriptionVoucher() works with any project.
    * The constraints are in CommercialProjectsVocabulary
      * WTF: The vocabulary uses launchpad.Moderate.
        but PersonVouchersView is using launchpad.Commercial.
        launchpad.Commercial is correct. launchpad.Moderate does not
        let the user redeem a voucher.
      * WTF, I see that the __contains__ checks permit
        deactivated and unmaintained projects for most users.
      * WTF: the filter rule is re-sorting the sorted data at the
        cost of instantiating ever project. The sort rules are identical.

    Out of scope
    * The vocab filters non-proprietary projects and creates terms
      that provide expiration information. The terms.tokens conflict
      with picker presentation rules and look like a Launchpad-Id
      This is out of scope fo this branch. I will report a bug suggesting
      that the picker entry rules include commercial information.
    * I reported bug 930309.

    Addendum
    * The Cancel button on the form does not conform to Lp form
      layout. Cancel should be a link.
    * The page title does not conform to 3.0 rules. It contains
      redundant user information.
    * The breadcrumbs are truncated because the page_title is too long.

QA

    * On qastaging verify you can apply a commercial subscription to
      any of these projects http://pastebin.ubuntu.com/834571/

LINT

    lib/lp/registry/vocabularies.py
    lib/lp/registry/browser/person.py
    lib/lp/registry/browser/tests/test_commercialsubscription.py
    lib/lp/registry/stories/vouchers/xx-voucher-redemption.txt
    lib/lp/registry/templates/person-vouchers.pt
    lib/lp/registry/tests/test_commercialprojects_vocabularies.py

TEST

    ./bin/test -vvc lp.registry.tests.test_commercialprojects_vocabularies
    ./bin/test -vvc lp.registry.browser.tests.test_commercialsubscription
    ./bin/test -vvc -t xx-voucher-redemption lp.registry.tests.test_doc

IMPLEMENTATION

I updated the existing test to use lp.testing.testcase. I added tests for
all the methods that the vocabulary implements. This rewrite is difficult
to read in the diff :(. I fixed the __contains__ method. I removed
_filter_projs() because it just duplicated sorting since we do not want
to filter on proprietary licenses. I changed the search method to use
ProductSet.Search because it is faster and it caches the commercial
subscription information. I replace the launchpad.Moderate checks with
launchpad.Commercial because those are the only users who can work
with commercial subscriptions. I replaced the use of product.title with
displayname becase the former was deprecated in 2007. I rewrote
"unsubscribed" because Lp often uses the term with in the verb form, but it
is not possible to unsubscribe form a commercial subscription. I reported
bug 930309 to remind the purple squad to include commercial information
in the the picker details.
    lib/lp/registry/vocabularies.py
    lib/lp/registry/tests/test_commercialprojects_vocabularies.py

I revised the page_title to provide only the unique information about the
vouchers page. I Replaced the cancel action with a cancel_url, then removed
the form exceptions from the page template. I added a test for the views
properties. I Removed an unneeded test.
    lib/lp/registry/browser/person.py
    lib/lp/registry/browser/tests/test_commercialsubscription.py
    lib/lp/registry/stories/vouchers/xx-voucher-redemption.txt
    lib/lp/registry/templates/person-vouchers.pt

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

Hi Curtis,

Thanks for all of the fixes here.

At 204 and 225 the failure message should reference self.num_commercial. You can probably just delete the custom message as the default is pretty descriptive.

236: typo: deactivated. In that comment I'd s/fails/returns no results/

345: s/projects user maintains, or all active project/
       project the user maintains, or all active projects/

Otherwise this looks like a great improvement, both in terms of adding the functionality needed and cleanliness.

review: Approve (code)

Preview Diff

Empty