← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~rharding/launchpad/nonpublic_1052659 into lp:launchpad

 

The proposal to merge lp:~rharding/launchpad/nonpublic_1052659 into lp:launchpad has been updated.

Description changed to:

= Summary =

The code was changed to only pull public specifications. In order to change
the method to be privacy aware it needed to be storm-ified and in the end much
of the query building tools already existed to do that.

== Pre Implementation ==

A little bit of chat with Aaron getting a sanity check on the storm-ification.

== Implementation Notes ==

Since the original method wasn't well tested, I copied over the product specification tests on Aaron's suggestion. The tests required some love and tweaking in order to work with a SpecificationSet vs a single Product, but I added them to trunk first with the only failing tests being those around non-public specifications. Then moved the tests over to this branch of work and they all passes first try.

Updated the specifications method to work via a constructed storm query adding
in the privacy tables and clauses via the existing visible_specification_query
method.

Then used the existing get_specification_filters to filter as required.

Then finally just added a quick test to make sure we get/don't get the right
specs when using the method.

== QA ==

Per the bug, having both public and non-public specs the owner should see
the non-public ones they have a grant to while the public only user should not
see them listed on the home page.

== Tests ==

New test in lib/lp/blueprints/tests/test_specification.py

The actual code is tested via a lot of other locations though.

For more details, see:
https://code.launchpad.net/~rharding/launchpad/nonpublic_1052659/+merge/136479
-- 
https://code.launchpad.net/~rharding/launchpad/nonpublic_1052659/+merge/136479
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/nonpublic_1052659 into lp:launchpad.


References