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

[Import] [Contribution] Cleanup templates & form variables, following contact pattern #23683

Merged
merged 4 commits into from
Jun 4, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jun 3, 2022

Overview

Cleanup templates & form variables, following contact pattern

Before

When doing an contribution import if you go forwards to the MapField screen and then BACK to the DataSource screen you get a fatal error

After

Fixed - in addition I have applied a bunch of shareable techniques from the contribution import to help align the code.

Technical Details

This came about because things were reviewed & merged in orders other than they were developed in. I wasn't entirely surprised - hence I have been doing r-run testing on what is merged

I suspect other imports will be impacted but I will test them one by one & put up PRs - it seems that for review in this area a lot of code area but a small r-run testing surface area is what reviewers want. This DOES have changes that affect contact too - I have r-run against both - but note the contact changes CAN be tested & merged separately via #23680

It would be nice to extend the queue usage to this - I need to work through the return codes for pledge, soft credit to sort that out

Comments

@civibot
Copy link

civibot bot commented Jun 3, 2022

(Standard links)

@civibot civibot bot added the master label Jun 3, 2022
@eileenmcnaughton eileenmcnaughton changed the title Cleanup templates & form variables, following contact pattern [Import] [Contribution] Cleanup templates & form variables, following contact pattern Jun 3, 2022
@eileenmcnaughton eileenmcnaughton force-pushed the import_cont branch 4 times, most recently from 6baa739 to 2506ef7 Compare June 3, 2022 10:39
@eileenmcnaughton
Copy link
Contributor Author

@monishdeb if this passes in time for the end of your day woudl you take a look - it was causing some test fails & those were because the test was testing the old functions - which I think are obsolete - but I pulled it back to not changing those

Without this there is a significant error in the Contribution import (and maybe other) form flow (master only) - I will keep testing the others to get us in good state for the rc

@eileenmcnaughton
Copy link
Contributor Author

@colemanw I just rebased this over the one you merged - it was passing & I expect it will again - it fixes a regression that will likely affect other imports - so I'm gonna work on them once this is merged

@colemanw colemanw merged commit 194e85b into civicrm:master Jun 4, 2022
@colemanw colemanw deleted the import_cont branch June 4, 2022 01:24
@colemanw
Copy link
Member

colemanw commented Jun 4, 2022

Cool, thanks @eileenmcnaughton

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.

2 participants