GNOME Bugzilla – Bug 585440
deleting a contact in multiple groups
Last modified: 2013-07-20 19:44:18 UTC
I have a contact in 2 groups. If I click on "remove" a dialog appears: "Do you really want to remove the contact 'Matteo'? Cancel, Delete" I think that if a contact is in more than one group, empathy should also ask if you want to just remove it from the group
I agree.
Its not very clear how one would go for it. I can easily obtain the total number of contacts an individual is in by fetching the RosterModel through RosterView in EmpathyIndividualMenu but I how will I get which group is the current Individual widget in? Should the group linger on after it's last contact has been removed or should it be deleted as well?
(In reply to comment #2) > Its not very clear how one would go for it. > > I can easily obtain the total number of contacts an individual is in by > fetching the RosterModel through RosterView in EmpathyIndividualMenu but I how > will I get which group is the current Individual widget in? > Wait I guess I found a solution.
Pushed fix in branch https://gitorious.org/glassrose-gnome/empathy/commits/deleting-a-contact-in-multiple-groups-585440
Created attachment 248101 [details] [review] Install roster-model property in EmpathyIndividualMenu
Created attachment 248102 [details] [review] Install property roster-contact in EmpathyIndividualMenu
Created attachment 248103 [details] [review] Show a 'Delete from Group' button in remove dialog
Can't you just use Folks's group API directly instead of using the EmpathyRoster{Contact,Model}? http://telepathy.freedesktop.org/doc/folks/c/FolksGroupDetails.html#folks-group-details-get-groups
I can use the folks api to get the groups an individual is a member of but still I will need the roster-contact to get the value of current group from which the individual has to be removed.
Created attachment 248783 [details] [review] Install property roster-contact in EmpathyIndividualMenu
Created attachment 248784 [details] [review] Show a 'Delete from Group' button in remove dialog
If these patches look fine should I go ahead and merge?
Review of attachment 248783 [details] [review]: ::: libempathy-gtk/empathy-individual-menu.h @@ +74,3 @@ +GtkWidget * empathy_individual_menu_new (EmpathyRosterContact *roster_contact, + FolksIndividual *individual, I'd prefer to pass only the group as a second argument, the individual menu shouldn't have to deal with EmpathyRosterContact at all. So something like: GtkWidget * empathy_individual_menu_new (FolksIndividual *individual, const gchar *active_group, ...);
Review of attachment 248784 [details] [review]: ::: libempathy-gtk/empathy-individual-menu.c @@ +718,3 @@ + * mnemonic so we have to create the button manually. */ + button = gtk_button_new_with_mnemonic ( + _("Delete from _Group")); Maybe "Remove from group '$group'" would be clearer? @@ +762,3 @@ + GError *error = NULL; + + FolksIndividual *individual = FOLKS_INDIVIDUAL (source_object); the \n should be after this line, not before. @@ +765,3 @@ + folks_group_details_change_group_finish (FOLKS_GROUP_DETAILS (individual), + res, &error); + GError *error = NULL; we usually use if (!function_finish (..) { error } rather than relying on the GError being set or not.
Created attachment 249632 [details] [review] Install property active-group in EmpathyIndividualMenu
Created attachment 249633 [details] [review] Show a 'Remove from Group' button in remove dialog
Review of attachment 249632 [details] [review]: Looks good except the arguments position (see inline comment). Feel free to merge once this has been fixed. ::: libempathy-gtk/empathy-individual-menu.c @@ +1171,2 @@ GtkWidget * +empathy_individual_menu_new (const gchar *active_group, As I said in my previous comment, 'active_group' should be the second argument of this _new() function, not the first one. The individual is the key mandatory argument here so it makes sense for it to be the first one passed.
Review of attachment 249633 [details] [review]: ::: libempathy-gtk/empathy-individual-menu.c @@ +716,3 @@ + GtkWidget *button; + gchar *button_text = "Remove from _Group "; + g_strconcat (button_text, active_group, NULL); This won't work for translators. You have to do something like that: button_text = g_strdup_printf (_("Remove from group '%s'), active_group); @@ +720,3 @@ + /* gtk_dialog_add_button() doesn't allow us to pass a string with a + * mnemonic so we have to create the button manually. */ + gchar *button_text = "Remove from _Group "; don't use _() here. It's to be used with static strings.
Comments incorporated and pushed changes upstream. Thanks for the reviews.