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

Participant import fix - broken uniqueName fields, mapping saving, ev… #23733

Merged
merged 1 commit into from
Jun 9, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jun 8, 2022

Overview

This addresses reviewer feedback by @aydun on the Participant import cleanup currently merged.
#23703 (comment)

It addresses

  • mappings not being saved - I added a test to lock in the saving of new mappings
  • event DOES work now for both id or title. Unit test locks this in
  • the label 'event' is used on both mapping & preview screens
  • the contact id label reverts to 'Contact ID (match to contact)'
  • if contact type organization is selected then that field is offered up for matching (ditto household)

This also switches away from using unique names in the participant import - which simplifies the code a little

Contact matching

This is a pre-existing confusion that we would ideally come up with a good answer for it - I think this PR gets us to the pre-existing behaviour & we should pick up over here https://lab.civicrm.org/dev/core/-/issues/3180

Note to self - re-test :"update mapping" works - UPDATE - yep UI test worked to update a field in a mapping

@civibot
Copy link

civibot bot commented Jun 8, 2022

(Standard links)

@civibot civibot bot added the master label Jun 8, 2022
@eileenmcnaughton eileenmcnaughton force-pushed the import_part branch 2 times, most recently from 038104a to 011ab68 Compare June 8, 2022 22:44
…ent_id

This switches to using 'normal' field names
for all participant fields.
@@ -88,11 +94,13 @@ protected function importCSV(string $csv, array $fieldMappings, array $submitted
$form = $this->getFormObject('CRM_Event_Import_Form_MapField', $submittedValues);
$form->setUserJobID($this->userJobID);
$form->buildForm();
$this->assertTrue($form->validate());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@demeritcowboy I managed to add these after two of the buildForm calls - the first one fails on the file upload rule which seems tricky

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool. Would need to look closer - there's that low-level php function move_uploaded_file or something that you used to have to use in the previous century to access the file for security reasons, so maybe there's an extra hurdle for those.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep - but running validate will def help find stuff

@colemanw
Copy link
Member

colemanw commented Jun 8, 2022

Looks good. Ok to merge once tests pass.

@eileenmcnaughton eileenmcnaughton merged commit bc6f923 into civicrm:master Jun 9, 2022
@eileenmcnaughton eileenmcnaughton deleted the import_part branch June 9, 2022 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants