Is it expected that this nifty module will be ported to Drupal 7?

(I realize that it depends on JQuery Menu, which has not yet been ported; cf. #996444: Port JQuery menu to Drupal 7.)

CommentFileSizeAuthor
#4 booknavigate.tar_.gz5.82 KBnmudgal

Comments

espirates’s picture

espirates commented

subscribe

shunting’s picture

shunting commented

Subscribe

Anonymous’s picture

Anonymous (not verified) commented

jQuery Menu 7.x-1.0 has now been released.

Anyone considering porting Advanced Book Blocks?

(Eventually I will probably do it if no one else does... but it may not be for another year...)

nmudgal’s picture

nmudgal commented
StatusFileSize
new booknavigate.tar_.gz5.82 KB

Coming from the issue #1508832: Support +/- expand/contract in Book navigation on Drupal.org, I actually started porting this module [namespace is different] to d7 so attaching the current work [though it may not be in working condition] so if someone is interested can take it forward or I will be trying to complete in upcoming days.
What is complete :

  • .info, .install files
  • hook_menu & menu structure
  • hook_permission
  • listing of blocks
  • Block add form

What remains mainly:

  • hook_block
  • hook_nodeapi
  • check if db queries are correct at all places
  • and everything else that I haven't covered :-)

Thanks

nouriassafi’s picture

nouriassafi commented
Assigned: Unassigned » nouriassafi

Okay, I have started porting this module to Drupal 7. Hopefully, will deliver in a couple of days.

Anonymous’s picture

Anonymous (not verified) commented

Hooray! Thank you very much!

nouriassafi’s picture

nouriassafi commented
Status: Active » Needs review

Okay, there we go.
I have uploaded the code to git:
http://drupal.org/sandbox/nouriassafi/1576288

As always, your feedback is welcome.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Needs work

Thank you very much indeed for this!

I've installed and enabled it, with jquery_menu, in a new local D7 instance, and have begun testing.

I have almost no experience with D7, so I may make some dumb mistakes.

Here's what I've seen so far:

After creating the first book, it took me to /admin, which doesn't exist. (There would, apparently, need to be a #overlay in the URL.) This might be a D7 bug rather than an ABB bug.

After assigning the block to a region, it didn't appear. After a bunch of poking around to see why not, I created a new dummy block and assigned that to the same region. The ABB block then appeared. After deassigning the dummy block, the ABB remained visible. (Again, this seems like it may be a D7 problem rather than an ABB one.)

I am getting a warning, everywhere the block appears:

* Notice: Undefined variable: edit_path in recursive_link_creator() (line 284 of /Applications/MAMP/htdocs/sites/all/modules/jquerymenu/jquerymenu.module).

There are as many copies of this warning as there are book pages, so presumably it is getting it once per page.

Taking a quick look at the code, it would seem from the structure of the IF branching that jquerymenu_edit_link must be turned on, but that the items have neither an edit_path nor an mlid.

There is something wrong with the sorting of books based on the weights set in the configuration:

With:
Book 1: weight -15
Book 2: weight 4
Book 3: weight -15

the order of appearance is Book 1, Book 3, Book 2; it should be Book 2, Book 1, Book 3.

With:
Book 1: weight -14
Book 2: weight 4
Book 3: weight -15

the order of appearance is Book 2, Book 1, Book 3, which is correct.

I've verified that "Automatically enable" and "Lock this block to new books" both work.

I haven't tried testing the taxonomy automation; it's not something I use in D6.

nouriassafi’s picture

nouriassafi commented

I have tested the above module against the latest dev release of jquerymenu on a fresh Drupal 7.14 installation and have not seen any of the above. Could you update jquerymenu to the latest dev and re-test?

Anonymous’s picture

Anonymous (not verified) commented

Thanks, good guess! I had an old dev version of jquerymenu. Updating it solved the undefined variable error.

I'm seeing some other things which I'll report in separate comments, to make it easier to keep track.

Anonymous’s picture

Anonymous (not verified) commented
Title: Drupal 7 version? » Initial Drupal 7 port: issues

[Updated title to reflect the current use of the issue]

I am seeing a problem with what level of the menu hierarchy is opened when I have several levels.

The problem seems only to affect leaf nodes (i.e. ones with no children).

When viewing a leaf node, the menu is open only as far as its parent. The leaf node itself is hidden.

Anonymous’s picture

Anonymous (not verified) commented

I am sometimes receiving the following series of error messages when saving the ABB configuration:

Notice: Undefined index: terms in book_blocks_config_submit() (line 317 of /Applications/MAMP/htdocs/sites/all/modules/drupal_7_port_of_advanced_book_blocks/advancedbookblocks.admin.inc).
Notice: Undefined index: terms in book_blocks_config_submit() (line 317 of /Applications/MAMP/htdocs/sites/all/modules/drupal_7_port_of_advanced_book_blocks/advancedbookblocks.admin.inc).
PDOException: SQLSTATE[HY000]: General error: 1366 Incorrect integer value: '' for column 'weight' at row 1: INSERT INTO {adv_book_custom} (delta, bid, enabled, weight) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3); Array ( [:db_insert_placeholder_0] => 0 [:db_insert_placeholder_1] => 14 [:db_insert_placeholder_2] => 0 [:db_insert_placeholder_3] => ) in book_blocks_config_submit() (line 353 of /Applications/MAMP/htdocs/sites/all/modules/drupal_7_port_of_advanced_book_blocks/advancedbookblocks.admin.inc).

