This feature is requested often, so i wrote a default argument plugin

CommentFileSizeAuthor
#80 684608-string-fix-6.x.patch1.04 KBDeFr
#80 684608-string-fix-7.x.patch1.05 KBDeFr
#78 0001-bug-684608-Fix-a-typo-in-user-facing-text.patch1.15 KBDeFr
#75 684608-75.views_.views-default-argument-tid-2x.patch2.5 KBjoachim
#67 bug-684608-Convert-new-taxonomy-settings-name.patch1.99 KBDeFr
#67 bug-684608-Convert-new-taxonomy-settings-name-take2.patch1.54 KBDeFr
#62 argument_default.patch7.3 KBdawehner
#60 argument_default.patch7.45 KBdawehner
#58 argument_default.patch7.45 KBdawehner
#55 684608-taxonomy-block-views-export.txt2.92 KBDeFr
#50 views-2x-default_arg_tid_node-684608-50.patch6.96 KBAgileware
#43 views-2x-default_arg_tid_node-684608-43a.patch6.96 KBAgileware
#43 views-2x-default_arg_tid_node-684608-43b.patch9.09 KBAgileware
#39 684608-39.views_.views-default-argument-tid-2x.patch3.49 KBjoachim
#36 views-2x-default_arg_tid_node-684608-36.patch5.64 KBAgileware
#35 views-2x-default_arg_tid_node-684608-35.patch2.31 KBAgileware
#30 684608-30.views_.views-default-argument-tid-2x.patch2.44 KBjoachim
#29 684608-29.views_.views-default-argument-tid-2x.patch2.45 KBjoachim
#19 views-default-argument-tid-2x-684608.patch4.89 KBManuel Garcia
#14 taxonomy_default_argument-2x-2-684608.patch4.56 KBManuel Garcia
#13 taxonomy_default_argument-2x-684608.patch4.21 KBManuel Garcia
#11 684608-taxonomy_default_argument-2.x.patch4.32 KBdropcube
#10 views-argument_default-tid-2x.patch5.48 KBpokurek
#7 views-argument_default-tid_0.patch4 KBdawehner
#2 views-argument_default-tid.patch3.99 KBdawehner
views-argument_default-tid.patch1.15 KBdawehner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Thats broken currently:
TODO

[17:27] <merlinofchaos> It should also optionally pull terms from a node if that's the current page and possibly optionally restrict what terms it will pick up by vocabulary(s).
dawehner’s picture

Here is a version with all this stuff.

Summit’s picture

Hi, will this handler work on node or term view? I need it for both:
1) On node_view to show nodes in a block which hold the same terms as current node
2) On term_view to show a block of nodes which hold all other terms of the nodes shown on the page

Great development!
Greetings, Martijn

dawehner’s picture

Sure this patch will allow it.

thekayra’s picture

Well this is what I call good timing =)

merlinofchaos’s picture

The name of the default argument should probably be Taxonomy Term ID from URL. Otherwise this looks really good. I'm not going to set needs work because I probably should just change that during the commit process, it's just a pain to edit and apply to multiple branches.

dawehner’s picture

Fixed this issue.

merlinofchaos’s picture

Status: Needs review » Fixed

Committed!

Status: Fixed » Closed (fixed)

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

pokurek’s picture

Version: 6.x-3.x-dev » 6.x-2.x-dev
Status: Closed (fixed) » Needs review
FileSize
5.48 KB

Hi,
There is a different approach how to implement default argument setting form in 6.x-2.x version.
I have failed with more then one form field, so I used the checkboxes type form field for settings.
Thnx for great work.

dropcube’s picture

Category: feature » bug
FileSize
4.32 KB

The options form of this plugin does not work. In 2.x is named argument_form and in 3.x options_form.

The form control dependencies do not work either and the selected options are not kept when updated.

The attached patch fixes these issues by renaming the function to correct name, and using the correct variables to populate the form and get the argument value.

Manuel Garcia’s picture

