If not, either reproduce this issue on the official demo or switch to Docker and see if the issue is resolved.
The issues do not occur on the official demo.
Describe the problem you’re having:
Hi there. I’m just here to report two no-brainer bugs that may or may not be only present on PDO + MySQL installations because your recent code changes rely on i.e. implicit type conversions that may not be relevant in other cases. Yes, I know your stance on ttrss’ MySQL support and about supporting anything other than the official docker setup. Still, since these are no-brainers (at least to me), I’m reporting them with 0 expectations. At the very least they become searchable.
Cannot save the edit feed form: when you added preliminary PHP8 support, and fixed a related bug by adding string fallbacks for some of the checkbox values, you didn’t apply the same fix for the non-null ttrss_feeds.feed_language, which isn’t passed by the form (not by default anyway), which is then attempted to be saved as null. See https://git.tt-rss.org/fox/tt-rss/src/branch/master/classes/pref/feeds.php#L713
The easy solution is to add the same string fallback for feed_language as you did for the checkbox values.
All edit feed options-tab checkboxes, and all edit filter checkboxes show as checked, regardless of their database state: when you changed to the new tab layout code for these dialogs, you didn’t take into account that with PDO + MySQL boolean flags are returned as strings. When these strings are passed to App.FormFields.checkbox_tag(), they are typecast implicitly as boolean in the checked conditional. However, as I’m sure you know, non-empty strings such as "0" or "1" are always cast as boolean true in JS. See https://git.tt-rss.org/fox/tt-rss/src/branch/master/js/App.js#L47
The easy solution is to change the condition to checked !== '0' && checked
The proper solution is to always explicitly typecast these to boolean when retrieving their values from database. (these are all the fields you have a boolean false fallback for anyway)
Include steps to reproduce the problem:
Edit feed, save feed
Edit feed and open the options tab OR edit filter
tt-rss version (including git commit id):
a42e8aad97b66fd4964263e37a1b40164e495b4c
Platform (i.e. Linux distro, PHP, PostgreSQL, etc) versions:
Any combination of:
Ubuntu versions 18.04 through 20.04
PHP versions 7.2 - 7.4
MySQL versions 5.7 - 8.0
this sadly sounds like something i would constantly forget. i’m not sure if adding a lot of explicit type-casts to bool everywhere is a good idea because of mysql. that would just be a colossal eyesore.
i think this kinda sounds like beginning of the end of mysql support in tt-rss.
that won’t work. it’s obviously ugly and i’m definitely going to forget why it’s there and inevitably refactor it away when i notice it in a few months.
realistically, mysql support in tt-rss is dead. i have no means to test against it, currently, because my development environment is the same compose setup.
Yeah, I figured as much. As these little issues will likely keep piling up, I would seriously consider disabling its support by default and just be rid of it.
I myself am going to move to postgres through your docker compose setup eventually, when I get around restructuring my own server. Until then, I’ll just patch it myself.
Either way, thank you for sticking with it for so long!
Ohoh, I didn’t understand that these were the single modifications. I corrected it myself.
While if rebase my commits over the fetched ones, I hope I’ll be able to constantly apply this fix ; but yeah, it seems the time is pretty over for MySQL DB
Sure. I didn’t think you’d accept PRs just to keep MySQL support going a little while longer.
Username is the same as here.
But if I’m going to do a PR, I’m going to look a bit deeper into why PDO is returning string values for the boolean/tinyints even though it’s perfectly capable to delegate their conversion to the mysqlnd driver. If I can avoid that ugly checked !== '0', all the better.
