Menu token module adds extra properties to menu items and there could be thousands of items with token support, that data should be stored in a database instead of a system variable. Use cache functions to improve performance.

CommentFileSizeAuthor
#1 menu_token_db_storage.patch4.93 KBdevelcuy

Comments

develcuy’s picture

develcuy commented
StatusFileSize
new menu_token_db_storage.patch4.93 KB

There is still some work to do: Make sure that menu_token data is deleted when the menu item is deleted. Improvements welcome.

develcuy’s picture

develcuy commented

This issue is being worked at branch feature-1087212.

dealancer’s picture

dealancer commented

Cool, I am testing it now, however version (of module 6.x-1.x) is not in the branch name .

dealancer’s picture

dealancer commented

Great job Fernando. The idea with using static var is really good. Here is my feedback:

1) I've got menu transfered from variables to database after secound or third update. I don't know the reason of that yet.
2) I would like to rename menu_token_update_6001 to menu_token_update_6000, cause many modules has update like last one.
3) Probably we should also delete all variables

This fix needed to be also ported to Druplal 7, before other issues should be fixed.

dealancer’s picture

dealancer commented

To solve it I have split creating table and performing queries in two different hooks, and for the second hook I am checking return values for that queries. If those queries was not performed it stops executing of them, and user probably knows about it. See http://drupalcode.org/project/menu_token.git/commitdiff/bbe7d2e6ea2428e6....

However when the first time performing drupal_write_record it returns false. This should be fixed.

dealancer’s picture

dealancer commented

Finally I get rid of those errors and updated has been done in single update. See changes: http://drupalcode.org/project/menu_token.git/commitdiff/6a2fd6732f268c6a...

This could be merged with main repository.

dealancer’s picture

dealancer commented
Status: Active » Needs review

We need to have many reviews about this issue, please update Menu Token module to latest dev version for Drupal 6 as it has new functional added for this issue. There are some feedbacks for new functional http://drupal.org/node/1095556#comment-4251878, but we are appreciate if you leave comments here.

dealancer’s picture

dealancer commented

I have completely rewrote update function, see http://drupal.org/node/1095556#comment-4389146

dealancer’s picture

dealancer commented
Status: Needs review » Needs work
dealancer’s picture

dealancer commented
Assigned: Unassigned » dealancer
dealancer’s picture

dealancer commented
Assigned: dealancer » develcuy
Status: Needs work » Needs review
dealancer’s picture

dealancer commented

We need to port this issue to D7 version of this module, so I am creating new brunch 7.x-1.x-feature-1087212

dealancer’s picture

dealancer commented
Assigned: develcuy » ygerasimov

In D7 version I have moved variable to db, have made minor fixes, refactored some code and added comments.
In D6 version I have also made some changes: refactored code, minor fixes and added Hicomments.

Hi @Yuri, could you please review this changes, test installation and test update from alfa version: variable -> db.

http://drupalcode.org/project/menu_token.git/shortlog/refs/heads/7.x-1.x...
http://drupalcode.org/project/menu_token.git/shortlog/refs/heads/6.x-1.x

dealancer’s picture

dealancer commented

I have merged branch 7.x-1.x with 7.x-1.x-feature-1087212. So now all changes should be in dev.

develcuy’s picture

develcuy commented
Assigned: ygerasimov » Unassigned
Status: Needs review » Fixed

Just did an update test from 6.x-1.0-alpha1, it works great!

dealancer’s picture

dealancer commented

cool :)

Status: Fixed » Closed (fixed)

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