Status: Needs review » Needs work
+++ modules/taxonomy/views_plugin_argument_default_taxonomy_tid.inc	16 Aug 2010 21:44:46 -0000
@@ -67,13 +72,13 @@
+    if (!empty($this->argument->argument->options['term_page'])) {

I think you have one too many ->arguement here to grab the option.

Other than that the code looks to make sense. I haven't tried applying the patch nor testing it (no time now about to run to have dinner), but it looks good.

Powered by Dreditor.

Manuel Garcia’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
4.21 KB

OK, I've managed to get some time to test this patch out, it indeed has that small problem, I've updated it just fixing that.

I've also spent some time testing the expected behaviour from both grabing the tid from the /taxonomy/term/TID paths, and from the currently viewed node. They both work as expected, and it's awesome =))

I'm going to be brave and set this on RTBC, since I only did a small fix to the patch. Imho it's ready to be committed.

Manuel Garcia’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
4.56 KB

I gave it a bit more thought, and since the limit by vocabulary is realy only used when grabing the tid from current node, I made it have a dependency on that setting, so it only appears if you choose "Load default argument from node page. Good for related taxonomy blocks."

Otherwise it'd be confusing, since it's not used if you only choose "Load default argument from term page".

Summit’s picture

Hi,

Why do this also on Views-2, and not move on to Views-3, and improve the Views-3, taxonomy default argument.
Isn't by patching differently on Views-2, not the case, that moving to Views-3 is more difficult?

I moved all views-2 instances to views-3 and had no problem on functionality.
Greetings, Martijn

Manuel Garcia’s picture

Status: Needs review » Reviewed & tested by the community

As per conversation with dereine in irc

merlinofchaos’s picture

This patch applies to 2.x but not 3.x -- can I get a reroll so I can apply to both?

dawehner’s picture

Status: Reviewed & tested by the community » Needs work

@manual

Are you sure you want to remove the limit checkbox? this would be a change to 6.x-3.x
Sorry that i didn't saw this. Let's get this in with the less changes as possible.
It can be polished later

Manuel Garcia’s picture

Status: Needs work » Needs review
FileSize
4.89 KB

Well, I was only fixing the previous patch... didn't think about that.

Here is a reworked patch, keeping the limit checkbox. Note that I'm not including 'expand checkboxes' as '#process' in limit, as it doesn't seem to be needed, but do correct me if I'm wrong. At least it works properly without it.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Thanks!

This is fine.

merlinofchaos’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 2.x -- I presume this does not need to be committed to any 3.x branch.

Status: Fixed » Closed (fixed)

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

merlinofchaos’s picture

Priority: Normal » Major
Status: Closed (fixed) » Active

This still does not work in Views 2.x -- the options are misnamed and it generates notices all over the place.

merlinofchaos’s picture

I'm tempted to just take it out. The feature can't work.

merlinofchaos’s picture

Status: Active » Fixed

Commented it out and committed.

Summit’s picture

Hi, May be stupid question, but why not on 3.x-dev branch? Is it already there?
greetings, Martijn

Status: Fixed » Closed (fixed)

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

joachim’s picture

Status: Closed (fixed) » Active

So what's the story with this? It was 'fixed' by ripping it out?

I've already got code that does this on 2.x, simply done as a new plugin for 'term ID from current node', with the vocabs limited by the validation options. Merlin: you want it as a patch?

joachim’s picture

Status: Active » Needs review
FileSize
2.45 KB

I need this for a project right now, so here's the patch :D

This just adds a new argument default handler which picks the term IDs from the current node (that is, the one implied by the current URL).

The terms are limited according to the vocabularies chosen in argument validation.

joachim’s picture

Argh. Left debug code in :/

(I can't even type debug code properly -- it was meant to say 'boogaloo' :(

merlinofchaos’s picture

Sorry, yes, I ripped out a feature to 2.x that never worked.

I'm committing fewer and fewer new features to 2.x as this goes forward. I want to get one more big 2.x release out, and then basically freeze it and do only critical bug fixes.

joachim’s picture

So, just to clarify (and so I know whether to put it into custom code or not ;) -- is this a wontfix, or a potential fix for a bug in 2.x that people should review?

merlinofchaos’s picture

I'm okay with other people fixing it, and reviewing it. Just let it be known: The only involvement I'll have at this point is when I know it's good enough to commit and I'll commit it. :)

