Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#13899 closed defect (fixed)

[patch] [cla] MVC - Dates don't work in StatefulModel

Reported by: ben hockey Owned by: rahul
Priority: high Milestone: 1.7
Component: DojoX MVC Version: 1.7.0b1
Keywords: Cc: bill, Ed Chatelain
Blocked By: Blocking:

Description

StatefulModel? does not handle Date values. see attached file for test case.

possible fix (not tested):

  • StatefulModel.js

     
    364364                       //              The input for the model, as a plain JavaScript object.
    365365                       // tags:
    366366                       //              private
    367                        if(lang.isObject(obj)){
     367                       if(lang.isObject(obj) && !(obj instanceof Date)){
    368368                               for(var x in obj){
    369369                                       var newProp = new StatefulModel({ data : obj[x] });
    370370                                       this.set(x, newProp);

Attachments (3)

13899.html (1.4 KB) - added by ben hockey 10 years ago.
shows issue with Date and RegExp?
mvc-13899.patch (13.5 KB) - added by Ed Chatelain 10 years ago.
Updated patch with fix for Dates and RegExp?., it includes a test html file, I will add a doh test later.
mvc-13899-tests.patch (109.0 KB) - added by Ed Chatelain 10 years ago.
This patch contains the updates for initializing the dates requested by Adam, along with a doh test for the dates and updates to the doh_mvc_form-kitchensink test which also had some date tests which needed to be updated. The doh tests run on IE and on Mac.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 10 years ago by ben hockey

the same issue applies for RegExp? (updated test case) but maybe not as critical as Date.

comment:2 Changed 10 years ago by Ed Chatelain

I had to make some other changes to get bindings and reset to work with dates. I will attach a patch of what I have so far.

There is a problem when trying to set a date string in this format: "Wed May 05 2010 01:00:00 GMT-0400 (Eastern Daylight Time)" into a into a TextBox? bound to a dijit.form.DateTextbox?, but that works in a Calendar

Another odd thing I found was that on a calendar I would get a different value from dijit.byId("cal").get('value') and dijit.byId("cal").value. dijit.byId("cal").value: Wed May 05 2010 01:00:00 GMT-0400 (Eastern Daylight Time) dijit.byId("cal").get('value'): Wed May 05 2010 00:00:00 GMT-0400 (Eastern Daylight Time) dijit.byId("cal").get('value') is getting 00:00:00 when it should be 01:00:00 but I have not gotten to look into why.

I have also not gotten to look into the RegExp? yet, I will try to do that soon.

comment:3 Changed 10 years ago by ben hockey

Cc: bill added

maybe i'm misunderstanding your comment so i'll see what the patch does but you shouldn't be concerned about date strings at all - just objects of type Date.

also, my understanding is that we should only be concerned about widget.get('value') as the "official" value of a widget. widget.value might not always be the underlying value represented by the widget - maybe i'm wrong about this. i cc'd bill so he can clarify/correct if needed.

comment:4 Changed 10 years ago by ben hockey

i see from your comments in the patch that you can't get a TextBox? bound to a DateTextBox? to work when the input of the TextBox? is "Wed May 05 2010 01:00:00 GMT-0400 (Eastern Daylight Time)". i don't think we need to support this case. i don't think that a user should expect that binding a String to a Date needs to work automatically. however, the inconsistency between DateTextBox? and Calendar might be something that interests bill.

comment:5 Changed 10 years ago by Ed Chatelain

Ben,

What problem are you seeing with RegExp?? I am not seeing it.

Thanks

Changed 10 years ago by ben hockey

Attachment: 13899.html added

shows issue with Date and RegExp?

comment:6 Changed 10 years ago by ben hockey

ed,

i've updated the test page to show the problem with RegExp?. you'll see that nothing is logged for the first line.

console.log('model.regex.get("value")', model.regex.get('value'));
console.log('regex', regex);

the problem is similar to the Date. lang.isObject(obj) will be true for a RegExp? and then the current code will try to iterate over the properties of obj to make a StatefulModel tree. the issue is that for a RegExp?, no tree should be made - it should be a leaf node.

i haven't tried this case with your patch but it looks like it would fail in the same way.

Changed 10 years ago by Ed Chatelain

Attachment: mvc-13899.patch added

Updated patch with fix for Dates and RegExp?., it includes a test html file, I will add a doh test later.

comment:7 Changed 10 years ago by Chris Mitchell

Resolution: fixed
Status: newclosed

In [26660]:

fixes #13899 mvc support for dates. thx [email protected] \!strict

comment:8 Changed 10 years ago by Chris Mitchell

In [26661]:

refs #13899 mvc support for dates. thx [email protected] \!strict

comment:9 Changed 10 years ago by Adam Peller

Cc: Ed Chatelain added
Resolution: fixed
Status: closedreopened

Dates should never be used with the long format in the constructor. Use separate year/month/day args or for ES5+ use ISO dates in the Date constructor.

comment:10 in reply to:  9 Changed 10 years ago by ben hockey

Replying to peller:

Dates should never be used with the long format in the constructor. Use separate year/month/day args or for ES5+ use ISO dates in the Date constructor.

this just means you want the tests changed right?

comment:11 Changed 10 years ago by Chris Mitchell

ed's going to be working on the tests more on monday. can update them with this info, thx.

Changed 10 years ago by Ed Chatelain

Attachment: mvc-13899-tests.patch added

This patch contains the updates for initializing the dates requested by Adam, along with a doh test for the dates and updates to the doh_mvc_form-kitchensink test which also had some date tests which needed to be updated. The doh tests run on IE and on Mac.

comment:12 Changed 10 years ago by Adam Peller

Resolution: fixed
Status: reopenedclosed

In [26715]:

Fixes #13899 fixes for Date and Stateful from edchat (IBM, CCLA)

comment:13 Changed 10 years ago by Chris Mitchell

In [26720]:

refs #13899 add missing test file. thx edchat @ ibm \!strict

comment:14 Changed 10 years ago by bill

Milestone: tbd1.7

comment:15 Changed 10 years ago by cjolif

#13823 is a duplicate of this ticket.

Note: See TracTickets for help on using tickets.