Problem/Motivation

When i do drush updb :

In ProcessBase.php line 171:

  Unable to decode output into JSON: Syntax error

  Fatal error: Declaration of Drupal\webform\WebformSubmissionListBuilder::getQuery($keys = '', $state = '', $source_entity = '') must be compatible with Drupal\Core\Entity\EntityListBuilder::getQuery():
  Drupal\Core\Entity\Query\QueryInterface in /data-ceph/www/kitbased28/public/modules/contrib/webform/src/WebformSubmissionListBuilder.php on line 1376

Steps to reproduce

Up webform to 6.2-dev with composer and launch updb

Proposed resolution

    • Use the : QueryInterface as a return type for the getQuery function in the WebformImageSelectImagesListBuilder, WebformOptionsCustomListBuilder, WebformEntityListBuilder, WebformOptionsListBuilder, WebformSubmissionExporter ,WebformSubmissionListBuilder

    • Use the : SelectInterface as a return type for the getQuery function in the WebformSubmissionLogManager class

    Remaining tasks

    User interface changes

    API changes

    Data model changes

    Thanks

Issue fork webform-3360357

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

devil2005 created an issue. See original summary.

Rajab Natshah’s picture

Facing the same issue with Drupal 10.1.0-beta1, PHP 8.1 on install from the browser and by drush.

PHP Fatal error:  Declaration of Drupal\webform\WebformSubmissionListBuilder::getQuery($keys = '', $state = '', $source_entity = '') must be compatible with Drupal\Core\Entity\EntityListBuilder::getQuery(): Drupal\Core\Entity\Query\QueryInterface in /var/www/html/test/drupal10-1-0-beta1/web/modules/contrib/webform/src/WebformSubmissionListBuilder.php on line 1369
 [warning] Drush command terminated abnormally. [71.1 sec, 187.8 MB]
Mon May 15 12:21:27.852407 2023] [php:error] [pid 202270] [client 10.50.111.41:44620] PHP Fatal error:  Declaration of Drupal\\webform\\WebformSubmissionListBuilder::getQuery($keys = '', $state = '', $source_entity = '') must be compatible with Drupal\\Core\\Entity\\EntityListBuilder::getQuery(): Drupal\\Core\\Entity\\Query\\QueryInterface in /var/www/html/dev/drupal10-1-0-beta1/web/modules/contrib/webform/src/WebformSubmissionListBuilder.php on line 1369, referer: https://localhost/dev/drupal10-1-0-beta1/web/core/install.php?rewrite=ok&profile=standard_with_webform_in_install&langcode=en
Rajab Natshah’s picture

Title: Error on updb on D10 » Fix Declaration for getQuery Webform Submission List Builder on D10

Rajab Natshah’s picture

Title: Fix Declaration for getQuery Webform Submission List Builder on D10 » Fix the declaration of the getQuery function for the Webform Submission List Builder in Drupal 10
Rajab Natshah’s picture

Status: Active » Needs review
Rajab Natshah’s picture

Issue summary: View changes
Rajab Natshah’s picture

Status: Needs review » Needs work

Check other getQuery functions

Rajab Natshah’s picture

Title: Fix the declaration of the getQuery function for the Webform Submission List Builder in Drupal 10 » Fix the declaration of the getQuery function for query building classes in Drupal 10
Rajab Natshah’s picture

Status: Needs work » Needs review
Rajab Natshah’s picture

devil2005’s picture

patch #11 rules :) thanks a lot

Berdir’s picture

Hm, this is ugly. The new core method was added in #3332872: Allow EntityListBuilder::getEntityIds() to easily extend the query and it just happens to conflict with the method name that webform uses a lot.

Possibly the method should be renamed instead here, it's not really an override of the parent method but something else, the switch between getEntityIds() and getQuery() is different.

Rajab Natshah’s picture

Issue summary: View changes

Is getTotalQuery affected by that too?

a.dmitriiev’s picture

I also don't think that getQuery from modules/webform_submission_log/src/WebformSubmissionLogManager.php has anything to do with core change to ListBuilder.

mherchel’s picture

Although I'm not versed enough to review the actual code, I verified #11 fixed the issue for me.

bbombachini’s picture

Status: Needs review » Needs work

Moving this to needs work as the solution previously presented might not be the right approach.

cosmicdreams’s picture

I disagree that we should reject the patch in #11 because of aesthetics. Typing the return object in the method signature is a valuable type hint that PHP allows. We should do it more often.

I'm using patch #11 for now.

Berdir’s picture

It's an unrelated change that implements its own interface which was not changed, it's not even the same return type, I fully agree that this doesn't belong in here. Same for WebformSubmissionExporter. This is tricky enough as it is, lets focus on the actually required changes.

The different arguments are tricky as well, but changing the name or the arguments (they seem to come from properties always, so they could also just be dropped) has its own risk of introducing more issues if people customize those classes on their own.

I updated the patch to only have the required changes. I am adding a change to require-dev though so we can actually test this on D10+, feel free to commit that separately. I didn't update the MR, might be easier to proceed with the patch as the MR also had unrelated changes it seems.

Berdir’s picture

DamienMcKenna’s picture

kreynen’s picture

THANKS! Applying the patch from #20 fixed the issue for me.

cosmicdreams’s picture

+1 on #20 for keeping the return type hints in the method signature.

Berdir’s picture

I also brought this up in slack and core maintainers would be open to rename the added core method to not break Webform: https://drupal.slack.com/archives/C1BMUQ9U6/p1684156596053419.

longwave’s picture

I opened #3361730: Rename EntityListBuilder::getQuery() to something less generic to solve this in core, but we do not have much time to get this in.

longwave’s picture

Anybody’s picture

Thanks @longwave, so that means we should close this works as designed?
Is the core fix perhaps worth a Drupal 10.1.0-beta2 release to fix this critical conflict? Or is betaX after beta1 also tied to a schedule?

jrockowitz made their first commit to this issue’s fork.

jrockowitz’s picture

Status: Needs review » Fixed
Berdir’s picture

Yeah, this could have been closed as a won't fix too as it will be fixed in the next core beta/rc release.

Status: Fixed » Closed (fixed)

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