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

EP-2378 Require amount to be submitted for recurring self-service #987

Open
wants to merge 6 commits into
base: ep-upgrade
Choose a base branch
from

Conversation

EJTison
Copy link
Contributor

@EJTison EJTison commented Oct 5, 2022

EP-2378 8.1 UI Self Service allows submit of new recurring gifts with no amount
I have no idea if this would work. I've only just begun looking at the front end, so this is all new to me. I'm trying to disable the submit button if the amount is invalid, same as it currently gets disabled if a payment method is invalid.

@EJTison EJTison changed the title I have no idea if this would work. Require amount to be submitted for recurring self-service Oct 5, 2022
@EJTison EJTison requested a review from wrandall22 October 5, 2022 19:53
@EJTison
Copy link
Contributor Author

EJTison commented Oct 5, 2022

@wrandall22 Definitely need some feedback on this. I'm flying by the seat of my pants, haha.
I don't even have the foggiest idea how I could run/test this locally, before deploying to staging. Is there perhaps a doc about how to set up your local environment for that, similar to how there was one for the back end?

@EJTison EJTison changed the title Require amount to be submitted for recurring self-service EP-2378 Require amount to be submitted for recurring self-service Oct 5, 2022
@wrandall22
Copy link
Contributor

This may not be something we want. I'll need to look at the code again, but it may be treating empty values as unchanged currently, which this would hinder.

@EJTison
Copy link
Contributor Author

EJTison commented Oct 6, 2022

but it may be treating empty values as unchanged currently,

Yes, I believe you're right.

which this would hinder.

From my understanding, this wouldn't have an impact on that? This new method only gets called in the context of checking whether the form submit button should be disabled or not. Theoretically this code shouldn't affect the actual processing of the data, later? That was my intent, at least.
I suppose in the context of "the user won't be able to submit a gift with no value" then yes, I am intentionally trying to hinder that because when submitting a new gift with no amount, it results in a 500 error from the back end (because it doesn't make any sense to create a gift with no amount), and when updating an existing gift, putting no amount will default to $1, which might not be what the user expected or intended... So I figured it would be better to simply not let them submit unless they actually have an intended amount value input...?

@wrandall22
Copy link
Contributor

There seems to be validation in place already that may not be firing when setting up a new gift here.

@EJTison
Copy link
Contributor Author

EJTison commented Oct 6, 2022

There seems to be validation in place already that may not be firing when setting up a new gift here.

Yep, I looked at that. That is working correctly, however it does nothing to actually disable the submit button. So even though an error message displays and there are checks in place to ensure invalid strings are not entered, you can still just hit the "submit" button anyway with a blank or "error-ing" input. That is what I am trying to remedy.

@EJTison
Copy link
Contributor Author

EJTison commented Oct 6, 2022

Tested in staging-- This change works perfectly for UPDATING gifts. Does not seem to work for creating new ones though. I guess I was mistaken that both actions used the same part of the code. I’ll figure out a fix.

@EJTison
Copy link
Contributor Author

EJTison commented Oct 10, 2022

Lee said I should probably leave this for Bill's team to take over at this point, since this is not in my scope of work. I am abandoning this branch for someone else to take over.
These last 4 commits are my incomplete attempt to set things up generally how they need to be, though I'm not sure how to finish setting up configureRecentRecipients.component.js and it's spec file properly... Hopefully this work is helpful and not buggy, considering I have not worked in this environment prior and did not have an IDE to keep me in check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants