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

[Ref] Extract getFormValues #18510

Merged
merged 1 commit into from
Sep 25, 2020
Merged

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Sep 18, 2020

Overview

Extract getFormValues on task preProcessCommmon

Before

Long chunk

After

Extracted function

Technical Details

Digging into this is seems the is always an object that inherits from this class

It is called statically primarily because the export task does not have
subclasses per form & does not know what variant it's passing in. However, we can
move functions onto the CRM_Contact_Form_Task class as non-static functions
and call them form form-> however unintuitive that seems.

I am trying to clean up the structure such that we can move on from the
static method but decided I should tackle this function a little first.

To test try doing an export from any search & this is called in static mode
(same goes for any task but the path is less ambiguous)

Comments

@civibot
Copy link

civibot bot commented Sep 18, 2020

(Standard links)

@civibot civibot bot added the master label Sep 18, 2020
$url = CRM_Utils_System::url('civicrm/contact/' . $fragment, $urlParams);
$session = CRM_Core_Session::singleton();
$session->replaceUserContext($url);

$cacheKey = "civicrm search {$qfKey}";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually this move is just to make cleaning up the url part easier

Digging into this is seems the  is always an object that inherits from this class

It is called statically primarily because the export task does not have
subclasses per form & does not know what variant it's passing in. However, we can
move functions onto the CRM_Contact_Form_Task class as non-static functions
and call them form form-> however unintuitive that seems.

I am trying to clean up the structure such that we can move on from the
static method but decided I should tackle this function a little first.

To test try doing an export from any search & this is called in static mode
(same goes for any task but the path is less ambiguous)
@eileenmcnaughton
Copy link
Contributor Author

test this please

@eileenmcnaughton
Copy link
Contributor Author

@colemanw this should be an easy extraction

@@ -88,7 +88,7 @@ public static function preProcessCommon(&$form) {

$isStandAlone = in_array('task', $form->urlPath) || in_array('standalone', $form->urlPath);
if ($isStandAlone) {
list($form->_task, $title) = CRM_Contact_Task::getTaskAndTitleByClass(get_class($form));
[$form->_task, $title] = CRM_Contact_Task::getTaskAndTitleByClass(get_class($form));
Copy link
Member

Choose a reason for hiding this comment

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

Cool. I didn't know about the square bracket shortcut for list(). Learn something new every day around here...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah phpstorm keeps making that change on me :-)

@eileenmcnaughton
Copy link
Contributor Author

@colemanw was this OK to merge

@colemanw colemanw merged commit 64c8be5 into civicrm:master Sep 25, 2020
@colemanw colemanw deleted the preprocess branch September 25, 2020 00:06
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