Skip to content

Fix minor b/c break in https://github.com/joomla/joomla-cms/pull/4292#5290

Merged
infograf768 merged 2 commits intojoomla:stagingfrom
wilsonge:patch-27
Dec 2, 2014
Merged

Fix minor b/c break in https://github.com/joomla/joomla-cms/pull/4292#5290
infograf768 merged 2 commits intojoomla:stagingfrom
wilsonge:patch-27

Conversation

@wilsonge
Copy link
Copy Markdown
Contributor

@wilsonge wilsonge commented Dec 2, 2014

This fixes a minor b/c break in #4292 where in some cases you could get the InvalidArgumentException being thrown when you got a 303 message.

Cases involved passing in empty string or null as the message parameter meaning the check at https://github.com/joomla/joomla-cms/pull/4292/files#diff-11a160a70413114ea70faf547cd097ecR976 failed and these values became the moved parameter (and weren't booleans or integers).

This adds a unit test for that scenario and fixes it. Of course all of this relates to the old redirect method and so of course is deprecated.

@cyrez
Copy link
Copy Markdown
Contributor

cyrez commented Dec 2, 2014

@test success!
Result : no B/C anymore.

@beat
Copy link
Copy Markdown
Contributor

beat commented Dec 2, 2014

👍
@test success!
Same Result: No B/C anymore on redirects using old API (tested with CB installed from WebStore: 3.3.6 worked, master without patch fails, and with this PR it works again. Also tested other redirects in Joomla, and they still work (which was 100% certain looking at this PR).
Code-review is ok for me too. (y)

Thanks @wilsonge for your great improvement in com_redirect, and the library, and your quick addressing of this little B/C! Keep up your great work, Joomla needs people like you 👍

@cyrez
Copy link
Copy Markdown
Contributor

cyrez commented Dec 2, 2014

Thanks @wilsonge for your great improvement in com_redirect, and the library, and your quick addressing of this little B/C! Keep up your great work, Joomla needs people like you 👍

The same as @beat : @wilsonge 👍 💯

infograf768 added a commit that referenced this pull request Dec 2, 2014
@infograf768 infograf768 merged commit 1e58493 into joomla:staging Dec 2, 2014
@Bakual Bakual added this to the Joomla! 3.4.0 milestone Dec 2, 2014
@wilsonge wilsonge deleted the patch-27 branch December 2, 2014 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants