Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[api provider - collections] UX Design - API Provider Collection Support #4312

Closed
dongniwang opened this issue Jan 18, 2019 · 39 comments
Closed
Assignees
Labels
group/uxd User experience (UX) designs notif/uxd Ping UX team that UX related changes are involved

Comments

@dongniwang
Copy link
Contributor

This is a...


[x] Feature request
[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Documentation issue or request

Description

Per request in #4291, opening this issue to track UX design for API Provider Collection Support.

The design will reflect:

  • Updates to the connection configuration page - adding an option to treat the produced data as a collection
  • Changes to step list - adding "For Each" step
  • Changes to IVP once the foreach step is added

Also need help and suggestion from @TovaCohen regarding the desired wording: open/closed foreach vs. open/end foreach.

cc: @lburgazzoli @christophd @amysueg @gaughan

@pure-bot pure-bot bot added the notif/triage The issue needs triage. Applied automatically to all new issues. label Jan 18, 2019
@dongniwang dongniwang self-assigned this Jan 18, 2019
@lburgazzoli
Copy link
Collaborator

lburgazzoli commented Jan 24, 2019

@dongniwang @gaughan

I was thinking if it really make sense to ad an option to treat the produced data as a collection or if it could be better if the back-end translates old style integration to the new style so:

  • the back-end will support old style integrations so if an old integration is started without modifications, it will work as today
  • the back-end translates the implicit split to the foreac/endforach when serving the data to the UI (similar to what happens to integration not using flows) so when the user saves the integration, such integration gets automatically upgraded

This has the advantage that it make components more consistent as the data shape is defined so they will have a single way of working and is - imho - less confusing for the users.

What do you think ?

@christophd @zregvart @heiko-braun

@christophd
Copy link
Contributor

If we intend to remove the implicit split capabilities completely from Syndesis in future I think the automatic translation is a good option as the user then will not see any split yes/no options on the connector and we should hide those because at some time we will remove that implicit split completely.

In case we would like to keep the implicit split capability for some reason I think the option split yes/no on the connector is a good way to go.

If we do the automatic translation to new style integrations the foreach steps just pop up all of a sudden when users open/edit old integrations. Compared to that an additional split yes/no option that is introduced on connectors is less disturbing I would say.

I think we should decide based on keeping the implicit split functionality or not.

@lburgazzoli
Copy link
Collaborator

lburgazzoli commented Jan 24, 2019 via email

@gaughan
Copy link

gaughan commented Jan 24, 2019

:) nice offload Luca!
I agree with the approach, it will be a new experience to have a step automatically be added but hands the user control now.
Moving datashape manipulation to a step seems more natural than a connection setting too.

@zregvart
Copy link
Member

Can we do this in the (database) migrations?

@lburgazzoli
Copy link
Collaborator

It is not needed but can be done there too (if really needed).

@zregvart
Copy link
Member

I kinda think that migrations, being a one-off thing, would reduce the tech debt. Though I would not insist on it, could be difficult to write/test, it is JavaScript... (sarcasm, right)

@lburgazzoli
Copy link
Collaborator

lburgazzoli commented Jan 24, 2019 via email

@zregvart
Copy link
Member

@lburgazzoli
Copy link
Collaborator

ah great, so let me open an issue for that, thx

@lburgazzoli
Copy link
Collaborator

@dongniwang @gaughan

Additional food for thought for UX / PM

Assuming we have a simple integration that just logs records from the db:

image

If we add forEach/endForEach block as a single "step" then the integration should become:

image

Which means that the log step is not more the final one, which introduces a new behavior as then one should be able to add a step after the endForEach

Things are going to be different for an API Provider

image

As in this case the final step is fixed

@gaughan
Copy link

gaughan commented Jan 25, 2019

👍
@dongniwang Look good to you?

@christophd
Copy link
Contributor

At the moment we can not change the order of steps once they have been added. Also we can not place new steps after the finishing step.

What does this mean in terms of how to add a foreach block? I see two options:

Option 1:

Treat foreach as a single step that adds both start and end with one single user action. The user then is able to add new steps to the foreach (= placing new steps between start and end)

Option2:

See start and end as separate steps that need to be added separately to the integration. So user can add a foreach start step at any place in the integration and then needs to add a foreach end somewhere else. Maybe also leave out the end step so the rest of the integration is automatically inside the foreach block

@christophd
Copy link
Contributor

In my demo I used option 2 where I added foreach start and stop as separate steps. So this would imply less changes in the UI I guess at it is working with what we have right now. Only thing we might need to add is the ability to add the endForeach step as a finishing step after an existing finishing step.

I think at the moment only connector steps can be placed as a finishing step and this has to be done at the very beginning of designing an integration. Basically we have to first specify the starting and finishing steps when creating an integration.

@gashcrumb
Copy link
Contributor

I think having another + after the last step and having a step instead of a connection would would be doable with #4322 implemented.

I wonder, do we technically really even need a last step/connection? We force it now but I don't recall the reasoning other than ensuring that an integration actually does something I guess.

@gashcrumb
Copy link
Contributor

Ah, I think it was because of the data mapper, i.e. you can't really add a data mapper step until you have 2 things to map between. But, we could just hide the data mapper step in that case easily enough.

@heiko-braun
Copy link
Collaborator

I think option 1 is more comprehensible – treat adding start and end as an atomic operation (in terms of the interaction model). Same for removal. If we keep them separate, it might lead to inconsistent flows.

@gashcrumb
Copy link
Contributor

Only problem I could foresee with option 1 is allowing the last step to be inside the foreach where it then operates on single items vs the collection, unless we also stop forcing the user to pick an end step.

@zregvart
Copy link
Member

