Opened 8 years ago

Closed 8 years ago

Last modified 7 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 8 years ago.
shows issue with Date and RegExp?
mvc-13899.patch (13.5 KB) - added by Ed Chatelain 8 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 8 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 8 years ago by ben hockey

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

comment:2 Changed 8 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 8 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 8 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 8 years ago by Ed Chatelain

Ben,

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

Thanks

Changed 8 years ago by ben hockey

Attachment: 13899.html added

shows issue with Date and RegExp?

comment:6 Changed 8 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 8 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 8 years ago by Chris Mitchell

Resolution: fixed
Status: newclosed

In [26660]:

fixes #13899 mvc support for dates. thx edchat@ibm \!strict

comment:8 Changed 8 years ago by Chris Mitchell

In [26661]:

refs #13899 mvc support for dates. thx edchat@ibm \!strict

comment:9 Changed 8 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 8 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 8 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 8 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 8 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 8 years ago by Chris Mitchell

In [26720]:

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

comment:14 Changed 8 years ago by bill

Milestone: tbd1.7

comment:15 Changed 7 years ago by cjolif

#13823 is a duplicate of this ticket.

Note: See TracTickets for help on using tickets.