Problem/Motivation

Custom non-entity paths may contain contextual arguments e.g., /custom/path/{node}. Providing Metatag route configuration via UI would be fabulous.

--

Really happy to find this start on solving this need in Metatag. Of course my second configuration tried to use a pattern like this. If we find time in our scope we’ll do some work on this.

Issue fork metatag-2952675

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:

Comments

jason.bell created an issue. See original summary.

marysmech’s picture

StatusFileSize
new7.98 KB

We needed this too so we implemented it :-) Please see attached patch.

Patch adds support for contextual arguments. So if you have route like this

mymodule.gallery:
  path: '/node/{node}/gallery'
  defaults:
    _controller: '\Drupal\mymodule\Controller\MyModuleController::gallery'
    ...

You can add (using "Add meta tag for custom route" button) URL for example

  • /node/1/gallery
  • /node/2/gallery
  • /node/3/gallery

and for each address you can define specific metatags. Everything is still exportable/importable and should be compatible with previous version of this module.

marysmech’s picture

Status: Active » Needs review
waldomero’s picture

Hi @marysmech. Thanks for your excellent contribution

I have tested your patch and I think it works fine but I didn't find a way to set a default metatag for the route without an specific parameter, for example, if you want to set a metatag for all the routes like /node/{id}/gallery.

I think that the users would like to have the posibility of set some default metatags for a route and to customize some routes with certain parameters. I think it would be excellent if an user, for example, could set the tag title: "Title for gallery" for all the /node/{id}/gallery routes and also he could set the tag title: "Title for gallery one" for the specific route /node/1/gallery.

What do you think?

zaporylie’s picture

This is very interesting issue. The main issue with patch in #2 is not allowing for wildcard-style route config as suggested in comment #4.

Let's take an example mentioned above and elaborate on it a bit:
/node/{node} <- this is canonical entity route
/node/{node}/gallery <- custom route displaying content of Media reference field stored on Node
/node/{node}/gallery/{media} <- custom route displaying single Media item

As of #4 let's say we want to customize metatags on per-media item level. In case we use current approach suggested in #2 we would have to create number of config entities - one per media entity in node, ex:
/node/1/gallery/1
/node/2/gallery/2
/node/2/gallery/3
/node/3/gallery/2 <- this is the same Media entity as in /node/2/gallery/2 but living in the context of Node 3

This solution is very explicit, it allows us to do all kinds of crazy metatag customizations but it just doesn't scale. If we have 10, 100, 1000, etc. media entities we will bloat the system with endless config entities.

That's why we should allow for wildcards in the path, ex.
/node/*/gallery/* (1)
/node/2/gallery/* (2)
/node/*/gallery/3 (3)
/node/3/gallery/2 (4)

In this scenario we have 4 patterns, where all, one or none param is transformed into wildcard.

Now, for the purpose od this discussion, let's assume we've declared all 4 paths via metatag_routes module. What would be the order in which they execute? For me the natural order would be from the most to the least specific - so in case Node ID is 3 and Media ID is 2, metatag number {4} will be used. If Node ID is 1 and Media ID is 1 the metatag config number (1) will be used. The problem becomes more tricky when Node ID is 2 and Media ID is 3 - which one of two (2) and (3) should take precedence? Which one is more specific?

Perhaps the easiest way of solving it before we decide on strategy (which I imagine will take forever) would be to sort matching config metadata (as collected in metatag_routes_metatags_alter) by weight? Disclaimer: I haven't looked in whether it's possible to add new property. For now we could give all of them weight 0. If the weight of two matching metetag entities is the same we will use sorting strategy we agree upon in a followup issue.

The second issue would be - how do we pass url param context to metatag? The natural way would be using tokens - metatag module use them already for content entities. Unfortunately only one entity can be passed as token replacement context which is an issue considering use case ilustrated above (both Node and Media entity passed as context). Furthermore why limiting to entities? Param can be string or any value, including objects, provided by ParamConverter. Perhaps we could explore global level tokens and pull the values from url similarly to `[current-page:url:args:value:?]`?

smavri’s picture

