It'd be nice to be able to control which vocabularies' terms show up on tagadelic pages. I recently ran across some use-cases based on reasoning (seo, internal use only, etc.) that nobody should be able to list terms belonging to some vocabularies without having proper permission to do so.
I attached a patch which will allow site-adminstrators to select which vocabularies' terms will eventually show up on tagadelic pages, though its settings are global and are not based on roles.
Please consider my patch (against 6.x-1.2) for the next release of tagadelic. Or, if that's ok with you, I'll wet my feet with trying out git for the first time ...
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | tagadelic.only_selected_vocabularies_1.patch | 21.32 KB | macedigital |
| #1 | tagadelic.only_selected_vocabularies.patch | 3.62 KB | macedigital |
Comments
Comment #1
macedigital commentedsorry, forgot to attach patch
Comment #2
Bèr Kessels commentedMakes sense.
However, I am not all too happy with the interface 'concept' yet. Not that I have a better idea right now.
I am not sure what mental model to follow, but i lean towards the "show unless" like you introduce, rather then an alternative "do not show untill a voc is chosen".
The other thing is where to configure this. I tend to lean towards putting this on the admin/content/taxonomy/vocabulary/12/edit pages. With a form_alter we could introduce a new checkbox "omit from tagadelic views".
Anotherm not-usability-related thing, is how this affects performance.
and a last, quite important thing I miss, is how you deal with the chunks. I saw only the part where you filter on 'list' but could not see how you filter on 'chunk'. More general: this should extend towards the cloud itself too, which I cannot find in your code (admitted: I did not look into the great detail of the patch yet).
That said, if we filter in the general cloud, and in the chunks, we must keep a keen eye on performance: we introduce another WHERE IN(), that may potentially get performance down. I don't ask for anything complex, just to either measure the SQL-queries before and after (mysql always returns the time it took, but beware of mysqls low level caching). Or an AB before and after, if anyone reviewing this patch is aware of AB.
Comment #3
macedigital commentedHi,
as I didn't want to introduce any surprises for people updating the module - if my patch was accepted - so I opted for implementing a show-unless-explicitly-turned-off interface. Answering in regard to your concerns:
Usability:
Providing a way to set preferences on respective vocabulary edit pages sounds like a great idea. Maybe we could still leave the "global" setting as is, as it provides a nice overview over which vocabularies are enabled/disabled at a glance.
Performance:
Right now the patch introduces one additional db-query for getting the settings variable. The result of this query is then re-used (written into a static variable) for all subsequent uses for the same page request. By offloading the WHERE-IN problem from the database there shouldn't be much of an impact on performance, but I haven't AB tested yet. So I don't know the concrete impact it'd have.
Consistency:
The patch already applies for 'list' and 'chunk' pages, with one caveat:
If a url like /tagadelic/chunk/1,2 is requested (same goes for 'list'), vocabulary 1 is enabled, but vocabulary 2 is disabled, it will show a page with all terms from vocabulary 1 only and not go 404, eventually creating duplicate content for /tagadelic/chunk/1.
An additional all-requested-vocabs-in-list-of-enabled-vocabs check would be needed here.
And you're right, I completely forgot about blocks.
As soon as I find the time, I'll implement the changes regarding usability and consistency; and also run a few benchmark rounds including those changes so we know how much of an performance impact is to be expected.
Comment #4
macedigital commentedA short status update (@see attached patch (against 6.x-1.2)):
Notable changes:
- checkbox in vocabulary edit form for enabling / disabling vocab
- partially permitted requests for e.i. /chunk/1,2,3 (1 + 2 enabled, 3 is not) result in 404-error
- blocks are done as well
- uninstall settings / cache on module uninstall
I couldn't AB test performance yet, but will hopefully find time tomorrow doing that.
Comment #5
Bèr Kessels commentedComment #6
Bèr Kessels commentedPlease open a pull request on github if you still want this feature.