Closed Bug 322386 Opened 14 years ago Closed 13 years ago

Views need to send all event creation/modification/deletion through the calICalendarViewController

Categories

(Calendar :: Internal Components, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jminta, Assigned: jminta)

Details

Attachments

(1 file, 2 obsolete files)

Right now, the views do some things (like change event times) through the controller, but other things they do on their own (like change event titles).  We need to change this so that all of these operations go through the calICalendarViewController (and the interface needs to be modified to support this) in order to support things like undo/redo.
Attached patch send title changes too (obsolete) — Splinter Review
Patch modifies the view controllers to also accept title changes, so that they can prompt for parent/occurrence and add the items to the undo/redo queue.  It also brings in some changes from occurrence editing that never made it into Sunbird.  (We really need to sync these controllers at some point.)  Patch is a prerequisite for bug 325477.
Assignee: base → jminta
Status: NEW → ASSIGNED
Attachment #216801 - Flags: first-review?(dmose)
Blocks: 325477
this doesn't seem to block 325477 anymore, because that bug is already fixed.
No longer blocks: 325477
Comment on attachment 216801 [details] [diff] [review]
send title changes too

patch is horribly bitrotted :(
Attachment #216801 - Attachment is obsolete: true
Attachment #216801 - Flags: first-review?(dmose)
Attached patch unbitrotted (obsolete) — Splinter Review
Same patch, but updated to 4 months of new code :)
Attachment #231695 - Flags: first-review?(dmose)
Comment on attachment 231695 [details] [diff] [review]
unbitrotted

Moving reviews to ctalbert and lilmatt per dmose
Attachment #231695 - Flags: second-review?(lilmatt)
Attachment #231695 - Flags: first-review?(dmose)
Attachment #231695 - Flags: first-review?(cmtalbert)
Unfortunately, this patch is bitrotted again. I had some difficulty in applying the changes to calendar-views.js, but I think I applied them in good faith. Please take a look at http://lxr.mozilla.org/seamonkey/source/calendar/base/content/calendar-views.js#93 for a look at the current code structure.
In particular, I changed
+            if (aNewTitle) {
+                instance.title = aNewTitle;
+                if (!aNewStartTime) {
+                    doTransaction('modify', instance, instance.calendar, itemToEdit, null);
+                    return;

to
+            if (aNewTitle) {
+                instance.title = aNewTitle;
+                if (!aNewStartTime) {
+                    doTransaction('modify', instance, instance.calendar, aOccurence, null);
+                    return;
because there is no longer an itemToEdit variable in this function, and that is what the other "doTransaction" statement was using later on in the function. 

Speaking of that other "doTransaction" statement in the "modifyOccurrence" method (line 113 in above lxr link).
It appears that the second hunk of the calendar-views.js section of this patch intends to remove that line. However, when I removed that line I could no longer edit the time of an event without using the event dialog. I could drag the events to different days, or lengthen them, but those changes would not be saved, and the event would revert to is original time. Putting this statement back into the code fixed the problem.

Joey (or lilmatt or mvl), could you take a look at this patch and the current code structure, and be certain that these changes are still correct?
Exactly the same patch. Joey made a refactored improvement to address further bitrot in calendar-views.js.
Attachment #231695 - Attachment is obsolete: true
Attachment #244128 - Flags: second-review?(lilmatt)
Attachment #244128 - Flags: first-review?(cmtalbert)
Attachment #231695 - Flags: second-review?(lilmatt)
Attachment #231695 - Flags: first-review?(cmtalbert)
Comment on attachment 244128 [details] [diff] [review]
Updating the patch with Joey's refactored calendar-views.js piece

Verified that we can edit items in Sunbird on the week view by simply clicking and dragging and doing in-line edit of the event title.
Attachment #244128 - Flags: first-review?(cmtalbert) → first-review+
Comment on attachment 244128 [details] [diff] [review]
Updating the patch with Joey's refactored calendar-views.js piece

r=lilmatt
Attachment #244128 - Flags: second-review?(lilmatt) → second-review+
patch checked in.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.