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

Convert Vocabularies to CMI #174

Closed
jenlampton opened this issue Feb 7, 2014 · 15 comments · Fixed by backdrop/backdrop#520
Closed

Convert Vocabularies to CMI #174

jenlampton opened this issue Feb 7, 2014 · 15 comments · Fixed by backdrop/backdrop#520
Assignees

Comments

@jenlampton
Copy link
Member

Vocabularies need to save settings into a config file.

For reference, here is the parallel issue from drupal.org

@quicksketch
Copy link
Member

I'm going to work on converting vocabularies to config files. A side-effect of this change is that vocabularies will no longer be "entities" by themselves. Akin to node types or user roles, vocabularies will be a type of taxonomy term.

@quicksketch quicksketch self-assigned this Oct 28, 2014
@quicksketch
Copy link
Member

This is coming along smoothly. Initial work is at https://github.com/quicksketch/backdrop/compare/backdrop:1.x...quicksketch:174/vocab_config?expand=1

Tests are already about 98% passing, but I'm worried about the comprehensiveness of the tests. We're pretty clearly changing Views as part of the upgrade, since things like the "taxonomy_vocabulary" table no longer exist at all, you can't join to them. I think that's going to be expected and not a problem, but the existing filters and arguments for terms should definitely be upgraded, especially given how frequently they would be used. This upgrade can be done by reading out the individual view config files, finding any arguments/filters using the taxonomy vocabulary filter, then updating them to use the new handlers.

@quicksketch
Copy link
Member

Tests are now fully passing and we have a working PR at backdrop/backdrop#520.

I've fully converted Views handlers to work without the taxonomy_vocabulary table, and added a new handler to display the label of a vocabulary directly from the taxonomy_term_data table, since we no longer need to join to get the vocabulary information (we just load it from config when necessary).

The default Views that Backdrop ships with actually don't need any modification. And it turns out the missing vocabulary relationship is not necessary in most views that display Taxonomy terms. Most of the fields/filters/sorts we use on Taxonomy term listings were already on the taxonomy terms directly, without the need to manually add a vocabulary relationship. On top of that, I'm not sure we could replicate the functionality 100%. So overall I think we should forego an upgrade for this particular change, as we can't adequately rewrite a view that used this relationship now that it no longer exists.

@quicksketch
Copy link
Member

API changes in this issue:

  • All vocabularies no longer have numerically indexed vid vocabulary identifiers. Instead, vocabularies are referenced exclusively by the $vocabulary->machine_name property, which existed previously but now is the only identifier.
  • Machine names may no longer be changed on vocabularies after they have been created.
  • Taxonomy $term variables previously used both $term->vid and/or $term->vocabulary_machine_name to reference their parent vocabulary. $term->vid no longer exists at all, and $term->vocabulary_machine_name has been renamed to just $term->vocabulary to bring it inline with $node->type and $user->roles, which both reference just machine names.
  • A number of functions that used to take $vid parameters now take machine names instead:
    • taxonomy_vocabulary_load($vid) => taxonomy_vocabulary_load($vocabulary_name)
    • taxonomy_vocabulary_delete($vid) => taxonomy_vocabulary_delete($vocabulary_name)
    • taxonomy_get_tree($vid) => taxonomy_get_tree($vocabulary_name)
    • taxonomy_term_load_children($tid, $vid = 0) => taxonomy_term_load_children($tid, $vocabulary_name = NULL)
    • taxonomy_get_tree($vid, $parent = 0, $max_depth = NULL, $load_entities = FALSE) => taxonomy_get_tree($vocabulary_name, $parent = 0, $max_depth = NULL, $load_entities = FALSE)
    • taxonomy_vocabulary_load_multiple($vids = array()) => taxonomy_vocabulary_load_multiple($machine_names = array())
  • taxonomy_get_vocabularies() has been restored to have parity with Drupal 7.
  • taxonomy_vocabulary_machine_name_load($name) is deprecated, since all vocabularies are now loaded by name.
  • Permissions for edit terms in x and delete terms in x now use the machine name instead of the vid.

@quicksketch
Copy link
Member

I've merged in #174.

This has quite a few API changes, tagging as needs change record.

@docwilmot
Copy link
Contributor

These are some big changes, but sound like they will make things simpler. Where are change records being documented?

@jenlampton
Copy link
Member Author

@docwilmot All change records will eventually be listed on api.backdropcms.org. Right now we have a handful of issues like this one that are tagged "needs change notice" - these have not yet been added to the API site.

@docwilmot
Copy link
Contributor

https://www.drupal.org/contributor-tasks/draft-change-record
This seems to be an open task in Drupal, how do we contribute draft change records here? I think given my below average coding skills and my probable ability "to read English well enough to understand an issue, and write English coherently" this would probably be a more reasonable place to contribute.

@jenlampton
Copy link
Member Author

@docwilmot wonderful! :)

Since I last commented here I think @cellear has started adding all the change records for our issues here on github, but there is a handful of change records on drupal.org that need to be added too.

@cellear can you add a comment here about how you'd like to split the responsibility of the remaining change records?

@docwilmot
Copy link
Contributor

Thanks @jenlampton, glad you approve. Please advise how change records are submitted and details about the approval process if any.

@jenlampton
Copy link
Member Author

Change records can be submitted on api.backdropcms.org: https://api.backdropcms.org/node/add/changerecord I've created an account for you there, please use the password reset feature to log in.

@docwilmot
Copy link
Contributor

@jenlampton it seems anonymous can post now too. In fact we already have some spam. Please check permissions.

@quicksketch
Copy link
Member

@jenlampton fixed it up. Thanks @docwilmot :)

@docwilmot
Copy link
Contributor

Cool
I'll work on change records now.
One last thing: I notice there is no way to tag a change record with what component of core it refers to (node, fields, layouts, whatever). I note that Drupal also does not. But I think it would be an idea to consider - Drupal now has 20 pages of unfiltered records. We should do better.

@docwilmot
Copy link
Contributor

Made a list at backdrop-ops/backdropcms.org#25. Will continue there since this issue was closed long ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants