refactor: replace legacy database result handling

[anonymoususer72041]

This PR removes obsolete legacy MySQL calls and reduces direct mysqli result handling in selected database-related areas by routing them through the existing DatabaseConnection API.

The install schema no longer uses obsolete mysql_* calls and install-module result handling now relies more consistently on the existing database helpers. Backup-related database access in settings and script code is also routed through the existing connection handling while preserving row-by-row processing where appropriate.

ControlPanel database result handling has been updated to use DatabaseConnection methods for row fetching, row counts, affected rows and inserted IDs instead of direct mysqli_* calls. The count-row handling was also adjusted to explicitly read the named count column rather than relying on the previous mysqli_fetch_row() pattern.

Database tests were extended to cover the normalized query handling patterns used by these changes, especially calling getAssoc() after a prior query() and iterating over the active result set. Obsolete database comments and dead examples referencing legacy mysql_* APIs were also cleaned up.

[anonymoususer72041]

@RussH before merging, we should discuss a few things:

1. I am not sure if changing previous migrations in f481abd is a good idea. Generally speaking, I have a problem with the current migration system and want to implement a new system (while removing the different legacy migration systems long term).

2. Updating the tests in fb229c6 was an idea of ChatGPT. I am not sure it this is a beneficial commit, so I am okay to drop it if it's just noise.

3. Generally, this PR might fix other previous opened issues, for example the backup functionality. I haven't tested them as the GitHub issues tab does not seem to be properly maintained. Could you take a look onto this and cleanup the affected issues after merging this PR, @RussH?

[anonymoususer72041]

@Frankli9986's PR #771 seems to be a partial duplicate of this PR.

3. Generally, this PR might fix other previous opened issues, for example the backup functionality. I haven't tested them as the GitHub issues tab does not seem to be properly maintained. Could you take a look onto this and cleanup the affected issues after merging this PR, @RussH?

As mentioned above, this PR may resolve multiple existing issues. One example could be #572, since @Frankli9986's PR specifically targets that issue and overlaps with the modules/install/Schema.php changes in this PR. I have not tested the demo content installation myself, because it is not my focus.

1. I am not sure if changing previous migrations in f481abd is a good idea. Generally speaking, I have a problem with the current migration system and want to implement a new system (while removing the different legacy migration systems long term).

Even if this PR fixes #572, I am still not sure whether changing old migrations is the best long-term approach.

For example, I am currently looking into a new standardized migration system, possibly based on something like Phinx. If we go in that direction, the current migration system and the migration definitions in modules/install/Schema.php may eventually be replaced. That is why I see both this PR's fix and #771's fix for #572 as temporary solutions rather than the final direction.

After checking the demo content installation flow in the current master branch, I think the more fundamental issue is that the demo install is based on an old backup snapshot. It loads db/cats_testdata.bak, executes the contained db/catsbackup.sql.* files and then continues through the legacy upgrade and module schema handling. This means that every fresh demo installation has to replay historical upgrade logic again, including old migration code that can break on newer PHP versions.

Maybe we could eventually solve this in a similar direction as #716: instead of keeping and patching another old database fixture, the demo data could be rebased onto the current canonical schema or regenerated in a way that no longer requires replaying old install migrations. That would make the changes in this PR and #771 useful as short-term compatibility fixes, but not necessarily the final solution for #572.

[RussH]

@anonymoususer72041
Thanks for this & on the specific points you raised first:

• Changing previous migrations

I agree this is not ideal as a long-term pattern. In general, I’d rather avoid rewriting old migrations unless we have to. In this case, though, the changes look acceptable as a short-term compatibility repair because they don't appear to change the migration intent; they replace PHP-incompatible mysql_* usage with the existing DatabaseConnection escaping/query helpers.

So I’m comfortable with this for PHP 7.x/8.x compatibility, but I agree it shouldn’t become the preferred future direction. Longer term, replacing the legacy migration/demo install flow with a cleaner migration system, or regenerated current demo data, sounds like the better fix.

• DatabaseConnection tests

I would keep these. They are not just ChatGPT noise from my point of view. The PR now relies on getAssoc() operating on the active result set after query(), plus getNumRows(), getAffectedRows(), and getLastInsertID(). Having tests like this is useful, especially while we are replacing direct mysqli usage.

3. Existing issues / backup functionality

I’m happy to review the affected issues after merge, but I would avoid claiming that this PR closes specific issues unless the relevant path has been tested. It may well help with #572 or overlap with #771, especially around demo install / legacy mysql_* usage, but because demo content installation and backup behaviour have historically been fragile, I’d prefer wording it as “may help with” rather than “fixes” until confirmed.

So my view is:

• fix: replace deprecated mysql_* with mysqli_* in upgrade scripts #771 does look at least partially duplicated by this PR.
• Demo content doesn't work #572 may be improved by this PR, but should only be closed after testing the demo install path.
• Backup-related issues should only be closed after testing backup creation and restore against a real/small OpenCATS install.

General PR review:

Overall, this looks like a good direction. The move away from direct mysqli/mysql result handling and towards DatabaseConnection API fits the project better, and the PHP 7.2 test run pass is a plus.

The ControlPanel.php changes look broadly clean, with getNumRows(), getAssoc(), getAffectedRows(), and getLastInsertID() replacing direct mysqli calls.

One thing I would like reviewed before merge: backupDB.php now uses getAllAssoc() when dumping table data. That is cleaner API-wise, but it changes the backup path from row-by-row result iteration to loading each table result set into memory first. That may be fine for smaller installs, but could be unsafe for larger OpenCATS databases. If practical, I’d prefer keeping the DatabaseConnection abstraction while preserving row-by-row processing for table dumps.

Before I can merge this, please resolve the current conflict in lib/ControlPanel.php and check the new Codacy issue? Once those are cleared, I’m happy to re-review/merge.

[anonymoususer72041]

Regarding the Codacy StaticAccess warning in modules/install/ajax/ui.php: This is related to the broader StaticAccess problem described in #682. The codebase already contains established static helper/singleton-style usages, while Codacy fails newly introduced occurrences even when they follow an existing local pattern. I think addressing that properly should be done as a separate cleanup/refactoring task rather than by adding artificial indirection in this PR.

For this PR, I would prefer to keep the installer change minimal and explicit. Your main substantive review point has been addressed separately, @RussH: backupDB.php now keeps row-by-row processing for table dumps, so large table result sets are not loaded into memory with getAllAssoc().