Entity Usage when a URL Redirect Exists

Created on 15 February 2023, about 2 years ago

Problem/Motivation

I would just like to know if this is known to be possible.

I have two pages lets say /cat and /dog . And a redirect exists that redirects /perro to /dog.

So lets say /cat has link to that redirect /perro (which redirect to /dog). I can't get entity usage to show me that /cat and /dog are connected by that redirect. I have only been able to get /dog to show that its connected to the /redirect.

Is this possible? I haven been trying all sorts of different combinations of checkboxes on the local tasks/source/targets but nothing has done it yet.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

πŸ’¬ Support request
Status

Active

Version

2.0

Component

Documentation

Created by

πŸ‡ΊπŸ‡ΈUnited States NicholasS

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @NicholasS
  • πŸ‡ͺπŸ‡ΈSpain marcoscano Barcelona, Spain

    Hi there! This is not possible with the current implementation, and I would think it falls out of scope for the module features. Since redirects can be created/edited outside of the scope of the entity's CRUD operations, it could potentially become complex to ensure consistency of the data over time.
    One option that comes to mind is to use the LinkIt β†’ module to store more structured references to linked content, instead of just storing a link with the alias on the href attribute.
    If you already have a lot of content, maybe you can create a batch process in custom code to try to update the links to no longer refer to the old aliases?
    Maybe there are other workarounds, but these are the first that come to mind... I hope this helps!

  • First commit to issue fork.
  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    I think we can do something here. Redirects are not in the routing system so need it’s own thing see \Drupal\redirect\EventSubscriber\RedirectRequestSubscriber. I think we can add something to \Drupal\entity_usage\EntityUsageTrackBase::findEntityIdByUrlString() to allow modules to determine an entity for a URL and then provide an implementation for redirects in the entity_usage module.

    Additionally at the moment we determine whether we look in an entity type's basefields based on a single boolean value - entity_usage.settings::track_enabled_base_fields - but we always want to track usages for redirect basefields so I think we should make basefield tracking per entity type.

  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    So....

    Completely refactor how we find an entity from a URL into something event based and split all of the related code out of \Drupal\entity_usage\EntityUsageTrackBase. Which is nice because that class is becoming an monster. The resulting change makes it easy to extend how we find an entity that corresponds to a URL in different cases as we now trigger an event which has the necessary info pre filled out for performance reasons and then allows us to add the necessary subscribers for each module. This means we'll do less if the file module is not installed for example.

    Also the new url to entity service is needed by the entity_usage_updater module. Currently that module creates a fake tracking plugin to get this functionality :D

    Need to add test coverage for the redirect code.

  • Pipeline finished with Failed
    17 days ago
    Total: 358s
    #415999
  • Pipeline finished with Failed
    17 days ago
    Total: 259s
    #416015
  • Pipeline finished with Failed
    17 days ago
    Total: 234s
    #416019
  • Pipeline finished with Failed
    17 days ago
    Total: 320s
    #416051
  • Pipeline finished with Failed
    17 days ago
    Total: 218s
    #416156
  • Pipeline finished with Failed
    17 days ago
    Total: 293s
    #416161
  • Pipeline finished with Failed
    17 days ago
    Total: 288s
    #416168
  • Pipeline finished with Failed
    17 days ago
    Total: 223s
    #416169
  • Pipeline finished with Success
    17 days ago
    Total: 228s
    #416170
  • Pipeline finished with Failed
    16 days ago
    Total: 292s
    #416367
  • Pipeline finished with Success
    16 days ago
    Total: 284s
    #416369
  • Pipeline finished with Failed
    16 days ago
    Total: 291s
    #416373
  • Pipeline finished with Success
    16 days ago
    Total: 287s
    #416374
  • Pipeline finished with Success
    16 days ago
    Total: 335s
    #416379
  • Pipeline finished with Failed
    16 days ago
    Total: 3375s
    #416619
  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    We have test coverage. This is now ready for review.

  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ
  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    The ability to only track some base fields mentioned #4 will be covered by #3326567: Track some base fields β†’

  • Pipeline finished with Failed
    16 days ago
    Total: 7060s
    #416683
  • Pipeline finished with Failed
    16 days ago
    Total: 229s
    #416829
  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ
  • Pipeline finished with Success
    16 days ago
    Total: 285s
    #416843
  • Pipeline finished with Success
    12 days ago
    Total: 210s
    #420437
  • πŸ‡ͺπŸ‡ΈSpain marcoscano Barcelona, Spain

    @alexpott Awesome work! Thank you very much for working on this. I agree things will get much cleaner and easier to modify going further.
    I was not paying too much attention to redirects so far since in my mind for some reason it wasn't "clicking" that we can track them just as first-class entities :)

    I have left a few minor suggestions / nitpicks as comments in the MR, but apart from that I believe there are 2 things we should still do here:

    1) Implicitly support redirect basefields

    I know #3326567: Track some base fields β†’ is asking to be able to granularly define which basefields we track and if we end up doing that, it could solve this, but to me there is no point in selecting redirects as source/target and not wanting to track basefields, so I would keep the two issues separate, and just special-case redirect basefields in this MR. If a site-builder enabled redirects as source/target, then we track its basefields regardless of the global checkbox. I think we can add some logic in \Drupal\entity_usage\EntityUsageTrackBase::getReferencingFields() to bypass the global setting for now if the entity is a redirect. (I also have reservations to the other issue, not sure how wild the form could get in order to accomplish that).

    (A quick aside, I believe a UX win we could consider sneaking into this same MR (although I don't feel too strongly if you prefer not) is to add states to the settings form, so that whenever redirect is enabled as source, we also enable it as target, and vice-versa. I see no point in selecting just one of them.)

    2) Do not display redirect entities in the UI

    Much like we special-case paragraphs and blocks when we display usage info on the UI, I believe we should do the same with redirects, which are just "bridges" to the other entities and aren't useful as rows in the usage page. It's unfortunate because \Drupal\entity_usage\Controller\ListUsageController::getRows() is getting more and more complex with yet another special case, but I think that's what we have to do here...

    An example of the issue users would find if we don't special-case them:

    - Create node Foo with path /foo
    - Create node Source1 with a link in body pointing to /foo, save node
    - Edit node Foo changing its alias /foo to /bar. (this creates a redirect from /foo to /bar)
    - Go to view the Usage page of Foo. It now displays 2 rows:
    - Source1 (correct)
    - Redirect entity (maybe we can remove this to avoid confusion?)
    - (note: At this point in DB we only have registered redirect as a source, since Source1 hasn't been processed)
    - Save node Source1 with no changes
    - View usages of Foo again:
    - Source1 (wrongly displays "Used in Old revision(s)")
    - Redirect entity (could be removed too)
    - (in DB we have now registered the redirect both as source and target)

    In my mind, the ideal scenario would be that we should either transparently display the source entity pointing to the redirect, or do something like we do with paragraphs, and indicate somehow that there is a redirect between source node and target node. Displaying a redirect in its own row, even if technically correct, would confuse editors IMO.

    What do you think?

    Thanks again!

  • Pipeline finished with Failed
    11 days ago
    Total: 263s
    #420840
  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    Re #11.1 Yes this makes sense. Implementing a solution and will update once ready and tested.

    I think I disagree with #11.2 - redirects are not really bridges - they are managed outside of the entity and have their own UI and knowing that a redirect is pointing to an entity is useful information. Still mulling this over though. I think maybe the example you given means not that we need to remove redirects but that we need to provide more info on redirects... complex.

  • Pipeline finished with Failed
    11 days ago
    Total: 225s
    #420900
  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    Added the code and test to address #11.1

  • Pipeline finished with Failed
    11 days ago
    Total: 245s
    #420910
  • Pipeline finished with Failed
    11 days ago
    Total: 257s
    #420929
  • Pipeline finished with Success
    11 days ago
    Total: 204s
    #420932
  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    Removing the "needs tests" added in #11 because everything I've added since then has test coverage and there was no additional test coverage requested by #11.

    I'd done some more thinking about #11.2 I agree that there is an issue with information going out of date when you create a redirect from a path alias as in the situation described. But; this is not caused by redirects per se. You can create exactly the same situation without them. For example:

    1. Install the standard profile.
    2. Install entity usage and configure the usage tab to appear on content
    3. Create node 1 and alias it to '/foo'
    4. Create node 2 and in the content add a link to '/foo'
    5. Edit node 1 and change the alias to '/foobar'
    6. Go to the usage tab of node 1 see that the usage shows node 2... even though the link is now broken and does not point to the entity.
    7. Create node 3 and alias it '/foo'.
    8. Go to node 3 usage - shows nothing
    9. Go to node 1 usage - still shows node 2...
    10. Go to node 2 and edit and save... things update...

    The point being we need to update usage when aliasing changes too. I think we should think about fixing all of this in a separate issue - that is likely to get quite complex.

  • Pipeline finished with Skipped
    11 days ago
    #421046
  • πŸ‡ͺπŸ‡ΈSpain marcoscano Barcelona, Spain

    Fair enough πŸ‘
    I was mainly considering the use-case of a redirect being created under the hood on node save, but it's true the problem goes beyond that and lies on the fact that the identifier we are using to track a relationship (path alias) can be changed outside of the source node form, and this can potentially break the relationship tracked.

    I have opened πŸ“Œ Consider updating usage when path aliases change Active so we can explore this problem in more detail. I agree it's going to be complex, so if we find it's almost unfixable in a reasonable way I'd also be OK to rescope that issue to implement workarounds to mitigate the issue as much as possible.

    Thanks again, this is a great win to the module! πŸ‘

Production build 0.71.5 2024