🇪🇸Spain @marcoscano

Thanks for contributing!

marcoscano → made their first commit to this issue’s fork.

Thank you @alexpott for helping with making the module better!

Nice, thanks for helping out! 👍

marcoscano → made their first commit to this issue’s fork.

marcoscano → made their first commit to this issue’s fork.

Thank you for filing and fixing it! :)

marcoscano → made their first commit to this issue’s fork.

Sorry I don't believe this is a need common enough to justify including it in this route. This controller is already too complex, and I'd rather we keep it as lean as possible (considering it already has a lot).
If others believe this would be useful for their projects feel free to add comments here. In the meantime you can use the patch or patch it locally in your project.
Thanks in any case for contributing!

Wut! I hadn't realized ✨ Linkit for Link field Fixed got in, actually quite a while ago... :)
Since our current plugin was meant to support the original usage of Linkit (embedded links in wysiwyg), let's adjust the \Drupal\entity_usage\Plugin\EntityUsage\Track\Link plugin to also support the format their widget is using. I believe this is the easiest and cleanest way to support this.
Thanks!

I am OK with the idea of simplifying the UI, but this controller is getting more and more convoluted to make everyone happy :)
After ✨ Differentiate pending/old revisions and default translations Active , there's more code dealing with these rows/columns, so we'll want to make sure we have tests that cover all possible scenarios.

EntityUsageListTrait is not being used anywhere inside ListUsageController.php... how is this a fix for the issue? 🤔
If drush updb was failing, all sites updating this module would have faced this problem. The fact that there are no other reports (I also didn't find it when I upgraded the sites I maintain) could suggest there is another issue interfering in your scenario? If we could have more details to troubleshoot it would be good (eg full stack trace, steps to reproduce on a clean install, etc).

Thanks!

Committed. Thanks all!

Thanks for helping! This is ready to go but needs a re-roll

Committed, thank you! 🙏

marcoscano → made their first commit to this issue’s fork.

I agree with the idea behind this, but I see a challenge in "filtering out the noise" on the listing page, because it could open the door to a scenario where the service reports usages of an entity (for example in the warning message when deleting an entity), and when editors go to the usage page to check what other entities reference it, they find nothing because the only usages are in past revisions. We'd need to find a way to let them know "there is nothing here because your site admin decided not to display past revisions" or something like that.

Tagged a new release with this 👍

Looks good to me, thanks both for contributing!

Thanks for contributing!

marcoscano → made their first commit to this issue’s fork.

If you end up implementing it as a block based on Type Tray's categories, I think a submodule like type_tray_block or something like that makes total sense! 👍

Thanks!

and we moved all files and placed in site/oursite/files/{file_name]

Are the files managed in Drupal or these are just links pointing to files in disk that happen to be at these locations? If the files are not file entities in Drupal (ie in the file_managed table), then this module won't track them, by design.

Happy to get this in. Does anybody feel like proposing a Merge Request?
(just clarifying that the proposed resolution in the ticket description has it backwards... :) if there is a field on the node that can reference media entities, then the node is the source, and media is the target. In other words, the field is always attached to the source, and what users reference is always the target)

Thanks!

Thanks for filing this and for the fix.
Is there a scenario where old URLs should be supported too? In other words, should be change the regex to the new format or should we just accept the new format as well?

Fixed, thanks!

marcoscano → made their first commit to this issue’s fork.

+1 for removing. Folks that know how to fix it probably don't need the reminder.

dan2k3k4 → credited marcoscano → .

I agree that the module could handle the misconfiguration in a more graceful way. Merge Requests are welcome if anyone feels like working on it.

Looks good to me! Thanks for the quick fix!

marcoscano → made their first commit to this issue’s fork.

Can we please open a new ticket for this and create the MR there?
@justcaldwell it would be great if you could compare the current fix with this new proposal and see if it solves the performance impact you are experiencing? Thanks!

Looks good to me, thanks for contributing! 👍

marcoscano → changed the visibility of the branch 1.0.x to hidden.

marcoscano → made their first commit to this issue’s fork.

Looks good to me, thanks for contributing! 👍

marcoscano → made their first commit to this issue’s fork.

Looks good to me, thank you for contributing!

