867797 - Syncing when viewing an event can sometimes cause event to disappear when it has been edited on server

Status: RESOLVED FIXED

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

Description

[James Lal [:lightsofapollo]]

Event will re-appear after restarting calendar.

Comment 1

[Alex Keybl [:akeybl]]

(In reply to James Lal [:lightsofapollo] from comment #0)
> Event will re-appear after restarting calendar.

Does syncing again bring it back? Is this a regression?

Comment 2

[James Lal [:lightsofapollo]]

its not really a sync error- the event _does_ exist in the calendar but there is some bug that prevents it from displaying. Its probably a regression between v1-train / v1.0.1

Comment 3

[Jason Smith [:jsmith]]

Actually, didn't read comment 0 when I requested qawanted in triage. I know what's going on here.

Sounds like this is temporary lack of display of the event if:

1. The imported calendar is being synced
2. The event being viewed during sync has been edited on the server
3. The event is no longer displayed until the calendar is restarted

This is a combination of an edge case and only temporary display issue while the calendar is active. I'm leaning towards not blocking on this as a result.

Comment 5

[Leo]

I believe, we need to block on this issue.

If the user wants to view his events after syncing, he will have to restart the application, which i think is not acceptable.

Practically the user is getting an impression, that he doesn't have any upcoming events, until he restart the application.

There is no guarantee that the user restarts the application everytime after syncing the calendar.

Comment 6

[Wayne Chang [:wchang]]

Triage - leo+ as regression per comment 2

Comment 7

[James Lal [:lightsofapollo]]

This does happen but not consistently (at least not on my account) some sort of race condition... I will continue testing but additional help narrowing it down would be useful.

Comment 8

[Jason Smith [:jsmith]]

(In reply to James Lal [:lightsofapollo] from comment #7)
> This does happen but not consistently (at least not on my account) some sort
> of race condition... I will continue testing but additional help narrowing
> it down would be useful.

I think the STR I did here to reproduce this easily was something like:

1. Import a google calendar
2. Select a day with google calendar events
3. Edit one of the events for the day selected on google calendar
4. Sync

Result - the day's calendar events disappear.

If that doesn't work, let me know.

Comment 9

[James Lal [:lightsofapollo]]

That was my own STR but it is not consistently failing

Comment 10

[Jason Smith [:jsmith]]

Okay, I'll take another look then.

Comment 11

[Jason Smith [:jsmith]]

Yeah, the STR in comment 8 reproduces the bug for me. What I noticed was:

1. The event remained visible on the month view, but if I selected it, I would get the infamous broken calendar UI (many UI pieces in wrong positions)
2. The event was not present in the week or day views

Comment 12

[sarsenyev]

As per Jason's comment 8, this bug reproduces on Inari.
Gmail, Yahoo, CaDav accounts are used for the test.
Events are disappeared in the calendar application, after syncing the accounts and then editing the events on the server account.
The events are not visible neither in "Month", "Week" or "Day" views

Environmental  Variables:
Inari Build ID: 20130515070203
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/0b6bcb1f4175
Gaia: 9648799c2e45917ff150fa9eef8aeac79a9ac008

Comment 13

[Jason Smith [:jsmith]]

Gareth - Dylan mentions you are running into issues reproducing this. If you would like, we could meet up and debug this if need be.

Comment 14

[Gareth Aye [:gaye] (back from PTO)]

That would be great! I'm planning on being in the SF office on Thursday and Friday... will you be around? If that doesn't work, a video where you walk through reproducing the issue would be hugely helpful. Let me know.

Comment 15

[Jason Smith [:jsmith]]

(In reply to gaye from comment #14)
> That would be great! I'm planning on being in the SF office on Thursday and
> Friday... will you be around? If that doesn't work, a video where you walk
> through reproducing the issue would be hugely helpful. Let me know.

I'm actually in NY this week, but I could still do a vidyo call. I'm on PTO on Thursday though. Name the day and time this week (Wednesday or Friday) and I'll be there.

Comment 16

[Leo]

(In reply to gaye from comment #14)
> a video where you walk
> through reproducing the issue would be hugely helpful. Let me know.

We have a video for STR in the bug 870349. Could you check if that is helpful else let me know if more information required to reproduce.

Comment 17

[Gareth Aye [:gaye] (back from PTO)]

Leo - James mentioned he had an idea for what he wanted to do here, so I'm passing it off now that he's back from PTO.

Comment 18

[James Lal [:lightsofapollo]]

I found what appears to be a race in caldav_pull_events.js in the method handleEventSync. Seems like we attempt to delete the old busytimes but we are not waiting for that operation to complete... This often means we hit conditions where we actually end up removing the _new_ busytime/event information from the cache rather then the old... this is fixed during restart.

Patch soon...

Comment 19

[James Lal [:lightsofapollo]]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10223

Pointer to Github pull-request

Comment 20

[James Lal [:lightsofapollo]]

Comment on attachment 758860 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10223

This is working well for me but I wanted your feedback / concerns before I clean up the patch...

the one change I plan on making is prefixing the busytime ids' with the event id which should dramatically reduce the likelihood UUID collisions.

Comment 21

[Leo]

The function "busytimeIdFor" removed from file "store/event.js" is used for modifying the events. This breaks the edit event feature.

Comment 22

[Leo]

(In reply to Leo from comment #21)
> The function "busytimeIdFor" removed from file "store/event.js" is used for
> modifying the events. This breaks the edit event feature.

Please ignore my comments i tested the patch with v1-train

Comment 23

[James Lal [:lightsofapollo]]

Clearing my needinfo- I think #22 answers #21?

Comment 24

[James Lal [:lightsofapollo]]

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

Comment 25

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1-train.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1-train, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1-train
  git cherry-pick -x -m1 cdeb777ff57c85fa236da621c179a1fd751cff20
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 26

[James Lal [:lightsofapollo]]

in v1-train: 43c9be6643ca5a649b04baeb23e53bc71cfe005b

Comment 27

[John Ford [:jhford] CET/CEST Berlin Time]

1.1hd: 43c9be6643ca5a649b04baeb23e53bc71cfe005b

Comment 28

[Jason Smith [:jsmith]]

lgtm on b2g18 on 6/18 - tried testing syncing while viewing events both not modified on server vs. modified on server.

Comment 29

[James Lal [:lightsofapollo]]

backed out of v1-train here: 258db40f9850558638c0fb3addbe7a203cbb14c4 there is a missing dependency that causes regressions on v1-train but not master.

Comment 30

[James Lal [:lightsofapollo]]

Adding missing dep this is needed for v1-train (otherwise we see the regressions)

Comment 31

[Jason Smith [:jsmith]]

Reopening for backout in comment 29

Comment 32

[James Lal [:lightsofapollo]]

landed back in v1-train here 30cf7138224dcfe8d731bef319227113606733f3. I tested out the path with the previous regressions and it looks good now that the dep has landed.

Comment 33

[Jason Smith [:jsmith]]

lgtm on b2g18 with regressions addressed