Problem/Motivation

The linkit.autocomplete route contains a slug for the {linkit_profile_id}. The controller then loads the LinkIt profile with the given id. However, Drupal supports parameter upcasting to do this automatically, see https://www.drupal.org/docs/8/api/routing-system/parameters-in-routes/pa...

Proposed resolution

Upcast the linkit profile in linkit.autocomplete route.

Remaining tasks

  1. Write a patch
  2. Review
  3. Commit

User interface changes

None.

API changes

None.

Data model changes

None.

Comments

idebr created an issue. See original summary.

idebr’s picture

Status: Active » Needs review
StatusFileSize
new3.6 KB

Attached patch upcasts the linkit profile in linkit.autocomplete route.

Status: Needs review » Needs work

The last submitted patch, 2: 3212820-2.patch, failed testing. View results

idebr’s picture

Status: Needs work » Needs review
StatusFileSize
new1.36 KB
new4.96 KB

Attached patch should fix the test failures.

Status: Needs review » Needs work

The last submitted patch, 4: 3212820-4.patch, failed testing. View results

idebr’s picture

Status: Needs work » Needs review
StatusFileSize
new677 bytes
new5.62 KB

Attached patch should fix the remaining test failures.

idebr’s picture

Version: 8.x-5.x-dev » 6.1.x-dev
StatusFileSize
new5.71 KB

Reroll against latest 6.1.x

idebr’s picture

StatusFileSize
new1.45 KB
new7.16 KB

Attached patch adds some additional changes to the test files to fix test failures.

  • mark_fullmer committed a1f663f2 on 6.1.x authored by idebr
    Issue #3212820 by idebr: Upcast linkit profile in linkit.autocomplete...

  • mark_fullmer committed 90c27c0a on 6.0.x authored by idebr
    Issue #3212820 by idebr: Upcast linkit profile in linkit.autocomplete...
mark_fullmer’s picture

Status: Needs review » Fixed

This change makes sense to me. Thanks for the clear explanation, and the updated test coverage! Merged into both the 6.0.x and 6.1.x branches.

Status: Fixed » Closed (fixed)

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

a.dmitriiev’s picture

This was actually a breaking change :) all the places in code where linkit_profile_id was passed to Url object are now broken

mark_fullmer’s picture

all the places in code where linkit_profile_id was passed to Url object are now broken

Thanks for reporting this. Can you clarify if you're talking about code within the Linkit module itself, or code which extends this module?

a.dmitriiev’s picture

I meant the third party code in custom modules (maybe also there are contrib modules that use linkit as a dependency, but I don't know any).

a.dmitriiev’s picture

I think this change deserves at least change record and a big section on the main project page.

mark_fullmer’s picture

Priority: Normal » Major
Status: Closed (fixed) » Needs work

I meant the third party code in custom modules (maybe also there are contrib modules that use linkit as a dependency, but I don't know any).

Thanks for clarifying. I honestly hadn't anticipated that there would be any 3rd party code that would extend Linkit in this way. Failure of imagination on my part. I agree that a change record would be a minimum thing to call out, but I'm leaning toward a more standard process of deprecating this before removing it altogether (in a new major version release)

samlerner’s picture

I just ran into this problem in my Drupal 10.1.4 site. Trying to add new menu items tries to load the linkit_profile slug in the autocomplete path, but it's named linkit_profile_id in some custom code we had. This triggered the following error:

Symfony\Component\Routing\Exception\MissingMandatoryParametersException: Some mandatory parameters are missing ("linkit_profile") to generate a URL for route "linkit.autocomplete". in Drupal\Core\Routing\UrlGenerator->doGenerate() (line 187 of core/lib/Drupal/Core/Routing/UrlGenerator.php).

Drupal\Core\Routing\UrlGenerator->getInternalPathFromRoute('linkit.autocomplete', Object, Array, Array) (Line: 300)
(...)

We can update our code, but it took us some time to figure out what was happening.

mark_fullmer’s picture

Status: Needs work » Needs review
StatusFileSize
new4.03 KB

We can update our code, but it took us some time to figure out what was happening.

Based on my reading of https://www.drupal.org/docs/8/api/routing-system/parameters-in-routes/pa... , we can maintain the legacy route name linkit_profile_id while still using the automatic upcasting that this issue provides. The variable name will be less than ideal, but I'm okay with that to maintain backwards compatibility in custom code.

Can someone who has experienced this issue in custom code test the attached patch? This should apply whether you're using 6.0.x or 6.1.x.

samlerner’s picture

@mark_fullmer tested the patch in #19 and it fixed the problem I was having earlier. Thank you!

We'll keep our updated code, but this patch will save some poor souls from WSODs.

trackleft2’s picture

Patch in Comment#19 worked for us too, thanks @mark_fullmer

  • mark_fullmer committed cb9da807 on 6.0.x
    Issue #3212820 by mark_fullmer, SamLerner, trackleft2: Provide backward...

  • mark_fullmer committed 2f77a656 on 6.1.x
    Issue #3212820 by mark_fullmer, SamLerner, trackleft2: Provide backward...
mark_fullmer’s picture

Status: Needs review » Fixed

Thanks for the input, everyone. Backwards compatibility has been included in follow-up releases 6.0.1 (Drupal 9.5->10.0) and 6.1.1 (Drupal >=10.1).

Status: Fixed » Closed (fixed)

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

joegl’s picture

We ran into the same issue on 6.0.1:

Symfony\Component\Routing\Exception\MissingMandatoryParametersException: Some mandatory parameters are missing ("linkit_profile") to generate a URL for route "linkit.autocomplete". in Drupal\Core\Routing\UrlGenerator->doGenerate() (line 181 of core/lib/Drupal/Core/Routing/UrlGenerator.php).

Confirming upgrade to 6.0.2 fixed this.

Thanks!