Repository navigation
fix: many to many case/dup ids, cursorPaginate key and null, restore, find(5, null), soft delete timestamps, forceFill errors, foreignId chain, rollback missing file - #37
Merged
Conversation
…me, a null row no longer gives page 1
Contributor
Author
Bot reviewI ran I also ran extra checks for
The soft delete change works with Blocking None. Non-blocking
|
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.
Bug fixes from the dox-orm#30 and dox-orm#31 reviews and other follow-ups. One commit per fix. No version bump. Each fix has a changelog line under Unreleased.
1. Many to many on a case-insensitive text key
COLLATE NOCASE). With a stored'ABC',sync(['abc'])added'abc', then deleted'ABC'.'abc 'on MariaDB.sync()andtoggle()ask the database if an id that did not match in Dart is the same as a stored row. If it is,synckeeps that row andtoggleremoves it. This check only runs for text ids, when there are stored rows, and it costs one extra query. The delete also leaves out every id that must stay (NOT IN), so the database never deletes a kept row.attach()anddetach()already behaved like the database, so they did not change.2. Many to many: same id twice
toggle(['1', 1])removed both'1'and'01'. On a number key with two stored rows of one id, the result haddetached: [1, 1].'1'and1) are now one id. The result lists show each id once.3. cursorPaginate
orderBy('author.id')on a join did not add the key as a tie breaker. Rows with the same value could be skipped. Fix: onlyid,<table>.idand<alias>.idcount as the key of the model.StateErrorinstead.'abc'for an int column) gives a Postgres "data exception". Without the savepoint, this breaks the whole transaction of the app. The test "a bad cursor in a transaction keeps the transaction" checks this.4. Query restore()
Blog().where(...).restore()returned true when any row matched, even a row that was not deleted. MySQL counts only changed rows, so it gave a different answer than Postgres and SQLite.deleted_at IS NOT NULL.5. find(5, null)
find(5, null)threw a type error with no clear message.ArgumentErrorthat says the first arg must be a column name. Otherfindcalls work as before.7. Soft delete
updated_at.blog.delete()also did not set thedeletedAtfield on the model in memory.updated_atwhen the model has timestamps, for both a model delete and a query delete.blog.delete()setsdeletedAtandupdatedAt.blog.restore()setsdeletedAtback to null, so the two stay in step.delete.dartis only used by plain queries, which do not know about timestamps, so the fix is inModel.delete().8. forceFill
setColumnValue(),forceFilldid nothing and gave no error. A value of the wrong type was also skipped with no error.ArgumentErrorwith a clear message. An ISO date text now sets aDateTimefield. A key that is not a column is still skipped, as before.forceFillis not released yet, so no released behaviour changes.9. foreignId(...).unique().constrained()
.unique(),.defaultTo(),.change()and other modifiers,constrained()did not compile.ForeignIdColumnDefinitionnow returns that type, like Laravel.10. Rollback with a missing migration file
Not in this PR
reload()keeps an old hasOne/belongsTo value: the fix must go in the code that dox_builder writes (theif (result[...] != null)guard in the relation getter). That is in thedoxrepo, so it is not changed here.Tests
many_to_many_text_key_testandmany_to_many_big_key_test, which only fail when they run after other files. This is the same on main. The cause is the stale prepared type bug in the pub.dev native driver used locally. Both files pass when run alone.dart formatanddart analyze --fatal-infosare clean.