Closed (fixed)
Project:
Administration Views
Version:
7.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
17 Feb 2011 at 18:05 UTC
Updated:
20 Jun 2012 at 10:11 UTC
Jump to comment: Most recent file
Comments
Comment #1
sunComment #2
dawehnerThe idea of this patch is to get the module which "provides" this view. Therefore the schema of the base_table is loaded.
To get this working i changed one base_table of a view, to be sure this is working
Comment #3
sunHm. I actually silently hoped that Views would have some hidden internal feature baked in that reveals the module dependencies of a view... but if that is not the case, then, hm.
We're going to face this issue at least one more time again, with the "Language" column in the node/content view, since that only really exists (or should matter) when Locale module is enabled. That's a more granular case though.
For this case, I wonder whether should perhaps simply rename the default views files? À la MODULE.WHATEVER-PATH-ID.inc - and extract and test for the MODULE part afterwards?
That said, but I definitely don't want to block this issue on that, I also recently started to play with the idea of possible options for contributed modules to plug in to admin_views - by default. I.e., if you have those modules installed, and you install admin_views, then you get the whole magic for free (whereas magic == default views using the System display handler). If not, those modules provide their own administrative UI, as usual. -- What I'm essentially after in the end is some sort of an algorithm to figure out module dependencies for (administrative, module-provided) default views, I guess...
Comment #4
sunThe drupal_get_schema() looks a bit scary in terms of performance, but I guess/hope that hook_views_default_views() is only invoked rarely.
This won't resolve the optional "Language" field/column issue I mentioned in #3, but that might be a separate issue anyway.
Thus, I'd be fine to go ahead with this as a stop-gap fix.
Comment #5
dawehnerYeah hook_views_default_views is only executed when you flush the caches.
This should actually work already, as long you don't get confused by "broken field" in the admin ui.
This will work.
Comment #6
sunFirst of all, I've committed @dawehner's patch in #2.
Attached patch is what I suggest.
Comment #7
sunSorry, missed a DRUPAL_ROOT in there.
Comment #8
sunAnd meh, the $paths are entirely obsolete.
Comment #9
sunTotally works. Thus, thanks for reporting, reviewing, and testing! Committed to all branches. (backported accordingly)
A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.
Comment #10
dimitriseng commentedI am using the latest dev, but the Language field/column is not included in the node/content view. Are there plans to include it or a separate issue needs to be created for this? Thank you.
Comment #11
sunIndeed, I did not have Locale module enabled when I re-exported the views. :-/
Comment #12
sunNote: I've enabled automated testing for admin_views now.
Thus, when restoring the Language field/filter, let's also add an automated AdminViewsLanguageTestCase test that ensures we don't introduce this regression again in the future.
Comment #13
dimitriseng commented@sun, thanks for the clarification. Is this just a case of you reexporting the views with Local enabled or is there more work involved to get Language field/filter availabe? This is quite useful for multillingual sites. Thank you for your great work.
Comment #14
sunI briefly scanned the commit history and apparently, the Language fields/filters seem to have gone lost much earlier in history. I wasn't able to locate when exactly.
If I understood @dawehner correctly, then we should be able to include the Language field/filter in the exported default views, but should naturally expect a Views notice about "broken handlers" on sites which don't have Locale module enabled, but which can be ignored.
FYI: This seems to be last issue that blocks a stable D7 release.
Comment #15
damiankloip commented@sun, what is going on with this? Is there anything we need to do? If so, let me know.
Comment #16
sunViews currently doesn't expose the Language field/filter when only Locale module is enabled. Created a patch for that:
#343178: Node language should be available without node translation being enabled
With that out of the way, the "separate display" approach would look like attached patch.
However, I'm not happy with having to override the entire field + filter configuration, when all we want is to add a single field + filter.
Comment #17
sunAlso, #16 blows up when Locale module is not installed.
This would be an alternative approach, but:
1) requires custom code
2) doesn't work, because "locale" alphabetically comes before "node"
So in turn, this pretty much leaves us with introducing a completely custom hook_admin_views_default_views_alter() in order to support this case.
Or... we just ignore this entirely.
Comment #18
damiankloip commentedI would be tempted to not include this functionality at all, I don't think it's the most common use case either. I think more people won't use this than will (I might be wrong on that though). I know we don't want to go down the route of custom code but if we did could we not use hook_views_default_views_alter to do what we needed (briefly discussed in IRC)? We could also do this directly in the exported view include, but this would make it more cumbersome when re exporting.
Comment #19
dimitriseng commented+1 for inlcuding the language filter/field in the node/content view.
I understand the technical complications of including this functionality, but I believe that this is very important for multilingual sites, if the language filter is not included then the node/content view would become unusable for these sites, and I am sure that these are a lot, so I hope that a way will be found to get this added. Many thanks for your great work.
Comment #20
sunAll of the above explored approaches look like a pain to maintain to me, and I'd totally place a bet that we'd forget to update one of the overridden views or displays or alter hook code in future exports.
It seems more reasonable to wait until Views supports "conditional plugins" so we're able to (natively) limit the addition of the Language field + filter in the exported views definition to when Locale module is installed.
I'm not sure whether there is a feature request against Views for that already, but if not, we should totally create one ;)
(And btw, the problem isn't unique to admin_views — I know that the exported views for drupal.org also contain custom edits in order to handle conditional definitions based on whether module XYZ is available or not.)
Comment #21
damiankloip commentedI think this is the right decision :) Removing tags.
Comment #22
dimitriseng commentedI guess also that this is the right decision based on the comments above. However, this makes this module almost unusable for multilingual sites, so I would suggest that somebody who understands what is required to get this working, submits a feature request for Views as suggested by sun in #20, if one does not exist already. Thank you.
Comment #23
damiankloip commenteddimitriseng: If you have a multilingual site, you can just add this filter to your admin views and override them, it will be no bother at all really, and will only take about 10 seconds.