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

Returns the Intent result #137

Merged
merged 2 commits into from
May 4, 2023
Merged

Conversation

symphony-jean-michael
Copy link
Contributor

@symphony-jean-michael symphony-jean-michael commented May 4, 2023

Changes

This PR adds some fixes to support GetResult from Intent of FDC3 v2.

  • I removed some mocked code (See 1st commit)
  • In some case, there was a race condition problem. The getIntentResult was called before the call to "setIntentResult". Consequently, the intent result was undefined. So, I made a change to return a promise that will only be resolved by setIntentResult. (See 2nd commit)

Demo

Before the fix

Before the fix the intent Result was empty

Screenshot 2023-05-04 at 14 10 29

And in the logs of Sail the intent result was returned before the call to setIntentResult

Screenshot 2023-05-04 at 14 10 48-2

With the fix

With the fix the Intent result is correctly returned

Screenshot 2023-05-04 at 13 44 08

And you can see that a promise is returned for getIntentResult
Screenshot 2023-05-04 at 13 44 30-2

@symphony-jean-michael symphony-jean-michael marked this pull request as ready for review May 4, 2023 12:16
@symphony-jean-michael
Copy link
Contributor Author

Hello @nkolba
I created this PR to support GetResult from Intent of FDC3 v2. It seems to me that maybe this can be linked to #73

Could you have a look on this PR ?

Thanks :)

@nkolba
Copy link
Contributor

nkolba commented May 4, 2023

thanks @symphony-jean-michael ! This looks great. Will merge

@nkolba nkolba merged commit 48566d8 into finos:main May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants