-
Notifications
You must be signed in to change notification settings - Fork 1
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-2558 - Update radio stations API #1126
base: master
Are you sure you want to change the base?
Conversation
… cannot be configured anymore.
@clonge in case you want to review any of the changes I made here for the new radio station API response model structure. |
Looks good to me. What you have should work, but not sure if you want to accommodate FL's defaults? The current form has the following default options:
These are there in case the api fails for any reason. In the current form the radio station field is required, but FL wants the user to still be able to complete the donation even if the api fails. I'm not sure the best way to implement these fallback values in your code. These default options are always returned by the api, so providing the defaults is only for a complete api failure (404, 500, etc). |
One reality is that this is technically meant to be usable by any ministry, not just FL (I know this means they would need to match the same API schema), which makes me a bit hesitant to add defaults on the client side which are specific to FL. |
If it's worth the trouble, I could have them configure defaults in the form of
|
Yep, understood. I don't think it's critical. We will need to test thoroughly when/if FL switches to branded-checkout anyway. If it's a big issue they will bring it up then. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started to review this, but I need more time to do a deep dive.
this.sessionStorage.setItem('radioStationName', Object.values(radioStationData)[0]) | ||
this.sessionStorage.setItem('radioStationCallLetters', Object.keys(radioStationData)[0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume we know the names of these properties or they change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The names of the properties change based on zip code/API response.
Jira
Part of sunsetting Studio Enterprise means reworking the radio stations API. Nobody is using it in BCO right now, but we want the code to work when they do.
Note: Currently the first call to the API takes a long time; we may want to put some sort of loading indicator in the radio station dropdown.