Have Views' taxonomy term filters use UUIDs instead of entity IDs

Unable to decode output into JSON: Syntax error
Error: Call to a member function uuid() on null in taxonomy_convert_entity_ids_to_uuids().
on line 78 app\core\modules\taxonomy\taxonomy.post_update.php

For this issue, we created a patch and patch is working fine.

Here it is just a re-roll of previous patch. i hope i didn't miss a thing.

Let's see if it passes. Maybe we'll have also a MR that could help chasing and keep it updated till ... it's done

Small update, attempting to fix CS/phpstan errors.

+ A version for 10.1.x, as it seems they are not the same.

Taking a brief look and looks like this is very close.

Seems like changing to UUID should be captured in a change record though, just to let others know.

There is still a lot of clean up happening here, which makes it really hard to review. While I like that logic is moved to its own methods instead of having one giant method, it makes reviewing this very hard, because its really hard to see which changes are needed here and which changes are just moving existing logic around. Please please please keep the patch/MR as small as possible

This feels very light on test coverage, but again, since it's hard to see what is actually new code here that should have test coverage, it might be sufficient.

Since we have an update hook, it should have an upgrade path test for that hook.

Moving to an MR for now, still needs addressing feedback from #94 as next step, ie:

1. Reduce scope as much as possible
2. Increase test coverage if needed

The failure is unrelated it seems. I am not sure why it still fails as it seems @dww had fixed that here https://www.drupal.org/project/drupal/issues/2640994#comment-15597379 🐛 Fix label token replacement for views entity reference arguments Fixed and it was committed to 10.3.x through to 11.x. @smustgrave, any chance you have seen this elsewhere as well? The MR/Fork is new from today so I would think it should not have that problem

Beyond that I think test coverage actually may not need much change as the existing test coverage before this issue should actually be validating that the existing complete process of updating, saving, filtering by continues to work.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

A small patch update for 10.3.x, as it seems they are not the same.

2761157-views-taxonomy-uuid branch still merges and passes fine, with expected config validation change (if I understand that warning right). So back to needs review since I believe #94 has been addressed/responded to. So it would be good if someone could confirm and change to RTBC since I made too many changes to this myself.

(Also hiding patches, for those adding them, please instead follow the documentation for downloading merge requests as patches → locally to avoid confusion + avoid issues with needs review bot pushing these issues back to needs work unecessarily)

Change record added here: https://www.drupal.org/node/3464158 →
Update issue summary to standard template but kept the original solution additional chapter.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Resolved conflict.

smustgrave → changed the visibility of the branch 2761157-views-taxonomy-uuid--10-3-x to hidden.

On the back-end (storage), store IDs as UUIDs

So for the schema update should it be of type: uuid?

Which is validated

Tests no longer passing, back to NW

My comment for uuid I could be wrong just seemed like it lined up

No, it really does look right. All 3 failures pass locally when I re-run so I triggered re-run via gitlab CI now

:facepalm, I hadn't pulled because the changes were applied via gitlab UI. They do fail locally. Not a valid UUID error, so could be our sample data...

Okay its really legitimately showing again what we are seeing now and then, that the Views configuration stored in test views & config install | config optional are not updated by update hooks and therefore out of date and need to be manually update.

However in this case, even when

1. Import test view single config
2. Resave
3. Config export
4. Running phpunit -c core/phpunit.xml core/modules/taxonomy/tests/src/Kernel/Views/TaxonomyIndexTidFilterTest.php

There is actually a problem with calculateDependencies() in the code itself revealed here. The 'content' key is not getting populated with the references to the UUIDs and therefore does not match the view. We need to fix that first. I updated the issue summary with this.

The patch doesn´t apply to Drupal 10.3.1. Rerolled the patch for that version

#115 not yet addressed, let's keep it at needs work as it needs work still. Hiding patches in favour of merge request.

2761157-views-taxonomy-uuid--10-3-x branch updated to apply to 10.3.2.

The patch for the 10.3.4

The patch for the 10.3.4

olmyr → changed the visibility of the branch 2761157-views-taxonomy-uuid--10-3-x to active.

1. Updated views wizard to also respect this
2. Fixed test failures
3. Updated issue summary

Very neat!

Left some comments on the MR.

Thanks for the feedback! Comments address + update test added + 2nd change record added