nextcloud / tables

🍱 Nextcloud tables app
https://apps.nextcloud.com/apps/tables
GNU Affero General Public License v3.0
148 stars 23 forks source link

Match default values when querying views #900

Closed juliusknorr closed 8 months ago

juliusknorr commented 8 months ago

When gathering rows that are visible in views we need to take rows into account that have no corresponding entry in the columns_TYPE tables. For those we can match the filter in PHP and include row ids from a left join where there is no entry.

Taking an example of a yes/no selection:

If the default value of a column doesn't match we end up using the same query as before for collecting row ids

...
    `id` IN (SELECT `row_id` FROM `oc_tables_row_cells_selection` WHERE (`column_id` = 7) AND (`value` LIKE '"false"'))
...

If the default value matches, we need to include those that do not have an entry

...
    `id` IN (SELECT `row_id` FROM `oc_tables_row_cells_selection` WHERE (`column_id` = 7) AND (`value` LIKE '"false"'))
OR
    `id` IN (select sl.id from oc_tables_row_sleeves sl LEFT JOIN oc_tables_row_cells_selection se ON sl.id = se.row_id AND se.column_id = 7 WHERE se.id IS NULL)
...

Steps to reproduce

juliusknorr commented 8 months ago

So I would love to have comprehensive tests for this but lack time at the moment. Also still a bit unsure how to cover that. Behat tests might just be to verbose for test definition, so I might go with a phpunit tests that still writes to the DB to cover the different value conversions more easily in a dataProvider.

Nevertheless I'd follow up this if you're fine with it @blizzz

blizzz commented 8 months ago

So I would love to have comprehensive tests for this but lack time at the moment. Also still a bit unsure how to cover that. Behat tests might just be to verbose for test definition, so I might go with a phpunit tests that still writes to the DB to cover the different value conversions more easily in a dataProvider.

Not a fan of unit tests for DB logic tbh, but would be a way to tackle it.

Nevertheless I'd follow up this if you're fine with it @blizzz

Welcome to the club :neutral_face: perhaps add an issue for tracking. You have my blessing ;)