project-flogo / flogo-web

Project Flogo Web UI
http://flogo.io
BSD 3-Clause "New" or "Revised" License
63 stars 36 forks source link

object type in activity settings should not be nested under mapping #1349

Open yxuco opened 3 years ago

yxuco commented 3 years ago

Environment

How are you running Flogo Web?

Describe the bug

A clear and concise description of what the bug is. When a variable in activity settings is of type object or array, and configured as, e.g., { "key": "mykey", "attributes": ["x", "y"]}, this configuration is exported as nested under "mapping" in the app's json file. Since the settings are not object mappings, it should be exported as the configure object as is, similar to what it does for trigger handler settings.

How has this issue affected you? What are you trying to accomplish? In my implementation of an activity that requires more complex configurations than simple primitive type variables, I would have to manually remove the wrapper of "mapping" in code for any config variable of object/array type. This would not be necessary if Web UI can export the settings without the nested "mapping" wrapper.

Providing context helps us come up with a solution that is most useful in the real world

To Reproduce

Steps to reproduce the behavior:

  1. define a new activity, and in descriptor.json, add a variable of object or array type under settings.
  2. use the activity in a model, and configure the variable in WebUI
  3. export the model, you should see that the configured setting contains an extra wrapper under "mapping"
  4. If in the activity implementation, you use the common function call to extract the configured object, such as metadata.MapToStruct(ctx.Settings(), settings, true). The call will fail, because you have to manually remove the nested "mapping".

Include error logs if applicable.

Expected behavior

export the app model without the extra nesting of "mapping" for activity settings.

A clear and concise description of what you expected to happen.

Screenshots

If applicable, add screenshots to help explain your problem.

Possible Solution

Not obligatory, but suggest a fix/reason for the bug, or ideas how to implement the addition or change

Additional context

Add any other context about the problem here.

fcastill commented 3 years ago

The UI assumes that Flogo allows the use of mappings in the activity settings. It seems that's not case and the UI should prevent the "mapping" wrapper from happening. So definitely a bug.

The other issue here is that the UI is always wrapping objects with the "mapping" props in the inputs section. And although the wrapper doesn't hurt for inputs, it is unnecessary when the object is static. Fixing that will require some work in the mappings parser so the parser can expose when an object is static or not.

yxuco commented 3 years ago

I agree that the nesting of "mapping" does not seem necessary even for inputs. I noticed that the Flogo OSS and Enterprise export/import trigger/activity settings differently. OSS actually handles settings and handler-settings better than FE when configured attributes are JSON objects. FE cannot import complex trigger settings, FE also requires fe_metadata for complex activity settings, while fe_metadata is not a standard core metadata attribute. So, I had to write code to convert models between OSS and FE models to support both UIs. I hope that these 2 UIs can get in sync on handling JSON objects in trigger/activity configurations.

fcastill commented 3 years ago

As noted in Gitter, there are exceptions to the mappings in settings. The "return" type activities do support the mappings in settings so the UI should still support using those in settings for actreply, actreturn and mapper activities.