898242 - [Buri][Calendar]The reminder time cann't be changed after changing calendar setting

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Calendar, defect, P1)

Description

[sync-1]

AU_LINUX_GECKO_ICS_STRAWBERRY.01.01.00.019.164
 Firefox os  v1.1
 Mozilla build ID:20130715070218
 
 
 
 Created an attachment (id=470199)
 PR pic
 
 DEFECT DESCRIPTION:
 The reminder time cann't be changed after changing calendar setting
 
 REPRODUCING PROCEDURES:
 1. Launch Calendar app ->  enter calendar settings page -> set all-day events reminders as 'None', press Done
 2. Create a all-day event, reminder defaults as 'None', save it  -> then view this all-day event, then to edit it    ->  find that reminder cann't be changed -> KO 
 
 EXPECTED BEHAVIOUR:
 Reminder should can be modified normally.
 
 ASSOCIATE SPECIFICATION:
 TEST PLAN REFERENCE:
 TOOLS AND PLATFORMS USED:
 USER IMPACT:
 Mid
 REPRODUCING RATE:
 5/5
 For FT PR, Please list reference mobile's behavior:

Comment 1

[sync-1]

Clone from brother

Comment 2

[sync-1]

Attachment: PR pic

Comment 3

[buri.blff]

please update status,thanks.

Comment 4

[Jason Smith [:jsmith]]

Confirmed on b2g18 on 7/26. Looks like we can't edit alarms during editing of an event that was created with no alarms specified.

Comment 5

[Lukas Blakk [:lsblakk] use ?needinfo]

I don't like this bug but I'm inclined not to block at this late stage in the game.  Since this only affects calendar events that are created with no alarm and the default is to have 1 alarm pre-created on a new event this might not be as terrible in our v1.1 feature release and we can polish this in v1.2, so I'd move this koi? but will let product make the final call here based on user story promises.

Comment 6

[buri.blff]

Dear lsblakk:
   is there any update here?

Comment 7

[Wilfred Mathanaraj [:WDM]]

its annoying and needs addressing. Should be done for v1.2.

Comment 8

[Ganesh Ghosh]

Attachment: Reminder can be changed even after defaults are set to none

Comment 9

[buri.blff]

Is the patch in comment8 ok?
if ok, please merge the patch to v1.1, thanks.

Comment 10

[Kevin Grandon :kgrandon]

Comment on attachment 786655 [details] [diff] [review]
Reminder can be changed even after defaults are set to none

Ganesh - I think you may have fixed another issue than what was in the original description?

I tested the branch and the original issue still repros for me. I think you will need to handle the modify_event screen as well?

Comment 11

[Ganesh Ghosh]

Attachment: Reminder can be changed even after defaults are set to none

Comment 12

[Kevin Grandon :kgrandon]

Comment on attachment 787385 [details] [diff] [review]
Reminder can be changed even after defaults are set to none

Made a few nits on github. Basically we need a test and to ensure that all desired actions work. Please verify the following cases:

Default All Day Alarm set to "None"
- You can create and add alarms to a new event
- You can add alarms to an existing all-day event with no alarms
- You can add alarms to an existing all-day event with 1 or more alarms

Default All Day Alarm set to a value
- You can create and add alarms to a new event
- You can add alarms to an existing all-day event with no alarms
- You can add alarms to an existing all-day event with 1 or more alarms

Normal Event Alarm set to a "None"
- You can create and add alarms to a new event
- You can add alarms to an existing event with no alarms
- You can add alarms to an existing event with 1 or more alarms

Normal Event Alarm set to a value
- You can create and add alarms to a new event
- You can add alarms to an existing event with no alarms
- You can add alarms to an existing event with 1 or more alarms

Integration tests are probably the best bet, but if we can restructure the code in a way to cover all of these with unit tests that would be nice as well.

Comment 13

[Ganesh Ghosh]

Attachment: Reminder can be changed even after alarm defaults are set to none with tests added

Comment 14

[Dylan Oliver [:doliver]]

triage: will be fixed in 1.2

Comment 15

[Kevin Grandon :kgrandon]

Comment on attachment 790585 [details] [diff] [review]
Reminder can be changed even after alarm defaults are set to none with tests added

Ganesh - hate to be the bearer of bad news, but this still doesn't cover all of the use cases above. I tried editing an all-day event which had alarms, and it didn't allow me to add any more alarms.

Have you verified that all cases above work as expected?

Comment 16

[Ganesh Ghosh]

Attachment: Reminder can be changed even after defaults are set to none

Comment 17

[Kevin Grandon :kgrandon]

We're looking pretty good here Ganesh! I added a few nits on github, if you could address those and rename the commit to include the bug number, we should be good. Thanks!

Comment 18

[Kevin Grandon :kgrandon]

Landed in master: https://github.com/mozilla-b2g/gaia/commit/d6b43217d4be63a9b0978886381b541fac7bfb02

Thanks for the patch!

Comment 19

[Peter Bylenga [:PBylenga]]

Verified Fixed:  User can change reminders for events regardless of default or initial setting.  Also tested cases mentioned in Comment 12.

Environmental Variables
Device: Buri v1.2 COM RIL
Build ID: 20131103004003
Gecko: http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/eec4da1b27eb
Gaia: cb981e2f47bc644b4d178d54378c3676c946facf
Platform Version: 26.0
RIL Version: 01.01.00.019.276 
Firmware Version: US_20131015