User should be able to change the name of a collection

I think this should be lower priority than the other features. We may be able to live without it in 2.0 given users tend to use the preconfigured collections.

I don't really see how this depends on bug 1016227, so removing it for now. Please re-add it if it's really necessary. Just trying to create a clean dependency tree. Thanks.

Thanks guys

Comment on attachment 8477368 [details] Github pull request I left a note on GH about using the gaia-header WC. I also think it would be good to wait to have Amir take a look at this. I think writing an integration test would be valuable if we can. I don't mind taking this up after this lands though, so I will file a but for it but we can land without it. Thanks!

I think we are missing some information about this feature. Renaming a collection should have the same result as creating a new custom collection. This means it must be a QueryCollection and the E.me search parameter should be {query: collection.query} where collection.query === collection.name and 'name' is the new value from the rename action. We also need to issue new web apps and background requests to update collection.webicons and collection.background. As an example, compare the collection created after applying the patch and renaming 'Music' to 'Musicals' with a custom 'Musicals' collection. The web results are different because the renamed collection is still using categoryId=142 (Music) as the search options. So, when renaming a QueryCollection we need to update: - collection.query - collection.name - collection.webicons - collection.background - collection.icon When renaming a CategoryCollection we also need to: - delete collection.categoryId - delete collection.cName A patch is worth a thousand words :) maybe this will do a better job than my explanation above: https://github.com/EverythingMe/gaia/commit/f6a804ef1ed232a2a16f1d7866fb0b42d3aca9a7

Hi all, Regarding to comment 5, I would like to have the opinion of UX and product as well. I can do that Amir explained perfectly but I prefer a double check with them before doing more stuffs. We have to scenarios with this example: Chris with his new FFOS changes the "Social" collection to "My friends" 1) Rename a collection and keep all the information doing the query with the original name Social will appear as "My friends" but the query to show web icons will be done with "Social" (basically just a change of name) or.... 2) Rename a collection and we will do the query with the new name to obtain different results and the new background related to new search Social will appear as "My friends" and the results show web icons related to "My friends" with a new background Summarizing, is the 'rename' action just a change of name or a semantic change? Thanks a lot guys

The behavior described in Comment 5 is what we implemented in v1.x (bug 911550): https://github.com/mozilla-b2g/gaia/pull/12553/files#diff-e1bb11dc3cd2b3a2d5a57c50eeed39c2R132

Amir, the pull request implements both solutions: 1) Just a change of name -> first commit https://github.com/crdlc/gaia/commit/508876c6e6f5d47c81eeda4e2c0801f73ac7e24d 2) Semantic change like previous versions -> squash the two commits https://github.com/crdlc/gaia/commit/dbf4f81ebc30b591712cfc77cdf8defe3c2043ac

