Trigger_menu() is a mess, because menu links to trigger pages are generated both manually and automatically. The patch removes trigger_access_check() and makes most of the logic inside trigger_menu() dynamic, except for the page to System's triggers, since System only exposes hook_cron() and the page it is on is called "Cron" and not "System".

Bottom line:
- Less code in total
- Less duplicate code (the manually declared menu links that were almost identical)
- Less menu links. The old code added some links even if the modules to which those links belonged weren't enabled.

CommentFileSizeAuthor
#12 544318-node-optional.patch730 bytesandypost
#8 rework_trigger_menu_03.patch2.15 KBxano
#6 rework_trigger_menu_02.patch5.23 KBxano
#4 rework_trigger_menu_01.patch5.1 KBxano
rework_trigger_menu_00.patch4.25 KBxano

Comments

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Issue tags: +makenodeoptional

This patch causes Node.module's human-readable name ('Node') to pop up in the menu. #544320: Rename Node to Content fixes this.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Issue tags: -makenodeoptional

Status: Needs review » Needs work

The last submitted patch failed testing.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Status: Needs work » Needs review
StatusFileSize
new rework_trigger_menu_01.patch5.1 KB

Status: Needs review » Needs work

The last submitted patch failed testing.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Status: Needs work » Needs review
StatusFileSize
new rework_trigger_menu_02.patch5.23 KB

Status: Needs review » Needs work

The last submitted patch failed testing.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Status: Needs work » Needs review
StatusFileSize
new rework_trigger_menu_03.patch2.15 KB

Hook_menu() was already rewritten during the major overhaul a short time ago. This patch removes some logic from trigger_assign() in favour of default local tasks and makes System the default module instead of Node in my attempt to make Node optional.

Status: Needs review » Needs work

The last submitted patch failed testing.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Xano, can you re-roll patch?

Marked as duplicate #310044: Trigger access callback not inherited

andypost’s picture

andypost
he/him
Primary language Russian
commented
Version: 7.x-dev » 8.x-dev

Actually this D8 material because node module is not going optional in D7

Another menu optimization #678606: Tab titles on Trigger page are not clear

Suppose this patch should be like bellow to not change current behaviur

+  $default_tab = module_exists('node') ? 'node' : 'system';
  $trigger_info = _trigger_tab_information();
  foreach ($trigger_info as $module => $module_name) {
    $items["admin/structure/trigger/$module"] = array(
      'title' => $module_name,
      'page callback' => 'trigger_assign',
      'page arguments' => array($module),
      'access arguments' => array('administer actions'),
+      'type' => ($module == $default_tab) ? MENU_DEFAULT_LOCAL_TASK : MENU_LOCAL_TASK,
      'file' => 'trigger.admin.inc',
    );
  }
andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new 544318-node-optional.patch730 bytes

Much easy with this patch :)

xjm’s picture

xjm
she/her
Primary language English
commented
Version: 8.x-dev » 7.x-dev

#764558: Remove Trigger module from core has been committed, so moving this issue back to D7. Thanks folks!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Hm. Does this still make sense in D7?

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Needs work » Closed (won't fix)

Err, nope. :) Oops. Don't think this is backportable. (Or... production-version-of-Drupal-portable?)

andypost’s picture

andypost
he/him
Primary language Russian
commented
Project: Drupal core » Trigger
Version: 7.x-dev »
Component: trigger.module » Code
Assigned: xano » Unassigned
Status: Closed (won't fix) » Needs review

Still valid for trigger module
Suppose all fixes should be moved not closed