To improve exportability of pages, it was helpful, if vocabulary settings could be exported with their machine name instead of the numeric 'vid'.

As far as I see, this should be the case in several contexts, arguments , ...

In the first comment I'll post the code for the 'term argument'. If the code fits the needs, I'd provide some rework for the other components too ;)

CommentFileSizeAuthor
#11 ctools-term-arg-machine_name-1542544-11.patch8.07 KBthepanz
#7 ctools-term-arg-machine_name-1542544-7.patch4.37 KBjeqq
#5 ctools-term-arg-machine_name-1542544-5.patch4.32 KBAngry Dan
#5 interdiff.txt4.48 KBAngry Dan
#3 ctools-term-arg-machine_name-1542544-3.patch3.83 KBderhasi
#1 ctools-term-arg-machine_name-1542544-1.patch3.77 KBderhasi

Comments

derhasi’s picture

derhasi commented
StatusFileSize
new ctools-term-arg-machine_name-1542544-1.patch3.77 KB

And there's the first part of the patch ;)

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Needs work 
+++ b/plugins/arguments/term.incundefined
@@ -161,3 +175,41 @@ function ctools_term_breadcrumb($conf, $context) {
+ *   array of either vids or machine names

Begin with a capital, end with a full stop

+++ b/plugins/arguments/term.incundefined
@@ -161,3 +175,41 @@ function ctools_term_breadcrumb($conf, $context) {
+ *   if set to TRUE (default), $args are handled as machine_names
+ *   else $args are expected as vids

Same as above, also use " Defaults to TRUE." at the end of the line instead.

+++ b/plugins/arguments/term.incundefined
@@ -161,3 +175,41 @@ function ctools_term_breadcrumb($conf, $context) {
+ *   either array of vocabulary ids ($to_vids == TRUE)
+ *   or array of machine names ($to_vids == FALSE)

Same as above

+++ b/plugins/arguments/term.incundefined
@@ -161,3 +175,41 @@ function ctools_term_breadcrumb($conf, $context) {
+  $args = array_flip($args);
+
+  if ($to_vids) {
+    $return = array_intersect_key($map, $args);
+  }
+  else {
+    $return = array_intersect_key(array_flip($map), $args);

Is all this flipping needed if array_intersect is used instead?

derhasi’s picture

derhasi commented
Status: Needs work » Needs review
StatusFileSize
new ctools-term-arg-machine_name-1542544-3.patch3.83 KB

Tim, I adjusted the comments, as u proposed.

The flipping is really needed, IMHO, as we need the values of the map to be returned, which are the counterpart of the input $args. In the case we use, array_intersect, we would have to flip the return array. In each case we get similar numbers of flips.

ousmaneo’s picture

ousmaneo commented

Tested !
Working patch makes possible exporting panels page taxonomy argument with machine_name instead of vids

Angry Dan’s picture

Angry Dan commented
StatusFileSize
new interdiff.txt4.48 KB
new ctools-term-arg-machine_name-1542544-5.patch4.32 KB

I've made some pretty big adjustments to this patch which should improve performance amongst other things.

  1. I've moved as much of the array_filtering into save time as possible.
  2. Prefer using machine_name over vid, to avoid having to db lookup all of the vocabularies on every request. Legacy support will still require the db lookup but it's better than nothing
  3. General refactoring
  4. Repurpose _ctools_term_vocabulary_machine_name_convert() to only be used for legacy support.
jeqq’s picture

jeqq commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Patch from comment #5 works for me. Thanks.

jeqq’s picture

jeqq commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new ctools-term-arg-machine_name-1542544-7.patch4.37 KB

Notice for Taxonomy term template pages.

Notice: Undefined index: vids in ctools_term_context() (line 42 of /var/www/features/sites/all/modules/contrib/ctools/plugins/arguments/term.inc).

DamienMcKenna queued 7: ctools-term-arg-machine_name-1542544-7.patch for re-testing.

davidwbarratt queued 7: ctools-term-arg-machine_name-1542544-7.patch for re-testing.

Angry Dan’s picture

Angry Dan commented
Status: Needs review » Reviewed & tested by the community

I've had this working for ages now, can we get a commit please?

thepanz’s picture

thepanz commented
StatusFileSize
new ctools-term-arg-machine_name-1542544-11.patch8.07 KB

Updated patch, rerolled against latest 7.x-1.x release.
Added the same edits also to the "term_depth" plug-in (that ships with CTools)

  • japerry committed c99a154 on 7.x-1.x authored by Angry Dan 
    Issue #1542544 by derhasi, Angry Dan, jeqq, thePanz: Support vocabulary...
japerry’s picture

japerry
Location KOMK
commented
Status: Reviewed & tested by the community » Fixed

I'm not sure why you'd want a vid over a machine name anyway. Tested fine for me. I'm a little worried about legacy, but the checks in the patch seemed to work for me.

Committed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.