Ancestor list not working with multiple parents

I'm bringing this back up because I see that it's also happening for me in ver 6.x-1.3 using Drupal 6.17. It's pretty annoying because my clients will probably attach a child to multiple parents and it'd be nice to have the ancestors show up.

I'm going to look into this and see if I can come up with anything.

I'm working on a patch for this, but it's a major one. I'm also including support for drilling down to 'ancestral children' because my application needs it. Should have something in the next week that will work gracefully but it won't be 'complete' for a while.

I'm working on a patch for this, but it's a major one. I'm also including support for drilling down to 'ancestral children' because my application needs it. Should have something in the next week that will work gracefully but it won't be 'complete' for a while.

---edit---

I'm attaching a patch for the maintainers to look at. Please feel free to ignore it if you want but I think it improves this module...did significantly for me. I started out addressing the main concern of ancestors not showing for nodes with multiple parents and then ended up expanding it out to meet my other needs. Might be some stuff to include in a new release.

I wanted to accomplish these things:
A) Show all ancestors and descendants of a given node and allow admin to select which types to display or hide and in what format (like the children display options currently available).
B) Add new links to the link operations to allow attaching children to parents from the child's node display (currently only let you link a parent to a child).
C) Remove the CSS from the code (indent styles in ancestor list) to allow some more sophisticated theming of the lists.
D) Make some prelim CSS for the module.

NOTE: I have not tested the views display options because I don't use them. I did my best to keep it consistent and should work. If not, I'm willing to fix.

Here's a list of my code changes in high-level detail. I tried to annotate well so you should be able to see what I did in the code:

1) Altered the load_ancestors function to use recursion and build an array of all ancestors.

2) Created a load_children function to do the same thing for children and decendents.

3) Altered the ancestors and children fieldset theme functions to use the arrays built in 1 and 2 and to display all ancestors and descendants recursively.

4) Altered nodeapi to accomodate the above changes and to make seperately themeable groups of ancestors and descendants based on the direct parent or child (indented or colored grand-children/parents etc).

5) Added new function list_parent_types to get an array of the types of ancestors attached to a child. This aided in creating links to attach a parent from a child node display.

6) Altered list_possible_children (now list_possible_attachments) to allow selecting which direction the add link would render (e.g. 'relativity/addparent/X/parent/Y' where X is child and Y is parent or vice versa). Also, 'parent' in this url can now be either 'parent' or 'child' depending on who is doing the attaching (the parent or the child) to let addparent know how to attach the nodes together.

7) Altered addparent to accommodate 6.

8) Altered hook_menu to accomodate 6 and 7.

9) Altered display_settings form to add display settings functionality for ancestor types. I'd like it to select only types that conceivable can be ancestors but this is beyond my scope. Added child display options for all nodes that can enter into relationships to allow display options for descendant children. It alerts you if it is not a direct child allowed type (**DESCENDANT CHILD).

10) Added ancestor_as _view (like current child_as view).

11) Removed display selection from the main theme functions into separate theme functions for clarity.

12) Added theme_relativity_show_children.

13) Cleaned up the get_operation_links code and added better classes for more flexible styling and to group by content type (eg 'Create new XXX' and 'Attach Existing XXX' should be in a div to allow them to be styled separate from YYY...).

14) Cleaned up some non conforming code standard.

15) Added a css file to style a little (attached in a zip).

If anyone could start testing this, it would be great!

Dave

So I just implemented this patch (manually... not cool)

It seems to have eliminated my "child" and "parent" list blocks.

And clicking the "Attach existing..." link brings me to a white screen only.
Clicking the "Attach this .. to a document of this type: ..." link brings me to a "page not found"

I also get a drupal message:

warning: Missing argument 3 for theme_relativity_show_children() in /homepages/17/d175755624/htdocs/martial_arts_genealogy/modules/relativity/relativity.module on line 1721.

Any advice?

Hi Chien_fu,

I'm so sorry to hear that- very frustrating. Which version of relativity are you working with? I'm changing the version to 6.x 1.3 because that's the one this patch was developed for. I did not create one for the 5.x branch.

