Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't crash when all action tasks are disabled #20940

Closed
wants to merge 2 commits into from

Conversation

mattwire
Copy link
Contributor

@mattwire mattwire commented Jul 23, 2021

Overview

This was found following release 1.2 of exportpermissions extension which supports disabling export/print/pdf/labels tasks. It turns out that disabling all of these can cause the advanced search to fail because some "modules" do not have any search tasks - see progressivetech/net.ourpowerbase.exportpermission#6.

Specifically Activities and Mailings trigger a "Network error" because when there are no tasks defined a "null" page gets added which then triggers a require_once('.php') here https://github.com/civicrm/civicrm-core/blob/master/CRM/Core/Controller.php#L452 because $className is not defined.

Before

When a search task has no actions (eg. Activity, Mailings) and it is expanded in Advanced Search it crashes.

After

All tasks can be disabled and everything continues working!

Technical Details

Explained above. Most of the task code changes here are fixing PHP notices - the change in CRM/Contact/StateMachine/Search.php is what stops the crash.

Comments

@civibot
Copy link

civibot bot commented Jul 23, 2021

(Standard links)

@mattwire mattwire changed the title Tasks Don't crash when all action tasks are disabled Jul 24, 2021
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jul 24, 2021
Alternate potential approach to the enotice portion of civicrm#20940
This approach adds support for permissions using a syntax like
https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_summaryActions/
and would potentially standardise that onto the search_tasks hook
https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_searchTasks/
- it's not quite clear what the search hooks supports -
https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_searchKitTasks/
@eileenmcnaughton
Copy link
Contributor

@mattwire what about this approach for the enotices side of it -
#20944

(I didn't get as far as looking at the crash fix - only at the weird & wonderful way those task functions turn themselves into knots)

eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jul 25, 2021
Alternate potential approach to the enotice portion of civicrm#20940
This approach adds support for permissions using a syntax like
https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_summaryActions/
and would potentially standardise that onto the search_tasks hook
https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_searchTasks/
- it's not quite clear what the search hooks supports -
https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_searchKitTasks/
@mattwire mattwire closed this Jul 31, 2021
@mattwire mattwire deleted the tasks branch July 31, 2021 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants