Hi,

I think it would be a good feature if the admin selects the menus in which Coffee searches.

Kálmán

CommentFileSizeAuthor
#10 menu-supporting-1435460-10.patch8.88 KBkalman.hosszu
#9 menu-supporting-1435460-9.patch8.15 KBkalman.hosszu
#7 menu-supporting-1435460-7.patch5.65 KBkalman.hosszu
#1 menu-supporting-1435460-1.patch6.24 KBkalman.hosszu

Comments

kalman.hosszu’s picture

kalman.hosszu commented
Status: Active » Needs review
StatusFileSize
new menu-supporting-1435460-1.patch6.24 KB

I created the patch.

I create separate commits to distinct the development parts. You can add it by using:

git am menu-supporting-1435460-1.patch

Cheers,
Kálmán

michaelmol’s picture

michaelmol commented
Assigned: Unassigned » michaelmol
Status: Needs review » Needs work

I like the idea to select the menus to include in Coffee.
I would prefer an option to select the menus per role, although there is a access check on each item. This will speed up the initial search.

For example;
So that the role anonymous only search in navigation.
And a content editor can also search in the management menu.

kalman.hosszu’s picture

kalman.hosszu commented

Hi,

It would be a default settings and a per role settings. The user's who is in more role, the menus would be merged.

What's your opinion? Could I help you in the development?

Best,
Kálmán

michaelmol’s picture

michaelmol commented

The patch fails when applying it to the 7.x-1.x branch.
You can help development by creating patches and suggest feature request.
When maartenverbaarschot and i decide to accept new co-maintainers we will let you know.

michaelmol’s picture

michaelmol commented
Assigned: michaelmol » Unassigned
michaelmol’s picture

michaelmol commented
Assigned: Unassigned » michaelmol
kalman.hosszu’s picture

kalman.hosszu commented
StatusFileSize
new menu-supporting-1435460-7.patch5.65 KB

There was a conflict on hook file because the patch created before you added the ordering so we modified the same line.

The conflict is resolved, the new patch attached.

Kálmán

michaelmol’s picture

michaelmol commented

Your patch results in an error when reaching the admin/config/administration/coffee page (at least in d7.12 minimal, not tested elsewhere).

PDOException: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'dev_d7min.menu_custom' doesn't exist: SELECT * FROM {menu_custom} ORDER BY title; Array ( ) in coffee_admin_page() (line 15 of /Users/michaelmol/Sites/Dev/drupal/d7-min/drupal-7.12/sites/all/modules/coffee/coffee.admin.inc).

Besides that make use of the DBTNG http://api.drupal.org/api/drupal/includes%21database%21database.inc/grou... instead of using db_query.
To make a list of menus make use of Drupal functions like: http://api.drupal.org/api/drupal/includes%21menu.inc/function/menu_get_n...

kalman.hosszu’s picture

kalman.hosszu commented
StatusFileSize
new menu-supporting-1435460-9.patch8.15 KB

Hi,

In static queries we should use db_query, for example I copied the query from the core menu module. The problem was that the "menu" module is not enabled during minimal Drupal installation.

The menu_get_names() function returns only with the menus machine names so I think we should use menu_get_menus() core function, but it's in "menu" module too, so I added the "menu" module to the dependencies array in info file.

The modifications what I created:

  • admin interface link added to info file
  • menu module added to dependencies
  • change menu listing with menu_get_menus() core API function
kalman.hosszu’s picture

kalman.hosszu commented
StatusFileSize
new menu-supporting-1435460-10.patch8.88 KB

Sorry, wrong file attached before.

Check this one.

Kálmán

michaelmol’s picture

michaelmol commented

getting close in
coffee_admin_page() you still make use of the query instead of menu_get_menus()

kalman.hosszu’s picture

kalman.hosszu commented
Status: Needs work » Needs review

I cloned the repository with

git clone --branch 7.x-1.x http://git.drupal.org/project/coffee.git
cd coffee

And downloaded the patch atteched in #10 and added the command:
git am menu-supporting-1435460-10.patch
and everything works fine, the coffee_admin_page() function use menu_get_menus().

Any problem?

heylookalive’s picture

heylookalive commented
Version: 7.x-1.0 » 7.x-2.x-dev
Status: Needs review » Fixed

This has now been done in the dev 2.x branch. There are new permissions and we now have a settings form, all works happily.

Status: Fixed » Closed (fixed)

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

  • Commit 33abf0b on 7.x-2.x, 8.x-1.x by heylookalive: 
    Issue #1435460 by kalman.hosszu: Select menus in admin interface