Code review comment for lp:~adiroiban/launchpad/bug-509252-take-2

Revision history for this message
Adi Roiban (adiroiban) wrote :

= Bug 509252 =
After landing the fix for bug 340662 (https://code.edge.launchpad.net/~adiroiban/launchpad/bug-340662-take-2/+merge/17598) the POTemplateSubsetNavigation will render the AdminPOTemplateSubset useless.

We should clean the security.py.

AdminPOTemplateSubset was added to fix bug 497438

== Proposed fix ==

Remove unused classed from security.py and refactor

== Pre-implementation notes ==

Talking with Henning we decide not to include security checking code in the model. This is why the logic of showing translations of a series was spitted between the model and the view and the security checks were done in multiple places.

This restriction lead to a strange implementation where the check was done both in the view and the model and it was a source of confusion.

I merged this logic in a browser helper function and now the security check is only done once.

== Implementation details ==

These changes renders the AdminDistroSeriesLanguage class from security.py useless, so I have also removed it.

== Tests ==
lp-test -t distroseries-translations -t distroseries-views

== Demo and Q/A ==
Make sure you are a member of Ubuntu Translation Coordinators team (ubuntu-l10n-coordinator).
https://launchpad.dev/~ubuntu-l10n-coordinator

Go to the translation page for a distribution and hide translations for this series
ie: https://translations.launchpad.dev/ubuntu/hoary/+admin

Next go to the distribution series +template page
https://translations.launchpad.dev/ubuntu/hoary/+templates

You should see the Administer page for Evolution templates, including disabled-template .

As a member of Ubuntu Translation Coordinators team you should be able to administer them, just like for a normal series.

Login as a normal user you should see a page informing that the translations are currently closed

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/security.py
  lib/lp/registry/browser/distroseries.py
  lib/lp/registry/browser/sourcepackage.py
  lib/lp/registry/interfaces/distroseries.py
  lib/lp/registry/model/distroseries.py
  lib/lp/translations/browser/browser_helpers.py
  lib/lp/translations/browser/distroseries.py
  lib/lp/translations/stories/distroseries/xx-distroseries-translations.txt

== Pylint notices ==

lib/lp/registry/browser/sourcepackage.py
    29: [F0401] Unable to import 'lazr.restful.interface' (No module named restful)

lib/lp/registry/interfaces/distroseries.py
    23: [F0401] Unable to import 'lazr.enum' (No module named enum)
    50: [F0401] Unable to import 'lazr.restful.fields' (No module named restful)
    51: [F0401] Unable to import 'lazr.restful.declarations' (No module named restful)
    117: [E1002, DistroSeriesVersionField._validate] Use super on an old style class
    447: [C0322, IDistroSeriesPublic.getPackageUploads] Operator not preceded by a space
    description=_("Return items that are more recent than this "
    ^
    "timestamp."),
    required=False),
    status=Choice(

    vocabulary=DBEnumeratedType,
    title=_("Package Upload Status"),
    description=_("Return only items that have this status."),
    required=False),
    archive=Reference(

    schema=Interface,
    title=_("Archive"),
    description=_("Return only items for this archive."),
    required=False),
    pocket=Choice(

    vocabulary=DBEnumeratedType,
    title=_("Pocket"),
    description=_("Return only items targeted to this pocket"),
    required=False),
    custom_type=Choice(

    vocabulary=DBEnumeratedType,
    title=_("Custom Type"),
    description=_("Return only items with custom files of this "
    "type."),
    required=False),
    )

    @operation_returns_collection_of(Interface)
    @export_read_operation()
    def getPackageUploads(created_since_date, status, archive, pocket,
    custom_type):

lib/lp/registry/model/distroseries.py
    1941: [C0301] Line too long (79/78)

« Back to merge proposal