-
Notifications
You must be signed in to change notification settings - Fork 205
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
Conversation
There was a problem hiding this 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
...tor/backend/src/test/scala/com/digitalasset/navigator/backend/query/ContractFilterSpec.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments
navigator/backend/src/main/scala/com/digitalasset/navigator/json/ModelCodec.scala
Outdated
Show resolved
Hide resolved
navigator/backend/src/main/scala/com/digitalasset/navigator/graphql/GraphQLSchema.scala
Show resolved
Hide resolved
d6236a9
to
4d87bd4
Compare
@bethaitman: I added a few lines to the release notes. Could I ask you to take a look when you have time? |
@bitonic: I added the integration test. I'll soon be able to remove the draft status :) |
...uite/scala/com/digitalasset/platform/tests/integration/ledger/api/TransactionServiceIT.scala
Outdated
Show resolved
Hide resolved
...uite/scala/com/digitalasset/platform/tests/integration/ledger/api/TransactionServiceIT.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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.
11a465f
to
8fc1f7f
Compare
@rautenrieth-da: could you check the navigator changes again please? I should've fixed all your comments. |
navigator/backend/src/main/scala/com/digitalasset/navigator/json/ModelCodec.scala
Outdated
Show resolved
Hide resolved
ea3d0f7
to
011dcba
Compare
Co-Authored-By: Francesco Mazzoli <f@mazzo.li>
Co-Authored-By: Beth Aitman <bethaitman@users.noreply.github.com>
011dcba
to
fa40588
Compare
There was a problem hiding this 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!
agreement_text
field to theCreatedEvent
in Ledger API.ContractInst
to theCreatedEvent
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
totrigger the build.