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

Cool! Please create generic solution (not only taxonomy term, not only filter) and attach patch. Thanks! :)

@nevergone should this be optional? seems like it would be better to just use uuid by default unless there is a reason to keep tid/nid as the stored filtered id? Not sure providing a user facing interface to switch between one or other is going to be helpful. We migrated a lot of content and id drift between views and migrations is an issue for us currently.

Has there been any new information on this topic? We would also like to use the UUID instead of NID and TID in Views filters. Our NID and TIDs are out of sync across server source and destination servers.

This causes an issue for us as well. Some of our view filters break each time configuration is imported.

Hopefully someone can address this.

Is there a patch that allows this to work? I am also having trouble when migrating configuration to different environments where taxonomies have different ids (but proper uuid)

I don't know if there is a patch for this but i managed to solve this problem with module Default Content , which will import taxonomies to different enviroment with same tid so views won't break.

Hope it helps.

As alluded to by the IS, it looks like the IDs are being set in core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid->valueForm(), which is tricky to sift through as it's way too long and has way too many nested ifs. It'll need to be refactored to make it understandable enough to work on.

Add a checkbox to the filter configuration form to allow users to use the uuid of the term as the filter value rather than the tid.

I agree with #7.

Here's the idea: Just use UUIDs, and forget about the checkbox. When reading them back from configuration, convert them to entity IDs. We can implement an update hook that migrates existing configuration with entity IDs to UUIDs. Then we won't need the checkbox.

Would there be any reason not to do that?

I'm trying to see what I can throw together.

There's still work to do, but I'd like to see what the testbot says.

I updated the IS with tasks, and was able to get "Support viewing, editing and saving the filter form with text/autocomplete" working. I'll look at select options next (and keep refactoring).

Questions for to-dos:

• Where in the code does Views save the actual view to config? I noticed that I'm getting small integers like 0, 6 and 20 instead of UUIDs (or even entity IDs, which should be in the hundreds range for my site).
• Where in the code does Views grab the stored values to execute the query? I'm not getting any results so I'm assuming we have to do a conversion here (although fixing the other item may fix this).

Where in the code does Views grab the stored values to execute the query? I'm not getting any results so I'm assuming we have to do a conversion here (although fixing the other item may fix this).

It happens in the

::init

method - in there it sets $this->value (an array)

Where in the code does Views save the actual view to config? I noticed that I'm getting small integers like 0, 6 and 20 instead of UUIDs (or even entity IDs, which should be in the hundreds range for my site).

You might have a config-schema issue here - taxonomy.views.schema.yml is probably telling the config API that the filter value is an integer

Thanks! init() really did the trick. I believe all that's left (other than tests) is an update hook to update the schema and data within it.

So, how do I change the data type from an integer to a string in existing config? I can change the values, but they still come out as integers, even though I updated the schema definition. It looks like there's a way to do this with field configuration, but not with views. Views configurations don't seem to have a toArray() method.

Would someone kindly point me to an example?

Fixed accidental status change.

Setting to NR so we're know what test failures come back; will turn to NW shortly.

