1.5.1 - Cancel changes on an array of complex types still has issues

[plwalters]

Currently line 3688 of breeze.debug.js is -

  function rejectChangesCore(target) {
    var aspect = target.entityAspect || target.complexAspect;
    var stype = target.entityType || target.complexType;
    var originalValues = aspect.originalValues;
    for (var propName in originalValues) {
      target.setProperty(propName, originalValues[propName]);
    }
    stype.complexProperties.forEach(function (cp) {
      var cos = target.getProperty(cp.name);
      if (cp.isScalar) {
        rejectChangesCore(cos);
      } else {
        cos._rejectChanges();
        cos.forEach(rejectChangesCore(co));
      }
    });
  }

But co is undefined in the rejectChangesCore() call in the cos.foreach. Judging by the other uses this should be

        cos.forEach(function (co) { rejectChangesCore(co)});

The problem I previously mentioned is noticeable here though. I am working on a jsBin to repro the issue but if you have the following schema -

            metadataStore.addEntityType({
                shortName: "Person",
                        someComplexTypes: { complexTypeName: "Identifier:#MyStuff", isScalar: false },
                    });

            metadataStore.addEntityType({
                shortName: "Identifier",
                namespace: "MyStuff",
                isComplexType: true,
                dataProperties: {
                    id: { dataType: "String" },
                                name: { dataType: "String" }
                }
            });

            var thisComplexType = manager.metadataStore.getEntityType('Identifier');
            var thisEntity = thisComplexType.createInstance({id: '1', 'name'});
            myPerson.someComplexTypes.push(thisEntity)
            myPerson.entityAspect.acceptChanges();
            myPerson.someComplexTypes.remove(thisEntity);
            myPerson.someComplexTypes.cancelChanges();

The person never regains the 'removed' complex type much as it would if it were not a complex type. I may be highly overlooking something here but I will try to get the jsBin working, it may be helpful to repro issues if you had a fiddle or bin with the latest version of breeze.js available via CDN (I believe cdn.js is serving 1.4.11 or so)

[plwalters]

That awkward moment when you live with something for a year and then when you actually repro it in a jsBin you realize the problem -

observableArray exposes a remove() method that the array of complexTypes does not. When you call remove on the array it appears to work fine to remove it from the underlying array but breeze doesn't seem to catch it. When I switched to using splice on the array instead breeze catches it and rejectChanges works.

Demo for ships and giggles.

http://jsbin.com/soraza/2/edit

Need to watch the console.

The issue with the foreach still exists

[wardbell]

The forEach fix is in the repro and will publish next release.

We still need to look at the KO remove problem. Stay tuned and stay on us

[jtraband]

@PWKad You state: "observableArray exposes a remove() method that the array of complexTypes does not.". I must be missing something because I don't see a remove method anywhere in breeze. What am I missing? and sorry for taking so long to look into this one. and can you confirm that the forEach issue has been fixed on the repo. If so, this will go out in 1.5.2 next week.

[plwalters]

@jtraband - The Knockout observableArray exposes a remove method that finds an item in an array and removes it without having to use a native for loop or something. It is a very handy method and appears to work great until cancelChanges is called because Breeze doesn't watch or override this method. In other words when using something like person.children.remove(child) this works fine except when inside of an array of complex types - the remove method works but breaks when you cancelChanges.

Checking the repo now

[jtraband]

Ok, I understand the disconnect now. Breeze has its own observableArray that implements all of the standard array fns. i.e. splice, push, pop, shift and unshift, but we do not attempt to wrap the knockout observableArray remove method ( this is a knockout specific extension). So your observation is correct, but I'm not sure we are going to get to this right away. ( unless you have a simple GitHub pull request :) But I will add a feature request for this now that I understand the issue. thx.

[plwalters]

Since this appears to be coming up more often in the issues and for me personally (one of our new developers just got stuck on this for two days) I think I am going to try to work up a PR sometime this weekend to address this. I will look at the current tests to see if I can add some specs that test out these scenarios but they may not be perfect.