backdrop-issues
backdrop-issues copied to clipboard
backdrop
[UX] Add Requirements check for max_input_vars to status report
Today @quicksketch mentioned that maybe we should split the status report update out of the PR for [UX] set warning messages if max_input_vars is not sufficiently large.
The status report message could stand on its own, and could get into the next minor version, and would be a benefit on its own.
We can keep working on the error messaging in the other issue and improve as needed.
Advocate: @jenlampton
working on tests.
My suspicion: the upgrade and update tests fail, because of the "requirement warning" - that seems to be a killer.
The other thing is the unrelated broken testCommentInterface, which should urgently get fixed.
I'm playing with the requirements checks: I wasn't aware a REQUIREMENT_WARNING would cause a problem during upgrade or install phase. I'll add an exception for those phases to downgrade to REQUIREMENT_INFO. Testing locally now... but I'm expecting tests to pass on the PR even without that change. 🤞
okay I rebased and added a special status for upgrade and install that's different from runtime. Pushed again but tests already seem to be passing 🎉
Thanks @jenlampton 🙏🏼 ...code LGTM, so I've RTBC'ed it.
I've added a small suggestion to wrap the message title in <code> tags, but otherwise this is good to go.
PS: can you please either close/reopen the PR, or rebase in order to trigger a tugboat sandbox?
...I've tested this on my local, and it looks fine:
You won't be able to trigger it in the PR sandbox, but this is what it looks like on my local after reducing the max_input_vars setting below the threshold of 1000 (I wasn't able to get ini_set() in settings.php nor directly setting it in .htaccess to work on my local - I had to do it within my lando setup instead, using a custom php.ini file):
https://github.com/backdrop/backdrop/pull/4197 has been merged into 1.x and 1.23.x. Thanks @jenlampton, @klonos, and @indigoxela!