I am not yet sure what circumstances trigger the error. I think it has something to do with taxonomy automation; I did not get these messages before starting to test that.

Anonymous’s picture

Anonymous (not verified) commented

Sometimes when saving a book, I get the error message:

Notice: Undefined variable: node_terms in _advancedbookblocks_node_op() (line 328 of /Applications/MAMP/htdocs/sites/all/modules/drupal_7_port_of_advanced_book_blocks/advancedbookblocks.module).

Anonymous’s picture

Anonymous (not verified) commented

I have tested the automation settings.

Three of the five work perfectly. The two that don't are the term-using ones.

Mass enable and mass disable apply the term settings correctly.

However, when using the term-based automation modes, creating a new book never adds it to the menu, regardless of what its term is.

Anonymous’s picture

Anonymous (not verified) commented

Sorting: it works. My report in #8 that it didn't was confused.

Testing finished for this round!

nouriassafi’s picture

nouriassafi commented

#11 This is a jquerymenu issue which can easily be resolved as described here:
http://drupal.org/node/344916#comment-4621198

that0n3guy’s picture

that0n3guy commented

@nouriassa, I agree with #11, there is an issue, and the solution you posted doesn't work (I'm using jquerymenu 3.x dev... this fix is in there) for this issue. This module needs to make the active menu

  • have an "open" class.

    I tested a solution where I went in with chrome and added an "open" class to the

  • for the menu item my page was on. Then I ran $('ul.jquerymenu .open').parents('li').removeClass('closed').addClass('open'); from within the Chrome console, it expanded the menu like it was supposed to.

    So basically we need to to tell jquerymenu what item is the "active" menu item.

    • that0n3guy’s picture

    that0n3guy commented

    #11 was easily fixed I figured out. No patch, but in adancedbookblock.module, in function advancedbookblocks_block_view, and after this code: 

            if (empty($trail)) {
          $trail = array();
        }

    Add: 

            else{
          array_unshift($trail, 'node/' . arg(1));
        }

    This will fix the issue.

    Anonymous’s picture

    Anonymous (not verified) commented

    I confirm that #18 works. Thanks!

    nouriassafi’s picture

    nouriassafi commented

    #12, #13, #14 Fixed and committed to Git.
    NB Once you enable the module or reset any of the blocks, you must save the configurations or the blocks will not appear.

    Anonymous’s picture

    Anonymous (not verified) commented

    Cool, thanks!

    It looks like you have not included #18?

    Anonymous’s picture

    Anonymous (not verified) commented

    New problem: I deleted all my existing book pages to start testing over again. On doing the mass delete operation, I got:

    Notice: Undefined property: stdClass::$book in _advancedbookblocks_node_op() (line 313 of /Applications/MAMP/htdocs/sites/all/modules/drupal_7_port_of_advanced_book_blocks/advancedbookblocks.module).

    Anonymous’s picture

    Anonymous (not verified) commented

    I'm still getting the #12 problem:

    Notice: Undefined index: terms in book_blocks_config_submit() (line 321 of /Applications/MAMP/htdocs/sites/all/modules/drupal_7_port_of_advanced_book_blocks/advancedbookblocks.admin.inc).

    I thought maybe I had failed to download the new version, but now the error is at line 321 instead of 317, which suggests I have new code.

    nouriassafi’s picture

    nouriassafi commented

    @that0n3guy #17 jquerymenu 3.x dev does not include the fix mentioned in #12. (It only deals with the .open class not the .active one.)

    Anonymous’s picture

    Anonymous (not verified) commented

    Regarding the NB in #20: I have observed this. Enable the module, save config, create first book, the block doesn't show, save the config again, now the block shows. This is probably the same thing I mentioned seeing in #8.

    This is not a good user experience—some people will decide that ABB just doesn't work and give up.

    It's not serious enough to hold off on marking this RTBC, but I'd open a new issue for it after that, if it's not easy to fix now.

    Anonymous’s picture

    Anonymous (not verified) commented

    I'm testing the taxonomy term based automation modes.

    Set the mode to "automatically enable new books".
    Create a new book (B1) with term "excluded".
    It appears in the block.
    Now set the mode to "automatically exclude books with selected terms" and set the term to "excluded".
    Save the config.
    B1 now disappears from the menu.

    The documentation label says "this will only alter the enable settings when books are added or edited", so the documentation doesn't fit the behavior.

    I don't know what the intended behavior was, or what it did in the 6.x versions. However, either the documentation or the behavior should change.

    Anonymous’s picture

    Anonymous (not verified) commented

    I have not seen problem #13 again (but I only got it occasionally before, so I can't be positive it's fixed).

    #14 seems to be fixed except for the issue I noted in #26.

    Substantial progress!

    nouriassafi’s picture

    nouriassafi commented

    #22 #23 #25 Fixed and committed to Git.

    cyrilarnaud’s picture

    cyrilarnaud commented

    The block works perfectly fine while i'm connected as an administrator but I have this error on each page displaying the block when I'm logged in as non-admin
    Exception: Query tagged for node access but there is no nid. Add foreign keys to node.nid in schema to fix. in _node_query_node_access_alter() (line 3250 of /home/users4/o/ocolot/www/dataforevents/modules/node/node.module).

    I'm using the latest git version.

    nouriassafi’s picture

    nouriassafi commented

    @cyrilarnaud #29 should be fixed now. Could you check and confirm?
    Thanks in advance.

    cyrilarnaud’s picture

    cyrilarnaud commented

    @nouriassafi : thanks the latest git version solved the problem #29
    Thanks a lot

    Anonymous’s picture

    Anonymous (not verified) commented

    OK, I have confirmed that #16 is correct (and #17 is incorrect). In the current jQuery menu -dev version, the patch at http://drupal.org/node/344916#comment-4621198 was mistakenly applied only to jquerymenu_no_animation.js, not to the main jquerymenu.js .

    So, #18 (although it works) is not the right solution.

    Anonymous’s picture

    Anonymous (not verified) commented
    Status: Needs work » Reviewed & tested by the community

    OK, at this point the only remaining issue seems to be #26 (taxonomy automation does not work as documented, although it may work the same way as in 6.x and/or as intended).

    I think it's time to mark this RTBC, and we can open a new issue for #26.

    Congratulations and thank you to @nouriassafi!

    Could the official maintainers please create a 7.x version?

    karibel’s picture

    karibel commented

    Any idea when it's possible to see an official (dev or beta) 7.X version? Keep up the good work.

    Anonymous’s picture

    Anonymous (not verified) commented

    It appears that the module was abandoned by the original developer/maintainer.

    At http://drupal.org/node/251466 there is an explanation for how someone (probably @nouriassafi) could become a (co)maintainer and release an official 7.x version.

    albert.aribaud’s picture

    albert.aribaud commented

    @nouriassafi: thanks for the porting. Just added it (and the latest dev of jquery menus) to my D7 site.

    Only issue I had was a succession of Notice : Undefined index: href in recursive_link_creator() (line 217 in .../sites/all/modules/jquerymenu/jquerymenu.module). I assumed it was due to some node(s) in my book not being published, so I did a dirty fix by making sure that href was defined even for unpublished pages, by copying line 230 and inserting it at line 235, i.e. turning:

    233     else {
234       $tree[$mlid]['link']['hidden'] = '1';
235     }

    into

    233     else {
234       $tree[$mlid]['link']['hidden'] = '1';
235       $tree[$mlid]['link']['href'] = 'node/'. $nid;
236     }

    This removed all the warnings I had.

    nouriassafi: if this fix is good enough, maybe you could push it in your git repo? Also, I'd like the module to be officially maintained for D7. :)

    Cheers,
    Albert.

    lolandese’s picture

    lolandese commented
    Title: Initial Drupal 7 port: issues » Advanced Book Blocks:l Drupal 7 port

    Changed title to make it more distinctive in the "Track posts" block on the dashboard.

    Bojhan’s picture

    Bojhan commented
    Status: Reviewed & tested by the community » Needs review

    Could this get some activity, the warning is removed but I doubt its the correct fix. As jQuery Menu changes.

    Anonymous’s picture

    Anonymous (not verified) commented

    I am still seeing the "Undefined index: href in recursive_link_creator" warning in the site log.

    anibal’s picture

    anibal commented

    subscribe

    anibal’s picture

    anibal commented

    Hello all,
    Any ideas when there will be a dev version available (not git) on drupal.org?

    Thank you for the effort on making this port.

    nasia123’s picture

    nasia123 commented

    This module works, tested it with drupal 7.22 and 7.x-4.0-alpha3 ...
    this is the notice I am getting

    Notice: Undefined offset: 116 in book_blocks_config_submit() (line 314 of C:\inetpub\wwwroot\wiki\sites\all\modules\advancedbookblocks\advancedbookblocks.admin.inc).

    webel’s picture

    webel commented

    Thanks for your work on this.

    With Jquery Menu 7.x-4.0-alpha3 when I save an Advanced Book Blocks configuration I also get dozens of times with different indices:

    Notice: Undefined index: 33 in book_blocks_config_submit() (line 314 of ... advancedbookblocks.admin.inc).
    webel’s picture

    webel commented
    Issue summary: View changes

    The error reported at #12 is definitely still present in the GIT snapshot from 2012-05-04. Whenever I change any Advanced Book Block settings of any kind I get 100s of messages like:

    Notice: Undefined index: 25 in book_blocks_config_submit() (line 314 of .../sites/all/modules/advancedbookblocks/advancedbookblocks.admin.inc).