Esri / geoform-template-js

GeoForm is a configurable template for form based data editing of a Feature Service.
http://esri.github.io/geoform-template-js/
Apache License 2.0
67 stars 83 forks source link

When "All Layers" is used, polygon layers are being allowed #425

Closed driskull closed 9 years ago

driskull commented 9 years ago

Seems like when "all layers" is used, we should have a check for only point feature layers. Currently, all feature layers are allowed.

the builder is allowing me to configure a form for all layers in a map, which ends up creating the conditions where i can select a form for a polygon feature layer.  if i do this, then fill out the form, select location, and submit. i get this error: "Sorry your entry cannot be submitted. Please try again."

iwittenmyer commented 9 years ago

More info: In the builder it will also allow me to explicitly select a single polygon feature layer. So the check for point features layers should restrict the list of editable layers in step 2 of the builder to points. Otherwise i can create an app with one layer that will never allow end users to successfully submit an entry.

driskull commented 9 years ago

@iwittenmyer I think we decided to just do a feature layer detection because checking if the layer is editable requires another network request or creating the layer to do. @CTDisasterManagement can you confirm?

If we have the webmap created in the bulder, we could probably just use this method to see if the layer is editable. https://developers.arcgis.com/javascript/jsapi/featurelayer-amd.html#iseditable

driskull commented 9 years ago

@CTDisasterManagement I'm seeing this issue:

 TypeError: undefined is not a function {stack: (...), message: "undefined is not a function"} "TypeError: undefined is not a function
    at null.<anonymous> (http://localhost/git/geoform-template-js/js/main.js:1613:52)
    at g.hitch (http://js.arcgis.com/3.13/init.js:177:296)
    at c (http://js.arcgis.com/3.13/init.js:76:221)
    at d (http://js.arcgis.com/3.13/init.js:76:10)
    at b.Deferred.resolve.callback (http://js.arcgis.com/3.13/init.js:77:350)
    at ia (http://js.arcgis.com/3.13/init.js:503:92)
    at Pa (http://js.arcgis.com/3.13/init.js:505:60)
    at oa (http://js.arcgis.com/3.13/init.js:504:397)
    at http://js.arcgis.com/3.13/init.js:513:482
    at c (http://js.arcgis.com/3.13/init.js:76:221)"

I don't think the layer is loaded yet so the function isn't available.

We may need a function like this to wait for it to be loaded first.

_featureLayerLoaded: function (layer) {
        var def = new Deferred();
        if (layer.loaded) {
          // nothing to do
          def.resolve();
        } else if (layer.loadError) {
          def.reject(new Error(this._dijitName + " Layer failed to load."));
        } else {
          var loadedEvent, errorEvent;
          // once layer is loaded
          loadedEvent = on.once(layer, "load", lang.hitch(this, function () {
            errorEvent.remove();
            def.resolve();
          }));
          // error occurred loading layer
          errorEvent = on.once(layer, "error", lang.hitch(this, function () {
            loadedEvent.remove();
            def.reject(new Error(this._dijitName + " Layer could not be loaded."));
          }));
        }
        return def.promise;
      },
driskull commented 9 years ago

Also, getting the layer twice isn't good. We should get the layer once and cache it as a variable. Calling getLayer() twice isn't very optimal.

driskull commented 9 years ago

@CTDisasterManagement I'm still seeing JS errors with this.

Here is my appid: be338760de9249f8b15df22a8e4ee586

Can you investigate?

CTDisasterManagement commented 9 years ago

Sure, we will look into this.

CTDisasterManagement commented 9 years ago

Hi @driskull , The error that we see in the appid you gave is due to a "length" property that is present as an enumerable property in the fields object of config. Here is the snapshot: image

Whereas the expected 'fields' object doesn't have any such property as we can see below: image

image

We have handled this issue in the code, although we might have to investigate on, how this "length" property got pushed in the fields object of config of this webmap.

Please share your insights on the same, if any!

Thanks!!

driskull commented 9 years ago

@CTDisasterManagement Not sure how that happened. very odd.

driskull commented 9 years ago

@CTDisasterManagement This app was created before we moved to multiple layers so I'm assuming that it got messed up when it was converted. Since it was an array before becoming an object.

During the conversion, we should make sure we're not saving the object with a length property or something.

I'm guessing that's what happened.

Can you look into the code and make sure that we don't do that if the app was configured with the version that was not supporting multiple layers?

CTDisasterManagement commented 9 years ago

Hi @driskull , The other two snapshots of objects that are working fine are also from apps created before multiple layer functionality was built. So chances are rare of this being the reason as the configs of these apps have also gone through the same updates. We are looking into this and will let you know if we find anything relevant!

Thanks!

driskull commented 9 years ago

Ok thanks