Agileware’s picture

I've been using this feature for a while to pull the tid from the taxonomy term pages and it has been working.
I have not tried getting the tid from a node.

Does this mean I have to stay on 2.12, even if there is a security release in future?
Or else apply the security fix manually myself.

I guess it isn't too major for me because I can use custom php code arguments (not everyone knows how though) but I wasn't expecting features to disappear on update and I can see other people also getting a bit stressed out when they update to 2.13 and their views break.

The solution in #30 also doesn't help my use case as it only works on terms from nodes so the tid from taxonomy term page url feature is still gone.

Agileware’s picture

Here is an updated version of the patch from #30 that applies cleanly to current dev.

Agileware’s picture

Here is a new patch.

This patch keeps the original checkbox options for term pages & node pages but replaces the code for node pages with the code in joachim's patch in #30.

This means it does away with the unusual limit options and instead uses the argument validator options to limit to specific vocabs.

This way we don't lose any settings that previously existed.

The only thing that is different from the old version is that on node pages it now only checks arg 1 instead of args 1, 2 & 3.
This can be changed back but seeing as we are providing an option for the default drupal node pages we only really have to care about arg 1.

dawehner’s picture

This means it does away with the unusual limit options and instead uses the argument validator options to limit to specific vocabs.

Mh. The limit's are helpful there, because they provide a common use case here:

Show nodes with the same terms as the current node, but only compare terms from a certain vid.
This can't be done here with a argument validator from my perspective.

joachim’s picture

> Show nodes with the same terms as the current node, but only compare terms from a certain vid.
This can't be done here with a argument validator from my perspective.

But that's exactly what the code I wrote does!

joachim’s picture

Updated version of my patch.

This adds the ability to choose whether multiple terms from the node are combined to make an OR argument (1+2+3) or and AND (1,2,3).

joachim’s picture

Just to explain what this patch does, as this issue has been meandering a bit...

With this default argument plugin, I can create a view that shows nodes with the terms taken from the current node, in the specified vocabulary/ies.

Suppose I have tagged a node with terms foo, bar from vocab 1 and term wibble from vocab 2. I can show on this node a list of other nodes tagged with foo AND bar, or foo OR bar, or tagged with wibble, or tagged with any (or all) of the three.

Agileware’s picture

The thing that concerns me about the latest patch is that unlike the patch in #36 we will still lose the functionality for getting the term id from the url on term pages.

If views-6.x-2.13 is created with this as it is in dev or with this newest patch this functionality will be lost and there will be a (possibly large) number of people who are already using this functionality that will all have their views broken, with no easy alternative to fix them (unless they know how to do it using PHP code).

Plus, AFAIK there was never a problem with the tid from term page functionality in the first place, but only with the tid from node page functionality.

joachim’s picture

It wouldn't be hard to improve my patch to do that:

- change the machine name
- change some of the UI text
- check for /taxonomy/term/TID before falling back to checking for a node.

Agileware’s picture

Here are a couple of new patches that merge the patches in #36 & #39 and also fixes support for the 'Allow multiple terms per argument.' option when using the term id from term page option (which it turns out never previously worked).

There are two patches for different approaches:
* patch a keeps the single handler views_plugin_argument_default_taxonomy_tid like the old method and the patch in #36
* patch b uses different handlers views_plugin_argument_default_taxonomy_tid and views_plugin_argument_default_taxonomy_tid_node like the patch in #39.

I don't mind which one gets preference but I would suggest a.
b makes the individual handlers simpler but means you can't have a view that takes arguments from both term and node pages, plus it would require an update to move peoples settings from the old way to the new way.

bryancasler’s picture

subscribe

bryancasler’s picture

Is this something that is already in D7? Because I'm looking for a solution to provide term OR arguments logic instead of AND.

