pkp/pkp-lib#6264 Remove adodb #6318
Conversation
…lfway generator aware; tinker with DAOs
| $returner = $this->_fromRow($result->GetRowAssoc(false)); | ||
| $query = Capsule::table($this->tableName)->where($this->primaryKeyColumn, '=', (int) $announcementId); | ||
| if ($assocType !== null) $query->where('assoc_type', '=', (int) $assocType); | ||
| if ($assocId !== null) $query->where('assoc_id', '=', (int) $assocType); |
There was a problem hiding this comment.
Just out of curiosity, does the query builder handle type conversions like this? I've often wondered whether I need to cast to int but never taken the time to experiment.
There was a problem hiding this comment.
Good question; I'll test when I have a sec. Using ADODB, PostgreSQL would complain if the data type didn't match the column type; I would expect PDO to behave the same. This could cause SQL failures when binding a user-supplied parameter, if the user entered a string instead of a number. Putting the casts in the binding resulted in more not-foundish behaviour than fatal-errorish.
classes/db/DAO.inc.php
Outdated
| if ($dbResultRange && $dbResultRange->isValid()) { | ||
| $sql .= ' LIMIT ' . (int) $dbResultRange->getCount(); | ||
| $offset = (int) $dbResultRange->getOffset(); | ||
| $offset += max(0, $dbResultRange->getPage()-1) * Config::getVar('interface', 'items_per_page', 30); |
There was a problem hiding this comment.
Have you checked to see how this interacts with the count/offset parameters supported by the REST API? In the new submission lists I have tried to move away from using the items_per_page setting on the backend and only supporting it on the reader-facing frontend.
Also, I think that there is a context setting for items_per_page, not just a config option.
There was a problem hiding this comment.
You're right, paging was broken. Fixed: asmecher@694b61a
| $arrFields = array_map(array($dataSource, 'qstr'), $arrFields); | ||
| return $dataSource->Replace($table, $arrFields, $keyCols, false); | ||
| $queryBuilder = new \Staudenmeir\LaravelUpsert\Query\Builder(Capsule::connection()); | ||
| return $queryBuilder->from($table)->upsert($arrFields, $keyCols); |
There was a problem hiding this comment.
Looks like we'll get upsert from Laravel 8 onwards.
There was a problem hiding this comment.
Ah, good. I've bumped to Laravel 7 in this PR but stopped short of Laravel 8 because of PHP requirements, if I recall correctly.
asmecher
force-pushed
the
remove_adodb
branch
from
2c2880e
to
0271262
Compare
|
Replaced by #6342 (rebase for Submission File overhaul) |
No description provided.