CCK/Views: location_instance fix

It has some unrelated changes in it, but here's a diff of my modification for relationships.

I have tested this patch but this does not work for me. This work for you bdragon ?

Thanks

You have to add relationships to the view for it to work. That's part of the problem -- it will increase the complexity of doing a location view.

Is #347030: location_cck needs a views relation a duplicate?

It looks to me like they are duplicate, although one patches location.views.inc and the other location_cck.

DISCLAIMER: Let me make it absolutely clear that I really appreciate the work being done on the location_cck module. Okay, time for criticism :)

I've tried this patch out (against 6.x-3.x-dev 2009-Mar-06) and it works as advertised but I think this may not be the best approach for another reason than what bdragon stated in #3:

If you want to create a search for locations on more than one content type you either have to:
a) use a single location_cck field for all content types
b) add a relationship for each location_cck field to the view

This, once again, adds complexity to location views.

This actually sounds kind of nice...

If the CCk location Is not the same, probably don't want to include it in search results... And if do, can by adding extra relationship

Also, I get the feeling that relationships are going to get more use and attention as people start using cck and views anyway. I'm not opposed to having to use relationships, but at least while "we" get used to it, some documentation and examples will probably be enough.

:) my non-expert opinion -C

I second the non-expert opinion with one of my own. I am just starting to get my head around relationships. While their role is not immediately obvious (like most things with Views), they really expand what you can do with Views. Even now, I am running into an instance where I wish another module had a relationship defined. So I would say using Relationships more would be a good thing.

I agree that relationships are important are going to be used more often but this is not the proper use of relationships. Relationships are there to connect different types of data together. The Filters section is there to restrict content types.

It seems there is already enough confusion surrounding relationships and this special behavior with regards to location_cck fields compounds that issue.

Doesn't seem to work for me. I've tried both rc1 and dev with and without relationships.

Tagging.

Tagging.

About how to get better relationships in without outright breaking everyone's site:

[13:00] <Bdragon> I was having issues with stuff taking the wrong path, coulda just been views bugs though
[13:00] <Bdragon> something about.... uh...
[13:00] <merlinofchaos> There's only one path =)
[13:00] <merlinofchaos> Which is probably the problem.
[13:01] <Bdragon> the author of a node linked to a location via a cck location or something
[13:01] <Bdragon> vs. the author of a node linked to a location via a native location
[13:01] <Bdragon> now if it were a mix of the two, I can't even solve that without a ******* UNION
[13:01] <merlinofchaos> Right. There is only one path, so you can't have both of those I think.
[13:02] <merlinofchaos> Bdragon: Here is my recommendation.
[13:02] <Bdragon> Yeah, but since there's already an implicit path natively
[13:02] <Bdragon> views was choosing that over the cck path
[13:02] <merlinofchaos> Create a version with both the implicit and explicit relationships. Document that the implicit relationships are deprecated and go away. Write an analyze tool that will look for these implicit relationships as being in use and say "In a future version these will go away, please remove them.'
[13:03] <YesCT1> bdragon: I went back though the cvs messages (to feb)  and I dont see a commit on 391160 (patch post dated March 4th)
[13:03] <Bdragon> oh, with that analyze hook that lets you output warnings?
[13:03] <merlinofchaos> Bdragon: Yes.
[13:03] <Bdragon> Hmm....
[13:03] <merlinofchaos> Actually the analyze hook is probably something nobody ever uses.
[13:03] <merlinofchaos> Bdragon: In the documentation, very loudly (and probalby in multiple places) tell people that they should go and analyze all their location related views.
[13:04] <merlinofchaos> Give them maybe 2 versions, and then rip out the implicit stuff.
[13:04] <Bdragon> Hmm, that is the best idea I've heard so far on the issue :)

sub

subscribe

I agree with YesCT that the more I learn about relationships the cooler they get. I think there is nothing wrong with relying on them and in fact I say the more the better.

sub

I'd suggest that, because location_cck entries are entirely dependent upon the core node and when handled in this fashion are not a separate data structure, they should be accessible without having to use a relationship. You don't use a relationship to display text field, or emfield videos, etc, so there's no reason that cck_location should either.

