1061614 - Call log database upgrade path to schema 4 wrongly puts calls one week apart in the same group

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Dialer, defect)

Description

[Anthony Ricaud (:rik)]

While working on bug 877971, we noticed that our reference workloads sometimes have incoherent data. The day used as part of the id for dialerGroups was not the same day as the lastEntryDate. Some records had a one week difference.

Comment 1

[Anthony Ricaud (:rik)]

Jon: You've worked on those, how where they generated? We may want to regenerate them because of those problems and maybe also to include some new data like bug 1047351.

Comment 2

[Jon Hylands [:jhylands]]

Anthony,

Right now regenerating reference workloads is problematic - I used to use a specialized version of b2g-desktop (RIL-enabled), but that option is no longer available.

We need in general to look at re-doing the generation of reference workloads so they can be generated on demand. Note however that once we generate with a specific version, the databases are forwards compatible with that and future version, but they are definitely not backwards compatible with older versions.

Comment 3

[Jon Hylands [:jhylands]]

And if you're getting incoherent data, you might want to double-check the migration code for the dialer database - that may be causing the problem.

Comment 4

[Gabriele Svelto [:gsvelto] (PTO)]

I stumbled upon this while investigating bug 1029142 and as it turns out our upgrade code is seriously broken. The issue is here:

https://github.com/mozilla-b2g/gaia/blob/8f01920c210841852b543a3775c750437dd4ec44/apps/communications/dialer/js/call_log_db.js#L388

We're creating a unique key which uses getDay() to extract the day from a Date object but getDay() returns the day-of-the-week, not the day-of-the-month (that's getDate(), so intuitive...), so calls one week apart are merged into the same group.

Changing the title to something more descriptive and setting the flags. This has been broken since v1.1, woot!

Comment 5

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH] Properly merge calls that happened in the same day into groups during database upgrades

One-liner fix. Unfortunately this is not covered by unit-tests at all and due to all the IndexedDB interaction writing one for this case would be a significant undertaking. I won't provide ones mostly because this code is unlikely to change in the future and thus we're also unlikely to introduce regressions.

Comment 7

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PULL REQUEST] Properly merge calls that happened in the same day into groups during database upgrades

Thanks for the review! Here's the PR + try run, waiting for Try to turn green before merging.

Comment 8

[Gabriele Svelto [:gsvelto] (PTO)]

Oh, integration tests, why dost thou always take three tries before finishing successfully? Anyway, green Try run:

https://tbpl.mozilla.org/?rev=59f3d6343148f392af96400b883e871dae69d589&tree=Gaia-Try

Merged to gaia/master 6d87102974b6b6f2d8baa0b7763534897258ff44

https://github.com/mozilla-b2g/gaia/commit/6d87102974b6b6f2d8baa0b7763534897258ff44

Comment 9

[Josh Cheng [:josh]]

Triage:
Low impact on Woodduck.