It's possible we won't actually need additional tests for this as there seems to be coverage (given the failures), but it may be worthwhile to check if conversions between the ID types are happening (even though this wouldn't be functional testing).

I'm giving up for now as I spend enough time trying to figure out #22. If someone can provide a hint, I'll pick it up again. Alternately, having someone else figure this out would be great. I left a @todo in the update hook.

Also, I refactored valueForm() some more, which has shrunken significantly. It should be much easier to see what's going on. (I can now see the entire thing in my IDE!)

Looks like the last part of views_update_8500() may be helpful here. (Thanks to @larowlan for the idea to look in there!)

I think I got it. The filters may be losing their vocabulary IDs, but this could just be my set-up. Further testing is required.

The above plan for an upgrade path wasn't workable because the way views configuration updates are done has changed. These now have to be done in Views post-save hooks. See #2989745: views_update_8500() inlines configuration changes instead of this being done on config save for bc for details.

Please test (as I'll be doing tomorrow) and report back.

I forgot to check for an unset array index.

Ways to test

These are the processes I used to test, in order from top to bottom (as they lead into each other). They all work for me.

Upgrade path

1. Switch to the latest dev branch.
2. Apply the patch.
3. Run DB schema updates.
4. Re-save all term filters in all Views displays containing them.

Exporting configuration

1. From the state left above, export the site's configuration.
2. Diff the configuration directory.
3. The entity IDs should be replaced by UUIDs.

Importing configuration

1. Install a new site with the above-generated configuration. (I used Config Profile and drush site-install.)
2. Import taxonomy terms with Structure Sync (assuming you've exported them there previously).
3. View/edit filters and Views results to ensure they haven't changed.
4. See Views results, and notice that filters are working properly.

Drupal.org test failures

There's now only one test failure, which looks like a test needs to be updated. I'll take a closer look. Updating status as not everything is green yet.

Given that, it looks like we have good coverage for this already so I don't believe we needs additional tests. Removing tag.

Updating title.

Hopefully this does it, changing the test ID value from an integer to a (UUID) string.

Here's a reroll for 8.9.x.

Rerolled patch 38 against Drupal 9.2-dev, commit hash 1bcb278060b75c4f45c82cdb8784262c2f45723b

Rerolled patch 40 against Drupal 9.2.x branch

Working on issues reported in test

Fixing tests, missing class "Drupal\Core\Config\Entity\ConfigEntityUpdater" addes, Please have a look.

Fixing failed tests of patch #52.

We've been using the patch from #38 successfully on an 8.9.x installation for a while now. We discovered one bug, using the patch causes the "Limit list to selected items" option to not work. We fixed that by changing line 10 of the getEntityIdsFromUuids function from

      $entity_ids[] = $term->id();

to

      $entity_ids[$term->id()] = $term->id();

Not sure of any other possible consequences of that, but it seems to fix the above option.

Re-roll patch to 9.3.x.

Trying to fix Fail test, Not sure these test fix are up to date or not. Please have a look and advise.

Fixing the fail tests because of "accessCheck()".

Updating status as per #56. Looks like that hasn't been included yet.

Worked on #56 as mentioned in #62, Please have a look.

Patch from #63 looks good and is working. I just had to make a quick reroll because taxonomy.post_update.php has been changed in 9.3.x.

Even if there are already a lot of coding standards issues in those files, shouldn't all new functions/methods have docblock comments?

1. +++ b/core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php
   @@ -95,6 +96,10 @@ public static function create(ContainerInterface $container, array $configuratio
   +    if (!empty($options['value'])) {
   +      $options['value'] = $this->getEntityIdsFromUuids($options['value']);
   
   @@ -102,6 +107,22 @@ public function init(ViewExecutable $view, DisplayPluginBase $display, array &$o
   +  protected function getEntityIdsFromUuids(array $uuids) {
   +    if (!empty($uuids) && !Uuid::isValid(reset($uuids))) {
   

   empty() called twice for no reason, it could always call function and have empty check inside

   Also protected method needs docblock

   Also reset() validates only first element of values

2. +++ b/core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php
   @@ -288,6 +246,88 @@ protected function valueForm(&$form, FormStateInterface $form_state) {
    
   +  protected function getFormValueForTextFields() {
   ...
   +
   +  protected function getFormValueOptions() {
   ...
   +
   +  protected function getFormValueOptionsWithHierarchyAndLimit() {
   ...
   +
   +  protected function getFormValueOptionsWithoutHierarchyAndLimit() {
   ...
   +
   +  protected function getDefaultValueForSingleExposedOption($default_value, $options) {
   
   @@ -427,4 +476,12 @@ public function calculateDependencies() {
    
   +  protected function getVocabulary() {
   ...
   +
   +  protected function getTermLabel($term) {
   
   +++ b/core/modules/taxonomy/taxonomy.post_update.php
   @@ -25,3 +29,45 @@ function taxonomy_removed_post_updates() {
   +
   +function taxonomy_convert_entity_ids_to_uuids(array $entity_ids) {
   

   missing doc-block

3. +++ b/core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php
   @@ -370,6 +410,14 @@ public function validateExposed(&$form, FormStateInterface $form_state) {
   +    foreach ($term_ids as $term_id) {
   +      $uuids[] = $this->termStorage->load($term_id)->uuid();
   

   better use loadMultiple() here

Post update hook does not update filters correctly, because it checks wrong array keys. I'd say we could just check for taxonomy_index_tid views filter plugin and update values.

if (($filter['plugin_id'] === 'taxonomy_index_tid') && isset($filter['value'])) {
  $filters[$id]['value'] = taxonomy_convert_entity_ids_to_uuids($filters[$id]['value']);
}

Updated patch for 9.4.0, addresses feedback in #66 and #67.

An interdiff was not possible because patch was rejected, but other than addressing the feedback, I had to add lines 166-171 in core/modules/taxonomy/tests/src/Functional/Views/TaxonomyIndexTidUiTest.php to have tests pass. The stored UUID in core/modules/taxonomy/tests/modules/taxonomy_test_views/test_views/views.view.test_filter_taxonomy_index_tid.yml does not match the UUIDs available, so no filter is applied. I therefore have the test update the View to select an existing Term and see if filtering works correctly. I feel a bit nervous that perhaps that is defying the point of this work, so please pay attention to that when reviewing. I could not find another way to resolve, but probably there is a better way than what I have done.

@scott_euser, just fixing the codesniffer errors in your patch.

1. +++ b/core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php
   @@ -27,6 +29,7 @@ class TaxonomyIndexTid extends ManyToOne {
   +  // @codingStandardsIgnoreLine
   
   @@ -71,6 +74,7 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition
   +      // @codingStandardsIgnoreLine
   
   @@ -113,6 +151,9 @@ public function getValueOptions() {
   +  /**
   +   * {@inheritdoc}
   +   */
   
   @@ -125,6 +166,9 @@ protected function defineOptions() {
   +  /**
   +   * {@inheritdoc}
   +   */
   
   @@ -153,7 +197,10 @@ public function buildExtraOptionsForm(&$form, FormStateInterface $form_state) {
   -      '#options' => ['select' => $this->t('Dropdown'), 'textfield' => $this->t('Autocomplete')],
   +      '#options' => [
   +        'select' => $this->t('Dropdown'),
   +        'textfield' => $this->t('Autocomplete'),
   
   @@ -169,8 +216,11 @@ public function buildExtraOptionsForm(&$form, FormStateInterface $form_state) {
   +  /**
   +   * {@inheritdoc}
   +   */
   
   @@ -280,7 +267,7 @@ protected function valueForm(&$form, FormStateInterface $form_state) {
   -      // Retain the helper option
   +      // Retain the helper option.
          $this->helper->buildOptionsForm($form, $form_state);
   
   @@ -303,6 +407,9 @@ protected function valueValidate($form, FormStateInterface $form_state) {
   +  /**
   +   * {@inheritdoc}
   +   */
      public function acceptExposedInput($input) {
   
   @@ -315,7 +422,7 @@ public function acceptExposedInput($input) {
        // If view is an attachment and is inheriting exposed filters, then assume
   -    // exposed input has already been validated
   +    // exposed input has already been validated.
   
   @@ -342,6 +449,9 @@ public function acceptExposedInput($input) {
   +  /**
   +   * {@inheritdoc}
   +   */
   
   @@ -374,10 +484,24 @@ public function validateExposed(&$form, FormStateInterface $form_state) {
   +  /**
   +   * {@inheritdoc}
   +   */
   

   out of scope

2. +++ b/core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php
   @@ -102,6 +110,36 @@ public function init(ViewExecutable $view, DisplayPluginBase $display, array &$o
   +    $terms = $this->termStorage->loadByProperties(['uuid' => $uuids]);
   

   should this use an aggregate query instead of loading the terms or do the terms end up getting loaded anyway?

3. +++ b/core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php
   @@ -102,6 +110,36 @@ public function init(ViewExecutable $view, DisplayPluginBase $display, array &$o
   +      $entity_ids[$term->id()] = $term->id();
   

   pretty sure the return is keyed by tid, so you can just array_combine with array_keys

There is no interdiff as the patch from #72 didn't want to apply to latest core on one of the inheritdoc tag that was mentioned in #76 as out of scope.

• #76.1 - Addressed all items plus couple more inheritdoc which wasn't mentioned but clearly out of scope
• #76.2 - Used aggregate query as suggested
• #76.3 - Addressed by aggregate query implementation
• Replaced {@inheritdoc} with actual description for introduced in the patch getFormValueOptions()

(Incomplete review, as about to serve up dinner.)

1. +++ b/core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php
   @@ -98,6 +104,38 @@ public function init(ViewExecutable $view, DisplayPluginBase $display, array &$o
   +   * Get entity IDs from UUIDs.
   

   Should be 'Gets'.

2. +++ b/core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php
   @@ -98,6 +104,38 @@ public function init(ViewExecutable $view, DisplayPluginBase $display, array &$o
   +        // Abandon conversion if we already have IDs and not UUIDs.
   

   Why would someone call this with IDs? What's the reason for this guard clause?

3. +++ b/core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php
   @@ -98,6 +104,38 @@ public function init(ViewExecutable $view, DisplayPluginBase $display, array &$o
   +      ->getAggregateQuery()
   

   What's the reason for the aggregate query and group by?

4. +++ b/core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php
   @@ -98,6 +104,38 @@ public function init(ViewExecutable $view, DisplayPluginBase $display, array &$o
   +      // Access was already checked during configuration time.
   

   What's configuration time?

5. +++ b/core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php
   @@ -98,6 +104,38 @@ public function init(ViewExecutable $view, DisplayPluginBase $display, array &$o
   +    return array_combine($tids, $tids);
   

   Is the return keyed by tid? The @return should say.

6. +++ b/core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php
   @@ -284,6 +259,123 @@ protected function valueForm(&$form, FormStateInterface $form_state) {
   +   * Get the form value for text fields.
   

   Gets.

7. +++ b/core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php
   @@ -284,6 +259,123 @@ protected function valueForm(&$form, FormStateInterface $form_state) {
   +   *   Form API render array for text fields.
   

   render array? But returned var is called $form_value, doesn't match up.

8. +++ b/core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php
   @@ -284,6 +259,123 @@ protected function valueForm(&$form, FormStateInterface $form_state) {
   +   *   Options array suitable for form API.
   

   Written FormAPI, usually.

Addressed the comment #78 Point 1,6,8
Keeping it needs work to address other points on #78
Attached patch against Drupal 10.1.x

The scenario is missed when there is no term associated with the TID which is causing drush updb to fail.
(In our case there are TIDs in the views config files but the same TID wont be present on environment as TID is dynamic)
Modifying patch to handle such scenario

I fixed CS error, and I have uploaded the patch. Please review it

Our site is running on 9.4.x version and we are facing below issue in build, we have also checked by upgrading platform version to latest 7.24.1 but still no luck

Error: > [notice] Update started: taxonomy_post_update_convert_term_views_filters_to_uuids
> [warning] Undefined array key "plugin_id" taxonomy.post_update.php:48
> [error] TypeError: Drupal\views\Plugin\views\PluginBase::init(): Argument #3 ($options) must be of type ?array, string given, called in /site-archive/pfpfizerforyoudk-1670742508/app/core/modules/views/src/Plugin/views/argument/ArgumentPluginBase.php on line 1131 in Drupal\views\Plugin\views\PluginBase->init() (line 137 of /site-archive/pfpfizerforyoudk-1670742508/app/core/modules/views/src/Plugin/views/PluginBase.php)

So we have added below patch by adding fix into core view module and that issue is resolved.

Fixed CCF for #83.
Please review.

Please stop posting wrong patches.

Patch in #80 introduces a change without the interdiff, comments after reduce patch size from 20 Kb to bytes...

If people want their patches to be helpful, please learn how to work with them first https://www.drupal.org/docs/develop/git/using-git-to-contribute-to-drupa... .

Updates to patches must be supplemented with the interdiff to simplify review process.

I believe there is one more thing that needs improvement - at the moment taxonomy_post_update_convert_term_views_filters_to_uuids() needlessly sets filters: { } on all displays views.

We should wrap $display['display_options']['filters'] = $filters; in an empty check to prevent this.

We needed this patch for 9.4.x so I have rerolled #79 here against that. Attaching in case its useful to someone else.

This patch may fail in the case of a Views filter configuration referencing a term which no longer exists in the site DB.

In taxonomy_convert_entity_ids_to_uuids():

    $uuids[] = Term::load($entity_id)->uuid();

While that's not a valid configuration state with reference to the DB content, it is possible to encounter it (eg: term was deleted but view configuration was not updated), and the patch could handle it better by checking the result of Term::load() before calling its ->uuid() method.

We could either drop the item from the list of UUIDs (which feels wrong, even though the configuration was already not valid with reference to content), or we could fail the DB update in order to notify the site owners. I believe the latter is safer?

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 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.

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

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

MR appears to need a rebase.

MR squashed and rerolled.
Patch attached for composer.

Closed the old MRs for. 10.3 as they were failing and probably missed the boat for them.

Rebased 11.x and still all green

All threads appear to be resolved, templated to tag 11.2 highlight but will let committer decide.

One (non-trivial) comment on the MR about the upgrade path.

MR rerolled
Patch attached for composer

Comment #133 is still to consider