marcoscano → made their first commit to this issue’s fork.

Thanks! :)

Thank you both, I think the approach that casts the variable to a string is simpler and more readable.

Thanks for contributing!

marcoscano → made their first commit to this issue’s fork.

Thank you for fixing the (unrelated) test failures! 🙏

marcoscano → made their first commit to this issue’s fork.

Just posted on https://www.drupal.org/project/media_remote/issues/3431910#comment-15753535 📌 Automated Drupal 11 compatibility fixes for media_remote Needs work . Unfortunately there are tests failing, we need to fix them prior to merging the D11 MR. Thanks!

We still need to fix the failing tests

Thank you for offering!
I believe the module doesn't need a new maintainer at this point. However, help from the community is always welcome.
Ways to help include:
- Help triaging issues / improving issue description with steps to reproduce, etc
- Reviewing patches / converting patches to Merge Requests
- Adding test coverage

I will do my best to avoid issues stalling when maintainer input is necessary.

Thanks again!

Thank you for offering!
I believe the module doesn't need a new maintainer at this point. However, help from the community is always welcome.
Ways to help include:
- Help triaging issues / improving issue description with steps to reproduce, etc
- Reviewing patches / converting patches to Merge Requests
- Adding test coverage

I will do my best to avoid issues stalling when maintainer input is necessary.

Thanks again!

Fixed using the second approach. Thanks! 👍

Never mind, it seems we do have another issue where this is proposed with test coverage, so I'm getting that one in: ✨ Track reusable Custom Block referenced by Layout Builder Fixed

Apologies for taking so long in reviewing this.
Thanks for contributing!

marcoscano → made their first commit to this issue’s fork.

I'm OK with this, but I'd be happier if we waited until that patch lands in core, so we can add this patch here with its corresponding test coverage.

Thanks for helping modernize the module's codebase. I don't feel strongly about the readonly property and/or docblocks, so I'd say let's get this in and iterate over in 📌 Fix the issues reported by phpcs Needs review , where we can fix these and other CS violations that have bubbled up lately.

Thanks again!

marcoscano → made their first commit to this issue’s fork.

Thank you all! I picked the last one and added it to the repo 👍

Thank you for working on this!

I wonder if there should be an arrowhead on one end?

I agree with this, if we could make the line an arrow, it may be more explicit the source->target hierarchy?

Thanks!

marcoscano → made their first commit to this issue’s fork.

Thanks for contributing!

marcoscano → made their first commit to this issue’s fork.

dww → credited marcoscano → .

Thank you for working on this! 🙏
I agree it would be ideal to have a visual representation of the "hierarchy" that the relationship represents: there is always a "source" entity and a "target" entity, these relationships are not bi-directional. In other words, this module tracks "what entity (or entities) is this entity pointing to". Do you think it's possible to iterate and try to capture that?
Thanks again!

hestenet → credited marcoscano → .

Thanks for filing this in! I am +1 with this idea, however I have 0 skills in designing or producing a logo. If anyone wants to contribute, I will gladly add it to the repo.

Thanks!

Thanks for contributing!

@silvi.addweb thanks for trying to help, but opening a MR with the same changes of the patch, when in the previous comment I mentioned what was missing in the patch, isn't enough to move this to RTBC

Apologies for not looking sooner into this. We'll need test coverage for the new feature we are introducing.

Thanks for opening and working on a fix.
I would have preferred that we add test coverage for the new scenario we are supporting, but it's fine as-is, I'm getting the fix in in the meantime.

marcoscano → made their first commit to this issue’s fork.

Fixed, thanks!

marcoscano → made their first commit to this issue’s fork.

Setting NW as per #3

OK, all green, I'm merging this.
Thanks again for working on this!

Fixed, thanks!

marcoscano → made their first commit to this issue’s fork.

Thank you for working on this! 🙏
I agree on the testing strategy you mentioned. Just removed the next minor/major and added the scheduled pipeline to be run weekly (thanks for the suggestion).
However, this run now failed, with error:

