GNOME Bugzilla – Bug 626544
Add unlinking UI for Individuals
Last modified: 2010-08-20 15:53:17 UTC
Created attachment 167529 [details] [review] Squashed diff of the unlinking-menu-entry branch This branch adds an "Unlink" menu entry just below the "Link…" menu entry. Clicking on "Unlink" will split a linked Individual into Individuals for all of its Personas, and is protected by a confirmation dialogue. http://git.collabora.co.uk/?p=user/pwith/empathy;a=shortlog;h=refs/heads/unlinking-menu-entry This depends on bug #626130 and bug #626543, though this can be committed before the libfolks bug is fixed (it will just fail to unlink things; there aren't any API changes in the libfolks bug).
Do we really want to add yet another entry to the contact menu? Can't we use the link dialog to unlink contacts as well?
(In reply to comment #1) > Do we really want to add yet another entry to the contact menu? Can't we use > the link dialog to unlink contacts as well? Not the way the linking dialogue is currently designed. The linking dialogue currently links by Individual (which I think is the right way to do things), meaning that the information about which Personas formed each Individual which you link into a new Individual is lost when you click "Link". This means that unlinking using the same dialogue is impossible, as we don't know which Individuals to have ticked in the list on the left to begin with. If a menu entry is too intrusive, we could perhaps put an "Unlink" button in the edit dialogue for linked Individuals?
Nick, do you have any thoughts about this?
(In reply to comment #2) > (In reply to comment #1) > > Do we really want to add yet another entry to the contact menu? Can't we use > > the link dialog to unlink contacts as well? > > Not the way the linking dialogue is currently designed. The linking dialogue > currently links by Individual (which I think is the right way to do things), > meaning that the information about which Personas formed each Individual which > you link into a new Individual is lost when you click "Link". This means that > unlinking using the same dialogue is impossible, as we don't know which > Individuals to have ticked in the list on the left to begin with. Can't we just have unlinked personas each get assigned to new Individuals? As a small optimization, we could track the original arrangement until Link is hit, so Cancel truly cancels. Hopefully this is how it works now anyhow (haven't checked yet).
(In reply to comment #4) > (In reply to comment #2) > > (In reply to comment #1) > > > Do we really want to add yet another entry to the contact menu? Can't we use > > > the link dialog to unlink contacts as well? > > > > Not the way the linking dialogue is currently designed. The linking dialogue > > currently links by Individual (which I think is the right way to do things), > > meaning that the information about which Personas formed each Individual which > > you link into a new Individual is lost when you click "Link". This means that > > unlinking using the same dialogue is impossible, as we don't know which > > Individuals to have ticked in the list on the left to begin with. > > Can't we just have unlinked personas each get assigned to new Individuals? > > As a small optimization, we could track the original arrangement until Link is > hit, so Cancel truly cancels. Hopefully this is how it works now anyhow > (haven't checked yet). As discussed on IRC, we would have to have the aggregator expose these new Individuals before the dialog is closed for this to work the way I'd imagined it. So we'll just stick with the separate unlinking dialog for now.
If we go with an "Unlink" button in the Edit dialogue, that work would depend on my work on EmpathyIndividualWidget. The first commit (4668dbdaf7d2a089db2a2074d18608f78aee19bf) out of the two in the branch in comment #0 can be committed any time, though, since Empathy now depends on libfolks 0.1.13.
(In reply to comment #6) > If we go with an "Unlink" button in the Edit dialogue, that work would depend > on my work on EmpathyIndividualWidget. The first commit > (4668dbdaf7d2a089db2a2074d18608f78aee19bf) out of the two in the branch in > comment #0 can be committed any time, though, since Empathy now depends on > libfolks 0.1.13. Actually, the "Unlink" button could be done in parallel to my work on EmpathyIndividualWidget. All we need to do is to add it to the button box at the bottom of the dialogue, probably left-aligned.
I didn't manage to do that this morning. Here's a branch with just the EmpathyIndividualManager changes: http://git.collabora.co.uk/?p=user/pwith/empathy;a=shortlog;h=refs/heads/unlinking-button
New branch with an EmpathyIndividualEditDialog and "Unlink" button. http://git.collabora.co.uk/?p=user/pwith/empathy;a=shortlog;h=refs/heads/unlinking-button-rebase1
Created attachment 168187 [details] [review] Squashed diff of the unlinking-button-rebase1 branch
Review of attachment 168187 [details] [review]: The code for this seems fine, except for the way it vertically stacks one contact editor per Persona. It's awkward for 2 Personas (even on a large screen), and unusably-tall beyond that. I think we should hold off until you can use the EmpathyIndividualWidget here to make this a reasonably-sized dialog.
Rebased branch: http://git.collabora.co.uk/?p=user/pwith/empathy;a=shortlog;h=refs/heads/unlinking-button-rebase2 This is now based on individual-widget-rebase2 (bug #626728).
Created attachment 168286 [details] [review] Squashed diff of the unlinking-button-rebase2 branch
Review of attachment 168286 [details] [review]: The code seems fine upon first glance, but I hit a couple issues: * I've got an Individual (A) with 3 personas: * user1@localhost (from user2@localhost) * user1@localhost (from user3@localhost) * user2@localhost (from user1@localhost) So the first two personas were auto-linked by the Aggregator, since they've got the same UID. If I unlink this Individual, I end up with: Individual B: * user1@localhost (from user2@localhost) * user1@localhost (from user3@localhost) Individual C: * user2@localhost (from user1@localhost) If I try to re-link Individuals B and C, I get some sort of bad state (and several warnings) that doesn't clear until I restart Empathy Similarly, if I link the persona (user3@localhost from user1@localhost) into Individual A, yielding Individual D, then I unlink Individual D, the contact list doesn't update at all. I don't know if it's relevant that I had no other Individuals in my roster at the time (or that Individual D was in Favourites and Ungrouped). Again, in that last case, if I restart Empathy, it appears as I'd expect (with 3 Individuals).
(In reply to comment #13) > Created an attachment (id=168286) [details] [review] > Squashed diff of the unlinking-button-rebase2 branch I just tried it with non-autolinked Individuals, and linking and unlinking and relinking ad nauseum worked fine.
I've pushed a change to Empathy which gets rid of the only warning I was getting when trying to reproduce the bug. Here's a folks branch which goes some of the way towards fixing it: http://git.collabora.co.uk/?p=user/pwith/folks;a=shortlog;h=refs/heads/626544-im-address-duplicates With this applied, I can still get into a bad state when linking two auto-linked Individuals together, then linking with a third (singleton) Individual. At this point, the resulting linked Individual lists some of the Personas twice. Unlinking then causes excess unreffing. I suspect it's a problem to do with the fact that we don't screen Individual.personas for duplicates, so I'll look into that.
With the second commit in 626544-im-address-duplicates, everything seems to work for me now, with no changes required in Empathy on top of the unlinking-button-rebase2 branch.
Here's a rebased Empathy branch. The squashed diff should be the same as before: http://git.collabora.co.uk/?p=user/pwith/empathy;a=shortlog;h=refs/heads/unlinking-button-rebase3
The Empathy branch has been merged. The folks branch still needs reviewing and merging. commit 5fa714c470951e828f8dab44856b8cc2d4c30340 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Wed Aug 18 11:04:06 2010 +0100 Add EmpathyIndividualEditDialog This replaces the edit dialogue from empathy-contact-dialogs.c, and is now used from the EmpathyIndividualMenu. Closes: bgo#626544 libempathy-gtk/Makefile.am | 2 + libempathy-gtk/empathy-individual-edit-dialog.c | 273 +++++++++++++++++++++++ libempathy-gtk/empathy-individual-edit-dialog.h | 59 +++++ libempathy-gtk/empathy-individual-menu.c | 8 +- po/POTFILES.in | 1 + 5 files changed, 337 insertions(+), 6 deletions(-) commit e445d9d5b2781ef59e1a07e97853f232c94d0dc2 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Tue Aug 10 11:54:48 2010 +0100 Allow unlinking individuals through EmpathyIndividualManager Wrap the FolksIndividualAggregator individual unlinking API in EmpathyIndividualManager with some basic error reporting (it isn't expected that unlinking will fail). libempathy/empathy-individual-manager.c | 34 +++++++++++++++++++++++++++++++ libempathy/empathy-individual-manager.h | 4 +++ 2 files changed, 38 insertions(+), 0 deletions(-)
Comment on attachment 168286 [details] [review] Squashed diff of the unlinking-button-rebase2 branch (Reviewed and OKed by Guillaume on IRC.)
Folks branch merged as well; resolving Fixed. commit 37c6361ec6dad777f657ac4f2b272d1628080c35 Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Fri Aug 20 08:37:04 2010 -0700 Clean up the variable naming for parallel data structures. folks/individual-aggregator.vala | 30 ++++++++++++++++-------------- 1 files changed, 16 insertions(+), 14 deletions(-) commit f2fc13b30ab64f82a8681f7466fed7d5b15cd1da Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Fri Aug 20 12:22:13 2010 +0100 Ensure duplicate Individuals don't enter candidate_inds when aggregating If Individuals matched on more than one property, they would appear in the list of candidate Individuals multiple times, causing the resulting linked Individual to contain duplicate Personas and Bad Things to happen. Helps: bgo#626544 folks/individual-aggregator.vala | 21 +++++++++++++++++++-- 1 files changed, 19 insertions(+), 2 deletions(-) commit ae5928a23c08501f0f47db8e29f51f5f6a838889 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Fri Aug 20 11:53:18 2010 +0100 Ensure the IMable.im_addresses property does not contain duplicates This can cause Personas to appear multiple times in a linked Individual, leading to bad state. Helps: bgo#626544 backends/key-file/kf-persona.vala | 14 ++++++++++++-- folks/imable.vala | 12 +++++++++--- folks/individual-aggregator.vala | 22 ++++++++++++++++++++-- 3 files changed, 41 insertions(+), 7 deletions(-)