A follow-up thought I had this morning:

1. Separate the built-in direct content type integration into its own module, node_location,
2. Change cck_location to work off the same nid/vid fields,
3. Change the rest of the module so that it behaves the same regardless of whether you use the cck_location or node_location.
4. At this point you have a core API for interacting with the locations system, the only difference comes down to how the user adds data to the system.

Thoughts? A bit of a departure from the current system, but the current system really is not cleanly structured when something so simple doesn't work.

So far, I love the relationship feature. I'm cool with the idea of it auto-discovering the relationship, but I would dig deep first to make sure there aren't some cases where the discovery is not the intent of the designer.

subscribe

Disclamer: I'm pretty new to the world of CCK/Views and I'm not yet sure what the location_instance is supposed to do :D

I just created a simple content type which includes a location field. Also created a very simple view which shows the node title and the location city. Without any patches Views creates a query like this:

SELECT node.nid AS nid, node.title AS node_title, location.city AS location_city FROM node node LEFT JOIN location_instance location_instance ON node.vid = location_instance.vid LEFT JOIN location location ON location_instance.lid = location.lid WHERE node.type in ('company')

I.e. it fetches all node rows with "node.type = 'company'", it joins that to the location_instance table on "vid" and it joins the result of that with the location table on "lid". All this without specifying relations or anything like that.

This results in a table with, in this instance, company names and a 'city' column filled with NULLs. This is caused by the location_instance table which seems to be missing some information:

+-----+-----+-----+---------------------+-----+
| nid | vid | uid | genid               | lid |
+-----+-----+-----+---------------------+-----+
|   0 |   0 |   0 | cck:field_address:3 |   3 |
|   0 |   0 |   0 | cck:field_address:4 |   4 |
+-----+-----+-----+---------------------+-----+

if I run: "update location_instance set vid = lid;"

The NULLs get replaced by the actual cities.

Bottomline: It seems to me that the location module doesn't properly fill the location_instance table which is supposed to be fixed by a patch in http://drupal.org/node/347030 (haven't tested that yet). This way location fields work out of the box without any manual relations in Views.

BartVB, there seems to be a thread here where a solution was posted that may address what you're talking about: CCK fields not showing

That thread is ongoing, i just subscribed...

Trace:
Views joins location with location_instance which joins to node but missus the required foreign key vid which along with equally important nid and uid does not get collected in hook_field() $op = 'insert' and/or 'update' before location_save_locations($items, $criteria) is called which updates location_instance table.

By preparing the $criteria thoroughly before location_save_locations() location is complete and both location_cck and location node works good with views.

No need for the views relationship fix anymore, which only complicates the SQL queries. Note to issue junkies:Remember to remove 'views data' Relationship definition, as soon as this is fixed.

But enough from me, lets give the patch the final say, review it and change the status as you wish as long as we get all these open issues consumed quick.

Who's feeling a little peckish, for grilled mapedelic issue kebabs, like me?

Patch rolled against DRUPAL-6--3 branch.

Duplicates marked and collected:
location_cck needs a views relation
CCK fields not showing
how to: display the location from a CCK field (the address, not a map or anything) in views has a template preprocess fix, this patch will make this obsolete.

Similar patch found here location_cck not saving right now marked duplicate.

Nick; your patch in #25 can't be retrieved, the attachment links to http://drupal.org/files/issues

Trying to attach file again...

Tested and patch failed.

Actually, never mind. I tried the dev and it appears to be working good. So, in theory, the patch works! My bad. It must have been the beer.

Can't say it's never happened to me :D
If you managed to find the patch working even when intoxicated, we can certainly mark this as reviewed and tested, yes.

Who is going to commit?

Patch failed for me:

location/contrib/location_cck> patch -b < location_cck_9.patch
patching file location_cck.module
Hunk #1 FAILED at 129.

Heh! Yah, well, this is the problem when patching and beer collide. As an aside, that is the same hunk error I got initially when I attempted the patch.

Re-rolled from module root folder. Hope this works for you!

The patch is against cvs branch DRUPAL-6--3

I have tried the patch #35 above and am getting an error.

