Repository navigation
fix: five small ORM fixes (limit, having, restore, find, select doc) - #35
Merged
Merged
Conversation
limit(-1) sent LIMIT -1, which is an error on Postgres. Now a negative value is ignored, like Laravel.
having('x', '!=', null) built x = '!=', because a null third arg was read as no arg. It now uses the same missing-arg marker and null check as where().
Blog().withTrash().where(...).restore() ran WHERE id IS NULL, changed nothing and returned true. Now it sets deleted_at to null on the rows of the query. With no where() it throws a StateError, like delete(). restore() on a loaded model is not changed.
A null second arg meant id only, so find('email', null) searched id = 'email'. Now a private missing-arg marker tells no arg from null. find(5) is not changed. findOrFail() gets the same fix.
The comment said a second select() adds columns. It replaces them, like Laravel. Only the comment changed.
Contributor
Author
Bot reviewVerdict: changes requested. The code is good. One thing must be fixed: the changelog. Blocking
What I checked (all fine)
Nits (no change needed)
|
Each Unreleased line of this PR was added more than once. Keep one line per item.
Contributor
Author
Fixer reply
|
Contributor
Author
Bot reviewLast commit eb229f7 only changes packages/dox-orm/CHANGELOG.md. It removes 10 duplicated lines.
Verdict: OK. |
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
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.
Five small ORM fixes. One commit per item.
limit(-1)sentLIMIT -1. Postgres gives an error. Now a negativelimit()ortake()is ignored, like Laravel.having('x', '!=', null)builtx = '!='. Now it buildsx IS NOT NULL, and=null buildsx IS NULL. It uses the same missing-arg marker and null check aswhere().Blog().withTrash().where(...).restore()ranWHERE id IS NULL. It changed nothing and returnedtrue. Now it setsdeleted_atto null on the rows of the query, and returnstrueonly when a row changed. With nowhere()it throws aStateError, likedelete().restore()on a loaded model is not changed.id = 'email'. Now a private "not given" marker tells no arg from null, so it findsemail IS NULL.find(5)is not changed.findOrFail()gets the same fix.Changelog lines are under
## Unreleased. No version bump.Tests
test/bug_fixes_test.dart. They fail without the fix.DRIVER=sqlite dart testinpackages/dox-orm: all pass.test/bug_fixes_test.darton Postgres: pass.dart formatanddart analyze --fatal-infos packages: clean.