Skip to content

feat: improve forgeStageTwoQuery to allow select/omit for singular model populate#1627

Open
DominusKelvin wants to merge 5 commits intobalderdashy:masterfrom
sailscastshq:feat/model-populate-select-omit
Open

feat: improve forgeStageTwoQuery to allow select/omit for singular model populate#1627
DominusKelvin wants to merge 5 commits intobalderdashy:masterfrom
sailscastshq:feat/model-populate-select-omit

Conversation

@DominusKelvin
Copy link

@DominusKelvin DominusKelvin commented Apr 11, 2025

Resolves balderdashy/sails#4667

I also account for the normal behaviour of Waterline projections not allowing both select and omit

CleanShot 2025-04-11 at 14 58 44@2x

Now this sort of subcriteria will be treated as valid subcriteria for populating associations

 const tickets = await Ticket.find({
      where: { event: event.id, status: { '!=': 'cancelled' } },
      sort: 'ticketHolderEmail DESC',
      select: ['ticketHolderEmail', 'status', 'publicId']
    })
      .populate('ticketType', { select: ['name'] })
      .populate('order')

@DominusKelvin DominusKelvin changed the title feat: improve forgeStageTwoQuery to allow select/omit as for singular model populate feat: improve forgeStageTwoQuery to allow select/omit for singular model populate Apr 11, 2025
@luislobo
Copy link
Contributor

luislobo commented Jul 7, 2025

Good improvement! It would be good to have unit tests

@DominusKelvin
Copy link
Author

Yes @luislobo

@DominusKelvin
Copy link
Author

Hey @luislobo. I just added three test cases for this.

@luislobo
Copy link
Contributor

@DominusKelvin great! I added some comments

- Add test for populate with select subcriteria for singular associations
- Add test for populate with omit subcriteria for singular associations
- Add test for populate with empty object (converts to true)
- Add test for error when both select and omit are provided together

These tests validate the new functionality that allows select/omit
subcriteria for singular (model) associations while ensuring proper
validation that prevents using both select and omit simultaneously.
- Change omit test to use 'car' field instead of 'id' (more realistic)
- Focus tests on validation logic rather than actual field filtering
- Ensure tests validate that select/omit subcriteria are accepted
  without throwing validation errors

The tests verify the core functionality works while being more
realistic about what fields can typically be omitted in an ORM.
@luislobo
Copy link
Contributor

I've added more comments, there is actually an issue on "select"

@DominusKelvin
Copy link
Author

I've added more comments, there is actually an issue on "select"

Hey @luislobo what's the issue you saw?

@luislobo
Copy link
Contributor

your changes do not do what the issue is asking: omit does not omit and select does not select, because the current waterline implementation does not process select and omit on populates, it only does it on the main model's select and omit. Your change just lets those fields to be validated in the configruation. If you add the tests that I mentioned on the code, you will see what I mean.

@luislobo
Copy link
Contributor

BTW, I do have an implementation for this version of Waterline that implements this, I could create a separate PR that does it. But since I'm not using this version of Waterline yet, I didn't create a PR.

@DominusKelvin
Copy link
Author

BTW, I do have an implementation for this version of Waterline that implements this, I could create a separate PR that does it. But since I'm not using this version of Waterline yet, I didn't create a PR.

Oh really I did test it out on my current Sails and Waterline versions

I'll test it out again to confirm

@luislobo
Copy link
Contributor

Here's the PR that takes care of the select/omit on propulated entities #1628

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

.populate('petType', {select: ['species']}) ("Ambiguous usage with singular model association")

2 participants