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.

Sign up for GitHub

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

Jump to bottom

Afform - Fix chainSelect to work with anonymous users #21759

Merged
merged 2 commits into from
Oct 7, 2021
Merged

Afform - Fix chainSelect to work with anonymous users #21759

merged 2 commits into from
Oct 7, 2021

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Oct 7, 2021

Overview

A few fixes following up from #21749

  • Fixes chainSelect to work with anonymous users
  • Correctly handle empty lists (e.g. countries with no states or provinces)

@colemanw
Afform - Rename submitFile param from entityName to modelName
9474f84 
$entityName is already a private property of APIv4 and has its own getter
which conflicted with a getter for a param named $entityName.
Better to just rename it.
@civibot
Copy link

civibot bot commented Oct 7, 2021

(Standard links)

@civibot civibot bot added the master label Oct 7, 2021
@rbaugh
Copy link

rbaugh commented Oct 7, 2021

I tested commit 2ab97fa and can confirm that it fixes the API call issue for anonymous on getting the state/province data for the field when form loads and when country changes. This was an issue for anonymous users once the chainSelect connection was fixed.

@colemanw colemanw added has-test merge ready PR will be merged after a few days if there are no objections labels Oct 7, 2021
@colemanw
Copy link
Member Author

colemanw commented Oct 7, 2021

Thanks for the testing. I've just added a unit test also, so between that and your review we should be good to go.

@rbaugh
Copy link

rbaugh commented Oct 7, 2021

I saw the unit test in the commit, but wasn't sure if you needed outside test on it. I saw it was merely changing/adding a variable, but wasn't sure what all was needed to be setup to test.

@colemanw
Copy link
Member Author

colemanw commented Oct 7, 2021

All good. The unit test will get automatically run by our CI infrastructure & the yellow status on this PR will turn green in a couple hours.

@rbaugh
Copy link

rbaugh commented Oct 7, 2021

Sounds good, thanks for working on this.

@colemanw
Copy link
Member Author

colemanw commented Oct 7, 2021

Merging per review & unit tests passing

@colemanw colemanw merged commit a185946 into civicrm:master Oct 7, 2021
@colemanw colemanw deleted the afformApiFixes branch October 7, 2021 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
has-test master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@colemanw @rbaugh