Just retested the patch (i use eclipse php) and it worked as expected. Not sure why you needed to do it manually, sorry about that.

Just re-retested in a my dev environment and I see what you mean. I totally ignored the blocks because I didn't use them in my applications. Give me a couple of days to fix this and repost.

In the mean-time maybe we can figure out why you had to manually apply the patch? That sucks and I'll do whats needed to fix that.

Thanks!

OK, it didn't take me very long.
Here is a modified patch (still created using Tortoise SVN..I maintain my code in SVN...) so hopefully it works. If not, let's please try to figure out why the patch is buggy...

The blocks didn't work at all for me as well but now they do. I removed the styling for the children block but it is still there for the ancestors block. You should be able to change it at will... ;) Also, you might need to rebuild the menu router as I changed some menu items.

-T

PS, I did just run into a different bug though and I'm not sure yet how to resolve it. If you have two nodes N1 and N2, and they are linked N1(parent)->N2(child), and then link N2(parent)->N1(child), it will loop forever. It should probably deny ability to link in the second instance, but I'm not sure yet how to do that, or if it's a problem in the original version 1.3.

So I found CygWIN for patching files in windows, but that seemed only slightly less cryptic than doing it manually.. Anyway... I got it installed. And I am using the latest 6x version.

It seems only very slightly different from before. The 'link operations' block gives me a blank page when I try to attach a child, and a page not found when attaching a parent.

There are no more drupal messages though :)

wow. this is a *very big* patch. It doesn't apply clean on dev snapshot of 6.1 branch:

relativity-DRUPAL-6--1$ patch < relativity-ancestor-list-not-working-with-multiple-parents-recursive-collect_0.patch 
(Stripping trailing CRs from patch.)
patching file relativity.module
Hunk #1 FAILED at 1.
Hunk #2 succeeded at 16 with fuzz 2 (offset 7 lines).
Hunk #3 succeeded at 66 (offset 7 lines).
Hunk #4 FAILED at 69.
Hunk #5 succeeded at 257 (offset 1 line).
Hunk #6 FAILED at 292.
Hunk #7 succeeded at 630 (offset 1 line).
Hunk #8 succeeded at 640 (offset 1 line).
Hunk #9 succeeded at 697 (offset 1 line).
Hunk #10 succeeded at 822 (offset 1 line).
Hunk #11 succeeded at 866 (offset 1 line).
Hunk #12 succeeded at 910 (offset 1 line).
Hunk #13 succeeded at 946 (offset 1 line).
Hunk #14 succeeded at 1101 (offset 2 lines).
Hunk #15 succeeded at 1147 (offset 2 lines).
Hunk #16 FAILED at 1155.
Hunk #17 succeeded at 1332 (offset 2 lines).
Hunk #18 succeeded at 1365 (offset 2 lines).
Hunk #19 succeeded at 1583 (offset 2 lines).
Hunk #20 succeeded at 1606 (offset 2 lines).
Hunk #21 FAILED at 1641.
Hunk #22 succeeded at 1749 (offset 2 lines).
Hunk #23 FAILED at 1770.
Hunk #24 succeeded at 1807 (offset -1 lines).
Hunk #25 succeeded at 1838 (offset -1 lines).
6 out of 25 hunks FAILED -- saving rejects to file relativity.module.rej

@tbenice I suggest to split the changes described in #3 into several issues and address each one separately. This will allow to better track each change to the code.
As you might have seen in #851962: Offering to co-maintain "Node Relativity" there's a new set of people with cvs access to this module. In my opinion, splitting down that big set of changes to allow everyone interested to test single fixes or improvements is the way to go. For example B) in #3 should be fixed in #877026: Looking to "Attach existing Parent" AS WELL AS "Attach existing Child".

That said, I also hit the bug originally reported here in a project of mine. I'll review your patch to strip off the part fixing the bug here and be back with a sorten patch. I'm not familiar with this so just in case, feel free to provide it quicker than me :)

Hi, did you rebuild the menu? There are menu changes.

@jonathan- That makes alot of sense. Yes it is a big patch. I didn't know how to best introduce the changes but wanted to share since it worked out well for me.

I'll try to address the 6.x-dev issue first, then begin taking the issues apart the best I can. In the mean time, could you please attach the .rej file to highlight the rejected hunks? It might help me out though I'm sure I can figure it out if not. I'll work on it this afternoon when my day job is done :)

@chien_fu- I'm still not clear about why you can't apply the patch w/o manually doing it...also, you will need to rebuild the menu after applying it because hook_menu was changed and that won't be reflected in the links because they are created in a theme function. Hope that works...(fingers crossed, toes too).

-T

Kickin!
I refreshed the menus and now we're hot to trot.
Thanks for the patch. I'm going to try to work with this for now, it seems to be just the ticket for me.

sorry I'm lazy to re-obtain the .rej file :). Also I've just commited a set of changes fror php 5.3 support so the diff/rej is probably even uglier. As said in #9 I'd like to just address the original target of this issue here.

I've set up a site with the hierarchy described in the initial comment.

1st attachment is a screenshot of your patch's rendering of ancestors with such a configuration. (sorry I forget css file).

2nd attch is obtained with this little change to the module:

@@ -828,8 +828,6 @@ function relativity_load_ancestors($node
         }
         if (count($siblings) > 0) {
           array_unshift($ancestors, $siblings);
-        }
-        if (count($siblings) == 1) {
           $keep_going = 1;
         }
       }

It does fix for this scenario but not for a more complex one. If I also make B2 a child of A1, this is the scenario:

A1
_B2
__C2

A2
_B2
__C2
_B3
__C2

So third attachment is the result of using your patch. I expected it to be ordered in a top-down form, as drawn above, yours is also right. Perhaps a settings option to let the user choose the order is of interest (can be solved in a follow up issue to this).

I started merging the part of your patch corresponding to this and found several things I decided to implement in other way, at least for the moment (think of the module code needs several improvements and I want to do them gradually):

* I slightly changed your recursive function.
** Now ancestors key is empty for root parents.
** There was a bug when parent_node is not an array (single parent case).
* I don't like indentation by padding the lines. I prefer a <ul><li> structure. I think this is the way to go.
* I think doing node_load inside a theme_ function is wrong although I haven't coded an alternative yet.

Finally, attached is a patch and a screenshot of the resulting ancestors list.

note: in a future I'd like to implement the ancestors array in compatibility with menu arrays (has_children, below, etc). So the ancestors block can be themed as a menu, taking advantage of standard drupal rendering, jquerymenu and possibly other modules.

Please comment on my proposal to get and agreement, commit, close this issue and jump to other things wanting to be improved :)

@jonhattan
Ok, great. Thanks for the quick work.

Yes, I'd like to implement a choice of rendering and make the array more compatible with menu system. Don't know quite how to do that.

The bottom up rendering is what worked for me because I wanted to have the closest relative at the top. Makes sense to have a more 'tree' like view as well.

When I have time, I'll open a new issue to add recursive descendants finding and post a patch for that part.

-Ted

I think I follow what you're saying.. If you're saying that you could reverse the order of the ancestry list so that closest relatives are on the bottom and more distant relatives are on top.. then I would be very interested in seeing a patch for that. Can't quite figure out how to do that on my own..

Thanks!

Ted. Looking forward to this. Has an issue been created for the 'tree'-like view?
Thanks!

Hi Chien_fu. Thanks for your continued interest. Unfortunately, this has dropped lower on my priority list for the time being...i just started a new job and am busy getting ready to launch a product. I'm actually hoping that someone smarter than me will do the array gymnastics to:
1) turn the ancestors array into a menu-tree compatible array
2) create a top-down version for a tree like view.

I promise to work on this at some point but the bottom-up list is working for me right now. If the above changes are/get committed to dev then you can start your own issue and...

It's in my TODO list. I've thinked on doing the function to return the hierarchy in a serie of arrays, each one with a complete path that can easily be reversed. But no time to spend in it yet.