[sip-comm-dev] Contact List Duplicate Groups Error


#1

Hey everyone,

I was organizing my contact list and I noticed that you're able to add the same group multiple times. There doesn't seem to be a logger error, but if you then try to add a user to the group, it tosses out a message that there is a problem. I think there may just be a missing case in the catch of CreateGroup in CreateGroupDialog.java of impl.gui.main.contactlist.addgroup.

I don't know if there is an appropriate catch when adding to the MetaContactListService clist, but if there isn't, I think the best course of action would be to pop up an error explaining what's wrong (duplicate group name), and then recreate the CreateGroupDialog with the group name that was passed in. This might require passing around the mainFrame, but we'll see. In any case, I just wanted to mention this to see if this was already known and to see what everyone thought.

Adam

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#2

Adam Goldstein wrote:

Hey everyone,

I was organizing my contact list and I noticed that you're able to add the same group multiple times. There doesn't seem to be a logger error, but if you then try to add a user to the group, it tosses out a message that there is a problem. I think there may just be a missing case in the catch of CreateGroup in CreateGroupDialog.java of impl.gui.main.contactlist.addgroup.

I tried this one and effectively you could add the same group multiple times, but when I try to add a contact it is successfully added in the group. I'll have a detailed look.

I don't know if there is an appropriate catch when adding to the MetaContactListService clist, but if there isn't, I think the best course of action would be to pop up an error explaining what's wrong (duplicate group name), and then recreate the CreateGroupDialog with the group name that was passed in. This might require passing around the mainFrame, but we'll see.

Yes, that's a good solution. I thought also of adding a text in red below the text field in the "Create group" dialog, as we're trying to avoid additional dialogs when possible.

In any case, I just wanted to mention this to see if this was already known and to see what everyone thought.

Thanks Adam! Nice catch!

damencho

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#3

Hey Adam,

I've just committed a fix to this problem. I've tried it locally and it seems to work. Right now it would simply report an error when you try to create a group that has the same name as another group under the same parent, and it won't create the group.

Could you please let me know if it works for you?

Thanks for the report!

Emil

Damian Minkov wrote:

···

Adam Goldstein wrote:

Hey everyone,

I was organizing my contact list and I noticed that you're able to add the same group multiple times. There doesn't seem to be a logger error, but if you then try to add a user to the group, it tosses out a message that there is a problem. I think there may just be a missing case in the catch of CreateGroup in CreateGroupDialog.java of impl.gui.main.contactlist.addgroup.

I tried this one and effectively you could add the same group multiple times, but when I try to add a contact it is successfully added in the group. I'll have a detailed look.

I don't know if there is an appropriate catch when adding to the MetaContactListService clist, but if there isn't, I think the best course of action would be to pop up an error explaining what's wrong (duplicate group name), and then recreate the CreateGroupDialog with the group name that was passed in. This might require passing around the mainFrame, but we'll see.

Yes, that's a good solution. I thought also of adding a text in red below the text field in the "Create group" dialog, as we're trying to avoid additional dialogs when possible.

In any case, I just wanted to mention this to see if this was already known and to see what everyone thought.

Thanks Adam! Nice catch!

damencho

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#4

Hi Emil,

Yup, it works for me too. I'll try to take a look at and add some sort of more permanent solution. I think Damian's point of fewer dialogs is good, and since there's a ton of space below the text field of the create group dialog, we can just stick in some red error text below.

Adam

Emil Ivov wrote:

···

Hey Adam,

I've just committed a fix to this problem. I've tried it locally and it seems to work. Right now it would simply report an error when you try to create a group that has the same name as another group under the same parent, and it won't create the group.

Could you please let me know if it works for you?

Thanks for the report!

Emil

Damian Minkov wrote:

Adam Goldstein wrote:

Hey everyone,

I was organizing my contact list and I noticed that you're able to add the same group multiple times. There doesn't seem to be a logger error, but if you then try to add a user to the group, it tosses out a message that there is a problem. I think there may just be a missing case in the catch of CreateGroup in CreateGroupDialog.java of impl.gui.main.contactlist.addgroup.

