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

N°7234 - Add the possibility of mapping the same CSV column twice or more #46

Merged
merged 14 commits into from May 13, 2024

Conversation

accognet
Copy link
Contributor

@accognet accognet commented Feb 9, 2024

example :
<PersonCollector>
<csv_file>collectors/Person_import.csv</csv_file>
<separator>,</separator>
<fields>
<primary_key>email</primary_key>
<first_name>first_name</first_name>
<name>name</name>
<status>status</status>
<org_id>org_id</org_id>
<email>email</email>
</fields>
</PersonCollector>

Copy link
Collaborator

@Hipska Hipska left a comment

Choose a reason for hiding this comment

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

It does make sense and very useful, however some minor change requests..

core/csvcollector.class.inc.php Outdated Show resolved Hide resolved
core/csvcollector.class.inc.php Outdated Show resolved Hide resolved
core/csvcollector.class.inc.php Outdated Show resolved Hide resolved
@accognet accognet self-assigned this Feb 12, 2024
@accognet accognet added the enhancement New feature or request label Feb 12, 2024
Copy link
Member

@Molkobain Molkobain left a comment

Choose a reason for hiding this comment

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

Unit test isn't working as expected:

  • If the duplicated field is something other than the primary key, it doesn't work
  • If 2 or more fields are duplicated it doesn't work, only the first one seem to work
  • Columns orders in the generated CSV changes when you add a duplicated field

Anne-Cath, you can talk to Denis or me about these comments, we reviewed it together.

core/csvcollector.class.inc.php Outdated Show resolved Hide resolved
core/csvcollector.class.inc.php Outdated Show resolved Hide resolved
@Molkobain Molkobain changed the title N°7234 - Collector Base: Add the possibility of mapping the same csv column twice N°7234 - Add the possibility of mapping the same CSV column twice or more Apr 25, 2024
Copy link
Collaborator

@Hipska Hipska left a comment

Choose a reason for hiding this comment

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

Please use correct php doc syntax.

core/csvcollector.class.inc.php Outdated Show resolved Hide resolved
core/csvcollector.class.inc.php Outdated Show resolved Hide resolved
@accognet accognet force-pushed the feature/7234-map_csv_column_twice branch from d605a38 to d2314c7 Compare April 29, 2024 11:51
accognet and others added 6 commits April 30, 2024 10:39
Co-authored-by: Thomas Casteleyn <thomas.casteleyn@super-visions.com>
Co-authored-by: Thomas Casteleyn <thomas.casteleyn@super-visions.com>
Co-authored-by: Thomas Casteleyn <thomas.casteleyn@super-visions.com>
@accognet accognet force-pushed the feature/7234-map_csv_column_twice branch from d2314c7 to 334edb4 Compare April 30, 2024 08:39
@Molkobain
Copy link
Member

All good, you can squash & merge. Mind to keep the PR number in the commit message.

@accognet accognet merged commit a44142e into master May 13, 2024
1 check passed
@accognet accognet deleted the feature/7234-map_csv_column_twice branch May 13, 2024 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
3 participants