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

VERIFIED FIXED

Status

Firefox OS
Gaia::Calendar
P1
normal
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: sync-1, Assigned: Ganesh Ghosh)

Tracking

unspecified

Firefox Tracking Flags

(blocking-b2g:koi+)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

4 years ago
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:

Updated

4 years ago
blocking-b2g: --- → leo?
(Reporter)

Comment 1

4 years ago
Clone from brother
(Reporter)

Comment 2

4 years ago
Created attachment 781410 [details]
PR pic

Comment 3

4 years ago
please update status,thanks.
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.
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.
Flags: needinfo?(ffos-product)

Comment 6

4 years ago
Dear lsblakk:
   is there any update here?
its annoying and needs addressing. Should be done for v1.2.
blocking-b2g: leo? → koi?
Flags: needinfo?(ffos-product)
(Assignee)

Updated

4 years ago
Assignee: nobody → gghosh
(Assignee)

Comment 8

4 years ago
Created attachment 786655 [details] [diff] [review]
Reminder can be changed even after defaults are set to none
Attachment #786655 - Flags: review?(kgrandon)

Comment 9

4 years ago
Is the patch in comment8 ok?
if ok, please merge the patch to v1.1, thanks.
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?
Attachment #786655 - Flags: review?(kgrandon)
(Assignee)

Comment 11

4 years ago
Created attachment 787385 [details] [diff] [review]
Reminder can be changed even after defaults are set to none
Attachment #786655 - Attachment is obsolete: true
Attachment #787385 - Flags: review?(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.
Attachment #787385 - Flags: review?(kgrandon)
(Assignee)

Comment 13

4 years ago
Created attachment 790585 [details] [diff] [review]
Reminder can be changed even after alarm defaults are set to none with tests added
Attachment #787385 - Attachment is obsolete: true
Attachment #790585 - Flags: review?(kgrandon)
triage: will be fixed in 1.2
blocking-b2g: koi? → koi+
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?
Attachment #790585 - Flags: review?(kgrandon)
(Assignee)

Comment 16

4 years ago
Created attachment 793378 [details] [diff] [review]
Reminder can be changed even after defaults are set to none
Attachment #790585 - Attachment is obsolete: true
Attachment #793378 - Flags: review?(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!
Flags: needinfo?(gghosh)
Attachment #793378 - Flags: review?(kgrandon) → review+
Landed in master: https://github.com/mozilla-b2g/gaia/commit/d6b43217d4be63a9b0978886381b541fac7bfb02

Thanks for the patch!
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: needinfo?(gghosh)
Resolution: --- → FIXED
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.