1089006 - [Smart Collections] Renaming Smart Collections will appear as default name after device is restarted.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Everything.me, defect)

Description

[John Thomas [:Johnt]]

Description: 
If the user renames a Smart Collection and then restarts the device, the smart collections will appear as their previous name. Example: Changing the "Social" smart collection to "Anything Else". After restarting device the smart collection will appear as Social.

Repro Steps:
1) Update a Flame to 20141024001204
2) Create or rename a Smart Collection.
3) Restart device.
4) Observe the  name of the Smart Collection.

Actual:
Smart Collection defaults to previous name.

Expected:
It is expected that the new name for the Smart Collections will remain even when the device is restarted.

Environmental Variables:
Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141024001204
Gaia: 0f76e0baac733cca56d0140e954c5f446ebc061f
Gecko: 7d78ff7d25b6
Gonk: 05aa7b98d3f891b334031dc710d48d0d6b82ec1d
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Repro frequency: 100%
See attached: Logcat & Screenshot

Comment 1

[John Thomas [:Johnt]]

Attachment: ChangeNameRestartLog.txt

Comment 2

[John Thomas [:Johnt]]

Attachment: AfterRestartLog.txt

Comment 3

[John Thomas [:Johnt]]

Attachment: RenamedtoThis.png

Comment 4

[John Thomas [:Johnt]]

Attachment: DefaultedtoThis.png

Comment 5

[John Thomas [:Johnt]]

This issue does NOT occur on the Flame 2.2 Kitkat (319mb) and the Flame 2.0 Kitkat (319mb)

Result: The 2.2 currently has a blocker bug when attempting to access Smart Collections.
https://bugzilla.mozilla.org/show_bug.cgi?id=1083965

Flame 2.2

Environmental Variables:
Device: Flame Master (319mb)(KitKat Base)(FullFlash)
Build ID: 20141024040202
Gaia: d893a9b971a0f3ee48e5a57dca516837d92cf52b
Gecko: a5ee2769eb27
Version: 36.0a1 (Master)
Firmware Version: v188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

The 2.0 currently does not have the ability to add or rename Smart Collections.

Flame 2.0

Environmental Variables:
Device: Flame 2.0 (319mb)(Kitkat Base)(Full Flash)
Build ID: 20141024000201
Gaia: 86d83f4b4111ca45ebc92ca779348cc966f43cff
Gecko: f8432250efb7
Version: 32.0 (2.0)
Firmware Version: v188
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 6

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

[Blocking Requested - why for this release]:

Nominating this to block 2.1. If the user renames a smart collection it is expected that their new name will be saved after a restart. The user should not lose settings or changes after a simple restart

Comment 7

[bhavana bajaj [:bajaj]]

Blocking as we regressed this and is broken functionality.

Comment 8

[Candice Serran (:cserran)]

Can we get a regression window and a reverse regression window for this please?

Comment 9

[Kevin Grandon :kgrandon]

Adding regressionwindow-wanted for comment 8.

Comment 10

[Joshua Mitchell (Inactive)]

