[bug] DataSource.compareValues() won't compare relative dates

[antanas.arvasevicius]

Hi,

We have modified slightly an original DateRangeItem return object by appending hidden property "__ExactValueToServer", I thought that would not affect anything, but I was wrong.
Then listgrid "date" field is filtered by any relative date (from: yesterday, to: tommorow) any click on filter issues fetch to server (but ).

Bug is found in DataSource.compareValues(..) method
The problem is that DateRangeItem which is used in filter UI submits AdvancedCriteria values not a Date but object:

{
    "_constructor" : "RelativeDate",
    "value" : "-5m[-0D]",
    "rangePosition" : "start",
    "logicalDate" : true,
    "_sendToServerValue" : "2015-08-13T21:00:00.000Z"
}

so original DataSource.compareValues() won't detect that these values are Date objects and compares it by unknown factor (as it always return that objects not equal)


patch we have applied to our system:

///<reference path='../Patch.ts'/>
module smartclient.patches.bugfix {

    export class ListGridCriteriaComparisonForRelativeDatesBugfixPatch extends Patch {
        public initialize(isc):void {
            var parent = isc.RestDataSource.getPrototype()['compareValues'];

            isc.RestDataSource.addMethods({
                compareValues: function (value1, value2, fieldName, ignoreCase) {
                    return isDate(value1) && isDate(value2) ?
                        this.compareDates(getAbsoluteDate(value1), getAbsoluteDate(value2), fieldName) :
                        parent.apply(this, arguments);
                }
            });
        }
    }

    function isDate(value) {
        return isc.isA.Date(value) || isc.DateUtil.isRelativeDate(value);
    }

    function getAbsoluteDate(value) {
        return isc.DateUtil.isRelativeDate(value) ? isc.DateUtil.getAbsoluteDate(value) : value;
    }
}

Affected: SmartClient_v100p_2016-01-12_LGPL and all previous

[Isomorphic]

This report is a bit scattered..

First, you refer to __ExactValueToServer but then in actual JSON you show _sendToServerValue - are those meant to be the same?

Second, why are you creating your own absolute date to send to the server - the built-in behavior DataSource.autoConvertRelativeDates already does this. Did you turn that off? Why?

Third, are you saying that this problem *only* arises when you add additional properties to your RelativeDate? If so, this is probably because you copied the object, so object identity comparisons no longer work. It's invalid to stuff extra properties in that object, hence your issue and a better approach would be to do the conversion in transformRequest, analogous to how the built-in autoConvertRelativeDates feature works.

[Isomorphic]

Note, although it's not valid to extend the RelativeDate object as you have, we do plan to change the RelativeDate comparison so it doesn't use object identity, since it could in theory cause unnecessary server fetches in other cases (like perhaps if someone's custom item simply *copied* the RelativeDate object each time, without adding properties).

Once we've made that change, your patch won't be necessary, but we'd still recommend fixing your usage: we don't support custom properties being added to a RelativeDate, so something else could break. And as we've covered, this whole approach seems like it might be unnecessary.

[antanas.arvasevicius]

Hi, Okay, forget what I have said previously, lets get fresh install of empty framework and:

Here is a code which shows that it's a really a BUG in framework itself.

steps to reproduce:
1) focus on filter "id"
2) focus on filter "some_other"
3) focus on filter "id"

see nothing happens? no fetches at all.

now:

4) select any RELATIVE date from a "date" field (from: yesterday, to: today) or any other relative (must be relative at least one)
5) focus on filter "id" (trigger fetch)
6) focus on filter "some_other" (trigger fetch again? wtf?)
7) focus on filter "id" (trigger fetch again? wwwtf?)
8) focus on filter "some_other" (trigger fetch again? wwwwttttfff?)

And you are saying that it's not a bug? Allright, then select from a "date" filter only absolute values. (from, to, any date from calendar, and then all works okay, not fetches occur )

So for that fix I've posted a patch for DataSource.compareValues(..) previously, hope you'll incorporate that into your sources :)

