@ptmkenny:
system.install
@ system_requirements()
is 1.5k lines of procedural goo β not exactly lending itself to a unit test. Further, the block emitting this apache-only error to apache and non-apache systems alike, is a system test.
If the hang-up is that this block of twenty lines of (test) code requires additional test coverage elsewhere in a test suite, then it would follow that a test supplying that coverage would pre-exist. I cannot find one. If someone could chime in on the rationale or logic for a test of this test, and where that test needs to live, that would be swell.
As this is merely a "bug" of omission in the first place β since system_requirements()
does in fact test the server $software
elsewhere for apache, but for some reason (of omission/oversight) not here β it seems to me to be an indication that there is no test coverage for this code now. Probably because it is a system test in and of itself.
Please also note that system.install
@ system_requirements()
around L219 runs another test for clean url (rewrite module) support, and in fact does already only conditionally proceed for apache webservers, and it conditionally does so as follows:
> && str_contains($software, 'Apache')
If there indeed does exist a separate unit or functional test supplying code coverage for this test, please point at it forthwith, as a guide.
If there is not any existing coverage of all this procedural goo, then this apache-only alarmism has been needlessly nagging maintainers of non-apache systems for years, for no good or justifiable reason, because of a bogus objection and obstruction to adding a simple && str_contains($software, 'Apache')
condition to an if statement, to bypass the code for non-apache web servers.
Lower quality software lingers on and on due to insistence on what is perceived to result only in higher quality software... clearly isn't working.
The real chore that evidently is being avoided is to boil away this procedural goo and get security checks much better organized somewhere other than inline'd in system_requirements()
. Forking procedurally for all these checks across all the possible web servers using if statements in a multi-thousand LoC function is very non-viable and non-testable, and as is self-evident, a maintenance headache.
5 years, and this remains obstructed with astonishingly unclear prescriptions as to why.
It is no wonder folks are reluctant to contribute.
ππΌ Thank you for the direction @greggles!
Can you post a screenshot of the message in the status report?
Certainly, attached. Thanks for taking interest.. although I'm sure this is not the best place to take up the issue.
If it matters, this is a site currently running Drupal 10.4.0, which had recently been upgraded from version 8, then version 9, etc. And it is running on Pantheon; my local/dev environment also uses nginx (valet).
I'm here because of a search for similar behavior in Drupal core's status report. First I thought I'd stumbled into just the right issue, then was scratching my head because of the closed status and note about this landing in a commit... yes, it took me a minute or three to realize I was in the issues for Security Review module, instead of drupal proper.
So the question is) does anyone have any insights as to whether any similar detection of web server has been proposed for Drupal core? Because if I'm not using Apache, I would much rather NOT have this warning about `.htaccess` always staring at me (or my clients) in the status report.
I'm five seconds into a forced experience of this new design, and am frantically searching for the option to opt-out!
A top-level primary navigation bar, with non-navigable menu items... ugh. Instead these appear to be hover-triggered, animated mega menus or some such... might as well start taking side bets on whether these are going to result in seizures, or merely headaches.
You have to offer something constructive, to invite constructive feedback. I'm old enough to remember an ancient time when Drupal cared about accessibility and user experience.
Same experience. Except that I completely removed the 2.0.x version and deployed the 1.x version, in order to get GTM connected and functioning as expected.
This project has one job, to integrate with GTM, and it fails out of the box. How does this get past even the most elementary of quality assurance steps?!
Like the others, I attempted to deploy the 2.0.x version of this google tag manager module on a client site, and it's a non-starter; impossible to configure with a GTM-*** identifier.
* Attempting to enter and save a GTM-*** container identifier results in a flash of error and then simply resets the form with no useful error message whatsoever.
The project page indicates this project is sponsored/supported by both Acquia and Google, and it (a) does not work out of the box for GTM, and (b) is confusing as hell, with either missing or incongruent documentation across D.O. and GTM getting started or support docs. As this issue was reported all the way back in January, and remains unaddressed, it would seem to reveal that neither Acquia nor Google either cares or actually supports this module.
It was necessary to completely remove 2.0.x and roll with the 1.x version, to get tag manager connected and functioning. At this point, 2.0.x should just be flagged as unsupported.
Same problem, reported in issue #3418254 π Upgrade to 2.0 failed, broke site: install hook(s) couldn't run; php 8 requirement not declared or evident Active .
And have encountered this painful upgrade blocker on a couple of projects now.
In issue #3421437 π PluginNotFoundException: The "field_item:daterange_all_day" plugin does not exist. Active , I see there are others that encountered the same problem, and at around the same time.
This isn't just inconvenient, it blocks the upgrade path β seemingly because the entity or field update has never been correctly integrated or deployed in a compatible way for Drupal >8.7.
It *breaks* the site, *breaks* drush actions, and it is an utterly maddening thing to try and sort through. All just because some date fields need to change a little bit, but we're more or less left to guess and stab.
emjayess β created an issue.
emjayess β created an issue.