Skip to content

Refactor Manualdb, part 2

Tilmann Becker requested to merge refactor-manualdb-2 into master

What does this MR do?

  • ManualDb is now empty, its methods are distributed to gateway classes
  • Fix some bugs introduced by !424 (merged), mostly caused by using db->fetch instead of db->fetchAll
  • Patch jquery tagedit to support integer ids, because PDO returns integers by default
  • Make Db.php non-abstract so it can get injected by DI
  • New high-level query functions (fetchValueByCriteria etc)
  • db->fetchValue() now throws an exception if no value was found - use db->exists instead if you want to check for existence
  • Tests: enable whoops while running tests because it gives helpful output (it still needs to be enabled in CI). Side effect: all "PHP Notices" become HTTP 500 errors
  • Typing: annotate some variables where manual injection is used (for better usage search in PHPStorm)
  • CI: preserve app and mailqueuerunner logs in artifacts

Idea: We should maybe rename the database methods semantically: "getXXX()" if it should expect a result, "listXXX()" or "filterXXX()" if it also accepts an empty result.

How confident are you it won't break things if deployed?

I expect that something will break, although I did quite some manual testing.

Some more testing could be useful, especially for:

  • sending emails
  • editing email templates
  • FAQ editing
  • Blog

Links to related issues

#9 (closed)

Checklist

  • added a test, or explain why one is not needed/possible...
  • no unrelated changes
  • asked someone for a code review
  • joined #foodsharing-beta channel at https://slackin.yunity.org
  • added an entry to CHANGELOG.md (description, merge request link, username(s))
Edited by Chris Oelmueller

Merge request reports