Skip to content

feat(mapper): allow to merge existing values by extracting identifiers #260

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

joelwurtz
Copy link
Member

This introduce the possibility to deep merge object with collections and fetching the correct value to update it instead of adding a new one into the collection

@joelwurtz joelwurtz force-pushed the feat/merge-existing-value-with-identifier branch 3 times, most recently from 745cb5b to 12a3e45 Compare April 6, 2025 17:26
@joelwurtz joelwurtz requested a review from Korbeil April 6, 2025 21:02
Copy link
Member

@Korbeil Korbeil left a comment

Choose a reason for hiding this comment

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

I do think it adds a lot of complexity to Array transformers (from/to), would it be possible to add some kind of abstraction to separate the pure mapping part & the identifier/existing values handling in separate class / traits or whatever ?

@joelwurtz joelwurtz force-pushed the feat/merge-existing-value-with-identifier branch from 12a3e45 to 1151b7f Compare April 16, 2025 10:19
@joelwurtz joelwurtz force-pushed the feat/merge-existing-value-with-identifier branch from 1151b7f to 98598c6 Compare April 16, 2025 15:48
@joelwurtz
Copy link
Member Author

I do think it adds a lot of complexity to Array transformers (from/to), would it be possible to add some kind of abstraction to separate the pure mapping part & the identifier/existing values handling in separate class / traits or whatever ?

Yeah but all of this is internal and it's only on implementation, i don't want to over abstract thing, as it may changes and other bugs may appear, we can always change it latter since it's internal, i first want feedback on using this feature before abstracting the code and public api is minimal on this (so it would not impact anything)

@joelwurtz joelwurtz force-pushed the feat/merge-existing-value-with-identifier branch from 98598c6 to 7355357 Compare April 16, 2025 15:52
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants