CiviReport - avoid error in test environments when using built-in php web server #21560
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(Standard links)
|
| @@ -626,6 +626,10 @@ public function preProcessCommon() { | |||
|
|
|||
| // set qfkey so that pager picks it up and use it in the "Next > Last >>" links. | |||
| // FIXME: Note setting it in $_GET doesn't work, since pager generates link based on QUERY_STRING | |||
| if (!isset($_SERVER['QUERY_STRING'])) { | |||
| // in php 7.4 can do this with less lines with ??= | |||
There was a problem hiding this comment.
ohhh - interesting - I didn't know that. Let's ditch php 7.3 support today....
|
Agree there is not much risk / downside here |
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Admittedly this is not something that comes up in core, but is a straightforward "if variable doesn't exist yet don't try to append to it".
Before
Error: $_SERVER['QUERY_STRING'] undefined
After
Technical Details
Came up during https://lab.civicrm.org/extensions/cdntaxreceipts/-/merge_requests/153#note_65360 where it seems in apache
$_SERVER['QUERY_STRING']is always at least set to '', whereas in php's built-in web server it might not exist.The point being in different web server environments you can't assume $_SERVER array members already exist.
Comments
I've labelled "Has Test" in the sense that the above noted unit test is intended to be a regularly run test just not as part of the core tests. And that's how this came up. Feel free to remove.