Syndesis: DataMapper : inconsistent field mapping visiualization

[lburgazzoli]

This is a...

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

The problem

I define the mapping between two large objects:

• a SalesForce Case object
• a ServiceNow Incident object

When defining the map, everything work as expected but when I'm getting back to my integration to change the mapping, the lines connecting the fields to map are inconsistent.

Expected behavior

Screenshot

Tasks involved / Steps to Reproduce

1. Create an integration between SalesForce and ServiceNow
2. Map Case to Incident
3. Publish the integration
4. Edit the integration

[igarashitm]

If runtime works correctly, it would be a line drawing issue in AtlasMap UI and might be related to atlasmap/atlasmap#565.
How can I create ServiceNow connection in the Syndesis to reproduce this? How can I get the credentials?

[gashcrumb]

Maybe it's worth creating a couple test connector extensions with some large objects to reproduce this, probably would be faster than getting ServiceNow access.

[lburgazzoli]

I can create a new account on our test instance but you may have a faster dev experience if you create a data shape with a large number of fields.

[igarashitm]

So far it doesn't reproduce with standalone AtlasMap. I would have to look in Syndesis anyway. It'll take a bit of time to understand how to create the extension...

[igarashitm]

@pleacu any guess? in Syndesis mapping definition is passed as a property and directly processed via MappingSerializer.deserializeMappingServiceJSON() here:
https://github.com/syndesisio/syndesis/blob/master/app/ui/src/app/integration/edit-page/step-configure/data-mapper/data-mapper-host.component.ts#L301-L315

[gashcrumb]

It could also be kinda related to the issue I'm fixing for https://github.com/syndesisio/syndesis/issues/3789 maybe? At least once I get a PR in for it it might be worth a test as it kinda tidies up how the view containing the mapper host component renders.

[igarashitm]

Even so still curious if there's some room to add safe guard from AtlasMap side instead of just showing broken lines.

[gashcrumb]

Yep, probably still worth investigating. That PR was merged so the changes are available on master.

Wonder, if it's a rendering problem, have we even just verified if it shows up or not with just twitter to salesforce perhaps? I know I haven't personally put that integration together in quite awhile...

[igarashitm]

This line drawing is the most dirty place in AtlasMap UI, it's not surprising if we break it again :-D

[igarashitm]

I managed to restore twitter/salesforce credentials... yeah that reproduced with twitter>salesforce integration. It seems lines are not redrawn unless some mapping is selected. Once you select one of fields that is involved in mapping, then all lines are redrawn. I don't think this is p0 btw.

[igarashitm]

It seems this only happens within Syndesis. Those inactive lines are drawn correctly from beginning even after reload or import in standalone. No idea why... could it be caused by the dependency difference, like Angular 6.0 vs. 6.1, etc?

[gashcrumb]

I'll give it a test tomorrow and have a look, agreed it's probably not p0.

[lburgazzoli]

I would say that it is not a p0 for technical reason but - imho - it is definitively a p0 for UX as the mapping step would be perceived as buggy even it is just a UI glitch.

[gashcrumb]

@igarashitm wonder if it's anything to do with how the initialization logic is done in the host controller, there's a call to re-initialize the data mapper angular services in 2 spots, one in the constructor of the host component and another that gets kicked off via ngOnInit(), I kinda think we can get rid of the one in the constructor.

Also I notice that if I just switch to the table of mappings and then back the lines behave okay.

Lemme just re-understand what's going on in the host component and see if we can tidy that up a bit :-)

[gashcrumb]

Also I notice that if I just switch to the table of mappings and then back the lines behave okay.

Nope, I lied there, they don't behave correctly :-)

[gashcrumb]

@igarashitm so adding this to the host component makes it behave better:

by manually triggering the event that the line machine component seems to act on for redraws. The setTimeout causes the code to execute outside of the current change detection cycle and then another change detection cycle runs. I'm not sure it's exactly the best fix though, what would normally cause that event to fire from the normal initialization path?

Worst case we could keep it for now but I do like to try and find alternatives :-) I wonder if this maybe came from reworking how the syndesis loading component there works perhaps, that kinda sped up a fair few pages and removed a lot of random code executing when it shouldn't have been.

[igarashitm]

@gashcrumb yeah it indeed solves the issue. I actually tried calling this.cfg.mappingService.notifyMappingUpdated() in the setTimeout() and it works. But it doesn't work without setTimeout().

In standalone, notifyMappingUpdated() is invoked via this handleMappingSaveSuccess()
https://github.com/atlasmap/atlasmap/blob/master/ui/src/app/lib/atlasmap-data-mapper/components/data-mapper-example-host.component.ts#L139

[gashcrumb]

Maybe we should keep it then? I'd definitely add a comment to explain why
the setTimeout is needed.

On Tue, Oct 16, 2018, 5:44 PM Tomohisa Igarashi [e-mail]
wrote:

@gashcrumb https://github.com/gashcrumb yeah it indeed solves the
issue. I actually tried calling
this.cfg.mappingService.notifyMappingUpdated() in the setTimeout() and it
works. But it doesn't work without setTimeout().

In standalone, notifyMappingUpdated() is invoked via this
handleMappingSaveSuccess()`

https://github.com/atlasmap/atlasmap/blob/master/ui/src/app/lib/atlasmap-data-mapper/components/data-mapper-example-host.component.ts#L139

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/syndesisio/syndesis/issues/3833#issuecomment-430412044,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAVdrAiJSRjbIrDJ85sRUXZj7GrE-EEjks5ullM5gaJpZM4XcAdP
.

[igarashitm]

+1, at least until we find better solution

[gashcrumb]

Sounds good, I'll submit a PR with that change in syndesis tomorrow morning-ish

[igarashitm]

For long term we'll need to clean up these messy state management in AtlasMap UI, maybe when we migrate to React.

[tplevko]

@mmelko please verify the fix