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-2558 - Update radio stations API #1126

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

wrandall22
Copy link
Contributor

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.

@wrandall22 wrandall22 requested a review from dr-bizz January 10, 2025 21:09
@wrandall22
Copy link
Contributor Author

@clonge in case you want to review any of the changes I made here for the new radio station API response model structure.

@clonge
Copy link

clonge commented Jan 10, 2025

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:

<option value="ZWEB">FamilyLife.com</option>
<option value="ZPOD">Podcasts</option>
<option value="ZYOU videos">YouTube videos</option>
<option value="RADIO">Radio</option>
<option value="ONE">OnePlace.com</option>
<option value="ZMFL">My FamilyLife App</option>
<option value="ZZZZ">None of the above yet!</option>

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).

@wrandall22
Copy link
Contributor Author

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.

@wrandall22
Copy link
Contributor Author

If it's worth the trouble, I could have them configure defaults in the form of

[
  { callLetter: 'name' },
  ...
]

@clonge
Copy link

clonge commented Jan 10, 2025

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.

Copy link
Contributor

@dr-bizz dr-bizz left a 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.

Comment on lines +375 to +376
this.sessionStorage.setItem('radioStationName', Object.values(radioStationData)[0])
this.sessionStorage.setItem('radioStationCallLetters', Object.keys(radioStationData)[0])
Copy link
Contributor

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?

Copy link
Contributor Author

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.

@dr-bizz dr-bizz self-requested a review January 10, 2025 22:01
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.

3 participants