FieldTypeUninstallValidator: TypeError: str_starts_with(): Argument #1 ($haystack) must be of type string, null given in str_starts_with()

Thanks for reporting this! But … it's a mystery to me how to reproduce this, since tests are passing and no backtrace is included.

Aha! Nice catch! 👏

That's something that we should be able to add explicit test coverage for pretty easily I think? 😊

@wimleers I just added the tests asserting that we are able to load the page which was not happening prior to this fix.Is there something else that needs to be a part of this test?Assigning it to you for the clarifications on this.

LGTM!

P.S.: I ran the test-only CI job and it passed tests: https://git.drupalcode.org/project/experience_builder/-/jobs/2379770 — which means that there was indeed a test failure, which means that this MR contains the appropriate test coverage. 👍

I was wrong:

P.S.: I ran the test-only CI job and it passed tests: https://git.drupalcode.org/project/experience_builder/-/jobs/2379770 — which means that there was indeed a test failure, which means that this MR contains the appropriate test coverage. 👍

That CI job should've failed. The test passes locally without the code changes.

Found the root cause: the test assertion was inverted … which meant it always passes.

Fixed.

If you agree with my changes, then the honor is all yours to merge this MR, @Utkarsh_33! 😄

Too bad, the test fails on PostgreSQL for some reason 🤷‍♀️ See https://git.drupalcode.org/project/experience_builder/-/jobs/2380752

To debug this, add

var_dump($this->getSession()->getPage()->getContent());

temporarily before the failing assertion.

https://issue.pages.drupalcode.org/-/experience_builder-3464830/-/jobs/2...

The website encountered an unexpected error. Try again later.

Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[42883]: Undefined function: 7 ERROR: function json_extract(jsonb, unknown) does not exist LINE 4: WHERE (JSON_EXTRACT(field_xb_demo_props, '$.*.*.expression')... ^ HINT: No function matches the given name and argument types. You might need to add explicit type casts.: SELECT "node__field_xb_demo"."entity_id" AS "entity_id", "node__field_xb_demo"."revision_id" AS "revision_id" FROM "test69047609node__field_xb_demo" "node__field_xb_demo" WHERE (JSON_EXTRACT(field_xb_demo_props, '$.*.*.expression')::text LIKE '%ℹ︎comment␟%'); Array ( ) in Drupal\experience_builder\FieldTypeUninstallValidator->checkContentEntityUses() (line 212 of /builds/issue/experience_builder-3464830/src/FieldTypeUninstallValidator.php).

Okay, so this error on PostgreSQL:

ERROR: function json_extract(jsonb, unknown) does not exist

is expected.

What is not expected, is that this runs at all, because we have

    if (!isset($connection_info['default']['driver']) || $connection_info['default']['driver'] !== 'mysql') {
      $this->markTestSkipped('This test only runs for the MySQL database driver. See https://drupal.org/i/3452756');
    }

in FieldTypeUninstallValidatorTest.

Conclusion: we expect that UninstallModulePageTest will fail on PostgreSQL too. We have #3452756: Ensure querying JSON nested values when parent keys are unknown is possible in all supported databases for this.

So this needs a slight adjustment to the new UninstallModulePageTest and then it'll be good to go 👍

As per #20 , tweak code of UninstallModulePageTest so that test skip for PostgreSQL only as it fails for this one.

Also observed there are some pipeline failures related : \ComponentValidationTest::randomMachineName() , for this rebased the MR. apart from it nothing seems to be left.

What is not expected, is that this runs at all, because we have

if (!isset($connection_info['default']['driver']) || $connection_info['default']['driver'] !== 'mysql') {
$this->markTestSkipped('This test only runs for the MySQL database driver. See https://drupal.org/i/3452756');
}
in FieldTypeUninstallValidatorTest.

This is kernel test, so here this var $connection_info['default']['driver'] return 'mysql', so this is not run for all which is expected.

Verified if we want var $connection_info['default']['driver'] returns value in string format like 'mysql' then test need to extend from InstallerTestBase.class for functional test otherwise it return Drupal\mysql\Driver\Database\mysql

Please review, moving NR

Getting close!

I have tried to address feedback & rebased the MR, Please review, moving to NR

Hi,

I reviewed MR!143, applied it against Experience Builder 0.x, and confirmed that I can go to the uninstall page without any errors. I was also able to uninstall the module and verified that the module is working correctly.

I’ve added a before-and-after screen recording for reference. RTBC+

Thank you.

The test-only CI job indeed reproduces the reported bug:

There was 1 error:
1) Drupal\Tests\experience_builder\Functional\UninstallModulePageTest::testUninstallModulePage
Exception: Warning: Undefined array key "expression"
Drupal\experience_builder\FieldTypeUninstallValidator->checkDefaultValueUses()() (Line: 140)

— https://git.drupalcode.org/issue/experience_builder-3464830/-/jobs/2575834

👍

Time to ship this! 🚢 Thanks all 🙏

P.S.: @atul_ghate: please do not post screenshots of a patch successfully applying. That is not remotely helpful. If the MR applies, we already know that it's an applicable patch …

Perhaps a follow-up issue is needed?

kristens-mbp-2:starshot-xb kristenpol$ ddun experience_builder

In PmCommands.php line 200:
                                                                                                                                         
  experience_builder: The <em class="placeholder">Experience Builder</em> field type is used in the following field: node.field_xb_demo  
                                                                                                                                         

Failed to run drush pm:uninstall experience_builder: exit status 1

#29: That's Drupal core's \Drupal\field\FieldUninstallValidator, not XB.

Before uninstall the XB module, you have to first delete all XB field instances. That's true for any module providing a field type.

So no, no follow-up is needed for that 😇

Ah yes of course… brain is mush.

I have seen some modules that maybe had a friendlier error message or maybe I’m hallucinating at this point 😜

@kristen pol The message is much friendlier also for this scenario … if you use the UI. There's only so much that's possible in a drush CLI context :)