Daniel Werner has submitted this change and it was merged.
Change subject: Removed time.Time's isValid() function
......................................................................
Removed time.Time's isValid() function
Since a time.Time object is supposed to be valid by design, passing invalid
values throws an
error in the constructor instead of having to check via an isValid() function.
Change-Id: I81f73a2d57f1454e3078eae46281673483a0f417
---
M DataValues/resources/time.js/src/time.Time.js
M DataValues/resources/time.js/tests/time.Time.parse.tests.js
M DataValues/resources/time.js/tests/time.Time.tests.js
M DataValues/resources/values/TimeValue.js
M ValueParsers/resources/parsers/TimeParser.js
M ValueView/resources/jquery.time/jquery.time.timeinput.js
M ValueView/resources/jquery.valueview/valueview.experts/experts.TimeInput.js
M
ValueView/tests/qunit/jquery.valueview/valueview.experts/experts.TimeInput.tests.js
8 files changed, 40 insertions(+), 47 deletions(-)
Approvals:
Daniel Werner: Verified; Looks good to me, approved
jenkins-bot: Verified
diff --git a/DataValues/resources/time.js/src/time.Time.js
b/DataValues/resources/time.js/src/time.Time.js
index f3ef2af..cde8c4a 100644
--- a/DataValues/resources/time.js/src/time.Time.js
+++ b/DataValues/resources/time.js/src/time.Time.js
@@ -55,6 +55,10 @@
utcoffset = '+00:00',
calendarname = Time.CALENDAR.GREGORIAN;
+ if( year === null ) {
+ throw new Error( 'time.Time object is invalid' );
+ }
+
if( options.calendarname ) {
calendarname = options.calendarname;
} else if ( result.calendarname !== undefined ) {
@@ -158,19 +162,6 @@
return time.getTextFromDate( precision, year, month,
day );
};
- /**
- * Returns whether the Object is representing any time at all.
This will be false if the
- * value passed to the constructor has not been interpreted as
time.
- *
- * TODO: throw an error if invalid Time, don't support invalid
data objects and deprecate
- * this function then.
- *
- * @return {boolean}
- */
- this.isValid = function() {
- return this.year() !== null;
- };
-
this.gregorianText = function() {
var result = this.gregorian();
return time.getTextFromDate( precision, result.year,
result.month, result.day );
@@ -196,8 +187,7 @@
return false;
}
- return this.isValid() && otherTime.isValid() // two invalid
times are not equal
- && this.precision() === otherTime.precision()
+ return this.precision() === otherTime.precision()
&& this.iso8601() === otherTime.iso8601();
};
diff --git a/DataValues/resources/time.js/tests/time.Time.parse.tests.js
b/DataValues/resources/time.js/tests/time.Time.parse.tests.js
index 58a61b3..aaaed52 100644
--- a/DataValues/resources/time.js/tests/time.Time.parse.tests.js
+++ b/DataValues/resources/time.js/tests/time.Time.parse.tests.js
@@ -38,10 +38,9 @@
// test integration with time.Time:
if( parsedTime !== null ) {
timeObject = new Time( parsedTime );
- assert.ok(
- timeObject.isValid(),
- '"' + timeInput + '" parser result can
be used to create new valid time.Time instance'
- );
+
+ assert.ok( timeObject instanceof Time, '"' +
timeInput + '" parser result can be '
+ + 'used to create new time.Time
instance' );
}
} );
diff --git a/DataValues/resources/time.js/tests/time.Time.tests.js
b/DataValues/resources/time.js/tests/time.Time.tests.js
index a45a6de..9b9a98d 100644
--- a/DataValues/resources/time.js/tests/time.Time.tests.js
+++ b/DataValues/resources/time.js/tests/time.Time.tests.js
@@ -24,7 +24,7 @@
var time,
valid = true;
try {
- var time = new Time( definition ); // throws an error
if failure
+ time = new Time( definition ); // throws an error if
failure
} catch( e ) {
valid = false;
}
@@ -39,6 +39,18 @@
$.each( validTimeDefinitions, function( name, definition ) {
testConstructByString( assert, name, definition );
} );
+
+ var t;
+
+ assert.throws(
+ function() { t = new Time( '' ); },
+ 'Trying to instantiate with an empty value throws an
error.'
+ );
+
+ assert.throws(
+ function() { t = new Time( 'foooo - invalid time' ); },
+ 'Trying to instantiate with an invalid value throws an
error.'
+ );
} );
function testConstructByString( assert, definitionName, definition ) {
@@ -51,7 +63,7 @@
}
assert.ok(
- valid && time.isValid(),
+ valid,
'New valid time.Time object built from "' +
definitionName + '" example definition'
);
}
diff --git a/DataValues/resources/values/TimeValue.js
b/DataValues/resources/values/TimeValue.js
index b747639..10484f2 100644
--- a/DataValues/resources/values/TimeValue.js
+++ b/DataValues/resources/values/TimeValue.js
@@ -14,10 +14,6 @@
if( !( value instanceof Time ) ) {
throw new Error( 'The given value has to be a
time.Time object' );
}
- if( !value.isValid() ) {
- throw new Error( 'The given time value has to
represent a valid time' );
- }
-
this._value = value;
};
diff --git a/ValueParsers/resources/parsers/TimeParser.js
b/ValueParsers/resources/parsers/TimeParser.js
index fa08fea..568fe90 100644
--- a/ValueParsers/resources/parsers/TimeParser.js
+++ b/ValueParsers/resources/parsers/TimeParser.js
@@ -27,17 +27,7 @@
* @return $.Promise
*/
parse: function( time ) {
- var deferred = $.Deferred();
-
- if( time.isValid() ) {
- // Valid time:
- var dataValue = new dv.TimeValue( time );
- deferred.resolve( dataValue );
- } else {
- // Invalid time:
- deferred.reject();
- }
-
+ var deferred = $.Deferred().resolve( new dv.TimeValue(
time ) );
return deferred.promise();
}
} );
diff --git a/ValueView/resources/jquery.time/jquery.time.timeinput.js
b/ValueView/resources/jquery.time/jquery.time.timeinput.js
index 78c0b93..98a7e47 100644
--- a/ValueView/resources/jquery.time/jquery.time.timeinput.js
+++ b/ValueView/resources/jquery.time/jquery.time.timeinput.js
@@ -32,7 +32,14 @@
this.element.addClass( this.widgetName );
this.element.eachchange( function( event, oldValue ) {
- var value = self._parse();
+ var value;
+
+ try {
+ value = self._parse();
+ } catch( e ) {
+ value = null;
+ }
+
if( value !== self._value ) {
self._value = value;
self._trigger( 'update', null,
[self._value] );
@@ -52,10 +59,11 @@
* Parses the current input value.
*
* @return {time.Time|null} Time object when parsing was
successful.
+ *
+ * @throws {Error} When no time.Time object could be
instantiated.
*/
_parse: function() {
- var timeValue = new Time( this.element.val() );
- return ( timeValue.isValid() ) ? timeValue : null;
+ return new Time( this.element.val() );
},
/**
@@ -69,8 +77,8 @@
return this._value;
}
- if( value !== null && ( !( value instanceof Time ) ||
!value.isValid() ) ) {
- throw new Error( 'Cannot set value: Neither
valid Time object nor \'null\' given.' );
+ if( value !== null && ( !( value instanceof Time ) ) ) {
+ throw new Error( 'Cannot set value: Neither
time.Time object nor \'null\' given.' );
}
if( value === null ) {
diff --git
a/ValueView/resources/jquery.valueview/valueview.experts/experts.TimeInput.js
b/ValueView/resources/jquery.valueview/valueview.experts/experts.TimeInput.js
index 5d2a36e..dc69d7a 100644
---
a/ValueView/resources/jquery.valueview/valueview.experts/experts.TimeInput.js
+++
b/ValueView/resources/jquery.valueview/valueview.experts/experts.TimeInput.js
@@ -186,7 +186,7 @@
} )
.on( 'timeinputupdate.' + this.uiBaseClass, function(
event, value ) {
self._updateCalendarHint( value );
- if( value && value.isValid() ) {
+ if( value ) {
self.$precision.data( 'listrotator'
).rotate( value.precision() );
self.$calendar.data( 'listrotator'
).rotate( value.calendarText() );
}
@@ -322,7 +322,7 @@
* @param {time.Time|null} time
*/
_setRawValue: function( time ) {
- if( !( time instanceof Time ) || !time.isValid() ) {
+ if( !( time instanceof Time ) ) {
time = null;
}
this._newValue = time;
@@ -344,8 +344,7 @@
return false;
}
- return time1.isValid() && time2.isValid()
- && time1.precision() === time2.precision()
+ return time1.precision() === time2.precision()
&& time1.iso8601() === time2.iso8601();
},
diff --git
a/ValueView/tests/qunit/jquery.valueview/valueview.experts/experts.TimeInput.tests.js
b/ValueView/tests/qunit/jquery.valueview/valueview.experts/experts.TimeInput.tests.js
index 1a0bea7..6188f85 100644
---
a/ValueView/tests/qunit/jquery.valueview/valueview.experts/experts.TimeInput.tests.js
+++
b/ValueView/tests/qunit/jquery.valueview/valueview.experts/experts.TimeInput.tests.js
@@ -22,8 +22,7 @@
],
unknown:
testExpert.basicTestDefinition.rawValues.unknown.concat( [
42,
- '1. 1984',
- new Time( 'foooo - invalid time' )
+ '1. 1984'
] )
},
relatedValueParser: TimeParser
--
To view, visit https://gerrit.wikimedia.org/r/66400
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I81f73a2d57f1454e3078eae46281673483a0f417
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/DataValues
Gerrit-Branch: master
Gerrit-Owner: Henning Snater <[email protected]>
Gerrit-Reviewer: Daniel Werner <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
