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

Export class cleanup - remove some unnecessary code #13120

Merged
merged 1 commit into from
Nov 18, 2018

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Remove hard-coded add ons to 'primary' (AKA all) fields - these are already present & tested. The provider_id is actually a duplicate of im_provider and by removing it we get rid of some other hacks

Before

Unnecessary code present

After

Unnecessary code removed

Technical Details

These are being merged into an array that already contains them. The only one not already present is provider_id - but this exports the same data as im_provider

Comments

CRM_Export_BAO_ExportTest::testGetSQLColumnsAndHeaders covers this pretty thoroughly

@civibot
Copy link

civibot bot commented Nov 17, 2018

(Standard links)

…KA all) fields

In testing all these fields except for provider_id are already in the primary field.

Provider ID is a psynonym for im_provider. Possibly it is the better name choice
but it has a lot of hacks associated with it that can go by removing it
@eileenmcnaughton
Copy link
Contributor Author

test this please

@mattwire
Copy link
Contributor

@eileenmcnaughton I've tested a contact export with and without this patch. As stated in the PR the "IM Service Provider" field is no longer exported, but it's a duplicate of "IM Provider" which is still exported. Ok to merge.

@eileenmcnaughton
Copy link
Contributor Author

thanks @mattwire

@eileenmcnaughton eileenmcnaughton merged commit 07d995f into civicrm:master Nov 18, 2018
@eileenmcnaughton eileenmcnaughton deleted the less_primary branch November 18, 2018 21:48
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