EntityFieldQuery uses incorrect ordered_results instead of orderedResults

#1: efq-ordered-results-case-1292930-1.patch queued for re-testing.

Rolled this against latest code. Location of the file has changed since OP.

Thanks for the reroll, could you please amend existing tests to check for this?

I'd love to, although I'll need a little guidance - it'll be the first tests I've written.

I'll take a few days to catch up on testing documentation and get back to this. Seems to be no rush here.

My first question, though: am I looking to amend EntityFieldQuery.php?

Thanks, chx.

Err, I mean EntityFieldQueryTest.php.

Yes, you are in the right file, I would probably patch up assertEntityFieldQuery.

Here's a reroll. The functionality of ordered_results has since changed to be ids instead of partial_entities. Tests coming soon...

#9: drupal-efq_ordered_results-1292930-9.patch queued for re-testing.

Just a reroll.

#13: 1292930-ordered_results-drupal_efq-13.patch queued for re-testing.

attached is a patch with a test that verifies $orderedResults is not empty after running execute on an EntityFieldQuery with an entityOrderBy called prior to execute.

16: drupal-ordered_results-1292930-16.patch queued for re-testing.

I just marked #2239355: pointless assignment in EntityFieldQuery::finishQuery() as a duplicate.

A couple quick comments on the patch:
- Since $this->ordered_results already exists (and is a public property by default) shouldn't we leave it around as a duplicate of $this->orderedResults for backwards compatibility? (And document it as a property on the class also.)
- It would be better if the test verified that the contents of $query->orderedResults were actually correct (i.e. in the correct order) rather than just that the variable is non-empty.

Is this still a valid issue? I couldn't find any mention of ordered_results in 8.0.x or anything similar to ordered_results.

I also think that this issue is no longer relevant. No occurences of "ordered_results" in code.

This is still an issue in Drupal 7.x core. It's a typo in a variable name, but some people may be using ordered_results instead of orderedResults, since it's a typo in the code. Documentation for it still says it's orderedResults. I don't see why we don't fix this, since it's an obvious typo in either the code or the documentation. Or is there another bug for this?

(EntityFieldQuery::execute)
* After executing the query, $this->orderedResults will contain a list of
* the same stub entities in the order returned by the query. This is only
* relevant if there are multiple entity types in the returned value and
* a field ordering was requested. In every other case, the returned value
* contains everything necessary for processing.

Because some people using ordered_results in code.

@sosyuki not really that good of an excuse. orderedResults never would have worked so changing the comment and the official way to "ordered_results" (or both) would be the fix.

This is throwing a deprecated message on PHP 8.2:

exception: [Deprecated] Line 1399 of includes/entity.inc:
Creation of dynamic property EntityFieldQuery::$ordered_results is deprecated

I think we need to update the execute() function doc block and rename the public property $orderedResults (which never worked and it is always empty) to $ordered_results.

Uploading the patch with this approach. I do not think we can do the opposite, as people are already using the $ordered_results in the code, so we need to preserve that. It is questionable if we have to preserve the currently non-working property $orderedResults, but I think it is not needed (is it possible someone is using it despite that is always empty?).

Draft CR: https://www.drupal.org/node/3322453

Thanks!