Fix ISO-8601 timezone parsing issue

[moumny]

Related to #3386
Druid driver outputs some datetime strings with the format +-hhmm for timezone offset.
In the time.js lib it expacts only +-hh:mm or Z.

Here we can also change Druid driver to output +-hh:mm format but i guess since +hhmm is also stantard its best to make time.js more robust.

Ps: we can do both as well :)

What do you think @salsakran @camsaul ?

[moumny]

I found a second problem in the tooltip of the charts.
While the data in the table is displayed in local time the tooltip is in the chart shows utc. This was because the Date was converted to moment obejct using .utc function.

Am I the only one seeing such issues ?

[salsakran]

@moumny thanks for submitting this so soon after we talked about.

Could you write a test that shows a specific failing combination of query and underlying data?

[moumny]

The fix for the tooltip breaks "computeTimeseriesDataInverval" which expects utc objects. I'll leave that for another PR.

Changed Druid driver to output timezone offsets as +HH:mm for consistency purposes.

I'm trying to write some small unit tests for the time lib and im having weird results. ISO timestamps with +09:00 are parsed correctly but timestamps UTC offset (Z, or +00:00) are parsed in local time. The same code works fine on a fiddlejs. Any idea ?

`

import { parseTimestamp } from 'metabase/lib/time';
import moment from 'moment';

describe('time', () => {
describe('parseTimestamp', () => {
const NY15_TOKYO = moment(1420038000000); // 2014-12-31 15:00 UTC
const NY15_UTC = moment(1420070400000); // 2015-01-01 00:00 UTC

    const TEST_CASES = [
        ['2015-01-01T00:00:00.000Z',        NY15_UTC],
        ['2015-01-01T00:00:00.000+00:00',   NY15_UTC],
        ['2015-01-01T00:00:00.000+0000',    NY15_UTC],
        ['2015-01-01T00:00:00Z',            NY15_UTC],

        ['2015-01-01T00:00:00.000+09:00', NY15_TOKYO],
        ['2015-01-01T00:00:00.000+0900',  NY15_TOKYO]]

    TEST_CASES.map(([str, m]) => {
        it(str + ' should be parsed as moment object', () => {
            let result = parseTimestamp(str);

            expect(moment.isMoment(result)).toBe(true);
            expect(result.unix()).toEqual(m.unix());
        });
    });

});

});`

[moumny]

OK its due to that moment/moment#3463.
Should we revert to last working version ?

[moumny]

@camsaul Can you take a look a this as well ?

[camsaul]

@moumny the Clojure stuff looks fine. @tlrobinson do you want to review the frontend stuff?

[tlrobinson]

I'll take a look at this. I think I was running into the same or similar issue and came up with a different workaround, but this might be better.

[tlrobinson]

@moumny I removed my workaround and updated npm-shrinkwrap.json to ensure we get moment@2.14.1 in https://github.com/metabase/metabase/tree/moment-tz-fix / c45e9ee The frontend LGTM (thanks for writing some tests!) so if you don't have anything else I'm happy to open a new PR and merge it.

Thanks for tracking down this down and fixing it.

[moumny]

Great !! thanks all for your help !
PS: more timezone fixes in the pipeline :D

[tlrobinson]

Superseded by #3453