Skip to content

Conversation

minottic
Copy link
Member

@minottic minottic commented Sep 26, 2025

Description

Reuse findOneComplete and findOneAll for v3 endpoints to enable scopes.

Motivation

Reintroduce loopback3 features to v3 endpoints (removing the support is non backward compatible change), by few findAllComplet branching

Changes:

  • findAll, findById and findOne in v3 all use the same function now. Avoids dup, shared with v4, same expected behaviour for the user
  • richer swagger for v3 endpoints
  • v3 findById supports filtering now
  • types extended to union v3 and v4

Tests included

  • Included for each change/fix?
  • Passing?

Documentation

  • swagger documentation updated (required for API changes)
  • official documentation updated

official documentation info

@minottic
Copy link
Member Author

depends on #2236 and it's opened against that branch. Will rebase after merge

@minottic minottic marked this pull request as ready for review September 26, 2025 11:29
@minottic minottic requested a review from a team as a code owner September 26, 2025 11:29
Copy link
Member

@nitrosx nitrosx left a comment

Choose a reason for hiding this comment

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

IMHO, using hardcoded "v3" and "v4" values in the functions calls will make the code less transparent, hard to manage and harder to separate by versions later on.
I would like to propose the creation of separated datasets.service.ts for v3 and v4.

@minottic
Copy link
Member Author

fine for me, so these services will then do the branching in your proposal?

@nitrosx
Copy link
Member

nitrosx commented Oct 13, 2025

@minottic if I understand your question correctly, I would create a service v3 which will be used by the all the endpoints of dataset api v3 and a service v4 which will be used by all the dataset endpoints v4.
It will introduce some code duplication for now, but it will make the code easier to read and it will be easier to remove obsolete V3 in the future

@minottic
Copy link
Member Author

ok, I will make a new commit for that

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