Skip to content
This repository was archived by the owner on Oct 29, 2020. It is now read-only.

get the user address form to show up#6195

Merged
katiecrane merged 1 commit intoDoSomethingArchive:devfrom
katiecrane:user-address
Feb 22, 2016
Merged

get the user address form to show up#6195
katiecrane merged 1 commit intoDoSomethingArchive:devfrom
katiecrane:user-address

Conversation

@katiecrane
Copy link
Contributor

What's this PR do?

There was an if statement that was unsetting all the form elements if there were fewer than 10 of them. However, we only want that to unset if only the default elements are there, and there are 2 of those.

How should this be manually tested?

  1. Go to a campaign and go to custom settings.
  2. Scroll down to "Signup Data Form"
  3. Check "Collect additional signup data"
  4. Expand "Configuration"
  5. Expand "Form Elements"
  6. Check "Collect User address"
  7. Fill in the necessary fields
  8. Login as a non-admin, sign up for the campaign and click the link under "Stuff you need" and the form should appear

Any background context you want to provide?

When I was submitting my address, it was saying it was invalid. I'm not sure if it just doesn't work on local or if that has to do with the UPS api.

What are the relevant tickets?

Fixes #6158

cc: @mikefantini, @DFurnes (also shoutout to him for help with drupal spelunking)

@mikefantini
Copy link

Hmmm @katiecrane did you use your NY address?

Did it just simply say it was invalid or did it suggest a way to fix it?

@katiecrane
Copy link
Contributor Author

@mikefantini no I used my parents' address in GA. It said it was invalid and did not suggest a way to fix it.

@mikefantini
Copy link

Yeah @katiecrane so on production all addresses work, but for testing purposes only NY and CA addresses work.

@katiecrane
Copy link
Contributor Author

@mikefantini the more you know. I just tried with an NY address and it worked! Suggested fixes and then accepted the address.

@angaither
Copy link
Contributor

@katiecrane did you check the issue that this fix was added for? to make sure that didn't break?
#5788

cc @weerd

@katiecrane
Copy link
Contributor Author

@angaither It looks like this now which is less extra space than in #5788, but I'm not sure if it looks the same as @weerd 's fix (but I think it should?)

image

@weerd
Copy link
Contributor

weerd commented Feb 22, 2016

I'm not 100% sure why 10 was the magic number I used...? But if it's tested and works fine then seems good to me :) Any chance this effects other modals w/ form elements? Maybe that's why 10 was the magic number?

@katiecrane
Copy link
Contributor Author

@weerd it's only in the dosomething_signup_user_signup_data_form function so I'd think it would only mess with that modal? It's possible that the magic number is larger than 2 I guess, but it's also definitely less than 10 if we want the address form to work

@weerd
Copy link
Contributor

weerd commented Feb 22, 2016

okie doke. sounds cool 💃

@angaither
Copy link
Contributor

👍

katiecrane added a commit that referenced this pull request Feb 22, 2016
get the user address form to show up
@katiecrane katiecrane merged commit b04a5f9 into DoSomethingArchive:dev Feb 22, 2016
@katiecrane katiecrane deleted the user-address branch February 22, 2016 19:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants