Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev/core#1865 Remove civiwp and option query strings from redirected … #17797

Merged
merged 1 commit into from
Jul 13, 2020

Conversation

seamuslee001
Copy link
Contributor

…url as they only relate to CiviCRM internal uses

Overview

This removes these additional query strings from the final redirected url as per dev/core#1865

Before

Some query strings that are only civi related passed onto the target url

After

only non civi related query strings passed onto the target url

ping @kcristiano @haystack @andrewpthompson

@civibot
Copy link

civibot bot commented Jul 12, 2020

(Standard links)

@civibot civibot bot added the 5.28 label Jul 12, 2020
…url as they only relate to CiviCRM internal uses
@eileenmcnaughton
Copy link
Contributor

So this seems to have support from @kcristiano & @christianwach which indicates it is good from a WP point of view

I feel a little less clear how strong the endorsement is on the Joomla! front. I see you pinged @andrewpthompson - have any Joomla! peeps weighed in?

@seamuslee001
Copy link
Contributor Author

test fail unrelated

@seamuslee001
Copy link
Contributor Author

ping @lcdservices also

@eileenmcnaughton eileenmcnaughton added the merge ready PR will be merged after a few days if there are no objections label Jul 13, 2020
@eileenmcnaughton
Copy link
Contributor

test this please

@eileenmcnaughton
Copy link
Contributor

Added merge ready because if @totten were standing by ready to release right now I'd be firmly in team 'better in than out' on this one.

@andrewpthompson
Copy link
Contributor

I have had a look at this patch and it makes sense. I've tested it on Joomla and it works as expected, with the "option" parameter being removed from the redirected URL and no problems seen.

@eileenmcnaughton
Copy link
Contributor

Thanks @andrewpthompson - appreciate you input & expert Joomla! eye

@eileenmcnaughton eileenmcnaughton added merge on pass and removed merge ready PR will be merged after a few days if there are no objections labels Jul 13, 2020
@eileenmcnaughton
Copy link
Contributor

Upped to MOP based on @andrewpthompson responding

@seamuslee001 seamuslee001 merged commit f24b4cc into civicrm:5.28 Jul 13, 2020
@seamuslee001 seamuslee001 deleted the dev_core_1865 branch July 13, 2020 05:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants