On lines 279 and 202 of openlayers_views_style_map.inc, functions from the content module are called to get a list of available fields. I'm working on a site in which the content module is disabled and it seems like this dependency could be easily eliminated. I'll have a patch with an hour.

CommentFileSizeAuthor
#1 remove_content_dependency.patch1.69 KBtmcw

Comments

tmcw’s picture

tmcw commented
StatusFileSize
new remove_content_dependency.patch1.69 KB

Sorry that took so long, this is an absurdly simple patch. (Since Geo requires Content, the dependency is transitive here...)

zzolo’s picture

zzolo commented
Status: Active » Needs work

Hey tmcw,

Thanks for the patch. I looked it over really quick and still seems like you are still using content_fields() but only if Geo data is being selected. Is the assumption that if Geo is installed, CCK will be too?

If that is the case, I have a couple problems:

1) Geo storage could still be used for some reason, though unlikely. And I dont think what we are doing here is assuming a Geo Field
2) If we are going to use that function, we need to actually do some checking if content is installed or make it a requirement for the whole module.

Thanks again for the work,
zzolo

tmcw’s picture

tmcw commented

Hmm, I hadn't considered that possibility... it seems odd that content_fields is called to basically just check that the fields are fields, when that is asserted on line 49. Can we not just even use that loop to re-check that geo fields are content fields?

rsoden’s picture

rsoden commented

The content() function is being called because the WKT data on geo fields isn't available to views at that point. So the field needs to be loaded and the data added to the query. I assume this is the whole reason that the views->query method is being mucked with in the first place.

I think this is a valid issue though. Can we solve it with some if_module_exists checks in strategic places?

tmcw’s picture

tmcw commented

I've been trying to test this with Geo Field / Geo Data and views integration there is definitely pretty broken. Since it's practically untestable to make this compatible with Geo Data minus Geo Field, I think it makes sense to just support users who have Geo Field installed, and thus checking for the existence of Geo Field before running that block of code.

Thoughts?

rsoden’s picture

rsoden commented
Status: Needs work » Reviewed & tested by the community

I spent a little more time looking at the geo_data module and older versions of the openlayers views integration last night. It seems to me that geo_field was all that was ever supported. Like Tom, I was unable to get geo_data working with any kind of view so I don't think testing is even a possibility at the moment.

Given that, I think the patch supplied here is a perfect solution for the time being. Those content functions will only ever get called if someone is trying to use a geo_field field in the view and in that case, they will obviously have CCK turned on as geo_field has this a dependency.

Once views integration for the geo_data module is a little more stable, we can look at supporting that, but this is a separate issue.

It would be great to get this committed.

zzolo’s picture

zzolo commented
Status: Reviewed & tested by the community » Fixed

Patched and commited: http://drupal.org/cvs?commit=224672

Any more related issues should go here: #485252: Views Style Plugin

Status: Fixed » Closed (fixed)

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