(In reply to Cristian Rodriguez (:crdlc) from comment #6) > Social will appear as "My friends" and the results show web icons related to > "My friends" with a new background > > Summarizing, is the 'rename' action just a change of name or a semantic > change? +Jacqueline for comment.

Also adding Maria

I'll bring this up with the UX team to discuss. I'm worried we don't explain to the user well enough how the smart collections work and what the relationship between the collection name and contents (whichever option we choose to go with) is. Without explaining the behavior I think we'll see lots of confusion from users. We also need to consider how this will relate to naming of groups. I'll respond back when I've chatted with the team.

Comment on attachment 8477368 [details] Github pull request Clearing r? until we have an answer from UX. Please ask again when relevant.

Let's have a rename just be a change of name that has no effect on the content in the collection. From our studies we know that users interpret the smart collections as full of preloaded apps. It's also unlikely that users are aware of the connection between the collection name and the query. Having the content change on a name change would be unexpected to the user.

Maria, should this apply to custom collections as well? If a user created a custom 'dogs' collections and later renames it to 'cats' - wouldn't it be confusing to get 'dogs' results in a collection named 'cats'? -- Cristian, it would be great if we can still use the second patch for collection.searchOptions and a modified version of collection.rename. Added a comment on Github on how to tell that a collection has been renamed: https://github.com/crdlc/gaia/commit/508876c6e6f5d47c81eeda4e2c0801f73ac7e24d#commitcomment-7602776

I prefer to wait for Maria's answer to your question because according to comment 13 the second commit will disappear (In reply to Amir Nissim (:amirn) from comment #14) > Maria, should this apply to custom collections as well? > If a user created a custom 'dogs' collections and later renames it to 'cats' > - wouldn't it be confusing to get 'dogs' results in a collection named > 'cats'? > > -- > > Cristian, it would be great if we can still use the second patch for > collection.searchOptions and a modified version of collection.rename. > > Added a comment on Github on how to tell that a collection has been renamed: > https://github.com/crdlc/gaia/commit/ > 508876c6e6f5d47c81eeda4e2c0801f73ac7e24d#commitcomment-7602776

Yes, the behavior should be consistent for preloaded and custom collections. Amir, I agree that in your example it may seem counter intuitive to not have the contents reflect the name of the collection. Most users won't be aware of the potential of a connection however, to them it will be no stranger than renaming a folder on their computer. If they do indeed want a folder with results for 'cats', they can just create a new one with that name.

The same idea than bookmarks

Comment on attachment 8477368 [details] Github pull request Addressed Maria's suggestion

Comment on attachment 8484084 [details] Edit collection screen screen shot looks good to me, thanks!

Comment on attachment 8477368 [details] Github pull request I think the code seems fine to me, but would like Amir to leave the final R+ if that's ok with you since he's already commented quite a bit on this. I guess this also depends on getting the bookmarks patch relanded?

Removing flag as Maria has answered for UX

Yes, you're right, it depends on getting the bookmarks patch relanded. I am working on that right now (In reply to Kevin Grandon :kgrandon from comment #20) > Comment on attachment 8477368 [details] > Github pull request > > I think the code seems fine to me, but would like Amir to leave the final R+ > if that's ok with you since he's already commented quite a bit on this. > > I guess this also depends on getting the bookmarks patch relanded?

code ready for review and happy tree

Comment on attachment 8477368 [details] Github pull request Looks great. Love the icon in the rename page :) One thing I noticed is the user can't tap 'done' if the name was not changed. Not sure if it's a bug, but it did confuse me at first.

Yes, you are right, the button is disabled while there is no changes by design like editing a contact for instance. Thanks for your time Amir!! (In reply to Amir Nissim (:amirn) from comment #24) > Comment on attachment 8477368 [details] > Github pull request > > Looks great. Love the icon in the rename page :) > > One thing I noticed is the user can't tap 'done' if the name was not > changed. Not sure if it's a bug, but it did confuse me at first.

Merged in master: https://github.com/mozilla-b2g/gaia/commit/91edecd83e33b4704e5774e15e5b8b45c0f28d48

Hi Peter, do you think that this feature should be in 2.1? If so I will ask for approval, thanks a lot

I don't think we could uplift this due to the fact that there's new strings and we're past string freeze =/ Clearing Peter's flag for now, but feel free to re-flag if needed.

ok, I thought that we can until 09/14. I was wrong obviously XD

(In reply to Cristian Rodriguez (:crdlc) from comment #29) > ok, I thought that we can until 09/14. I was wrong obviously XD Actually, I'm sorry - I think we can wait, and I had the wrong dates. It's my fault XD. We can request uplift and see.

Comment on attachment 8477368 [details] Github pull request [Approval Request Comment] [Bug caused by] (feature/regressing bug #): This is a new feature. [User impact] if declined: Same as 2.1, users will just not be able to edit collections. This is a feature we previously had in 1.4. [Testing completed]: Manual and unit testing. [Risk to taking this patch] (and alternatives if risky): It's fairly well silo'd so should be low risk. [String changes made]: Yes, new strings to support the feature.

I suppose we should flag this as late-l10n because we are requesting uplift, but let's remove it if it's not needed or we don't receive uplift approval.

v2.1: https://github.com/mozilla-b2g/gaia/commit/daf91971902cd8e70e6cc5616159260cfcfce7c5

Fabrice - Can we chat about the approval request here? While I get the code risk was called out as low risk here, I'm concerned that we're adding non-critical strings to the release in this patch. At this point of stabilization, while we do allow for strings in landings, they really need to be critical strings for a particular release (e.g. string change needed for a required 2.1 feature). This feature however isn't critical to 2.1 (it's a nice to have at best), so adding this feature into this release seems like it's more risk than it's value due to the localization concerns.

I hear you, and I maybe should have rejected this one. But we're not yet in a hard lock down this week.