I'd go with option 2, and with different visualisation than other steps. Perhaps something looking like a bracket encompassing steps within.

@christophd
Copy link
Contributor

@gashcrumb technically we do not need the end step (we did not have it before with the implicit split) but if we have it we are much more flexible in what we can do in an integration and we definitely need it for all those use cases where we need to do some things after a foreach

@christophd
Copy link
Contributor

Adding the endForeach step to an integration also has impact on follow-up data-mapping steps as these should not see the output messages of foreach internal steps but only the overall foreach out message. see #4355

@heiko-braun
Copy link
Collaborator

heiko-braun commented Jan 25, 2019

Adding the endForeach step to an integration also has impact on follow-up data-mapping steps as these should not see the output messages of foreach internal steps but only the overall foreach out message. see #4355

Does that explain why we should use option 1 or 2?

@christophd
Copy link
Contributor

Does that explain why we should use option 1 or 2?

Not really - it just points out that we need the endForeach

@christophd
Copy link
Contributor

I think option 1 & 2 totally influence the way how people are writing integrations. With option 1 you will have to add foreach first and then afterwards adding more steps into that. With Option 2 you can do that same thing plus add a foreach around some existing steps that are already there.

@gaughan
Copy link

gaughan commented Jan 25, 2019

I like option 2. We should rrename here to split and merge also. Spilt could later be expanded to add more functionality like spilt the first 3 records etc.

@dongniwang
Copy link
Contributor Author

Just wanted to capture the discussion with @gaughan and @amysueg - it seems like we can go ahead with two steps: a split step, and a merge step, given the fact that in the future we'll want a fully functional split step. And to support the current use case of api provider collection support, we are supplying users with a split step with limited functionality. Does this make sense?

Also to note, it's a UX challenge to remind users to add the merge step once they added a split step when creating an API Provider integration...but we can certainly explore a solution for this challenge.

@gashcrumb
Copy link
Contributor

@gaughan @dongniwang Please consider join instead of merge, I think it's a more accurate word in this context really.

@amysueg
Copy link

amysueg commented Jan 25, 2019

@gashcrumb as @gaughan pointed out "Join" has SQL connotations... that is why we are recommending merge, a little more neutral.

@lburgazzoli
Copy link
Collaborator

lburgazzoli commented Jan 25, 2019 via email

@christophd
Copy link
Contributor

Why not using the integration pattern name "aggregate". As the aggregate pattern per definition can also live alone in an integration we might enhance that step at some time in future to also work without a previous split.

For instance there could be an integration collecting a fixed number of events from an event stream and then fire up a new event or do something else when the aggregate is completed.

We could reuse the aggregate step that we introduce right now for this in future.

@TovaCohen
Copy link
Collaborator

Are "Iteration" and "Collection" worth considering for the step names? The current step names are nouns. The descriptions have the verbs. In context, this would look like:

Advanced Filter Continue the integration only if criteria you define in scripting language expressions are met.
Basic Filter Continue the integration only if criteria you specify in simple input fields are met.
Collection Combine integration data and execute subsequent integration steps once.
Data Mapper Map fields from the input type to the output type.
Iteration Split integration data and execute subsequent integration steps once for each result.
Log Send a message to the integration's log.
Template Upload or create a Freemarker, Mustache or Velocity template to define consistent output data.

@heiko-braun heiko-braun added group/uxd User experience (UX) designs notif/uxd Ping UX team that UX related changes are involved labels Jan 28, 2019
@heiko-braun heiko-braun added this to the Sprint 41 (2/4) milestone Jan 28, 2019
@gaughan
Copy link

gaughan commented Jan 28, 2019

@gashcrumb using join to me, may lead the user to think they are performing a SQL join. wdyt? Future DV functionality will only add to this.
@christophd +1 on aggregate, especially on expanding in the future.
@TovaCohen I think the name has to be strongest as user tend to gloss over the description, returning to read in detail only when they encounter unexpected behavior.

@lburgazzoli
Copy link
Collaborator

@dongniwang what is the status for this ?

@dongniwang
Copy link
Contributor Author

The design is ready for review. Please provide your comment in InVision. Let me know if you need to set up a call to walk through the design. Thanks! https://redhat.invisionapp.com/share/JNQ9E1A4AVQ

@TovaCohen - Please review the wording and provide your feedback. Thank you!

cc: @lburgazzoli @christophd @gaughan

@TovaCohen
Copy link
Collaborator

I added comments to the design on pages 3, 8, 11, 12, 16, 24. Just suggesting text changes.

@lburgazzoli
Copy link
Collaborator

@paoloantinori @gashcrumb please have a look

@gashcrumb
Copy link
Contributor

I had a look, I think it's fine. I suspect some elements of the design may not make the initial effort (the bits resembling the guided tour mostly) but it gives us a direction.

@dongniwang
Copy link
Contributor Author

I had a look, I think it's fine. I suspect some elements of the design may not make the initial effort (the bits resembling the guided tour mostly) but it gives us a direction.

@gashcrumb - that's fine. Alternatively, we can just use the modal dialog and have the modal on the homepage when users first log on to the app after the release.

@heiko-braun heiko-braun removed notif/triage The issue needs triage. Applied automatically to all new issues. labels Jan 31, 2019
@amysueg
Copy link

amysueg commented Feb 18, 2019

The design for API Provider collection support (split/aggregate) is complete, see design tracker: https://syndesisio.github.io/syndesis-ux/

Closing this issue.

@christophd Please feel free to open additional UX issues.

@amysueg amysueg closed this as completed Feb 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
group/uxd User experience (UX) designs notif/uxd Ping UX team that UX related changes are involved
Projects
None yet
Development

No branches or pull requests

9 participants