Grouping patch for _addRecordToGroup

[senordhuff]

Hi,

I've had to develop a patch for _addRecordToGroup. I have been unable to recreate this in your SDK. But, we were seeing some incorrect grouping behavior and it was the result of a call to getUnderlyingField sometimes returning a string object instead of the field object. This occurred because we were overriding getGroupValue to return unique values for some fields but they were getting ignored.

I tried patching getUnderlyingField to check and see if a string was returned and then try to lookup the field object. But, that seemed to create other problems so I abandoned that fix. Here is that fix I tried

		// get a reference to a field by name - this might be in fields, completeFields or ds.fields
		getUnderlyingField : function (fieldId) {
		    if (!this.fields && !this.completeFields && !this.dataSource) {
		        this.logWarn("fields and completeFields are null and there is no DataSource");

		        return null;
		    }
		    var item = null;
		    if (this.fields) {
		        item = isc.Class.getArrayItem(fieldId, this.fields, this.fieldIdProperty);
		    }
		    
		    //6/3/13 check to see if item is a string and then check completeFields
		    if ((!item || isA.String(item)) && this.completeFields) {
		        item = isc.Class.getArrayItem(fieldId, this.completeFields, this.fieldIdProperty);
		    }
		    
		    //6/3/13 check to see if item is a string and then check completeFields
		    if ((!item || isA.String(item)) && this.dataSource) {
		        if (!isc.isA.DataSource(this.dataSource)) this.dataSource = this.getDataSource(this.dataSource);
		        item = this.dataSource.getField(fieldId);
		    }
		    return item;
		}

So, instead I created this patch for _addRecordToGroup which solves our problem. It checks to see if a String was returned by getUnderlyingFields and, if so, then checks completeFields to try to get the actual field object.

		$52u:function(_1,_2,_3){

			var _4,_5=this.groupTree.getRoot(),_6=this.getGroupByFields();for(var i=0;i<_6.length;i++){var _8=_6[i],_9=this.getUnderlyingField(_8),_10=this.data.indexOf(_1);var _11=this.getRawCellValue(_1,_10,_8,true);
			
			//6/3/13..patch to check completeFields is string returned by getUnderlyingFields
			if(isA.String(_9) && this.completeFields!=null){
				var fieldToFind= this.completeFields.find("name",_9)
				if(fieldToFind!=null){
					_9=fieldToFind;
				}
			}
			if(_9.getGroupValue){_11=_9.getGroupValue(_11,_1,_9,_8,this)}else if(_9.userFormula){_11=this.getFormulaFieldValue(_9,_1)}else if(_9.userSummary){_11=this.getSummaryFieldValue(_9,_1)}else if(_9.displayField){var _12=this.getDisplayValue(_9.name,_11,_1);if(_12){_11=_12}}else if(_9.$62&&_9.$62.getGroupValue){_11=_9.$62.getGroupValue(_11,_1,_9,_8,this)}
			if(_9.getGroupValue==null && (_9.valueMap&&_9.valueMap[_11])){_11=_9.valueMap[_11]}
						
			if(_11==null||isc.isAn.emptyString(_11)){_11=this.nullGroupTitle}
			var _13=this.groupTree.getChildren(_5);if(_13==null)_4=null;else _4=_13.find('groupValue',_11,_9.type=="datetime"?Array.DATETIME_VALUES:null);if(!_4){_4={groupName:_8,groupValue:_11,$52e:true,canDrag:false,canSelect:false};if(this.singleCellGroupHeaders())_4.singleCellValue=_11;else _4.groupTitle=_11;_4[_8]=_11;_4[this.recordCustomStyleProperty]=this.groupNodeStyle
			_4[this.recordEditProperty]=false
			if(_3!=null){_4[this.groupTree.openProperty]=_3.find(_8,_11)?true:false}
			this.groupTree.add(_4,_5)}
			_5=_4}
			this.groupTree.add(_1,_5);if(_2){var _14=this.groupTree.getParents(_1);for(var i=_14.length-2,j=0;i>=0;i--,j++){var _16=this.getGroupTitle(_14[i]);if(!this.singleCellGroupHeaders())_14[i].groupTitle=_16;else _14[i][this.singleCellValueProperty]=_16}}

		},

We've had a lot of very difficult to debug problems with grouping in the past. But, our users are increasingly interested in grouping capabilities so I am going to have to spend some more time trying to debug these problems we've had. And, I'm wondering if this fix where getUnderlyingField returns a string instead of the actual object may be the root of other problems.

Can you comment on why getUnderlyingField sometimes returns a string instead of the Field object and whether my fix is something you can apply to the product so I don't have to maintain this patch indefinitely?

[Isomorphic]

First a quick clarification - mainly for anyone without much SC experience stumbling on this thread - to point out that you are of course modifying internal (undocumented) APIs here. getUnderlying field is not a public method so we don't support interacting with it directly and don't make any explicit guarantees about its return type or behavior.

Having said that - in normal usage this method should be returning a field object, not a string. We can't really comment on why it isn't doing so in your app as we have not seen this behavior.
Similarly we wouldn't want to add this speculative patch since it's doing essentially what getUnderlyingField should already be taking care of - looking for the field object with the appropriate ID in the completeFields array.

All we can surmise here is that you have a misconfiguration in your app that's causing the bug (an invalid entry in the fields array that's confusing things, perhaps), or there's some subtle special case we're unaware of.
The best way to approach this would probably to go back to the original problem - the grouping misbehaviors and show us how to reproduce those.

Regards
Isomorphic Software

[senordhuff]

Hi, if you'd prefer that I email questions like this about undocumented APIs instead of posting on the forums in the future, please let me know.

I have now figured out the root cause of the issue. There were a couple of areas in our code where we were creating new strings as String objects via new String() rather than string primitives. So, once we fixed that, this issue was resolved with no patches needed.

You may want to consider a defensive patch on your side to avoid such an issue affecting someone else in the future. My two suggestions are following

1. Patching getArrayItem which is called by getUnderlyingField because this is what caused our problem. It first checks to see if the param passed in is an object and it was returning true for our String objects. Here is what I propose there;

    isc.Class.addClassProperties({
    	
    	getArrayItem:function (_1,_2,_3){
    		if(_2==null)return null;if(isc.isA.Number(_1))return _2[_1];
    		//6/5/13
    		//if object and string, do not return, instead evaluate as string in next step
    		if(isc.isAn.Object(_1) && !isc.isA.String(_1))return _1;
    		if(isc.isA.String(_1))return _2.find(_3||this.$cp,_1);
    		return null
    	}
    	
    })

2. Refine your isA.Object method to check for objects of String and Number and return false for those ?

[Isomorphic]

Hi, if you'd prefer that I email questions like this about undocumented APIs instead of posting on the forums in the future, please let me know.

It's ok for such questions to go on the forums - we just wanted to make it clear up front that we're dealing with a some very advanced customizations and working with them directly is not truly "supported" - especially important since the discussion could be quite confusing to someone new to the system.

Glad to hear you resolved your issues, and thanks for your suggestions - we'll pass them along to the dev team for consideration.