Code review comment for lp:~bac/launchpad/bug-488762-snapshot

Revision history for this message
Brad Crittenden (bac) wrote :

= Summary =

Editing large teams fails because taking a snapshot of the team includes all of the
collections for that team, big stuff like all of the members. There is no reason for
those collections to be part of the snapshot.

== Proposed fix ==

lazr.lifecycle has a new method 'doNotSnapshot' that can encapuslate a Field and make
it not be included in the snapshot. On IPerson all of the collections have been
wrapped in that method to prevent their inclusion.

== Pre-implementation notes ==

Chats with Curtis.

== Implementation details ==

As above.

== Tests ==

I've added a test to show that the fields I currently expect to be omitted from the
snapshot are in fact omitted. It's not a very interesting test but there to prevent
regression.

bin/test -vvm lp.registry -t test_person_snapshot

== Demo and Q/A ==

On staging become an admin and try to edit the team in the bug report. It should
succeed.

= Launchpad lint =

Bah!

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

Linting changed files:
  lib/canonical/launchpad/webapp/snapshot.py
  versions.cfg
  lib/lp/registry/doc/person.txt
  lib/lp/registry/interfaces/person.py
  lib/lp/registry/model/product.py
  lib/lp/registry/tests/test_person.py

== Pylint notices ==

lib/canonical/launchpad/webapp/snapshot.py
    11: [F0401] Unable to import 'lazr.lifecycle.interfaces' (No module named lifecycle)

lib/lp/registry/interfaces/person.py
    52: [F0401] Unable to import 'lazr.enum' (No module named enum)
    53: [F0401] Unable to import 'lazr.lifecycle.snapshot' (No module named lifecycle)
    54: [F0401] Unable to import 'lazr.restful.interface' (No module named restful)
    55: [F0401] Unable to import 'lazr.restful.declarations' (No module named restful)
    62: [F0401] Unable to import 'lazr.restful.fields' (No module named restful)
    406: [E1002, PersonNameField._validate] Use super on an old style class
    1401: [C0322, IPersonEditRestricted.addMember] Operator not preceded by a space
    status=copy_field(ITeamMembership['status']),
    ^
    comment=Text(required=False))
    @export_write_operation()
    def addMember(person, reviewer, status=TeamMembershipStatus.APPROVED,
    comment=None, force_team_add=False,
    may_subscribe_to_list=True):
    1433: [C0322, IPersonEditRestricted.acceptInvitationToBeMemberOf] Operator not
preceded by a space
    comment=Text())
    ^
    @export_write_operation()
    def acceptInvitationToBeMemberOf(team, comment):
    1445: [C0322, IPersonEditRestricted.declineInvitationToBeMemberOf] Operator not
preceded by a space
    comment=Text())
    ^
    @export_write_operation()
    def declineInvitationToBeMemberOf(team, comment):
    1737: [C0322, IPersonSet.newTeam] Operator not preceded by a space
    defaultmembershipperiod='default_membership_period',
    ^
    defaultrenewalperiod='default_renewal_period')
    @operation_parameters(
    subscriptionpolicy=Choice(
    title=_('Subscription policy'), vocabulary=TeamSubscriptionPolicy,
    required=False, default=TeamSubscriptionPolicy.MODERATED))
    @export_factory_operation(
    ITeam, ['name', 'displayname', 'teamdescription',
    'defaultmembershipperiod', 'defaultrenewalperiod'])
    def newTeam(teamowner, name, displayname, teamdescription=None,
    subscriptionpolicy=TeamSubscriptionPolicy.MODERATED,
    defaultmembershipperiod=None, defaultrenewalperiod=None):

lib/lp/registry/model/product.py
    28: [F0401] Unable to import 'lazr.delegates' (No module named delegates)

lib/lp/registry/tests/test_person.py
    20: [F0401] Unable to import 'lazr.lifecycle.snapshot' (No module named lifecycle)

« Back to merge proposal