StatusFileSize
new8.39 KB

Here is a the correct patch after updating to 8.x-1.1 version.

id.conky’s picture

StatusFileSize
new8.48 KB

Patch compatibility fix after upgrate to 8.x-1.2.

id.conky’s picture

StatusFileSize
new8.33 KB

Patch re-upload.

judapriest’s picture

Thanks for the work, and thanks the patch. Reroll on #8 seems to work.

Don't hesitate to make an interdiff. Easier to review ;).

musa.thomas’s picture

hello just try the #8 patch but still a problem, you can try with path argument in letter like /test/{wild}=> /test/toto
The problem come from the hash is differenet of the route generate and the config saved.

musa.thomas’s picture

StatusFileSize
new8.42 KB

here a quick patch start from #8 I just replace the has with array_keys

johnsicili’s picture

The last patch failed to apply for me on 8.x-1.3.

My two cents/use-case on this is to use one meta title and description for every variation.

Furthermore, I need to have the slugs (I have two in my URL) available as a token and with the ability to modify them (e.g. replace dashes with spaces, capitalize, etc.).

Maybe it's possible to have a callback in the modules route? I'd be good with defining metas there and have PHP to help out.

BTW, hi @Jason.bell! Hope you've been well.

anybody’s picture

Status: Needs review » Closed (won't fix)

This projects functionality is now part of Metatags. Please see #3295162: Deprecate Metatag Routes.

damienmckenna’s picture

Project: Metatag Routes (obsolete) » Metatag
Status: Closed (won't fix) » Needs work

Let's move this to the Metatag issue queue. The patch will need a reroll.

justskew’s picture

Version: 8.x-1.x-dev » 8.x-1.22
StatusFileSize
new8.78 KB

Patch for 8.x-1.22

lamp5’s picture

Version: 8.x-1.22 » 8.x-1.x-dev
alvar0hurtad0’s picture

Status: Needs work » Needs review
StatusFileSize
new8.69 KB

Please find the attached patch.

alvar0hurtad0’s picture

StatusFileSize
new8.69 KB

I've been testing the patch in real life and I found an issue, this new patch prevents some issues in when you need to save several paths for same controller.

biancaradu27’s picture

biancaradu27’s picture

biancaradu27’s picture

biancaradu27’s picture

damienmckenna’s picture

Version: 8.x-1.x-dev » 2.0.x-dev
Status: Needs review » Needs work
Parent issue: » #3423884: Plan for Metatag 2.1.0

Let's try to resolve this for the upcoming 2.1.0 release.

The patch needs to be rerolled against the 2.0.x branch.

anybody’s picture

@alvar0hurtad0 or @biancaradu27 could you perhaps reroll this against 2.0.x or even better turn this into a MR against it?
Based on the results we should eventually consider this for 2.2.0 to not block 2.1.0 here? Seems it's not that important for the community (not much activity)?

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

loze’s picture

I created a MR with the latest patch

This still doesn't address using wildcards in paths, see #5

loze’s picture

Status: Needs work » Needs review
anybody’s picture

Status: Needs review » Needs work

Thanks! Tests are failing. NW!

damienmckenna’s picture

Status: Needs work » Needs review

I reran the tests because the failure was unrelated to this issue.

damienmckenna’s picture

This is a separate issue to deal with the test that failed: #3464483: Fix MaintenanceModeTest test failures

damienmckenna’s picture

Version: 2.0.x-dev » 2.1.x-dev
Status: Needs review » Needs work

The MR needs to be updated to work off the 2.1.x branch.

anybody’s picture

Seems like I have no permission to change the target branch. Can you do that @damienmckenna?

damienmckenna’s picture

Status: Needs work » Needs review
damienmckenna’s picture

Status: Needs review » Reviewed & tested by the community

  • damienmckenna committed e685ba70 on 2.1.x authored by loze
    Issue #2952675 by biancaradu27, loze, alvar0hurtad0, id.conky,...
damienmckenna’s picture

Status: Reviewed & tested by the community » Fixed

Committed. Thanks everyone!

Status: Fixed » Closed (fixed)

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