Agileware’s picture

@animelion:

That is not what this issue is about.
This is just about providing the ability to take a default argument from the url on taxonomy term pages and from the node on node pages.

joachim’s picture

I've tested patch views-2x-default_arg_tid_node-684608-43a.patch in #43 above -- works find for getting taxonomy terms from the current node.

+1 from me :)

DeFr’s picture

I've been playing with patch 43a, and I can confirm it does work fine for extracting taxonomy terms from either a taxonomy term page or a node page in a single display.

Diving into the patch, grabbing the vocabularies from which the terms should be extracted from the argument validator instead of having them as a separate argument sorts of make sense from a UI point of view, but it creates a tight relationship between the argument_default_taxonomy_tid and argument_validate_taxonomy_term: if you create your own term validation plugin that's extending views_plugin_argument_validate_taxonomy_term, then the settings don't apply anymore (because $this->argument->options['validate_type'] wouldn't be 'taxonomy_term') . Don't know if its a blocker.

For choosing between 43a and 43b, I guess the real question is of what is acceptable for Views 2.x. I would favor 43a because it's a lot closer to what's in Views 6.x-3.x and 7.x-3.x, which should ease the maintenance quite a bit, but if it's deemed too risky for Views 2.x, then at a bare minimum the hunk of 43b that makes views_plugin_argument_default_taxonomy_tid.inc work on taxonomy term pages should be commited, and the plugin re-activated in taxonomy.views.inc, otherwise people upgrading from Views 2.12 will find a lot of their views broken.

Additionally, if 43a gets in, I guess patches will be needed to add the node_operator settings to both 6.x-3.x and 7.x-3.x, to keep them in sync. I would be happy to provide them.

Agileware’s picture

I guess it would be up to the maintainer on whether or not we need to support the case of custom validators, or if the creator of the custom validator will have to create a custom default argument handler as well.

Also, this line should probably be changed but i haven't had time to do it. Can do it on the weekend:

+++ ./modules/taxonomy/views_plugin_argument_default_taxonomy_tid.inc	2011-03-19 16:15:04.000000000 +1100
@@ -39,83 +38,78 @@ class views_plugin_argument_default_taxo
+          if (!in_array(-1, $this->argument->value)) {

This should be

if ($this->argument->value != array(-1)) {

Powered by Dreditor.

Agileware’s picture

The consensus seems to be to go with the 43a patch, so this is that again with the one line fix mentioned in #49.

Also, as merlin mentioned in #33:

I'm okay with other people fixing it, and reviewing it. Just let it be known: The only involvement I'll have at this point is when I know it's good enough to commit and I'll commit it. :)

And I don't expect a sudden rush of new reviews from other users so I guess we have to mark RTBC soon to get him to look over the current solution.

If 6.13 gets released without a solution to this there will be a lot of cranky people.

- Although, as DeFr said, we might first need an upgrade path for any changed settings but I have not the time to do that right now.

dawehner’s picture

The central problem with this patch is that it will basically have to be rewritten fro 6.x-3.x and 7.x-3.x

Additional i don't get why do you remove the 'vid' feature... Do you want to use validation instead?

dawehner’s picture

Additional this will break some existing views.

Can't we just port the argument default plugin from d6--3 to d6--2 and that's it?

Agileware’s picture

I don't mind what the solution is that ends up being in there, as long as it isn't the current dev state of no functionality at all as that would be a regression.

Basically, as long as my views don't break between 6.12 & 6.13 like they did from 6.12 to 6.x-2.x-dev I'll be very happy.

DeFr’s picture

Status: Needs review » Active

Let's step back here, completely forgets about the patches that were attached, and try to get back to the root.

Basically, the issue is that the commit referenced by merlinofchaos in #25 ( commit ffa8f33f ) removed an handler that used to work just fine in previous Views version (extracting the tid from the taxonomy term's page and using it as a default value for the arguments), which can break various things if you're upgrading from Views 6.x-2.12 to 6.x-2.x-dev. Quoting merlin, it was removed because the options are misnamed and it generates notices all over the place ; fixing the notices and the options naming should thus be enough.

While testing though, I start to wonder if there wasn't a problem with the setup used for the tests: using the latest -dev, and uncommenting the argument default value in 6.x-2.x, I don't get any notice, which makes it hard to fix them. When using PHP 5.3 and enabling strict warnings, there's one more of those caused by the definition of views_plugin_argument_default_taxonomy_tid::options_submit , but there's already a bunch of those as is.

I do get a notice when using 6.x-3.x when adding an argument though, notice: Undefined index: #options in /var/www/ows/core/6-v3/includes/form.inc on line 1930. and the indendation kinda looks messed up there: the vocabularies are shown even if the load taxonomy term from node page isn't checked.

I'm thus not quite clear on what really needs to happen to make taxonomy's argument default plugin more suitable for a release; any clarifications would be more than welcome: as is, the 6.x-2.x code looks more production-ready than the 6.x-3.x one.

DeFr’s picture

This issue is now in the wild with the release of Views 2.14.

To try to help this issue move forward, in addition to #54, I'm going to outline the use case that works fine in 2.11/2.12/2.13 and was broken by the commit in #25 and attach an export of a View exhibiting this problem: imagine that you want your taxonomy term page body to only show Story nodes, and that you want a block on the same page to only list the titles of the Page nodes that have the same taxonomy term. It's rather easy to do with Views 2.13, you can clone the provided taxonomy_term default view, add your filter, and then create another view like the one attached to get a block that you can just use in your Blocks page. That looks like a not that uncommon pattern, and all those views are broken by the removal of the "taxonomy term id from url" default plugin removal.

alexkb’s picture

We're also having this issue with 2.14. We've used this functionality (default argument -> Taxonomy Term ID from URL) on a number of blocks which can't get fed the argument as they're blocks. For now, we've done a bit of a quick hack using "PHP code" as an argument type, and then using:

$t =  taxonomy_get_term(arg(2));
return $t->tid;

Which seems to work ok for now, but is probably not the most efficient method.

rooby’s picture

This is going to catch a lot of people out now that it is in a recommended release.

It would be good to make more of a mention of it in the release notes for 2.14.
Currently it just has:

#684608: Removed taxonomy tid default argument plugin, for it is non-functional in 2.x

in the middle of everything else.

Maybe it needs a notice at the top that stands out more to let people know to review any views that use the default argument for taxonomy tids.

dawehner’s picture

Status: Active » Needs review
FileSize
7.45 KB

It's kind of funny that people complain about a feature which never worked, anyway here is a patch which works for me,
but i justed just the storage & display of the storage, but i guess the code from manuell's patch should have worked.

DeFr’s picture

Status: Needs review » Needs work

@dereine: The problem is that while the "extract taxonomy terms from node" didn't work in 2.11 / 2.12 / 2.13 (in fact, you couldn't see that the option was there, and the label only state "Taxonomy Term ID From URL, so no-one assumed it would work), the "extract taxonomy term id" from the taxonomy/term/TID part of the plugin worked just fine, and that's what people used it for.

The block could maybe have had some weird results when used on a node page, but, well, that's what Block visibility options are for: the block is designed to be seen on taxonomy term path, so the block was probably restricted to the taxonomy/term page in the settings. ;-)

WRT to the patch in #57, it's adding a console.log in dependent.js and a dsm in views_plugin_argument_default_taxonomy_tid.inc, which I guess are leftovers from a debugging session ;-) More seriously, I think it's also missing a hunk to update the option names in the option_definition of the class, I guess 'term_page' should also become $this->option_name . '_term_page' there. I don't really get why the options need to be renamed here; I guess the goal is to avoid a nameclash should multiple defaut value plugins want to use the 'vids' key, and that the patch would be applied to 6.x-2.x , 6.x-3.x and 7.x-3.x ? If this is a case, maybe something's needed the old options names to the new ones, otherwise I guess Views saved on those branch would need to be re-configured after the patch is applied.

dawehner’s picture

Status: Needs work » Needs review
FileSize
7.45 KB

Ah yes, but this is just by accident :)

Removed the two dsm/console.log instances, thanks for spotting them.
The options have to be renamed here because default argument plugins in views2 kind of suck:

* You don't have option_definition
* you don't have options_form, but only argument_form
* you don't have options_submit but no similar thing.

The argument default form is merged into the options form of the argument, so you have to prefix the values to be able to fix similar prefixes, see any other default argument plugin in views2.

This patch will ONLY be applied to views2, because in views3, it works fine.

DeFr’s picture

Status: Needs review » Needs work

Patch in #60 is the same as the one in #58 ? Wrong file attached ?

dawehner’s picture

Status: Needs work » Needs review
FileSize
7.3 KB

Another try :)

