Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change record missing for not being able to rename vocabulary machine names (feature removal compared to D7) #5446

Open
klonos opened this issue Jan 8, 2022 · 5 comments

Comments

@klonos
Copy link
Member

klonos commented Jan 8, 2022

We have this bit of code commented out in testEntityBundleRenamingDeleting():

    // Rename the vocabulary's machine name, which should cause its pattern
    // variable to also be renamed.
    // @todo: Renaming vocabularies in Backdrop is not supported. Remove?
    //$vocab->machine_name = 'new_name';
    //taxonomy_vocabulary_save($vocab);
    //$this->assertEntityPattern('taxonomy_term', 'new_name', LANGUAGE_NONE, 'bundle');
    //$this->assertEntityPattern('taxonomy_term', 'old_name', LANGUAGE_NONE, 'base');

I guess we've inherited that from early D8.

D7 allows editing the machine name of vocabularies:

In D8+ it is not possible:

We should decide:

  1. Keep things as is, completely remove this commented-out code from the test, and make sure there is a change record for this.
  2. Re-introduce the feature, and reinstate the commented-out code in the test.
@klonos
Copy link
Member Author

klonos commented Jan 8, 2022

I believe that the change that introduced this in D8 was Remove ability to update entity bundle machine names.

@argiepiano
Copy link

You can change the name (human label) of a Vocabulary but not its machine name.

At the risk of stating the obvious, vocabularies are now stored as configs in Backdrop (they are not full-fledged entities), and they don't use a numerical ID like all entities stored in the database. In D7, vocabularies were full-fledged entities, were stored in the table taxonomy_vocabulary, and had separate columns for vid and machine_name.

In D7 it was possible to change the machine_name of vocabularies because taxonomy terms referred to vocabularies through their vid or vocabulary id. This is not the case anymore. In Backdrop taxonomy terms refer to vocabularies through their machine names within the column vocabulary. The ability to change the machine name of vocabularies would imply changing this column for all existing taxonomy terms.

This is, in my opinion, potentially risky, which outweighs the benefit of changing a machine name which is not visible to the end user.

Perhaps I'm misunderstanding your suggestion, @klonos ?

@herbdool
Copy link

I see very little upside to this suggestion. I'm with @argiepiano on this.

@klonos
Copy link
Member Author

klonos commented Jan 16, 2022

@herbdool the suggestion is 2 options. The first one is this:

  1. Keep things as is, completely remove this commented-out code from the test, and make sure there is a change record for this.

So I'm basically suggesting to clean up a @todo by removing already commented-out code, and then also add a change record about this. If there was a function used for this in D7, we'll also need a wrapper that includes only a call to watchdog_deprecated_function() (which links to the change record).

@ghost
Copy link

ghost commented Jan 23, 2022

@klonos Maybe edit the issue title appropriately to avoid confusion with people thinking you're asking to revert this change...

@klonos klonos changed the title D7 feature regression: Cannot rename vocabulary machine names Change record missing for not being able to rename vocabulary machine names (feature removal compared to D7) Jan 24, 2022
@klonos klonos self-assigned this Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants