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

Expose a contract's agreement text on the Ledger API #1151

Merged
merged 15 commits into from
May 17, 2019

Conversation

gerolf-da
Copy link
Contributor

@gerolf-da gerolf-da commented May 15, 2019

  • Added agreement_text field to the CreatedEvent in Ledger API.
  • Changed java bindings + java codegen
  • Changed utilities for scala codegen
  • Made necessary changes in Sandbox to propagate the agreement text from ContractInst to the CreatedEvent
  • Made changes to the navigator to show the agreement text in the contract details page when it is set and not empty

Fixes #1110

Pull Request Checklist

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

Copy link
Contributor

@bitonic bitonic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there seem to be no ledger-api-integration-test testing this -- please add

Copy link
Contributor

@rautenrieth-da rautenrieth-da left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments

@gerolf-da
Copy link
Contributor Author

@bethaitman: I added a few lines to the release notes. Could I ask you to take a look when you have time?

@gerolf-da
Copy link
Contributor Author

@bitonic: I added the integration test.

I'll soon be able to remove the draft status :)

Copy link
Contributor

@bethaitman bethaitman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Release notes look excellent. I've left a couple of small comments and suggestions, but this is good - really comprehensive.

@gerolf-da gerolf-da force-pushed the 1110-agreements-on-ledger-api branch from 11a465f to 8fc1f7f Compare May 16, 2019 08:22
@gerolf-da gerolf-da marked this pull request as ready for review May 16, 2019 08:23
@gerolf-da
Copy link
Contributor Author

@rautenrieth-da: could you check the navigator changes again please? I should've fixed all your comments.

@gerolf-da gerolf-da requested a review from rautenrieth-da May 16, 2019 11:49
@gerolf-da gerolf-da force-pushed the 1110-agreements-on-ledger-api branch from ea3d0f7 to 011dcba Compare May 16, 2019 15:10
@gerolf-da gerolf-da force-pushed the 1110-agreements-on-ledger-api branch from 011dcba to fa40588 Compare May 17, 2019 06:35
Copy link
Contributor

@rautenrieth-da rautenrieth-da left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Navigator changes look good!

@gerolf-da gerolf-da merged commit c645348 into master May 17, 2019
@gerolf-da gerolf-da deleted the 1110-agreements-on-ledger-api branch May 17, 2019 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/java-ecosystem Java development experience component/ledger Sandbox and Ledger API component/navigator component/scala-ecosystem Scala development experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extracting the contents of the agreement field of a contract instance through the Sandbox API
5 participants