Merge lp:~sinzui/launchpad/person-merge-job-4 into lp:launchpad
Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Merged at revision: | 12803 | ||||||||
Proposed branch: | lp:~sinzui/launchpad/person-merge-job-4 | ||||||||
Merge into: | lp:launchpad | ||||||||
Diff against target: |
782 lines (+154/-262) 16 files modified
lib/canonical/launchpad/browser/logintoken.py (+5/-4) lib/lp/registry/browser/peoplemerge.py (+18/-30) lib/lp/registry/browser/person.py (+8/-0) lib/lp/registry/browser/tests/peoplemerge-views.txt (+9/-21) lib/lp/registry/browser/tests/person-views.txt (+1/-0) lib/lp/registry/browser/tests/test_peoplemerge.py (+50/-26) lib/lp/registry/browser/tests/test_person_view.py (+18/-4) lib/lp/registry/browser/tests/test_team.py (+17/-0) lib/lp/registry/interfaces/person.py (+12/-7) lib/lp/registry/interfaces/persontransferjob.py (+2/-2) lib/lp/registry/model/person.py (+3/-4) lib/lp/registry/stories/person/merge-people.txt (+1/-35) lib/lp/registry/stories/person/xx-adminpeoplemerge.txt (+4/-14) lib/lp/registry/stories/person/xx-merge-person-with-hidden-email.txt (+0/-45) lib/lp/registry/stories/team/xx-adminteammerge.txt (+4/-68) lib/lp/registry/tests/test_person.py (+2/-2) |
||||||||
To merge this branch: | bzr merge lp:~sinzui/launchpad/person-merge-job-4 | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Benji York (community) | code | Approve | |
Review via email: mp+57037@code.launchpad.net |
Description of the change
Enable async person/team merges from the UI.
Launchpad bug:
https:/
https:/
Pre-
Test command: ./bin/test -vv \
-t test_is_
-t /xx-adminpeople
- Update call-sites to use mergeAsync() where appropriate.
- UI to show if a user is the subject of a scheduled merge request.
-------
RULES
* Add an info notification to the person/team index page that states
the team is involved in a merge.
* Ensure a merge cannot be requested is the user/team is involved
in a queued merge
* Change the view the call Person.
explaining that the merge is queued.
* ADDENDUM. Fix the ambiguous "Constraint not satisfied" message.
QA
* Using one of the unmergeable users, perform a merge.
* Verify (quickly) you see the notification that the user is being merged.
* Verify the user is merged after a few minutes.
* Using and undeletable teams, perform a delete.
* Verify (quickly) you see the notification that the team is being merged.
* Verify the team is merged after a few minutes.
LINT
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
lib/
IMPLEMENTATION
Fixed the signature of mergeAsync() and create() methods and updated the
documentation. Added reviewer to Merge() to be compatible with the
aforementioned methods.
lib/
lib/
Revised the rule for is_merge_pending because we care about merges in
the duplicate role. It is okay for a person or team to have several
dupes pending to merge. We expect this to be the case for team deletions
which are merges into ~registry.
lib/
lib/
I deleted the duplicate validate() from the direct subclass. I think it
was left over from a refactoring. I discovered that the common validate()
is used by forms that do not provide a target user since it is assumed to
be the requester. If fixed it to fix the tests I was writing.
Added a check to ensure the duplicate and target do not have a pending merge.
Updated the logintoken and peoplemerge views to use merge_async().
Changed the messages to explain the merge is queued.
I was startled by the "Constraint not satisfied" in a test, since that is
a UI problem. I found a bug was reported about it so I fixed the message.
lib/
lib/
lib/
lib/
I deleted corner cases from these story tests because there are testing in
the view tests. I updated the stories to verify the messages users see.
lib/
lib/
Added a notification to explain that the team or person is being merged.
lib/
lib/
lib/
This branch looks good.
One thing you might want to look at, I /think/ there is a hole in the
testing around the "is_merge_pending" property. I don't see that there
are any tests that the property is ever true.