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

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

= Summary =

Users can choose to hide their email addresses and we should always honor that
request. Currently anyone who attempts to merge a person with hidden email addresses
is shown all addresses for that user.

Also did a drive-by fix to the person picker vocabulary that was not catching an
Unauthorized exception which prevented people with hidden emails from being selected
in the picker.

== Proposed fix ==

Rather than displaying the addresses and letting the user choose which ones to
contact we should just show a count of addresses and send notification to them all.

== Pre-implementation notes ==

Quick call with Curtis as a sanity check of the approach.

== Implementation details ==

As above.

== Tests ==

bin/test -vvm lp.registry -t merge-people.txt

== Demo and Q/A ==

= Launchpad lint =

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

Linting changed files:
  lib/lp/registry/browser/peoplemerge.py
  lib/lp/registry/stories/person/merge-people.txt
  lib/lp/registry/interfaces/person.py
  lib/lp/registry/templates/people-requestmerge-multiple.pt
  lib/canonical/launchpad/browser/vocabulary.py

== Pylint notices ==

Non-issues:

lib/lp/registry/browser/peoplemerge.py
    250: [C0322, AdminTeamMergeView.deactivate_members_and_merge_action] Operator not
preceded by a space
    name='deactivate_members_and_merge')
    ^
    def deactivate_members_and_merge_action(self, action, data):

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)
    407: [E1002, PersonNameField._validate] Use super on an old style class
    1405: [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):
    1444: [C0322, IPersonEditRestricted.acceptInvitationToBeMemberOf] Operator not
preceded by a space
    comment=Text())
    ^
    @export_write_operation()
    def acceptInvitationToBeMemberOf(team, comment):
    1456: [C0322, IPersonEditRestricted.declineInvitationToBeMemberOf] Operator not
preceded by a space
    comment=Text())
    ^
    @export_write_operation()
    def declineInvitationToBeMemberOf(team, comment):
    1754: [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):
    1823: [C0322, IPersonSet.findPerson] Operator not preceded by a space
    created_after=Datetime(
    ^
    title=_("Created after"), required=False),
    created_before=Datetime(
    title=_("Created before"), required=False),
    )
    @operation_returns_collection_of(IPerson)
    @export_read_operation()
    def findPerson(text="", exclude_inactive_accounts=True,
    must_have_email=False,
    created_after=None, created_before=None):

lib/canonical/launchpad/browser/vocabulary.py
    27: [F0401] Unable to import 'lazr.restful.interfaces' (No module named restful)

--
Brad Crittenden
<email address hidden>

« Back to merge proposal