Problem/Motivation
Any sites using the original version of the patch from #3358555: Method to bypass redirection attached to this issue will need the m4032404.settings configuration updated to use the new name: disabled -> negate
Steps to reproduce
Install 8.x-1.0-alpha6
Add pages to exclude by adding them to the pages textbox
Set "Do not redirect the above paths to 404"
Upgrade to 8.x-1.0
Expected path matching behaviour is reversed until the settings are re-saved.
Proposed resolution
Provide a hook_update_N for those sites which had been actively using a patched 8.x-1.0-alpha6.
User interface changes
n/a
API changes
n/a
Data model changes
n/a
OP
Latest version of https://git.drupalcode.org/project/m4032404/-/merge_requests/3 wouldn't apply to 1.0-alpha6, and requires upgrading to 2.x.
This is a temporary issue to upload the previous state of the patch to be used, until we can safely upgrade to 2.x.
This issue can be ignored by the maintainers.
Related to: #3358555
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | 3530038-bypassredirection-temp.patch | 9.75 KB | vj |
Issue fork m4032404-3530038
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
lnunesbrComment #3
vj commentedApplying temp fix
Comment #4
elc commented#3358555: Method to bypass redirection was merged into 8.x-1.x branch and released as 8.x-1.0: https://www.drupal.org/project/m4032404/releases/8.x-1.0
What is the cause of needing to be patch off 8.x-1.0-alpha6 instead of just using 8.x-1.0?
Is it because of the use of a patch in production using the different config name for negating the path matching? I hadn't considered that it would need a update to help sites using patched version move to main line. It probably does need one.
Comment #6
elc commentedI've tested this here locally both with and without the "disabled" key and it's working to fix up the config here. Can you use this to patch onto 8.x-1.0 and get it working?
Since using #3358555: Method to bypass redirection was widespread, this should be merged and released quickly to help out others trying to update.
Comment #8
lnunesbrHello @elc, thanks for chiming in. We intend to upgrade to 1.0 indeed and later to 2.x, but for now, we just created the patch with yesterday's state as we were using the MR's patch url (#3358555 didn't contain patch files, just the MR changes), and with the new commits, made the mr patch url incompatible with the module's version we were using.
As an emergency matter, to prevent our builds to fail, we added this patch file without the recent commits. This was an edge case, and might not apply to others.
Comment #9
elc commentedYour site will need an update to go from the config from the original patch on #3358555 to what ended up in the final patch in 8.x-1.0 or 2.0.0. There's also a bug in the patch from #3358555 where if the "pages" config is empty, it is still used in the matching logic.
If you were relying on "disabled = TRUE" then the logic will also silently reverse as the outcome of
$negate = $this->config->get('negate') ?? FALSE;will be FALSE.There will a non-zero number of sites using the original patched 8.x-1.0-alpha6 other than yourselves and I'd rather not have sites ending up with unexpected behaviour.
The patch will still be here, but I seem to have usurped your issue of just posting a patch with the realisation that there needs to be a hook_update_N to fix the config for any of those sites.
That's good news because I would have missed updating the config key for those sites again in #3529941: Use ConditionManager instead of custom code which will once again need to change the configuration.
Comment #12
elc commentedGoing to leave this issue open as you did create a new one instead of adding the patch to #3358555, making me think that it's not possible for you to add a patch file to a "fixed" issue.
When you're done with it, set status to "Fixed".
Comment #13
elc commentedMarking as fixed as this fix has been merged and there's no need for the issue to be open for the link to the temp patch to work, and I don't believe it's going to need another update.