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] Remove handling for non-existent 'savedMapping' field #23283

Merged
merged 1 commit into from
Jun 3, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

[REF] Remove handling for non-existent 'savedMapping' field

Before

The mapField class has handling for the field savedMapping - however this field is NOT present on the mapField form - it IS present on the DataSource form - but that isn't accessed by the mapField class - so I can't see any way the inside of the removed IF is reachable.

MapField does have the field saveMapping - the difference is not a typo - the field on DataSource would ideally be saved_mapping_id whereas saveMapping is an instruction to save the mapping

After

poof

Technical Details

This pattern is repeated across all the MapField classes so my analysis here is right then I can remove in the others too

Comments

I did an r-run in the Membership import

@civibot
Copy link

civibot bot commented Apr 22, 2022

(Standard links)

@civibot civibot bot added the master label Apr 22, 2022
@@ -288,68 +288,66 @@ public function buildQuickForm() {
public static function formRule($fields, $files, $self) {
$errors = [];

if (!array_key_exists('savedMapping', $fields)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@darrick
Copy link
Contributor

darrick commented May 26, 2022

I ran headlong into this when I was trying to create a function to submit imports via the command line. I started with the importCSV you are familiar with. So I had to unset this variable in the submittedFields before calling the forms post_process.

@eileenmcnaughton
Copy link
Contributor Author

@darrick it looks like this is stale so I would need to rebase to bring back to life - are you saying a rebased version of this SHOULD be merged?

@eileenmcnaughton
Copy link
Contributor Author

Also - I should note @darrick - my hope is that there will be a simple way to run the import in the end - the goal is that we can use the civicrm_queue table - which requires it to be easily runabe

@eileenmcnaughton
Copy link
Contributor Author

OK - it's rebased - if you are able to confirm you agree with the change I should be able to get someone to merge it

@eileenmcnaughton
Copy link
Contributor Author

@mlutfy do you feel you could merge this too?

@darrick
Copy link
Contributor

darrick commented Jun 3, 2022

I did revisit this again last night as I was unsure exactly how forms were processed. I can confirm this PR does what is intended.

@eileenmcnaughton
Copy link
Contributor Author

@darrick ie - you are saying it should be merged?

@darrick
Copy link
Contributor

darrick commented Jun 3, 2022

@eileenmcnaughton yes.

@totten totten merged commit 24f0a1c into civicrm:master Jun 3, 2022
@eileenmcnaughton eileenmcnaughton deleted the import_saved_map branch June 3, 2022 07:12
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.

3 participants