Problem/motivation

The problem is that it's split on /, then the first part is 'https:', which includes the derivative separator. hasDefinition() with a derivative separator now triggers a LogicException in \Drupal\Core\Entity\EntityTypeManager::getDefinition()

See #3301682: Define bundle classes via attributes

Steps to reproduce

Enter an absolute URL into a linkit field/autocomplete widget

Proposed resolution

Verify that the first part is a valid entity type?

Issue fork linkit-3596529

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:

About issue forks

Comments

berdir created an issue. See original summary.

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

dkmishra opened merge request !169

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Active » Needs work

I suppose an explicit checks works as a quickfix, although I wonder if the whole thing shouldn't be refactored, for example by using a regex that covers the different formats and only allows a string that consists of valid entity types.

The kernel test mixes using provider in one method and testing multiple things in another, kernel tests are relatively quick to set up but not free, I think that could all be one single method.

That said, I'm almost certain this causes test fails on the existing next minor tests, but unfortunately, that's not enabled in .gitlab-ci.yml.

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

mark_fullmer’s picture

mark_fullmer
he/him
Primary language English
Location Tucson
commented

That said, I'm almost certain this causes test fails on the existing next minor tests, but unfortunately, that's not enabled in .gitlab-ci.yml.

I've add Next Minor tests to the CI.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

So this passes on next minor then, maybe you could the next minor job on https://git.drupalcode.org/project/linkit/-/pipelines/844996 manually to verify whether that fails without this change?

your call then if you want to merge this with or without extra test coverage.

mark_fullmer opened merge request !171

mark_fullmer’s picture

mark_fullmer
he/him
Primary language English
Location Tucson
commented

Confirming that Next Minor does fail when the "fix" staged here is not included, both in the test coverage this adds as well as in an existing test (Drupal\Tests\linkit\FunctionalJavascript\LinkFieldTest::testLinkFieldWidgetAndFormatter): https://git.drupalcode.org/project/linkit/-/pipelines/847165 .

In my mind, then, I don't think there's need for new test coverage here.

Given the "fix" is a one-line conditional, I'm inclined to adopt it as-is rather than working out a more comprehensive regex.

mark_fullmer’s picture

mark_fullmer
he/him
Primary language English
Location Tucson
commented
Status: Needs work » Fixed

Fixed by https://git.drupalcode.org/project/linkit/-/commit/2dfe33447ed463922df9f...

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Thanks.

You likely just forgot (if so, ignore this), but I find it useful to use the recommended commit message on the contribution record page, that includes the issue number and then the commit shows up automatically in the issue and makes it easier to generate release notes and track commits back to their issue. You have to tick the edit commit message checkbox on the MR, I also forget it sometimes.