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] export code simplification #14758

Merged
merged 1 commit into from
Jul 9, 2019

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Removes an unnecessary variable check, simplifies tests

Before

Less logical

After

More logical

Technical Details

On review I find the value ['exportOption'] is a parameter used at the form later to determine whether to
export 'Primary fields only' (ahem) or the user should select an array of fields. By the time the form is submitted we
either have an array of selected fields or NULL denoting that there is no selection (& hence the defaults should be used)
and the selection at the form layer is redundant. However it is still being referenced in one place (and hence the tests
are passing it in to avoid an enotice). This fixes that one place & removes it from the tests. It also extracts part of
a test to move towards making it easier to alter the signature of exportComponents without messing with so many places in
the code

Comments

@civibot
Copy link

civibot bot commented Jul 9, 2019

(Standard links)

@civibot civibot bot added the master label Jul 9, 2019
On review I find the value ['exportOption'] is a parameter used at the form later to determine whether to
export 'Primary fields only' (ahem) or the user should select an array of fields. By the time the form is submitted we
either have an array of selected fields or NULL denoting that there is no selection (& hence the defaults should be used)
and the selection at the form layer is redundant. However it is still being referenced in one place (and hence the tests
are passing it in to avoid an enotice). This fixes that one place & removes it from the tests. It also extracts part of
a test to move towards making it easier to alter the signature of exportComponents without messing with so many places in
the code
@eileenmcnaughton
Copy link
Contributor Author

test this please

@eileenmcnaughton
Copy link
Contributor Author

@colemanw this & #14762 are small steps in the right direction

@colemanw
Copy link
Member

colemanw commented Jul 9, 2019

I've reviewed the code and it seems to make sense.

@colemanw colemanw merged commit 2c53b6a into civicrm:master Jul 9, 2019
@colemanw colemanw deleted the export_test_improve branch July 9, 2019 12:05
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