fix: getGetPost() and getPostGet() when index is null

[michalsn]

Description
This PR fixes the problem below:

$_GET['foo'] = 'bar';
d(service('request')->getPostGet());
// returns an empty array

Basically, we return only one stream:

• for getPostGet() method - POST array
• and for getGetPost() method - GET array

And that's fine when the index is specified.

But not when index is null. We should check both streams for values and return them all with priority for GET or POST values - depending on the used method.

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[kenjis]

I think this fix is okay and consistent with the current implementation.

But when do you use the methods without index?
Or do you really need the methods? I have never used it and I don't see any use case.
This is just my question.

[michalsn]

I started experimenting with Controlled Cells, and this approach seemed to be appropriate to get the values I wanted before filtering everything. And that's how I discovered this bug.

To be fair, I can't recall that I used this method without an index before, but I can imagine it can be helpful sometimes, especially when we want to create some wrapper around other libraries. I think Datatables would be a good example. Of course, we can always debate that it is better to refer to specific indexes (that we should know), but I think the choice should be left to the developer, and the method should do what it advertises.

[kenjis]

I'm not sure using these methods is really no harm,
but I agree that the bugs should be fixed as long as we provide these methods.

[MGatner]

I think this is worth more discussion. getVar() reads as follows:

To return an array of all POST items call without any parameters.

So sets some precedence for returning just $_POST. Both getPostGet() and getGetPost() state:

Returns: $_POST if no parameters supplied, otherwise the POST value if found, or null if not

Which is obviously wrong because it never mentions $_GET, but if the goal is to bring the code in line with the UG I think we need to decide what that is first. Alternatively, if we would rather define appropriate behavior for these then it would be better to do that before setting a precedent.

[kenjis]

Good catch. It certainly does not seem to describe the correct behavior when no parameters are supplied.

In addition, there are a few utility methods for retrieving information from either $_GET or $_POST, while maintaining the ability to control the order you look for it:

$request->getPostGet() - checks $_POST first, then $_GET
$request->getGetPost() - checks $_GET first, then $_POST
https://codeigniter4.github.io/CodeIgniter4/incoming/incomingrequest.html#retrieving-input

This method works pretty much the same way as getPost() and getGet(), only combined. It will search through both POST and GET streams for data, looking first in POST, and then in GET:
https://codeigniter4.github.io/CodeIgniter4/incoming/incomingrequest.html#CodeIgniter\HTTP\IncomingRequest::getPostGet

I think this PR's behavior is logical. So we should fix the UG like that.

[michalsn]

I think this is worth more discussion. getVar() reads as follows:

To return an array of all POST items call without any parameters.

So sets some precedence for returning just $_POST.

I think that the getVar() method is quite different since it also covers the JSON data. In fact, if we set null as an index, in some scenarios (JSON request) it will return JSON data from the input stream.

When it comes to the getPostGet() and getGetPost() methods... the description for the "return" is indeed terrible and should be fixed.

[michalsn]

I have updated the description, so now there should be no doubt about how these methods work... although IMO there should have been no doubt before either (except for the incorrect description for the return).

As kenjis mentioned, the UG states:

This method works pretty much the same way as getPost() and getGet(), only combined.

And we all know how getPost() works when no index is specified.

[MGatner]

I'm good with that. Thanks for the UG update!

Refactor request, maybe v5: either make input variables a dedicated class ($request->vars->post('foo')) or just collapse all of these into 1-2 methods with parameters.