changes for timeInterval #6493
changes for timeInterval #6493
Conversation
|
@hanbollar, thanks for the pull request! Maintainers, we have a signed CLA from @hanbollar, so you can review this at any time. I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome. 🌍 🌎 🌏 |
|
@hpinkos I updated the documentation and added in the throw statement. lmk if this is what you were looking for / if there's anything else you think I should do. |
|
I don't think this change is necessary, There are way more rules to an ISO8601 interval than containing a slash and the doc already links to the spec. |
|
Actually, looking at the code some more this should do some extra validation. After |
|
@mramato updated to match that |
|
Thanks @hanbollar, can you add a quick test as well? |
|
@ggetz added |
Specs/Core/TimeIntervalSpec.js
Outdated
| @@ -100,6 +100,12 @@ defineSuite([ | |||
| expect(interval.data).toEqual(data); | |||
| }); | |||
|
|
|||
| it('fromIso8601 throws error when given Invalid ISO 8601 date.', function() { | |||
| expect(function() { | |||
| expect(TimeInterval.fromIso8601({ iso8601 : '2020-08-29T00:00:00+00:00' })); | |||
There was a problem hiding this comment.
You don't need to wrap this in the second expect:
expect(function() {
TimeInterval.fromIso8601({ iso8601 : '2020-08-29T00:00:00+00:00' });
});
|
@ggetz removed second expect |
|
Thanks @hanbollar ! |
Source/Core/TimeInterval.js
Outdated
| @@ -143,6 +147,9 @@ define([ | |||
| //>>includeEnd('debug'); | |||
|
|
|||
| var dates = options.iso8601.split('/'); | |||
| if (dates.length !== 2) { | |||
| throw new DeveloperError('Invalid ISO 8601 date.'); | |||
There was a problem hiding this comment.
options.iso8601 is an invalid ISO 8601 interval
Fixes #6164