-
Notifications
You must be signed in to change notification settings - Fork 59
Implement amulet form to action encoder #2471
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
Conversation
34be433 to
980d09c
Compare
980d09c to
5c01928
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.
Pull Request Overview
This PR implements an amulet form to action encoder that builds AmuletConfig objects from configuration changes. The implementation enables conversion of form data to the required configuration format for submission.
- Adds a utility function to construct AmuletConfig objects from ConfigChange arrays
- Updates the transfer fee steps indexing to be 1-based for consistency
- Integrates the new encoder into the SetAmuletConfigRulesForm for action creation
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| buildAmuletRulesConfigFromChanges.ts | New utility function that converts ConfigChange arrays to AmuletConfig objects |
| buildAmuletConfigChanges.ts | Fixes transfer fee steps indexing and adds required synchronizers handling |
| SetAmuletConfigRulesForm.tsx | Integrates the new encoder to create actions for form submission |
| buildAmuletRulesConfigFromChanges.test.tsx | Comprehensive test coverage for the new encoder utility |
apps/sv/frontend/src/__tests__/utils/buildAmuletRulesConfigFromChanges.test.tsx
Outdated
Show resolved
Hide resolved
5c01928 to
35d18eb
Compare
Fixes #2413 Signed-off-by: fayi-da <fayimora.femibalogun@digitalasset.com>
35d18eb to
ec7acf1
Compare
Signed-off-by: fayi-da <fayimora.femibalogun@digitalasset.com>
pawelperek-da
left a comment
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.
LGTM
| defaultValues, | ||
| onSubmit: async ({ value }) => { | ||
| onSubmit: async ({ value: formData }) => { | ||
| console.log('submit amulet config form data: ', formData); |
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.
Should we leave the logs?
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.
Yeah i'll remove it in the PR that implements the full submission flow
| }, | ||
| }, | ||
| }; | ||
| console.log('action for submission', action); |
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.
Should we leave the logs?
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.
Yeah i'll remove it in the PR that implements the full submission flow
|
|
||
| const allSynchronizers = [ | ||
| ...new Set([...beforeRequiredSynchronizers, ...afterRequiredSynchronizers]), | ||
| ].sort(); |
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.
.sort is fishy as it's modifying the original array, I'd rather use the immutable .toSorted in modern code but in this case it's not all that important
Fixes #2413
Pull Request Checklist
Cluster Testing
/cluster_teston this PR to request it, and ping someone with access to the DA-internal system to approve it./hdm_teston this PR to request it, and ping someone with access to the DA-internal system to approve it.PR Guidelines
Fixes #n, and mention issues worked on using#nMerge Guidelines