DeFr’s picture

Ok, I've been playing with this patch, and it seems to work fine. The last hunk introduces a pattern that's already used in views_plugin_argument_default_user in 6.x-2.x, and is present in 6.x-3.x, so I guess it makes sense to include it in this patch.

The fact that Views 2 doesn't use the option_definition means that the intended behavior of auto-checking the "Load default form term page" can't work, and site builders will need to check it when upgrading from 2.11, but I think that's workable.

The only nitpick-y thing would be that the pattern for the other option names seems to be along the line of 'default_argument_X' (for example, default_argument_user, default_argument_date, ...), which would make this option_name default_argument_taxonomy instead of default_taxonomy_tid. That's really minor though.

The fact that the option names doesn't match means that Views don't upgrade cleanly between Views 2 and Views 3; the fix seems to be adding a convert_options function in the Views 3 branch, just like the one that's present on views_plugin_argument_default_user, to convert the options' name, right ? I've a patch to do that locally, I can attach it if you want, but I think the final option name for Views 2 should be sorted out first.

And most importantly: thanks for working on this ! :-)

dawehner’s picture

Thanks for the review and testing! I would be very happy if you could attach your patch to the conversion, and the renamed variable.

dawehner’s picture

I actually think that the name here should reflect the name of the plugin, not the module.

