ListGrid trigger an unexpected on delete/post error

[gdeimat]

Hi,

We experience a problem with ISC 9.0p. I can reproduce it on Chrome, FF and IE10.

The scenario is as follows :
1 - List is populated with an initial fetch
2 - I select one item to remove, and call ListGrid.removeSelectedData with a callback to add an extra-field to give some feedback to the user if an error occurs on the server.
3 - Server return an error (403 - Access denied in my case)

At this point, I expect to get my extra-field with an error image on the selected row. (That's the behaviour we had with 8.2p or 8.3p)

But, I can see that a new fetch request is triggered. And my extra-column is empty. If I re-select the item, and click on the delete button, the delete request is not sent and another fetch request is triggered.

I did a Log.getStackTrace() in my dataSource's transformRequest(), here is the result :

    [a]MathFunction.getStackTrace(args=>undef, ignoreLevels=>undef, maxLevels=>undef, skipFBugTrace=>undef, extensionTrace=>undef)
    BaseDataSource.transformRequest(dsRequest=>Obj)
    DataSource.getServiceInputs(dsRequest=>Obj)
    DataSource.sendDSRequest(dsRequest=>Obj)
    DataSource.performDSOperation(operationType=>"remove", data=>Obj, callback=>null, requestProperties=>Obj)
    Canvas.deleteRecords(records=>Array[1], deleteOperation=>Obj{ID:dsUser_remove}, context=>Obj, dataSource=>[BaseDataSource ID:dsUser])
    Canvas.removeSelectedData(callback=>afterFlowCallback(), requestProperties=>undef)
    [a]Dialog.callback(value=>true)
    [c]Class.fireCallback(callback=>[a]Dialog.callback(), argNames=>"value", args=>Array[1], target=>[Dialog ID:isc_globalWarn], catchErrors=>undef) on [Class Dialog]
    [a]MathFunction.fireCallback(callback=>[a]Dialog.callback(), argNames=>"value", args=>Array[1], catchErrors=>undef)
    ** recursed on [c]Class.fireCallback
 baseDataSource.js?meiTpmVersion=${meitpm.build.version.full}:481
DELETE http://localhost:8080/TradeInsight/resources/Acme/user/2077 403 (Forbidden) ISC_Core.js?meiTpmVersion=${meitpm.build.version.full}:25410
XHR finished loading: "http://localhost:8080/TradeInsight/resources/Acme/user/2077". ISC_Core.js?meiTpmVersion=${meitpm.build.version.full}:25410

    [a]MathFunction.getStackTrace(args=>undef, ignoreLevels=>undef, maxLevels=>undef, skipFBugTrace=>undef, extensionTrace=>undef)
    BaseDataSource.transformRequest(dsRequest=>Obj)
    DataSource.getServiceInputs(dsRequest=>Obj)
    DataSource.sendDSRequest(dsRequest=>Obj)
    DataSource.performDSOperation(operationType=>"fetch", data=>Obj, callback=>Obj, requestProperties=>Obj)
    DataSource.fetchData(criteria=>Obj, callback=>Obj, requestProperties=>Obj)
    ResultSet.fetchRemoteData(serverCriteria=>Obj, startRow=>0, endRow=>75)
    ResultSet._fetchRemoteData()
    [c]Class.fireCallback(callback=>"_fetchRemoteData", argNames=>undef, args=>Array[0], target=>[BaseResultSet ID:isc_BaseResultSet_0 (created by: isc_ListGrid_0)], catchErrors=>undef) on [Class EventHandler]
    [c]Class._fireActionsOnPause()
    ** recursed on [c]Class.fireCallback
 baseDataSource.js?meiTpmVersion=${meitpm.build.version.full}:481
XHR finished loading: "http://localhost:8080/TradeInsight/resources/Acme/user". ISC_Core.js?meiTpmVersion=${meitpm.build.version.full}:25410

But I can't be able to find why the fetch request is triggered.

Any ideas ? Thanking you in advance for your assistance.

[Isomorphic]

removeSelectedData() issues a "remove" operation and it's not valid to return a validation error - only a total failure (negative dsResponse.status) is valid.

Prior versions would not have shown validation errors on the record - not unless you added other logic. Probably, it's this other logic, which you haven't shared, that's misfiring in some way and triggering a fetch. Perhaps something in editComplete or editFailed.

As far as the stack trace for the fetch, since it's a fetch for the beginning of the dataset (startRow 0, endRow 75), it suggests you have possibly called invalidateCache(). You can also set the "ResultSet" log to "DEBUG" in the Developer Console to see more details about when and why the ResultSet initiates fetches.

[yavery]

Found where the issue is coming from ...

Hi,

After further investigating this issue, it appears that what is triggering this extra fetch() is the addition of
"this.data.resort()" in the setSort() method near line 56,629 (line# based on modules-debug/ISC_Grid.js):

9.0p Code

    if (this._sortSpecifiers && this._sortSpecifiers.length>0) {
        if (calledFromResort && this.data && this.data.resort && this.data._sortSpecifiers) {
            // if called from resort() and the data has a resort() method (ResultSet), AND
            // the data has already been sorted, call resort() instead of setSort() (which
            // will no-op for the same specifiers)
            this.data.resort();
        } else if (this.data &&
            (this.data.setSort || this.data.length > 0
                || isc.isA.ResultTree(this.data) || isc.isA.Tree(this.data)))
        {
            // do the actual sorting
            ...

8.3p Code

    if (this._sortSpecifiers && this._sortSpecifiers.length>0) {
        if (this.data &&
            (this.data.setSort || this.data.length > 0
                || isc.isA.ResultTree(this.data) || isc.isA.Tree(this.data)))
        {
            // do the actual sorting
            ...

If I revert that particular piece of code, everything behaves as it did before. No extra fetch, etc ...

Any idea how I can work around this or is this a bug that can be fixed on your side so that this extra fetch()
following a failed remove() doesn't get fired?

Thanks,

[yavery]

Anyone? Anyone? Bueller?

Has someone had a chance to look into this?

Thanks!

[Isomorphic]

We're looking at this. We're trying to reconcile the bug that this fixed vs its apparent affect on your use case.

[Isomorphic]

Actually, could you clarify - why did you post in this thread? Did you mean to post elsewhere? Because we're not finding a codepath that relates to removeSelectedData().

Can you capture a stack trace showing you are ending up in this code? As you'll note, the line of code you're having trouble with is only supposed to execute on an explicit resort() (note variable "calledFromResort").

[yavery]

I was posting to the right thread alright ... Here's the requested stack and more details as to the specific use case ...

console.trace()
isc.ListGrid.addMethods.setSort ISC_Grids.js?meiTpmVersion=${meitpm.build.version.full}:56645
isc.ListGrid.addMethods.resort ISC_Grids.js?meiTpmVersion=${meitpm.build.version.full}:56079
isc.ListGrid.addMethods.setFields ISC_Grids.js?meiTpmVersion=${meitpm.build.version.full}:31431
BaseList.addProperties.displayRecordFeedback baseList.js?meiTpmVersion=${meitpm.build.version.full}:1580
(anonymous function) baseList.js?meiTpmVersion=${meitpm.build.version.full}:1483

As well as our custom code referenced in the call stack above (anonymous callback and dispayRecordFeedback() method):

...

listGrid.removeSelectedData(
   function (dsResponse, data, dsRequest)
   {
      // The callback is called once for each item to delete because ISC performs atomic delete operations.
      if (dsResponse.errors)
      {
         dsRequest.originalData[BaseDataSource.FLD_FEEDBACK] = dsResponse.errors[BaseDataSource.FLD_FEEDBACK];
         dsRequest.originalData[BaseDataSource.FLD_FEEDBACK_MSG] = dsResponse.errors[BaseDataSource.FLD_FEEDBACK_MSG];
         baseList.displayRecordFeedback(dsRequest.originalData);  // ***** BaseList.js line 1483 in stack trace *****
      }
   });

...

displayRecordFeedback : function (record)
{
   var fields = this.listGrid.fields;

   if (fields.find('name', BaseDataSource.FLD_FEEDBACK))
   {
      // fields exists, refresh the feedback of this record

      var rowNum = this.listGrid.getRecordIndex(record);
      var colNum = this.listGrid.getFieldNum(BaseDataSource.FLD_FEEDBACK);

      this.listGrid.refreshCell(rowNum, colNum);
   }
   else
   {
      // add feedback field

      fields.add(BaseDataSource.createFeedbackField(), 0);
      this.listGrid.setFields(fields); // ***** BaseList.js line 1580 in stack trace *****
   }
}

The idea here is that, following a failed removeSelectedData(), we add an extra temporary column to
the listGrid to have it display a ERROR icon beside each failed rows. Hovering the error icon provides
the user with more details as to the reason for the error that occurred with that particular row.

I guess, calling setFields() fires the resort() method which is screwing things up for us with the new 9.0 release ...

What next now ? Thanks for your assistance with this.

[Isomorphic]

The problem, in a nutshell, is that setFields() potentially completely reconfigures the grid - just about everything could change, and so a lot of cleanup/resetting logic is run.

We'd recommend one of two approaches:

1. Always have your extra field in the fields array, and just use hideField/showField to manipulate it

OR

2. Handle this error differently - it's not clear what error condition is occurring here, perhaps it can be avoided entirely. If it can't, a dialog box or similar error display might be better than showing something that looks like inline validation errors (which we would generally consider a semantically different thing), especially with the awkwardness of revealing a special field to show an error icon.

[yavery]

Hi,

Thanks for looking into this ...

This is a multi-delete scenario, which can trigger different responses on a per-row basis. This is
the reason why we need/want to provide feedback on a per row basis. A popup would not allow for that.
We don't want to have 20 popups popping up one after the other either ... hence the reason for the
feedback column which can contain feedback for rows that failed, if/when appropriate.

This so far has worked perfectly (until 9.0) and has been well received by our users. It's a clear and
easy to understand paradigm that we have in place since the beginning of our app's rollout (2009).

If we go down the path of having the show/hide column, would that also mean a user could
manually turn it visible by right-clicking the listGrid header and checking the column ? If that's
the case, is there a way to use the show/hide column and prevent a user from manually
making that column visible ?

Thanks for the clarification.

[Isomorphic]

Well if the users like it, no reason to review it...

You can omit a field from the column picker by setting listGridField.canHide to false. Note that, unless you were already using this setting, your users have already been able to see and manipulate the field in the column picker menu (adding a new field via setFields() will generally make it available in the column picker).

Note that we're also looking at whether we really really need to do a resort() for every call to setFields(). We might be able to avoid it, which would also resolve your issue.

[yavery]

Great!

Let me know if you do change that behaviour. In the meantime, I'll look into the show/hide field approach
and see if this is something we could get by with.

Thanks,

[Isomorphic]

We've made a change to address this - instead of resort(), setFields() will now call setSort(getSort()) which will no-op if the sort-spec hasn't changed. Please retest with a build dated January 17 or later.

[yavery]

I'll grab it is as soon as available and will let you know.

Thanks!

[yavery]

Hi guys,

There's hasn't been any 9.0p builds both for 01/16 and 01/17 ... Anything wrong with your build server ?

Thanks,

[yavery]

Build from 01/19 appears to have fixed the issue. Thanks!