After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 585440 - deleting a contact in multiple groups
deleting a contact in multiple groups
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: Contact List
2.27.x
Other Linux
: Normal minor
: ---
Assigned To: empathy-maint
Depends on:
Blocks:
 
 
Reported: 2009-06-11 15:31 UTC by Nicolò Chieffo
Modified: 2013-07-20 19:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Install roster-model property in EmpathyIndividualMenu (7.07 KB, patch)
2013-06-30 19:47 UTC, Chandni Verma
none Details | Review
Install property roster-contact in EmpathyIndividualMenu (7.18 KB, patch)
2013-06-30 19:47 UTC, Chandni Verma
none Details | Review
Show a 'Delete from Group' button in remove dialog (6.20 KB, patch)
2013-06-30 19:48 UTC, Chandni Verma
none Details | Review
Install property roster-contact in EmpathyIndividualMenu (7.05 KB, patch)
2013-07-09 23:28 UTC, Chandni Verma
reviewed Details | Review
Show a 'Delete from Group' button in remove dialog (6.19 KB, patch)
2013-07-09 23:29 UTC, Chandni Verma
reviewed Details | Review
Install property active-group in EmpathyIndividualMenu (7.50 KB, patch)
2013-07-19 13:44 UTC, Chandni Verma
reviewed Details | Review
Show a 'Remove from Group' button in remove dialog (6.49 KB, patch)
2013-07-19 13:44 UTC, Chandni Verma
reviewed Details | Review

Description Nicolò Chieffo 2009-06-11 15:31:53 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
Comment 1 Guillaume Desmottes 2009-06-23 14:22:01 UTC
I agree.
Comment 2 Chandni Verma 2013-06-29 09:05:01 UTC
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?
Comment 3 Chandni Verma 2013-06-29 11:13:42 UTC
(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.
Comment 5 Chandni Verma 2013-06-30 19:47:51 UTC
Created attachment 248101 [details] [review]
Install roster-model property in EmpathyIndividualMenu
Comment 6 Chandni Verma 2013-06-30 19:47:57 UTC
Created attachment 248102 [details] [review]
Install property roster-contact in EmpathyIndividualMenu
Comment 7 Chandni Verma 2013-06-30 19:48:04 UTC
Created attachment 248103 [details] [review]
Show a 'Delete from Group' button in remove dialog
Comment 8 Guillaume Desmottes 2013-07-09 08:29:53 UTC
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
Comment 9 Chandni Verma 2013-07-09 12:55:35 UTC
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.
Comment 10 Chandni Verma 2013-07-09 23:28:55 UTC
Created attachment 248783 [details] [review]
Install property roster-contact in EmpathyIndividualMenu
Comment 11 Chandni Verma 2013-07-09 23:29:01 UTC
Created attachment 248784 [details] [review]
Show a 'Delete from Group' button in remove dialog
Comment 12 Chandni Verma 2013-07-17 15:14:39 UTC
If these patches look fine should I go ahead and merge?
Comment 13 Guillaume Desmottes 2013-07-18 12:45:10 UTC
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,
...);
Comment 14 Guillaume Desmottes 2013-07-18 12:45:29 UTC
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.
Comment 15 Chandni Verma 2013-07-19 13:44:38 UTC
Created attachment 249632 [details] [review]
Install property active-group in EmpathyIndividualMenu
Comment 16 Chandni Verma 2013-07-19 13:44:46 UTC
Created attachment 249633 [details] [review]
Show a 'Remove from Group' button in remove dialog
Comment 17 Guillaume Desmottes 2013-07-19 13:50:35 UTC
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.
Comment 18 Guillaume Desmottes 2013-07-19 13:53:48 UTC
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.
Comment 19 Chandni Verma 2013-07-20 19:44:18 UTC
Comments incorporated and pushed changes upstream. Thanks for the reviews.