Contact List Duplicate Groups Error

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-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

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-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

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-unsubscribesip-communicator.dev.java.net
> For additional commands, e-mail: dev-helpsip-communicator.dev.java.net
> 
> 

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

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-unsubscribesip-communicator.dev.java.net
>> For additional commands, e-mail: dev-helpsip-communicator.dev.java.net
>>
>>
>
>
------------------------------------------------------------
---------
> To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
> For additional commands, e-mail: dev-helpsip-communicator.dev.java.net
>
>

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

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-unsubscribesip-communicator.dev.java.net
>>> For additional commands, e-mail: dev-helpsip-communicator.dev.java.net
>>>
>>>
>>
------------------------------------------------------------
---------
>> To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
>> For additional commands, e-mail: dev-helpsip-communicator.dev.java.net
>>
>>
> 
>
------------------------------------------------------------
---------
> To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
> For additional commands, e-mail: dev-helpsip-communicator.dev.java.net
> 
> 

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

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-unsubscribesip-communicator.dev.java.net
>>>> For additional commands, e-mail: 
>>>> dev-helpsip-communicator.dev.java.net
>>>>
>>>>
>>>
------------------------------------------------------------
---------
>>> To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
>>> For additional commands, e-mail: dev-helpsip-communicator.dev.java.net
>>>
>>>
>>
>>
------------------------------------------------------------
---------
>> To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
>> For additional commands, e-mail: dev-helpsip-communicator.dev.java.net
>>
>>
>
>
------------------------------------------------------------
---------
> To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
> For additional commands, e-mail: dev-helpsip-communicator.dev.java.net
>
>


------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

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-unsubscribesip-communicator.dev.java.net
>>>>> For additional commands, e-mail: 
>>>>> dev-helpsip-communicator.dev.java.net
>>>>>
>>>>>
>>>>
------------------------------------------------------------
---------
>>>> To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
>>>> For additional commands, e-mail:
dev-helpsip-communicator.dev.java.net
>>>>
>>>>
>>>
>>>
------------------------------------------------------------
---------
>>> To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
>>> For additional commands, e-mail: dev-helpsip-communicator.dev.java.net
>>>
>>>
>>
>>
------------------------------------------------------------
---------
>> To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
>> For additional commands, e-mail: dev-helpsip-communicator.dev.java.net
>>
>>
> 
> 
>
------------------------------------------------------------
------------
> 
>
------------------------------------------------------------
---------
> To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
> For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net