Issue

As per #2156089-52: Remove comment_get_thread() in favour of method on CommentStorage, CommentStorage::loadThread() can be called with argument $comments_per_page = 0, even though this is not used in core currently.

I didn't test correctly though. Setting $comments_per_page = 0 yields
Call to undefined method Drupal\Core\Database\Driver\mysql\Select::setCountQuery() - because that method is only available in Database\Query\SelectExtender.

Solution

Reshuffled code and added a little test. (Whose purpose is to just run CommentStorage::loadThread() with the argument that made it throw an exception before.)

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because fatal errors under certain conditions
Unfrozen changes Unfrozen because it only changes internal logic ordering to prevent fatal errors
Prioritized changes The main goal of this issue is to reduce fragility
CommentFileSizeAuthor
#2 comment-loadthread-test-only-should-fail.patch966 bytesmarvil07
comment-loadthread.patch3.97 KBroderik

Comments

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

@roderik, could you upload a patch that contains just the test you've made to illustrate the current failure?

marvil07’s picture

marvil07 commented
StatusFileSize
new comment-loadthread-test-only-should-fail.patch966 bytes

test only, should fail

Status: Needs review » Needs work

The last submitted patch, 2: comment-loadthread-test-only-should-fail.patch, failed testing.

marvil07’s picture

marvil07 commented
Status: Needs work » Needs review

back to NR

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

This looks good to me.

I've added a beta phase evaluation to the issue summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 7866484 and pushed to 8.0.x. Thanks!

  • alexpott committed 7866484 on 8.0.x 
    Issue #2346119 by marvil07, roderik: Fix call to undefined method Select...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented

Thanks for picking this up!