The location data is now being output by the View query: a list of all the Node titles and their locations grouped by country.

But the location data is repeated on the node itself - so I get the node title followed by two identical addresses.

The second one doesn't pay any attention to the location config settings.

Thanks

Thank you for the great review lamoo!

If you previously had nodes added with location_cck fields there will already be incomplete references stored in the location_instance table. Because there is no primary key on this table all the fields are used when inserting/updating the records. This is why the new correctly populated records are inserted leaving the old broken references around for views to choke on.

The patch now includes an update script to fix the broken records and any duplicate records, from editing nodes before running update.php, will also be removed.

I like this patch better...

Hi Nickl

I tried patch #38 and am still getting the same errors, please see below.

Steps:
Updated the original, unpatched location_cck.install and location_cck.module files.
Flushed out all my location tables in the dbase
Deleted all content type instances using this module.
Recreated a new test node with a location.

Errors are:
1) On the business directory node the location address is being repeated as before.
2) My User profile view is displaying my test business location. When I move the GMAP marker in one, it affects the other.
3) In Content Management > Content Types > My Content Type > Manage Fields > Locative information > Display settings > Hide fields from display
I changed the settings and saved, but neither of the location displays is affected by my hidden field choices.

Further info:
I am using the location module in two places:
As a CCK field in a content type I created for a site business directory of addresses,
In user profiles so people can set their own location.

Modules I have enabled are:
GMAPs
GMAP location
Location
Location CCK
Node locations
User locations

I applied this patch and had the following results:

1) nid, vid, uid are all now correctly populated inside location_instance. This should solve my problem creating a view. All data seems correct in the location* tables.

2) I created a new content type created with a CCK field added of type location. I created an instance of this content type. After creation the view node page shows the location twice.

Since the location was displayed when the nid was not stored (before this fix), it was clearly resolving the relationship through the genid field. Now that nid is ALSO populated, it is resolving the relationship through both links (genid & nid).

(To be clear, I applied the content of location_cck.3.patch. Not being an expert at PHP/drupal yet, I couldn't seem to apply this patch with the linux "patch" command. I therefore typed in the exact contents. I don't know if this was a user issue (me being the user) or there is something wrong with the patch configuration.)

Node locations doesn't play nice with location_cck after this patch is applied. Duplicate values in node view etc. What is the reason for wanting to use both location_cck and location_node? It seems to me to be an either or scenario.

More info on this here all the way back from the D5 days.
Another good comparison here of cck vs non-cck.
In this camp users are embracing the cck version punting for an upgrade path to convert node locations into their equivalent cck fields.
Another question why hide cck fields from the rest of the functionality available for locations.
Same frustrations here with a similar fix.

But this seems to be the way we will go forward #383244: location_cck back compat and views support

I've written some code to make location_node avoid location_cck, if it will help anyone. Not sure I like this approach myself though, I would suggest pick one of the two to use, not both.

@lamoo I managed to reproduce your problem. Assuming it was required I added the uid to the location_instance table as well. This is not correct. Attached is a new patch that works with User locations.

subscribe

Hi

I have applied patch #44 thank you.

It looks like the error number 2 is fixed, now the location in the User node and the Business Directory nodes are acting independently of one another, which is great.

Error number 3 (hiding location fields from display) is also fixed, to an extent.

However error number 1 is still happening: The location is still being repeated in the Direectory nodes (but not on the user nodes).

Note that the repeated output of the address isn't affected by the field display options - the entire address is output regardless of what you specify there.

I have figured out why the location address was repeated on the Directory nodes.

There are two separate places where you set location settings for a content type:

mysite.com/admin/content/node-type/contenttypename
mysite.com/admin/content/node-type/contenttypename/fields/field_location

You don't actually have to add the location field to your content type manually as it is included by default when you set your global settings. When I removed the manually-added one, the repeating address problem was resolved.

I'm popping in after a break from locations. This looks promising. I'm interested to see a new batch of commits, I wonder when bdragon will have time.

Someone wanting to use this functionality asked me to look at this issue so they could create block of nodes based on proximity to the primary node. It's easy enough to write a custom block when location is added to the content type.

