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

feat: add support for custom id selector in dataservice entities #69

Conversation

alcaidio
Copy link

Introduced a mechanism to specify a custom id selector for entities in the dataservice. (like for ngrx entities)
When an entity does not have a default identifier named 'id', the custom selector can be used.
The custom id selector must return either a string or a number. enhances flexibility for handling entities with non-standard identifiers in the dataservice.

I'm not sure about what I did; this is my first PR here, and I hope I'm not too far from the solution. Looking forward to discussing any potential improvements.

@alcaidio
Copy link
Author

I dont know why my PR has 9 file changed ? I only change one file and create one under libs directory??

@rainerhahnekamp
Copy link
Collaborator

rainerhahnekamp commented Jul 20, 2024

@alcaidio, thanks but can you please add .nx/workspace-data to .gitignore?

@alcaidio alcaidio force-pushed the feature/add-custom-id-selector-to-data-service-entity branch 4 times, most recently from fb68539 to 8dbf3e1 Compare July 20, 2024 21:57
@rainerhahnekamp
Copy link
Collaborator

Hey @alcaidio, thanks also for the other PR, but I suggest we make sure that this one gets merged first before continuing.

So you still have the nx/workspace in the PR and the package-lock.json. Both shouldn't be there. Can you please fix that?

Btw, thanks for that unit test. It was urgently needed...

@alcaidio alcaidio force-pushed the feature/add-custom-id-selector-to-data-service-entity branch 2 times, most recently from e9ff269 to 9041b4e Compare July 24, 2024 07:18
@alcaidio
Copy link
Author

I've updated my PR @rainerhahnekamp , thanks for your work :)
I see the withPagination feature it didn't see it in the readme, is that on purpose?

@rainerhahnekamp
Copy link
Collaborator

Thanks. It is not on purpose that withPagination is missing in the docs :)

@rainerhahnekamp
Copy link
Collaborator

Hey @alcaidio,

sorry to bother you again:

  • The CI is failing because of an issue which has been fixed in the meantime. Please rebase
  • You have an any in some of your methods. Can you replace it?
  • Unfortunately, we don't have any tests for withDataService. You are not introducing here a breaking change, do you?

@alcaidio alcaidio force-pushed the feature/add-custom-id-selector-to-data-service-entity branch from 9041b4e to ce54c9e Compare July 24, 2024 21:23
introduced a mechanism to specify a custom id selector for entities in the dataservice. when an entity does not have a default identifier named 'id', the custom selector will be used. the custom id selector must return either a string or a number. enhances flexibility for handling entities with non-standard identifiers in the dataservice.
@alcaidio alcaidio force-pushed the feature/add-custom-id-selector-to-data-service-entity branch from ce54c9e to 2a46305 Compare July 24, 2024 21:28
@alcaidio
Copy link
Author

I don't know how to solve the problem, I've updated my branch and I can't get the tests to work :/ sorry, it's too complicated for me, I'll come back to it if I have time. thanks anyway.

@alcaidio alcaidio closed this Jul 26, 2024
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