[Commits] r1298 - in core/trunk/geoext: lib/GeoExt/data tests/lib/GeoExt/data

[commits at geoext.org]

Author: tschaub
Date: 2009-07-29 19:50:35 +0200 (Wed, 29 Jul 2009)
New Revision: 1298

Modified:
   core/trunk/geoext/lib/GeoExt/data/FeatureStore.js
   core/trunk/geoext/tests/lib/GeoExt/data/FeatureStore.html
Log:
Take care not to add feature attributes when calling record.set - unless the record contains custom fields.  This deserves a better solution when have mappings for all fields to feature properties. Thanks for the review and patch modifications Andreas.  r=ahocevar (closes #132)

Modified: core/trunk/geoext/lib/GeoExt/data/FeatureStore.js
===================================================================
--- core/trunk/geoext/lib/GeoExt/data/FeatureStore.js	2009-07-29 14:57:11 UTC (rev 1297)
+++ core/trunk/geoext/lib/GeoExt/data/FeatureStore.js	2009-07-29 17:50:35 UTC (rev 1298)
@@ -371,16 +371,25 @@
      */
     onUpdate: function(store, record, operation) {
         if(!this._updating) {
+            /**
+              * TODO: remove this if the FeatureReader adds attributes
+              * for all fields that map to feature.attributes.
+              * In that case, it would be sufficient to check (key in feature.attributes). 
+              */
+            var defaultFields = new GeoExt.data.FeatureRecord().fields;
             var feature = record.get("feature");
             if(record.fields) {
                 var cont = this.layer.events.triggerEvent(
                     "beforefeaturemodified", {feature: feature}
                 );
                 if(cont !== false) {
+                    var attributes = feature.attributes;
                     record.fields.each(
                         function(field) {
-                            feature.attributes[field.mapping || field.name] =
-                                record.get(field.name);
+                            var key = field.mapping || field.name;
+                            if (!defaultFields.containsKey(key)) {
+                                attributes[key] = record.get(field.name);
+                            }
                         }
                     );
                     this._updating = true;

Modified: core/trunk/geoext/tests/lib/GeoExt/data/FeatureStore.html
===================================================================
--- core/trunk/geoext/tests/lib/GeoExt/data/FeatureStore.html	2009-07-29 14:57:11 UTC (rev 1297)
+++ core/trunk/geoext/tests/lib/GeoExt/data/FeatureStore.html	2009-07-29 17:50:35 UTC (rev 1298)
@@ -187,19 +187,28 @@
         }
 
         function test_featuremodified_update(t) {
-            t.plan(6);
+            t.plan(8);
 
             /*
              * Set up
              */
             var feature, id, layer, store, recordType, record;
-
+            
             feature = new OpenLayers.Feature.Vector(null, {
                 foo: "foo",
                 bar: "bar"
             });
 
             id = feature.id;
+            
+            function keys(obj) {
+                var list = [];
+                for(var k in obj) {
+                    list.push(k);
+                }
+                return list;
+            }
+            var originalAttrs = keys(feature.attributes);
 
             recordType = GeoExt.data.FeatureRecord.create([
                 {name: "foo"}, {name: "bar"}
@@ -244,6 +253,19 @@
 
             t.eq(layer.getFeatureById(id).attributes.bar, "bar3",
                  "update event causes update of feature property \"bar\"");
+            
+            // make sure calling record.set didn't add any attributes
+            var currentAttrs = keys(feature.attributes);
+            t.eq(originalAttrs.length, currentAttrs.length,
+                 "no new attributes added: " + currentAttrs.join(", "));
+            
+            var feature2 = new OpenLayers.Feature.Vector(null, {
+                "foo": "foo_f2"
+            });
+            layer.addFeatures([feature2]);
+            store.getById(feature2.id).set("bar", "bar_f2");
+            
+            t.eq(feature2.attributes.bar, "bar_f2", "previously undefined attribute set correctly");
 
          }
     </script>