Merge lp://staging/~sinzui/launchpad/delete-packaging-link into lp://staging/launchpad

Proposed by Curtis Hovey
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 15987
Proposed branch: lp://staging/~sinzui/launchpad/delete-packaging-link
Merge into: lp://staging/launchpad
Diff against target: 642 lines (+86/-199)
11 files modified
lib/lp/registry/browser/tests/test_sourcepackage_views.py (+28/-31)
lib/lp/registry/doc/sourcepackage.txt (+0/-64)
lib/lp/registry/interfaces/packaging.py (+2/-3)
lib/lp/registry/model/packaging.py (+7/-6)
lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging-concurrent-deletion.txt (+2/-2)
lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging.txt (+2/-1)
lib/lp/registry/tests/test_packaging.py (+9/-20)
lib/lp/registry/tests/test_sourcepackage.py (+16/-59)
lib/lp/testing/factory.py (+6/-1)
lib/lp/translations/browser/tests/test_sharing_details.py (+12/-11)
lib/lp/translations/tests/test_translationpackagingjob.py (+2/-1)
To merge this branch: bzr merge lp://staging/~sinzui/launchpad/delete-packaging-link
Reviewer Review Type Date Requested Status
Richard Harding (community) Approve
Review via email: mp+124809@code.staging.launchpad.net

Commit message

Allow the Ubuntu community to manage their upstream packaging links.

Description of the change

Launchpad's zcml states that anyone can unlink a package from a series.
The rule is used on the distribution and source package pages, and on
the project's package pages. When translation message sharing was added,
a condition was added to the code that will raise a forbidden error if
the user attempts to unlink a package from a series that is sharing
translations.

There is no role or action that a user can do that definitely qualify a
user to manage packaging. We know that project users are more likely to
create bogus packaging links and that users create more packaging links
then are deleted.

The Packaging.userCanDelete() restrictions are naive. The rules assume
that shared messages are more important then the true links to the
upstream project. The ubuntu community cannot fix bogus data created by
project maintainers who do not understand packaging. Ubuntu encourages
projects to link to them by making it easy to create the link, and the
community gardens the packaging links. The restrictive code is solving a
problem that never existed, and introduces a new problem that distro
communities cannot manage their data.

The broken rules and tests favoured the package or productseries owner,
but these properties recored registrants, not people with
responsibility. In fact, most of these Packaging permission checks are
asking if the user is ~sinzui or ~jelmer because we created most of the
packaging links :(

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

RULES

    Pre-implementation: jcsackett
    * Either remove Packaging.userCanDelete() or simplify it to verify
      if the user has launchpad experience.

QA

    * Visit https://qastaging.launchpad.net/ubuntu/q-series/+source/tweak
    * Verify there is a delete and edit icon next to the upstream series
    * Open each action and verify the pages loads without error
    * Visit https://translations.qastaging.launchpad.net/ubuntu/precise/+source/tweak/+sharing-details
    * Verify the edit and delete icons are show next to the package
    * Open each and verify the pages loads without error

LINT

    lib/lp/registry/interfaces/packaging.py
    lib/lp/registry/model/packaging.py
    lib/lp/translations/browser/tests/test_sharing_details.py

TEST

    ./bin/test -vvc -t packaging lp.translations.browser.tests.test_sharing

IMPLEMENTATION

I updated Packaging.userCanDelete() to check if the user is not
probationary, which is the experience test we use elsewhere when we do
not want novices creating bogus data. I updated the tests that care
about sharing and packaging to make it clear probationary users cannot
change collection packaging data.
    lib/lp/registry/interfaces/packaging.py
    lib/lp/registry/model/packaging.py
    lib/lp/translations/browser/tests/test_sharing_details.py

To post a comment you must log in.
Revision history for this message
Richard Harding (rharding) wrote :

Thanks for clearing up in irc that the karma changes are needed to get the user to not be probationary. For devs not totally aware of the distinction a comment to that effect would be helpful. Thanks for the change.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.