Fix traversal when parent model is loaded#853
Merged
Conversation
DarkSide666
requested changes
Apr 16, 2021
Member
DarkSide666
left a comment
There was a problem hiding this comment.
Overall I don't think this is correct approach.
Loading record shouldn't add condition to model because that will limit how getTitles(), export() and other dataset-wise methods will work when model is or is not loaded. That shouldn't happen.
f519577 to
14a1a34
Compare
Member
Author
|
All changes reverted and addressed differently, |
mvorisek
commented
Apr 19, 2021
| // add entity ID to scope to allow easy traversal | ||
| if ($model->id_field && $model->getId() !== null) { | ||
| $query->group($model->getField($model->id_field)); | ||
| $query->having($model->getField($model->id_field), $model->getId()); |
Member
Author
There was a problem hiding this comment.
FYI: having with group by is like where, but it does not apply after aggregate functions.
DarkSide666
approved these changes
Apr 19, 2021
Member
DarkSide666
left a comment
There was a problem hiding this comment.
Approved in advance. Need to fix this approach for MS SQL
d23156b to
83077c2
Compare
8c2ab17 to
02df81d
Compare
5588b33 to
6834cd4
Compare
cee8d38 to
bf48e54
Compare
3781e5f to
85c1345
Compare
602f778 to
a121105
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
finish after #855, #860, #861
discussion here: c8cbce3#r49718142
fixes https://github.com/mvorisek/atk4-hintable-mirror/blob/1.2.1/tests/Data/HintableModelTest.php#L120
must traverse from model with ID = 2 only (before this PR, ID = 2 condition was not applied and it was required to be added manually, now with entity concept we can assume locked ID)
note: this works also with
$model->load{One, Any}()->simpleManyand also withModel::duplicate()which will unset the ID correctly.