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

Feature/save agent form #119

Merged
merged 28 commits into from
Sep 18, 2024
Merged

Feature/save agent form #119

merged 28 commits into from
Sep 18, 2024

Conversation

lukavdplas
Copy link
Contributor

@lukavdplas lukavdplas commented Aug 30, 2024

Syncs changes in the agent form with the backend.

This also includes a FormService and FormStatusComponent to show the user whether changes are saved - those can be reused in other forms.

Part of #78

@lukavdplas lukavdplas marked this pull request as ready for review September 9, 2024 14:01
Copy link
Contributor

@XanderVertegaal XanderVertegaal left a comment

Choose a reason for hiding this comment

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

Works very well! I'll be sure to implement this in my other PRs as well.

I have a few suggestions that are mostly based on personal preferences, so have a look whether you want to implement any. The only change I'd really like you to consider is the outfactoring of Gender and SourceMention into two enums that are reused across ObjectTypes and InputTypes, if you haven't done so in another PR yet.

return this.form.valid;
}

private toMutationInput([data, id]: [Partial<FormData>, string]): UpdateAgentInput {
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to get rid of the Partial (since you are not disabling controls anyway, so all should always be present), you could add form.getRawValue() to the pipe of changes$. Not required/necessary by any means.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a non-issue here because there are not disabled controls, but in general, if you did disable controls, I would expect that those values are not specified in the update request.

@lukavdplas lukavdplas merged commit ed377d9 into develop Sep 18, 2024
2 checks passed
@lukavdplas lukavdplas deleted the feature/save-agent-form branch September 18, 2024 13:47
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.

2 participants