(In reply to Candice Serran (:cserran) from comment #8)
> Can we get a regression window and a reverse regression window for this
> please?

Candice - because of the time it takes to do these windows I don't really think it is appropriate to call for both at the same time until one is deemed not helpful and the other is needed.

Because the reverse window has more utility (in that it can find what fixed the issue and that patch can then hopefully be uplifted to the affected branch) I'm going to have the QA-Contact find that one first. If all works well then the regular window won't even be needed.

Comment 11

[Adam Alldredge [:AdamA][Inactive]]

When I was beginning my regression window I found that this is reproducing in the latest Flame 2.2.

Environmental Variables:
Device: Flame 2.2
Build ID: 20141106045123
Gaia: 068b9711277b06c7d633517f9e1fcb5624bb39b3
Gecko: 0c66a9fd9085
Version: 36.0a1 (2.2)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Comment 12

[Joshua Mitchell (Inactive)]

[Blocking Requested - why for this release]:

It seems there has been some confusion on this bug. 

This issue DOES repro in 2.2 when not blocked (comment 11) ((Reverse-regression window unavailable)
<flipping 2.2 to affected>

This issue did not repro in 2.0 because smart collections were not fully implemented. This is not considered unaffected.
<flipping 2.0 to ----> 
Meaning that this is not a regression but instead that this feature was implemented broken.  (regression-window unavailable)

Because it was initially blocked on based on this being a regression we might want to  re-evaluate based on this new information. (IMO - it should still be blocked on for data loss)

Comment 13

[Kevin Grandon :kgrandon]

Bugs. I like bugs.

Comment 14

[GitHub Autolander Bot]

Attachment: [PullReq] KevinGrandon:bug_1089006_collection_renaming_restart to mozilla-b2g:master

Comment 15

[Kevin Grandon :kgrandon]

Comment on attachment 8519290 [details] [review]
[PullReq] KevinGrandon:bug_1089006_collection_renaming_restart to mozilla-b2g:master

Hey - could either of you guys help me review this patch? It's fairly simple, we were just missing a setter to take a path when getting the name. I looked at options for setting up a real integration test for this, but it's quite difficult as we don't have the default collections setup properly currently. I think it's a test we can add in the future, but for now I just have a simple unit test for sanity. Just need one review here, thanks!

Comment 16

[Chris Lord [:cwiiis]]

Comment on attachment 8519290 [details] [review]
[PullReq] KevinGrandon:bug_1089006_collection_renaming_restart to mozilla-b2g:master

This is an r+, but I think the unit test could be shorter and I see it's +autoland, so just removing my review.

If crdlc disagrees, I have no problem with this landing, otherwise it'd be nice to remove the redundant part of the unit test (after which you can assume my r+).

Comment 17

[Kevin Grandon :kgrandon]

Sure thing - autoland is just on for testing now, and it won't actually autoland until checkin-needed is added to the bug as well :) I'll send an email to dev-gaia to clarify this soon.

Comment 18

[Chris Lord [:cwiiis]]

Comment on attachment 8519290 [details] [review]
[PullReq] KevinGrandon:bug_1089006_collection_renaming_restart to mozilla-b2g:master

ah, in that case, r+ then :)

Comment 19

[Chris Lord [:cwiiis]]

(assuming my comment about the unit test is either wrong or addressed)

Comment 20

[Kevin Grandon :kgrandon]

Comment on attachment 8519290 [details] [review]
[PullReq] KevinGrandon:bug_1089006_collection_renaming_restart to mozilla-b2g:master

Thanks Chris - you're right, we were able to simplify it. I've done so, and am clearing the review on Cristian. Thanks!

Comment 21

[GitHub Autolander Bot]

Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.

Comment 22

[Kevin Grandon :kgrandon]

Comment on attachment 8519290 [details] [review]
[PullReq] KevinGrandon:bug_1089006_collection_renaming_restart to mozilla-b2g:master

Oops, I need to get Autolander fixed to recognize when the review is coming from the component.. Or stamp it myself I suppose.

Comment 23

[Kevin Grandon :kgrandon]

In master: https://github.com/mozilla-b2g/gaia/commit/7d5ca310f3082b6abaff0c0f8a3cd2d9d2a04244

Comment 24

[Kevin Grandon :kgrandon]

Comment on attachment 8519290 [details] [review]
[PullReq] KevinGrandon:bug_1089006_collection_renaming_restart to mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Edit feature implementation.
[User impact] if declined: Edits to default collection names will not stick.
[Testing completed]: Manual, and a small unit test to test sanity. Will work on a more comprehensive integration test, but we currently lack the setup to do so.
[Risk to taking this patch] (and alternatives if risky): Low risk, one-liner.
[String changes made]: None.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

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

Comment 26

[Yeojin Chung [:YeojinC]]

This issue is verified fixed on Flame 2.1 and 2.2.

Result: The edited Smart Collection name remains after restarting the device.

Device: Flame 2.1 (319mb, KK, Shallow Flash)
BuildID: 20141112001201
Gaia: 4c159e75a1568afbbf0c83c1235ec56facfbe87d
Gecko: b9849b3c6aaa
Version: 34.0 (2.1) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.2 (319mb, KK, Shallow Flash)
BuildID: 20141112040208
Gaia: 5ae28ff11b982e2bd7d1aa097cda131536952bdc
Gecko: 688f821edcd4
Version: 36.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0