In future I'll post only this kind of "exploits" without any project specific details, as I can see, you are using these details back as it's only my fault of bad usage, bad extending and that I am a only problem source, not your framework.

    var testData = new Array(500).map(function (v, i) {
        var date = new Date();
        date.setDate((i % 29) + 1);
        return {id: "" + (i + 1), date: date}
    });

    var ds = isc.DataSource.create({
        ID: "ds",
        clientOnly: true,
        fields: [{name: 'id', primaryKey: true}, {name: 'date', type: 'date'}],
        testData: testData,
        fetchData: function (criteria, callback, requestProperties) {
            console.log('fetchData: ', criteria, requestProperties);
            this.performDSOperation("fetch", criteria, callback, requestProperties);
        }
    });

    isc.ListGrid.create({
        width: 400,
        height: 200,
        autoDraw: true,
        dataSource: "ds",
        autoFetchData: true,
        useAllDataSourceFields: false,
        showFilterEditor: true,
        filterByCell: true,
        fields: [
            {
                name: 'date',
                width: "30%"
            },
            {
                name: 'id'
            },
            {
                name: 'some_other'
            }
        ]
    });

\

[Isomorphic]

Yup, this new test case shows there are cases where unnecessary fetches can be performed, without the need for bad usage shown in the previous test case.

The fix we were already planning to apply will handle this case too, but thanks for pointing out that it can be triggered more easily, that bumps the priority up.

Just as an aside, it's best not to ascribe bizarre, plainly incorrect motives to Support. It's really quite simple: with any bug, we need to figure out how likely other people are to hit it, and prioritize accordingly, so we fix the worst bugs first.

We'd still suggest fixing your usage from the previous case - that extra property you added to the RelativeDate object is, again, unsupported, and it would be perfectly valid for future versions of the framework to skip it in serialization, drop it when copying or storing criteria, or do something similar. And there's no apparent need to do things in the particular way you have.

[antanas.arvasevicius]

Thank you!

Yes, that additional property to RelativeDate object causes some troubles, we need to patch isc.clone and other to pass that value around, and some other tricky things to do. We'll try to setup more nonobstructive way by using Map and singleton repository for additional data attachment. The problem we are solving that way, is that by selecting DateRangeItem from: today, to:today we need to send "yyyy-mm-dd 00:00:00" "yyyy-mm-dd 23:59:59" instead of "yyyy-mm-dd" as we are storing all date in DateTime format, but we want to display on ui only "date", so DataSource field type is "date" instead of "datetime".
So we are transforming relative date to absolute and append to "__sendToServerValue" to object, because original relative to absolute date transformer will transform value like that "yyyy-mm-dd" as type is "date".

As I'm writing this I thought maybe we must use field type "datetime" but somehow force appearance to display only "yyyy-mm-dd" and date picker to not show a "time" inputs? is that possible?

PS. our database need to have all fields as DateTime, as business works in multiple timezones, so only "Date" it's not sufficient

[Isomorphic]

It seems like you want all the behaviors of a "date" field (in terms of display, editing, etc) *except* you need a last minute transform on the way to the server. If so, DataSource.transformResponse() is the right API for this; you should be able to modify DSRequest.data so that you replace any RelativeDate instances with the absolute Dates you want (or even the specific Strings you want), then call Super() to have the rest of the serialization take place as usual.

[antanas.arvasevicius]

! oh, wait a minute, "rangePosition" property will give me a needed information whether is a "from" or "to" range?

[Isomorphic]

Yes, exactly. You can pass that to DateUtil.getAbsoluteDate() and it will do the work of returning the first second of the range vs last second of the range as you want it.

[antanas.arvasevicius]

oh, that is cool, I am going to remove a bunch of my hacks, Thank you!
It was a main reason to do that "__sendToServer" and calculate in DateRangeItem because in transform I couldn't know is it "from" or "to" (add 00:00:00 or 23:59:59)

But I think I could just use schema field type: "datetime" instead and it should already convert "yesterday" "today" ranges for me. But how could I make ListGrid and DateRangeItem (no time selection) to look like a type: "date" ?
no time part maybe there is a properties or something.
Because on server there is a DateTime not a Date, so it would be more conceptually correct.

[Isomorphic]

You could, via a bunch of settings like dataSourceField.format and various SimpleType properties and overrides, cause a "datetime" field to appear and be edited pretty much like a "date" field. It would take quite a few settings, because there are a lot of automatic behaviors to override.

However, in this case we'd argue that your field is conceptually a logical "date". It sounds like your end user's conceptual model of the field is a logical date with no notion of time attached. If so, then the fact that it is actually stored as a "datetime" server-side is more of an implementation detail: something to be handled as close to the storage layer as possible. So, you could handle this entirely server-side, but if not, then DataSource.transformResponse() is the closest in-browser API to the storage layer.

[Isomorphic]

Note that we've added support for relative date comparison, including shortcut strings like "$today" and relative strings like "+0D[+1D]" - so you should be able to remove the patch code that enforced it for you for RelativeDate objects locally.