Adapt to the calIChangeLog interface changes (gdata offline support)

RESOLVED FIXED in 1.0

Status

Calendar
Provider: GData
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Fallen, Assigned: Fallen)

Tracking

unspecified
Bug Flags:
blocking-calendar1.0 +

Details

(Whiteboard: [needed beta][no l10n impact])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

7 years ago
Created attachment 569773 [details] [diff] [review]
Fix - v1

Since offline support has landed, both gdata and wcap need some updating. This bug covers gdata support.

Mohit, I would be delighted if you could take a look at this patch to see if the code is doing the correct thing and also give the patch a test in both uncached and cached mode.
Attachment #569773 - Flags: review?(mohit.kanwal)
Flags: blocking-calendar1.0+
(Assignee)

Comment 1

7 years ago
Created attachment 571482 [details] [diff] [review]
Fix - v2

This works much better. I've tested these cases:

* add/modify/delete while:
  - online with network
  - offline
  - online without network
* playback offline items
  - add/modify/delete working
  - all failing (easy to test with a bogus proxy server set)
  - Higher amount of items changed

* retrieve items:
  - offline / online
  - cached / uncached

* conflict testing (both while online and when coming back online):
  - modified on server, deleted locally
  - modified on server, modified locally
  - deleted on server, modified locally
  - deleted on server, deleted locally (no error!)
Attachment #569773 - Attachment is obsolete: true
Attachment #569773 - Flags: review?(mohit.kanwal)
Attachment #571482 - Flags: review+
(Assignee)

Comment 2

7 years ago
Pushed to comm-central changeset 4a14fc769a0d
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Trunk
(Assignee)

Comment 3

7 years ago
Backported to releases/comm-aurora changeset f2486981acc2
Target Milestone: Trunk → 1.0b9
(Assignee)

Comment 4

7 years ago
Backported to releases/comm-beta changeset 053fae2d6e0f
Target Milestone: 1.0b9 → 1.0b8
(Assignee)

Comment 5

7 years ago
Comment on attachment 571482 [details] [diff] [review]
Fix - v2

Mohit, despite this has landed, I'd appreciate if you could look at the code and give it a whirl.
Attachment #571482 - Flags: review+ → review?(mohit.kanwal)
Comment on attachment 571482 [details] [diff] [review]
Fix - v2

Review of attachment 571482 [details] [diff] [review]:
-----------------------------------------------------------------

The patch looks fine. How ever can you tell me about doing !!this.mOfflineStorage in the calls to XXXItemOrUseCache?
I shall test it out, especially the conflict cases. 
Cheers,
Mohit

::: calendar/base/src/calCachedCalendar.js
@@ +496,1 @@
>                              }

Nice! changed the this_.supportsChangeLog to try-catch!

::: calendar/providers/gdata/components/calGoogleCalendar.js
@@ +337,1 @@
>  

Is there a typo in the above line?  !!this.mOfflineStorage
Attachment #571482 - Flags: review?(mohit.kanwal) → review+
Just tested. And it works well. 

+1
(Assignee)

Comment 8

7 years ago
Hey Mohit, thanks for the review. Could you elaborate more on what you meant in the review? The splinter review is a bit strange sometimes, you need to click directly in the line where the issue is :-)

About !!this.mOfflineStorage, thats just a way to "cast" a variable to boolean. See the following example:


var foo = { some: "object" };
var firststep = !foo;        // firststep is now "false"
var secondstep = !firststep; // secondstep is now "true"
var in_one_step = !!foo;     // The same thing but in one step

The idea in XXXItem was to use the cache if we have an offline storage set, so that on an uncached call useCache is false.
Ok cool, that explains it then.
Blocks: 362645
You need to log in before you can comment on or make changes to this bug.