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

dev/core#4295 SearchKit: check controller class for qfKey when loading legacy search actions for contributions #26235

Merged

Conversation

ufundo
Copy link
Contributor

@ufundo ufundo commented May 16, 2023

Overview

Fix for https://lab.civicrm.org/dev/core/-/issues/4295

Before

Legacy search actions for contributions don't work if they aren't going through CRM_Contribute_Controller_Task

For example: https://lab.civicrm.org/ufundo/ukgiftaid/-/tree/searchkit-actions

After

They work :)

Technical Details

At first I tried using the $task['class'] for the qfKey, but this required modifying the core qfKey validation. An attempt going down that route here https://github.com/ufundo/civicrm-core/tree/task-controller-qfkey-name

I think this is probably much safer!

@civibot
Copy link

civibot bot commented May 16, 2023

No issue was found matching the number given in the pull request title. Please check the issue number.

@civibot
Copy link

civibot bot commented May 16, 2023

(Standard links)

@civibot civibot bot added the master label May 16, 2023
@ufundo ufundo changed the title dev/core#4295 check route callback for generating qfkey dev/core#4295 SearchKit: check controller class for qfKey when loading legacy search actions for contributions May 16, 2023
@colemanw
Copy link
Member

Ooh, this is clever looking up class from menu path. Well done.
I wish it didn't involve an additional sql query, but oh well, that's what you get for supporting legacy stuff.
PS CiviBot has been confused lately about issue numbers... don't worry about it 🤷

@colemanw colemanw merged commit 70c2b33 into civicrm:master May 16, 2023
@ufundo
Copy link
Contributor Author

ufundo commented May 16, 2023

Glad you like it! 😀

Thinking about it, might you get similar mismatches between the controller class and the task class for legacy contact actions? And so maybe this line should use the same approach?

$key = \CRM_Core_Key::get(\CRM_Utils_Array::first((array) $task['class']), TRUE);

@colemanw
Copy link
Member

Thinking about it, might you get similar mismatches between the controller class and the task class for legacy contact actions? And so maybe this line should use the same approach?

Good idea!

olayiwola-compucorp added a commit to compucorp/civicrm-core that referenced this pull request Sep 22, 2023
olayiwola-compucorp added a commit to compucorp/civicrm-core that referenced this pull request Oct 2, 2023
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