Export - add tests out header output, move phone_type_id to metadata #12587
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(Standard links)
|
|
Jenkins re test this please |
|
@eileenmcnaughton i think this needs a rebase now |
This has the side effect of making it such that it only gets 16 char rather than 255. I thnk this is 'ok' here but there is a separate issue open to increase the varchar for all fields on export
eileenmcnaughton
force-pushed
the
header_test
branch
from
0d1d4e6 to
219c47d
Compare
| @@ -1756,6 +2063,7 @@ protected function getBasicSqlColumnDefinition($isContactExport) { | |||
| 'country' => 'country varchar(64)', | |||
| 'phone' => 'phone varchar(32)', | |||
| 'phone_ext' => 'phone_ext varchar(16)', | |||
| 'phone_type_id' => 'phone_type_id varchar(16)', | |||
There was a problem hiding this comment.
@eileenmcnaughton is the change here because it is now able to use the field definition and not fall back to a default length of 255?
There was a problem hiding this comment.
@seamuslee001 yep - it's an unintended consequence but one which I think is OK since many other fields are given 16 char. I want to look at that side of things more once I get to the end of the cleanup
|
@seamuslee001 is this good to merge? |
|
Yay lots of tests! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Add lots of tests to export. Use metadata for phone_type_id
Before
Header output not tested
After
Header output tested.
Phone Type ID column is with the other phone fields. As a side effect the definition is now only varchar(16). I think this is no worse than all the other fields with the 16 limit & changing this field should be OK. But there is a separate issue open to look at a higher varchar number for all columns in the output https://lab.civicrm.org/dev/core/issues/181
Technical Details
Support export of phone_type_id by marking the field as exportable. It would be nice to clean up campaign_id fields & im_provider but the latter requires a bit more untangling and the former requires a title change on the field (from 'Campaign' to 'Campaign ID') - this is not dissimilar to changes made in other places & should better support search builder but I have left it out of scope for now
Comments