dawehner’s picture

So from my perspective this patch could be commited, @defr?

DeFr’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
1.54 KB
1.99 KB

Re-reading the patch, yes, reflecting the name of the plugin seems fine, so, I agree it's ready to be committed => RTBC for the patch in #62.

I'm also attaching the patch I mentionned above for the 3.x... In fact, I'm attaching two of them: the first one is rather conservative, and use the approach used in all the convert_options functions (migrate the settings explicitely). The second one finds all the options names by calling option_definition, and iterates over it to migrate the settings. Feel free to pick whichever you like best :-)

dawehner’s picture

Status: Reviewed & tested by the community » Fixed

Both seems to look fine!
Commited to 6.x-2.x and the other patch(Variant 2) to 6.x-3.x.

ccshannon’s picture

I don't understand this comment. It ("Taxonomy Term ID from URL" default argument handler) was working fine for my company until 6.13-6.14 update, unless you're referring to something else.

Anyway, glad to see it was patched, so thanks all. I'll happily wait until it makes its way into a release.

Thanks, again.

Stolzenhain’s picture

Wow! This snippet really saved my day (or at least the two hours of searching through taxonomy template behaviour and getting it right).

I too was a bit dumbfounded, when parts of my page blocks ceased to work after the Views update with a formerly available argument option missing.
Now I actually hope that the committed patch could be in a soon-to-come Views release, so I don't have to quickfix all my sites using this argument code after the Views update.

joachim’s picture

Status: Fixed » Active

Did the text "Load default argument from node page, thats good for related taxonomy blocks" get committed as part of this issue? It really needs rewriting!

joachim’s picture

Also, we've lost the ability to OR the terms rather than AND them, which was in my patch further up.

DeFr’s picture

@joachim: The text was sort of introduced in this issue, all the way back in #2 ; it was tweaked a bit in the last patch, from "Load default argument from node page. Good for related taxonomy blocks" to "Load default argument from node page, thats good for related taxonomy blocks". Before the last fixes though, the text was present in the source but hidden, so you probably couldn't see it.