I tried this one and effectively you could add the same group multiple times, but when I try to add a contact it is successfully added in the group. I'll have a detailed look.

I don't know if there is an appropriate catch when adding to the MetaContactListService clist, but if there isn't, I think the best course of action would be to pop up an error explaining what's wrong (duplicate group name), and then recreate the CreateGroupDialog with the group name that was passed in. This might require passing around the mainFrame, but we'll see.

Yes, that's a good solution. I thought also of adding a text in red below the text field in the "Create group" dialog, as we're trying to avoid additional dialogs when possible.

In any case, I just wanted to mention this to see if this was already known and to see what everyone thought.

Thanks Adam! Nice catch!

damencho

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#5

Adam,

Yes sounds nice indeed. Just send it over when you are done.

Emil

Adam Goldstein wrote:

···

Hi Emil,

Yup, it works for me too. I'll try to take a look at and add some sort of more permanent solution. I think Damian's point of fewer dialogs is good, and since there's a ton of space below the text field of the create group dialog, we can just stick in some red error text below.

Adam

Emil Ivov wrote:

Hey Adam,

I've just committed a fix to this problem. I've tried it locally and it seems to work. Right now it would simply report an error when you try to create a group that has the same name as another group under the same parent, and it won't create the group.

Could you please let me know if it works for you?

Thanks for the report!

Emil

Damian Minkov wrote:

Adam Goldstein wrote:

Hey everyone,

I was organizing my contact list and I noticed that you're able to add the same group multiple times. There doesn't seem to be a logger error, but if you then try to add a user to the group, it tosses out a message that there is a problem. I think there may just be a missing case in the catch of CreateGroup in CreateGroupDialog.java of impl.gui.main.contactlist.addgroup.

I tried this one and effectively you could add the same group multiple times, but when I try to add a contact it is successfully added in the group. I'll have a detailed look.

I don't know if there is an appropriate catch when adding to the MetaContactListService clist, but if there isn't, I think the best course of action would be to pop up an error explaining what's wrong (duplicate group name), and then recreate the CreateGroupDialog with the group name that was passed in. This might require passing around the mainFrame, but we'll see.

Yes, that's a good solution. I thought also of adding a text in red below the text field in the "Create group" dialog, as we're trying to avoid additional dialogs when possible.

In any case, I just wanted to mention this to see if this was already known and to see what everyone thought.

Thanks Adam! Nice catch!

damencho

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#6

Emil

Alright, I made a little solution, but I'm not sure if it's a little dirty.

I added a JLabel to CreateGroupPanel called errorLabel that shows whatever is wrong, and I made it so that rather than displaying an ErrorDialog if there's a problem, the errorLabel becomes visible and shows the error message. To do this, I pass the CreateGroupDialog itself to the private CreateGroup class so that the disposal of the dialog is handled there. If there is an exception, the errorLabel is updated, and we return out of the run method. If there is no exception, the code will reach the dispose statement. I left what used to be there in comments.

Let me know what you think of this. I hope it's alright for a little fix.

Adam

Emil Ivov wrote:

CreateGroupPanel.java (3.22 KB)

CreateGroupDialog.java (8.34 KB)

···

Adam,

Yes sounds nice indeed. Just send it over when you are done.

Emil

Adam Goldstein wrote:

Hi Emil,

Yup, it works for me too. I'll try to take a look at and add some sort of more permanent solution. I think Damian's point of fewer dialogs is good, and since there's a ton of space below the text field of the create group dialog, we can just stick in some red error text below.

Adam

Emil Ivov wrote:

Hey Adam,

I've just committed a fix to this problem. I've tried it locally and it seems to work. Right now it would simply report an error when you try to create a group that has the same name as another group under the same parent, and it won't create the group.

Could you please let me know if it works for you?

Thanks for the report!

Emil

Damian Minkov wrote:

Adam Goldstein wrote:

Hey everyone,

I was organizing my contact list and I noticed that you're able to add the same group multiple times. There doesn't seem to be a logger error, but if you then try to add a user to the group, it tosses out a message that there is a problem. I think there may just be a missing case in the catch of CreateGroup in CreateGroupDialog.java of impl.gui.main.contactlist.addgroup.

