Exercise by key #4049
Exercise by key #4049
Conversation
leo-da
force-pushed
the
leo-4009-exercise-by-key
branch
from
dc15468 to
93966b0
Compare
|
|
There was a problem hiding this comment.
LGTM modulo forthcoming docs, but @hurryabit may prefer that reference's JSON format always be flattened within the containing object, which is unambiguous and also backward-compatible with the current API.
Yep, I thought about keeping the existing format. But decided to change it so it is consistent with our domain model. However if @hurryabit insists, |
| @@ -189,8 +189,10 @@ class Ledger { | |||
| */ | |||
| async exercise<T extends object, C, R>(choice: Choice<T, C, R>, contractId: ContractId<T>, argument: C): Promise<[R , Event<object>[]]> { | |||
| const payload = { | |||
| templateId: choice.template().templateId, | |||
| contractId, | |||
| reference: { | |||
There was a problem hiding this comment.
Why? I think the old API was fine.
There was a problem hiding this comment.
because reference can be of ContractLocator type:
sealed abstract class ContractLocator[+LfV] extends Product with Serializable
final case class EnrichedContractKey[+LfV](
templateId: TemplateId.OptionalPkg,
key: LfV
) extends ContractLocator[LfV]
final case class EnrichedContractId(
templateId: Option[TemplateId.OptionalPkg],
contractId: domain.ContractId
) extends ContractLocator[Nothing]
There was a problem hiding this comment.
you see it groups templateId with key and optional templateId with contractId. That is what I currently have in my domain model, which I am not going to flatten that is for sure (I assumed you wanted to have something similar in TypeScript)... JSON can be flattened, but I think reference field kind of makes sense. Again up to you.
There was a problem hiding this comment.
plus I hate writing JSON formatters manually :), if we have a reference field, the formatter can be completely derived.
ExerciseCommand got a new required element: `reference` of polymorphic type.
CHANGELOG_BEGIN - [JSON API - Experimental] Support Exercise by Key. See #4009. CHANGELOG_END
leo-da
force-pushed
the
leo-4009-exercise-by-key
branch
from
db78b00 to
7053493
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| fields += "choice" -> obj.choice.toJson | ||
| fields += "argument" -> obj.argument.toJson | ||
| if (obj.meta.isDefined) | ||
| fields += "meta" -> obj.meta.toJson |
There was a problem hiding this comment.
There's no value here from not building fields in the functional style; use ++.
There was a problem hiding this comment.
The API and the docs look good to me. Many thanks.
I'll leave the review of the implementation and the approval to @S11001001.
Closes: #4009
TODO
referencefield, actually flatten it.referencefield is removed.Pull Request Checklist
CHANGELOG_BEGINandCHANGELOG_ENDtagsNOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with
/AzurePipelines runtotrigger the build.