The ability to OR the term instead of AND-ing them definitely needs to be a new issue, with patch for all the active branches. Given that 2.x is supposed to be frozen, and that this issue got in only because it was a regression from 2.11, it seems unlikely to ever land in 6.x-2.x.

joachim’s picture

> to "Load default argument from node page, thats good for related taxonomy blocks".

A tweak full of typos :(

> The ability to OR the term instead of AND-ing them definitely needs to be a new issue, with patch for all the active branches. Given that 2.x is supposed to be frozen, and that this issue got in only because it was a regression from 2.11, it seems unlikely to ever land in 6.x-2.x.

But it was in my patch on this issue in #39.

joachim’s picture

Status: Active » Needs review
FileSize
2.5 KB

One last time, hopefully:

- fixes typos in UI text
- fixed "return (complexexpression)" which makes debugging argument code a PITA
- re-added option to choose the operator.

rooby’s picture

Yeah, it seems like all the work put in from #29 to #50 was pretty well ignored.

+1 for the additions in #75.

bryancasler’s picture

I'm not even on D6 anymore, but I'm glad to see this getting some love.

DeFr’s picture

Well, to be honest, I would say that this is what happens when one tries to put to many things in one issue; the focus is put on fixing the root issue. The any/all should be dealt with in #1197030: No way to use "any tid" as default contextual filter, which was already commited in 7.x-3.x and should be ported to 6.x-3.x and mayyybe if dereine/merlinofchaos feels like it to 6.x-2.x.

That leaves us with the string fix. I take back a bit of what I've said in #73, in that that's the text that is already used in 6.x-3.x and was used also in 7.x-3.x until 2edecc68 went in to tweak it. It looks like merlin fixed the string when committing to D7 while it wasn't part of the patch, which means that it didn't land in the 6.x-3.x branch, which means that it's not in 6.x-2.x either.

Attaching the one-liner patch to bring them all back to sync. It should be applied to both 6.x-2.x and 6.x-3.x

joachim’s picture

Status: Needs review » Needs work
+++ b/modules/taxonomy/views_plugin_argument_default_taxonomy_tid.inc
@@ -35,7 +35,7 @@ class views_plugin_argument_default_taxonomy_tid extends views_plugin_argument_d
+      '#title' => t('Load default argument from node page, that\'s good for related taxonomy blocks'),

Sorry but that is still not grammatical English. If you're going to throw away the work I've done in my patch, at least use the fix for the UI text I put in too.

DeFr’s picture

Status: Needs work » Needs review
FileSize
1.05 KB
1.04 KB

@joachim: Well, that's still what's in 7.x-3.x, and what's needed if we want to synchronize all the branches. Sure, we could have two patches, one for 6.x-3.x and one for 7.x-3.x to change it everywhere at the same time, but every time you do something like that, you decrease the likelihood of your fix ever being commited.

All that being said, and to not lose your translation, I'm uploading the two patches, the one labelled 6.x should be applied to the two 6.x branches, and the one labelled 7.x should be applied to 7.x-3.x

joachim’s picture

Views 7.x-3.x has an option to choose the operator, just it's presented differently:

Multiple-value handling
Filter to items that share all terms
Filter to items that share any term

Once again: the patch to review is at comment #75.
The string fix in 684608-string-fix-7.x.patch should may be go on another issue.

DeFr’s picture

It's not only that it's presented differently, it's that the option name isn't the same, that the logic is different, and that if the patch in #75 is applied, then, the views you make in 2.x will lose their settings when you upgrade to views 6.x-3.x / views 7.x-3.x.

So really, the thing that should be dealt with in a separate issue is the OR-ing of the term, which already have a dedicated issue, which explicitely aims to backport what's in 7.x-3.x to 6.x-3.x and maybe 6.x-2.x

MustangGB’s picture

Issue summary: View changes
Status: Needs review » Closed (won't fix)