Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 349785

Summary:

Able to add the same file to Favourites more than once

Product:

[ECD] Orion

Reporter:

Pradyut Sarma <pradyutksarma>

Component:

Client

Assignee:

Susan McCourt <susan>

Status:

RESOLVED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

andrew.eisenberg, simon_kaegi

Version:

unspecified

Target Milestone:

0.4 M2

Hardware:

OS:

Windows 7

Whiteboard:

Attachments:

Description	Flags
Favourites View Discrepancy	none

Description Pradyut Sarma

2011-06-20 03:44:33 EDT

Build Identifier: M8

Hi,

I am able to select the same file from my Navigator and add it to my Favourites more than once. Should I be allowed to do that, since it is a reference to the same file in the Navigator.
Also, there is another behavior that I have observed.

When I rename one of the favourite reference, all the other references are also renamed. But when I delete one, the others are not deleted.
Nonetheless, you wouldn't see this discrepancy if you don't allow the same file to be added to the Favourites more than once.

Reproducible: Always

Steps to Reproduce:
1. Add a file to the Favourites
2. Add the same file again
Check Details section for more info.

Comment 1 Pradyut Sarma

2011-06-20 03:45:45 EDT

Created attachment 198239 [details]
Favourites View Discrepancy

Comment 2 Andrew Eisenberg

2012-01-17 13:59:16 EST

Susan, are you working on this right now?  I was going to take a stab at fixing this if you currently are not.

Comment 3 Andrew Eisenberg

2012-01-17 18:40:42 EST

Pull request: 
https://github.com/eclipse/orion.client/pull/11

I wrote all this code and have the rights to contribute it to Eclipse under the eclipse.org web site terms of use.

Some comments on this:

1. I did not write any tests.  I didn't see any test infrastructure for this area, but if you would like me to add a test (and can point me to something similar), I will.
2. I made a change from 
-        if (faves[i].directory) {
to
+        if (faves[i].isFavorite) {
I am not sure I fully understand what the previous code was doing.  It looks like it may have been a bug.

Comment 4 Susan McCourt

2012-01-18 11:36:34 EST

thanks, Andrew!  I'll take a look a little later today

Comment 5 Susan McCourt

2012-01-18 21:56:00 EST

(In reply to comment #4)
> thanks, Andrew!  I'll take a look a little later today

it'll be tomorrow, after our conf call.

Comment 6 Susan McCourt

2012-01-20 18:05:30 EST

*** Bug 369010 has been marked as a duplicate of this bug. ***

Comment 7 Susan McCourt

2012-01-20 18:07:57 EST

per duplicate bug by Andrew, the original fix was not complete.
To reproduce: 
1. In the navigator, click on the '+' icon next to 'Favorites'
2. In the text box that appears, type 'http://eclipse.org'
3. repeat


When all cases are fixed, you can update this bug. 
Note also that I removed the function "favoriteExists" from the fix for bug 368896 since it wasn't technically part of that fix.

So you may need to pull the latest and add it back.  When done, let me know the pull request.  Thanks for working on this.

Comment 8 Susan McCourt

2012-01-20 18:23:04 EST

sorry if this ends up causing more trouble.  I originally thought you had reopened this bug and so decided to try to commit bug 368896 first.  Then I pulled the function out.  Then I realized you intended to commit this bug, then bug 368896, then the one I marked duplicate.  

At any rate, at this point, it would be cool to have one commit that fixes both problems.  Not trying to haze you, just got confused a little.

Comment 9 Andrew Eisenberg

2012-01-20 18:46:01 EST

Sorry for the confusion.  I'm just trying to get a handle on committing things via git.  I'll try to get this all done in one commit.

Comment 10 Andrew Eisenberg

2012-01-21 01:26:55 EST

Here we go.  This one is better.
https://github.com/aeisenberg/orion.client/commit/2acd664edb624537ba2885c009dbaea922faf16d

As always, I wrote all this code and have the rights to contribute it to Eclipse under the
eclipse.org web site terms of use.

While I was trying to squash all the changes into one commit, I wound up deleting the repository and simply losing the changes.  :(  Thankfully, not hard to recreate everything.  The commit here fixes this bug, Bug 369010 (for dealing with duplicate favorites).  Also, there is a change for ensuring that case insensitive sorting of favorites occur (the current solution does a case-sensitive sort and looks quite strange).

Hope this works for you.

Comment 11 Susan McCourt

2012-01-21 16:53:13 EST

(In reply to comment #9)
> Sorry for the confusion.  I'm just trying to get a handle on committing things
> via git.  I'll try to get this all done in one commit.

No worries, we're just learning about the best way to get contributions (and will probably modify Orion to make this simpler).  For now, it's helpful to mention not just the commit but what branch in your repo it is in.  That way I can find it via a git log in Orion and cherry pick it.  (A better way is for us to be able to cherry pick from the git commit page, I'll open a bug for that.)

Pushed the fix.

There was typo in one of the messages, so while I was there I changed the message to specify what item is the duplicate.  That can be helpful if the user fumble fingers the item and wonders what happened/if they selected the right one.

I also noticed that it was still possible to add duplicate folders so I fixed it since I was there anyway.  The problem is that the favorites service stores item.ChildrenLocation if there is one (for folders) and item.Location for files.  So the code checking for dups has to provide the ChildrenLocation if there is one.  It's a bit of knowledge leak that the client code knew this, but I'm not really up for reorganizing the favorites code right now.  (If you were interested in doing so see also bug 334201.  Ultimately I'd like to see all kinds of URLs stored as favorites, with user groupings, etc.  The way it's organized now (with searches as a separate thing) is quite arbitrary.

Thanks again for the fix!