I tried this one and effectively you could add the same group multiple times, but when I try to add a contact it is successfully added in the group. I'll have a detailed look.

I don't know if there is an appropriate catch when adding to the MetaContactListService clist, but if there isn't, I think the best course of action would be to pop up an error explaining what's wrong (duplicate group name), and then recreate the CreateGroupDialog with the group name that was passed in. This might require passing around the mainFrame, but we'll see.

Yes, that's a good solution. I thought also of adding a text in red below the text field in the "Create group" dialog, as we're trying to avoid additional dialogs when possible.

In any case, I just wanted to mention this to see if this was already known and to see what everyone thought.

Thanks Adam! Nice catch!

damencho

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#7

Hi Adam,

I've just committed your patch. Thanks for the report and the patch! Good work!

I've made some minor modifications that I've listed below. If you have something to add you're welcome.

- Removed the default value of the error label ("Everything's going fine") - we don't need this message, as we'll show the error only when something goes wrong

- Added the errorLabel in the labelsPanel instead of the rightPanel - it seems more compact like this, the error appears just below the textfield.

- Removed the CreateGroupDialog from the list of parameters in the CreateGroup constructor - we don't need that, because we're already in the dialog and we could access it directly from the inner class

- I liked very much that you thought of moving the dispose(); from the actionPerformed to the thread! However we should add in the actionPerformed a case for the "Cancel" button, so I added an else there:

  if (name.equals("create"))
  {
             new CreateGroup(clist, groupPanel.getGroupName()).start();
         }
         else if(name.equals("cancel"))
         {
             dispose();
         }

Once again nice catch!
Yana

Adam Goldstein wrote:

···

Emil

Alright, I made a little solution, but I'm not sure if it's a little dirty.

I added a JLabel to CreateGroupPanel called errorLabel that shows whatever is wrong, and I made it so that rather than displaying an ErrorDialog if there's a problem, the errorLabel becomes visible and shows the error message. To do this, I pass the CreateGroupDialog itself to the private CreateGroup class so that the disposal of the dialog is handled there. If there is an exception, the errorLabel is updated, and we return out of the run method. If there is no exception, the code will reach the dispose statement. I left what used to be there in comments.

Let me know what you think of this. I hope it's alright for a little fix.

Adam

Emil Ivov wrote:

Adam,

Yes sounds nice indeed. Just send it over when you are done.

Emil

Adam Goldstein wrote:

Hi Emil,

Yup, it works for me too. I'll try to take a look at and add some sort of more permanent solution. I think Damian's point of fewer dialogs is good, and since there's a ton of space below the text field of the create group dialog, we can just stick in some red error text below.

Adam

Emil Ivov wrote:

Hey Adam,

I've just committed a fix to this problem. I've tried it locally and it seems to work. Right now it would simply report an error when you try to create a group that has the same name as another group under the same parent, and it won't create the group.

Could you please let me know if it works for you?

Thanks for the report!

Emil

Damian Minkov wrote:

Adam Goldstein wrote:

Hey everyone,

I was organizing my contact list and I noticed that you're able to add the same group multiple times. There doesn't seem to be a logger error, but if you then try to add a user to the group, it tosses out a message that there is a problem. I think there may just be a missing case in the catch of CreateGroup in CreateGroupDialog.java of impl.gui.main.contactlist.addgroup.

I tried this one and effectively you could add the same group multiple times, but when I try to add a contact it is successfully added in the group. I'll have a detailed look.

I don't know if there is an appropriate catch when adding to the MetaContactListService clist, but if there isn't, I think the best course of action would be to pop up an error explaining what's wrong (duplicate group name), and then recreate the CreateGroupDialog with the group name that was passed in. This might require passing around the mainFrame, but we'll see.

Yes, that's a good solution. I thought also of adding a text in red below the text field in the "Create group" dialog, as we're trying to avoid additional dialogs when possible.

In any case, I just wanted to mention this to see if this was already known and to see what everyone thought.

Thanks Adam! Nice catch!

damencho

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net