There was 1 failure:
    
    1) Drupal\Tests\entity_usage\Functional\Update\UpdateTest::testUpdate8206
    Failed to run installer database tasks: Database drupal not found. The
    server reports the following message when attempting to create the
    database: SQLSTATE[HY000]: General error: 1007 Can't create database
    'drupal'; database exists., Failed to CREATE a test table on your database
    server with the command CREATE TABLE {drupal_install_test} (id int NOT NULL
    PRIMARY KEY). The server reports the following message: SQLSTATE[3D000]:
    Invalid catalog name: 1046 No database selected: CREATE TABLE
    "test56035940drupal_install_test" (id int NOT NULL PRIMARY KEY); Array
    (
    )
    .Are you sure the configured username has the necessary permissions to
    create tables in the database?
    
    /builds/project/entity_usage/web/core/tests/Drupal/FunctionalTests/Update/UpdatePathTestBase.php:226
    /builds/project/entity_usage/web/core/tests/Drupal/FunctionalTests/Update/UpdatePathTestBase.php:141
    /builds/project/entity_usage/web/core/tests/Drupal/FunctionalTests/Update/UpdatePathTestBase.php:113
    /builds/project/entity_usage/web/core/tests/Drupal/Tests/BrowserTestBase.php:367
    /builds/project/entity_usage/web/core/tests/Drupal/FunctionalTests/Update/UpdatePathTestBase.php:92
    /builds/project/entity_usage/tests/src/Functional/Update/UpdateTest.php:41
    /builds/project/entity_usage/vendor/phpunit/phpunit/src/Framework/TestResult.php:729
    
    FAILURES!
    Tests: 1, Assertions: 1, Failures: 1.
---- Drupal\Tests\entity_usage\Functional\EntityUsageLayoutBuilderTest ----

Does this ring any bell for you? Has anything changed in CI between yesterday and today? :) 🤔

marcoscano → made their first commit to this issue’s fork.

While allowed, it's a best practice to avoid underscores in URLs. There's plenty of documentation on the internet on the reasons for that, I believe the one more widely accepted is that Google won't parse words separated by underscores, while it will if they are separated by hyphens.

If that's important to you, you can always bypass the validation with the hook and implement your own validation that allows underscores...

This is done but it's not ideal to have to patch a dev-dependency. I have opened 📌 Remove lenient treatment from paragraphs as dev-dependency in CI Active as a follow-up so we can clean things up once paragraphs is D11-ready.

marcoscano → created an issue.

Looks good, thanks for helping!

marcoscano → made their first commit to this issue’s fork.

I'll be looking into this shortly
(this is in fact just a test comment for our RSS feed integration :) )

That's fine 👍

My question was probably because from memory, the only projects where I recall non-node entities having their own standalone page were commerce projects. Most scenarios of other custom entities I remember were components of the page, in which case a type-tray-style "Add..." page wouldn't make too much sense (because components are normally more easily created inline, in the context of their parent page). That said, maybe this is a fair request just to support commerce sites, if nothing else... :)

Also, as long as the new entity type implements ThirdPartySettingsInterface, it should be possible to abstract our controller and other relevant code to be agnostic of the type of the entity we should affect. So it would be fine to refactor the existing implementation to be generic, if we can do that without too much work 👍

I could see a new global setting with checkboxes along the lines of "Entity Types to apply Type Tray", with only "Node" selected by default, where admins could opt in other entity types, does that align with what you had in mind?

Thanks again!

I agree this is a good idea, at least for the grid view, where I think just trying to adjust the CSS to have a closer look to what core provides would be beneficial.

For the list view, maybe someone with design expertise could possibly help us come up with a solution that is aligned with the grid view, but accomodates both the thumbnail and a possible longer description text as well.

Thanks!

@kaszarobert thank you for offering to help. I have just added you as maintainer. Feel free to work on issues, commit fixes and tag releases as you see appropriate. I will still try to keep an eye from time to time, but realistically, that may not happen as often as I'd like so feel free to move forward as needed.

Thanks again, and welcome aboard! 👍

Today I saw https://www.drupal.org/project/iconify_icons/ → , which provides integration with a library of open-source icons. Here's an example when searching for "events" : https://icon-sets.iconify.design/?query=event

Maybe we could have a submodule extending the iconify_icons API and allowing people to select their Type Tray icons directly from the UI?