alarm goes off for completed tasks

VERIFIED FIXED in 0.9

Status

Calendar
Alarms
VERIFIED FIXED
14 years ago
10 years ago

People

(Reporter: Matthew Shanley, Assigned: Geoff Lawrence)

Tracking

unspecified

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

14 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040514
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040514

Alarms go off for a task which has been completed. This occurs both for alarms
set from the start time or end time of the task. It also occurs regardless of
whether the task was marked as completed before the start time or after. 

Reproducible: Always
Steps to Reproduce:
1. Add a task with an alarm set
2. Check completed checkbox for that task before the time of the alarm

Actual Results:  
At the start time for the task, the alarm still goes off.

Expected Results:  
Since the task is completed you shouldn't be notified of it.
Confirming for TB 0.8+ (20041024) and extension 2004100812
Status: UNCONFIRMED → NEW
Ever confirmed: true
Target Milestone: --- → Sunbird 0.3

Comment 2

13 years ago
*** Bug 284042 has been marked as a duplicate of this bug. ***

Comment 3

13 years ago
*** Bug 298911 has been marked as a duplicate of this bug. ***

Updated

12 years ago
QA Contact: gurganbl → general
Reassigning all automatically assigned bugs from Mostafa to nobody@m.o

Bugspam filter: TorontoMostafaMove
Assignee: mostafah → nobody
Not going to make the 0.3 train.
Target Milestone: Sunbird 0.3 → ---
Component: General → Alarms
OS: Windows 2000 → All
QA Contact: general → alarms
Hardware: PC → All
(Assignee)

Comment 6

10 years ago
I am running Thunderbird 2.0.0.9 and Lightning 0.7 (build 2007102304) and this bug is still an issue for me.

Hopefully someone can fix it, please?

Comment 7

10 years ago
I'm starting to use Lightning for some some weeks, and I'm facing this problem too. It would be appreciated that checking the completed checkbox would remove this task from the warning list (if any).

Thank you in advance if anybody would handle that.
(Assignee)

Comment 8

10 years ago
I have tried looking at the source code but as someone new to Lightning code I am a little lost. It looks to me like when the task is marked completed and saved there needs to be code to remove the corresponding alarm. If someone can confirm this I can try and see if I can figure out the code changes. I guess also a test case is probably needed. Again if someone can point me in the right direction maybe I could write that too!
Flags: wanted-calendar0.9+
Geoff, thanks for looking into this.

You should take a look at calAlarmService.js, the onModifyItem function, or rather the onAddItem function.

Here you can check if the item is a task and filter occurrences (or check on the single item) that are not completed.

Hope this helps, I look forward to your patch :-) Feel free to contact me in #calendar on irc.mozilla.org (nick is Fallen), or per email if you require further help.

For help on setting up a build environment, see http://wiki.mozilla.org/Calendar:Build
Assignee: nobody → geoff.j.lawrence
Only release drivers are allowed to set '+' on the wantet-calendar0.9 flag.
Flags: wanted-calendar0.9+ → wanted-calendar0.9?
(Assignee)

Comment 11

10 years ago
Created attachment 320055 [details] [diff] [review]
Patch to fix 248342

Sorry but I am not sure what flags to set
(Assignee)

Comment 12

10 years ago
This fix seems to have been much simpler than I expected but it seems to work! Hopefully Philipp or someone can review my patch and check it in, it would be great to see this in 0.9. Please also accept my apologies if I am not using Bugzilla correctly.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Comment on attachment 320055 [details] [diff] [review]
Patch to fix 248342

I recommend Philipp. ;)
Attachment #320055 - Flags: review?(philipp)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED
Comment on attachment 320055 [details] [diff] [review]
Patch to fix 248342

>Index: base/src/calAlarmService.js
>===================================================================
>RCS file: /cvsroot/mozilla/calendar/base/src/calAlarmService.js,v
>retrieving revision 1.8.2.32
>diff -u -U 8 -p -r1.8.2.32 calAlarmService.js
>--- base/src/calAlarmService.js	7 May 2008 08:37:18 -0000	1.8.2.32
>+++ base/src/calAlarmService.js	8 May 2008 21:26:59 -0000
Great! Correct patch headers on your first try ;-)

>         if (!alarmTime) {
>             // If there is no alarm time, don't add the alarm.
>             return;
>         }
>+        if (aItem.isCompleted) {
>+            // Do not add an alarm when the item is 100% completed BUG  #248342
>+            return;
>+        }
I'd prefer putting the isCompleted condition together with the first if() block and extending the comment. This makes the code a bit more compact. Also, you don't have to note the bug#, since the bug is fixed with this patch. We usually put bug# into the source if there is an outstanding but that needs to be fixed.

Since the changes are small, if its ok with you I'll fix this myself before checkin?

r=philipp with if() block fixed.

For next time, here is the process:
* When uploading a patch, request review from someone specific. See http://wiki.mozilla.org/Calendar:Module_Ownership for a list of module specific reviewers.
* If the patch contains UI changes, then request ui-review from christian (not in this bug, obviously)
* When everyone has given r+, upload a new patch with eventual nits fixed, unless the reviewer agrees to do this himself.
* Until you have CVS access, the reviewer usually takes care of checkin. If he forgets, add the keyword "checkin-needed" to the bug.
* When the patch has been checked in, the bug is set to FIXED. The reviewer again takes care until you have CVS access.
* If you can confirm this works in a subsequent nightly after the patch has been checked in, you can set it to VERIFIED.

Thanks for the patch, we really appreciate it and look forward to further patches :-)
Attachment #320055 - Flags: review?(philipp) → review+
(Assignee)

Comment 15

10 years ago
Philipp, I am very happy if you make the minor changes and check-in. Are those steps above in the Wiki somewhere, if not I could add them in? I got the patch headers right by reading the docs!
Geoff, thanks for the tipp. I put the steps a bit more elaborate into the Calendar:Build guide, feel free to modify if I got something wrong or if the wording can be improved.
Created attachment 320157 [details] [diff] [review]
Patch to fix - v2

I found another issue I fixed before checkin. If the passed item is an event, then aItem.isCompleted will fail. I added an extra check.
Attachment #320055 - Attachment is obsolete: true
Attachment #320157 - Flags: review+
Checked in on HEAD and MOZILLA_1_8_BRANCH

-> FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago10 years ago
Flags: wanted-calendar0.9?
Resolution: --- → FIXED
Target Milestone: --- → 0.9
(Assignee)

Comment 19

10 years ago
I have checked this fix in the nightly builds from 2008-05-12 and 2008-05-13 and it is working nicely.
Status: RESOLVED → VERIFIED

Updated

10 years ago
Duplicate of this bug: 436950
You need to log in before you can comment on or make changes to this bug.