$result = db_query("SELECT n.nid, n.title, truncate((degrees(acos(sin(radians(latitude)) * sin( radians(%f)) + cos(radians(latitude)) * cos( radians(%f)) * cos( radians(longitude - %f) )) ) * 69.09),1) as distance  FROM location l JOIN location_instance li ON li.lid = l.lid JOIN node n ON li.nid = n.nid HAVING distance < %f ORDER BY distance LIMIT 0, %d", $lat, $lat, $long, $distance, $number);

But when using Location CCK, the genid column is populated with $genid = 'cck:'. $field['field_name'] .':'. $node->vid;

This makes it difficult to write JOIN on that column. I'm really curious why the data is being stored that way and not in 3 separate columns?

subscribe

Patch in #43 works as expected for me (so far). Thanks!

Subscribing

subscribing

Subscribing...

OMG... It worked! THANK YOU, from one "Nick L" to another. :-) (Used Patch in Comment #43)

subscribe

+1 for patch in comment #43

It works but is glitchy. If I have 3 nodes with 2 of them located in one town and the other in a different town, locations get placed in the wrong spot. Has anyone run into this?

Never mind. I feel like an idiot. I had set up the view for grouping by city which caused some weirdness. This patch works like a charm.

Patch in #43 works for me so far, except for the update part trying to fix existing locations: assuming that the genid has a fixed length obviously doesn't work, since field name lengths may vary. As a result, the location_instance table was empty. Instead, we need to cut off the node vid after the second occurrence of the colon. Updated patch attached.

This patch is really great and works perfectly :)

Thanks!

#59 very, very good. Thanks.

+1 #59

Sad we cannot use the proximity filter :(

But we can change the status :D

+1 for #59! When can we get this committed?

+1 more for #59! It works! Commit it!

when I using the relationship method to pull the cck location infos the cck content permission settings are ignored. I get the info of the location even if I have no access to view the cck location field

#59 patch works great
please commit

instructions that might be helpful

1) add relationship (Content: Location)
2) add Location field and choose previous Location relationship ((Location) Location: Address)
NOTE: on step 2 don't choose "Content: Location"

GiorgosK,

Thank you so much for the step by step!

Tim

Thank you nickl and smk-ka. Patch from #59 committed to DRUAPL-6--3.

Went ahead and committed based on the subsequent comments vouching for it. If it doesn't work out, good thing it's a development branch ;-)

:) Hopefully dev is really going to become dev! Lots of new patches and lots of problems.. which we can then work together to fix for everyone! I think right now, most people have old dev or rc versions running with various old patches, so it will be good to bring everyone together over the new dev. Thanks Ankur!

I'm also hoping that once people see more patches being committed, that more people will be willing to a) write patches, and b) really test them! Yeah! :)

I have updated the Location module from the 2010-Jan-23 to the 2010-Mar-25 6.x-3.x-dev version, but cannot see any improved functionality. I am only using the Location CCK sub module (along with its required Location mother module), and using Location data in Views still works the same way (via Relationships).

A bug still remains: if a relationship to a location field (let's say Content: Country) contains more than one entry, the Views table prints a separate row for each entry, instead of one row containing all entries in the corresponding column.

roball, can you attach a screen shot(s)?

I'm not sure I understand what is happening and also what you expect to happen. Maybe a screen shot would help. Also, a views export might be useful ... for people who really understand views! :)

Here are the steps to reproduce:

1. Create a new content type with a Location CCK field that only has "Country" set to "Allow" in the Collection settings, while all other elements are set to "Do not collect". Set the "Number of values" to Unlimited.

2. Add some nodes of that content type, each selecting more than one country.

3. Create a node view filtering only that content type.

4. Create a relationship to that country field.

5. Add a Location: Country field and select the relationship created in step 4. You will recognize that a "Group multiple values" option will not be available :-( However, it should be, as at all other fields that may contain more than one items.

@Ankur tip of the hat to you.
@Roball I suggest creating a new issue which will get you more attention than this stale old thread.

Marking fixed again for the langoliers to eat up and remove from my issue list.

subscribe