Minor refactor preparatory to function extraction #12468
Conversation
|
(Standard links)
|
CRM/Export/BAO/Export.php
Outdated
| @@ -487,7 +487,18 @@ public static function exportComponents( | |||
| } | |||
| } | |||
| else { | |||
| $primary = TRUE; | |||
| $extraProperties = self::defineExtraProperties($queryMode); | |||
There was a problem hiding this comment.
@eileenmcnaughton not sure why the movement of these 4 lines as they should be done whether we have select fields or not i would have thought, I agree with L495 - L500 tho
There was a problem hiding this comment.
@seamuslee001 so moving them doesn't affect whether they are run or not - it just moves them to the outside of a code chunk they don't logically fit in the middle of
eileenmcnaughton
force-pushed
the
export_return
branch
from
2391353
to
26caa4e
Compare
|
@seamuslee001 I updated this to remove the move of those lines - mostly because I have a new plan for them which means I'll probably address them before rather than after the follow up code extraction -(once #12485 is merged a whole lot of cleanup options open up) |
CRM/Export/BAO/Export.php
Outdated
| 'im_provider' => 1, | ||
| 'phone_type_id' => 1, | ||
| 'current_employer' => 1, | ||
| ]; |
There was a problem hiding this comment.
I'm not sure what provider_id even is but it's missing from the diff. Was that omission intentional?
There was a problem hiding this comment.
ah yeah - it's a pseudonym for im_provider - I'll put it back until we figure out which one we don't need
eileenmcnaughton
force-pushed
the
export_return
branch
from
26caa4e
to
be1b612
Compare
|
@colemanw updated ^^ |
|
@monishdeb are you ok to merge this - test fail has a fix in #12490 |
|
Yes, did a quick check on my local after applying this patch and is safe to merge. |
|
thanks! |
|
Just a note that this change manipulates the ordering of the columns in the exported CSV. Eg: The export used to start with the contact id before this PR.
|
| 'phone_type_id' => 1, | ||
| 'provider_id' => 1, | ||
| 'current_employer' => 1, | ||
| ]; | ||
| $fields = CRM_Contact_BAO_Contact::exportableFields('All', TRUE, TRUE); | ||
| foreach ($fields as $key => $var) { | ||
| if ($key && (substr($key, 0, 6) != 'custom')) { |
There was a problem hiding this comment.
maybe, move(array_merge) the above initialization after this for loop?
|
@jitendrapurohit ok I'll wait til other open related PRs are merged & then add a test + follow up on this |
Overview
Minor code simplication preparatory to function extraction
Before
$isPrimary is defined & is always TRUE & then used in an if. Detail relating to extraProperties is half way through the code that makes sense to extract.
After
$isPrimary removed, extraProperties code re-ordered
Technical Details
This is highly toxic code. We have some fixes to do but it needs many incremental cleanups first IMHO
Comments
@jitendrapurohit @monishdeb