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

NFC Code cleanup to core task class #12316

Merged
merged 1 commit into from
Jun 20, 2018

Conversation

mattwire
Copy link
Contributor

@mattwire mattwire commented Jun 15, 2018

Overview

Just commenting and variable renaming towards making this more generic.

Identified during investigation for dev/core#158

Technical Details

CRM_Core_Form_Task::preProcessCommon() is currently only used by CRM_Case_Form_Task.

@civibot
Copy link

civibot bot commented Jun 15, 2018

(Standard links)

*
* @var int
*/
static $queryMode = CRM_Contact_BAO_Query::MODE_CASE;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this used?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find any instances?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/civicrm/civicrm-core/pull/12316/files#diff-8e58e0ef24ffee40975a92a797f7acecR135

It's a baby step towards making that preProcessCommon function generic enough to handle more than just the case task.

@eileenmcnaughton
Copy link
Contributor

eileenmcnaughton commented Jun 15, 2018

Just a testing note - this messes with the $useTable variable so we need to check that it is still possible to export more than 500 contacts - eg from manage groups and normal searches only retrieve the first 500

UPDATE I confirmed useTable not used here

@eileenmcnaughton
Copy link
Contributor

@mattwire I worked through this refactor locally & it makes sense. It changes 'queryMode' to be more correct - does that fix any bug? And if so is there something still needing backporting to the rc?

I'm merging this but can we follow up by adding a getter on queryMode ?

$form::getQueryMode();

is, IMHO, less ugly than

$form::$queryMode

@eileenmcnaughton eileenmcnaughton merged commit 500b95a into civicrm:master Jun 20, 2018
@mattwire mattwire deleted the core_task_code_cleanup branch September 25, 2018 11:01
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.

4 participants