Skip to content

[5.x] Replace UsesCallbackRelations with afterQuery#1219

Open
Jade-GG wants to merge 7 commits intomasterfrom
feature/remove-callback-relations
Open

[5.x] Replace UsesCallbackRelations with afterQuery#1219
Jade-GG wants to merge 7 commits intomasterfrom
feature/remove-callback-relations

Conversation

@Jade-GG
Copy link
Collaborator

@Jade-GG Jade-GG commented Mar 4, 2026

ref: RAP-1811

Since Laravel v12.50.0 we can once again use afterQuery on a relation without the keys being reset (thanks to a fix made by this weird jade-gg person). This means that we don't need to use the UsesCallbackRelations trait (along with the custom relations) anymore.

This also means I was able to remove one of our TODOs relating to manually keying the prices relation after the fact during indexing.

Obviously this pins the minimum Laravel version to ^12.50.

@Jade-GG
Copy link
Collaborator Author

Jade-GG commented Mar 4, 2026

Neat, the prefer-lowest tests are failing because they seem to ignore the version constraint.

royduin
royduin previously approved these changes Mar 4, 2026
@royduin
Copy link
Member

royduin commented Mar 4, 2026

Seems like all the product prices are gone?

@Jade-GG Jade-GG force-pushed the feature/remove-callback-relations branch from c6b6ed0 to d697478 Compare March 5, 2026 09:58
@Jade-GG
Copy link
Collaborator Author

Jade-GG commented Mar 5, 2026

Seems like all the product prices are gone?

As it turns out, we can't use non-unique keys for the afterQuery keyBy, because those will (in hindsight: obviously) overwrite each other when you're doing a query with more than one product at the same time.

This affected superAttributes and prices. So I've reverted how prices works and simplified how super attributes work in the cart, as the actual attribute id is only necessary when specifically adding the product to the cart.

@Jade-GG Jade-GG force-pushed the feature/remove-callback-relations branch from 7e6ee8c to 26eba45 Compare March 5, 2026 10:23
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