Admin Banner

[ailZhou]

Description

Behold, we have admin banner page & form!

Related ticket(s)

CMDCT-4089
CMDCT-4090

---

How to test

1. Sign into HCBS, adminuser@test.com
2. In the top right corner, click [My Account] -> [Manage Account]
   
3. On the My Account page click [Banner Editor]
   
4. This will take you to the Banner Admin page. Fill out the form completely and click [Replace Current Banner]
   • Note: We do not have validation yet so there will still be some bugginess until that is added
     
5. You should see the Banner filled in Current Banner section
   
6. Click [Delete Current Banner] and that should remove the banner. That should complete all the test for the banner.

Important updates

---

Author checklist

• I have performed a self-review of my code
• I have added thorough tests, if necessary
• I have updated relevant documentation, if necessary

---

convert to a different template: test → val | val → prod

[codeclimate]

Code Climate has analyzed commit 1877c17 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 28.7% (90% is the threshold).

This pull request will bring the total coverage in the repository to 89.7% (-3.5% change).

View more on Code Climate.

[benmartin-coforma]

I've got lots of comments below, but overall I think this is really good work! Thanks so much for taking on this story.

[benmartin-coforma]

This is looking really good! My remaining comments are quibbles about types.

[benmartin-coforma]

I think this needs a change, but I'm not sure what it should be.

For one, the ? in bannerData?.startDate was never necessary here; we know that bannerData exists from the check on line 51.

More importantly, the ! worries me. We're telling Typescript "trust me, this value will always exist"... but you and I know that it won't always exist, because the field in AdminBannerForm is optional. What will we display for a banner with no defined start date? Today's date? An error? Nothing?

I haven't tested this, but I suspect (based on my reading of convertDateUtcToEt) that it will be today's date, which would be incorrect behavior, right? Do you think the best behavior would be to display nothing instead? Something like: { bannerData.startDate && <span{convertDate... perhaps?

If you've already tested this scenario and are satisfied with the behavior, my apologies - but maybe we should make that behavior explicit? Something like: bannerData.startDate ?? new Date().valueOf() perhaps? Or maybe a change within convertDateUtcToEt, so that it accepts number | undefined?

And whatever change we make here, we should also make 4 lines down for endDate.

[ailZhou]

I'm glad you gave feedback on this because I was thinking about this before starting work today and wasn't certain what to do.

The start & end date field isn't suppose to be optional when filling out the admin banner form as there's a set time frame when the banner would run, allowing for scheduling ahead of time. I think this is why it was split as a separate type that inherited from a banner parent type. There's suppose to be a validation check that would make sure the user enter's the date fields in but we currently don't have validation set up yet.

Anyway, I decided to go with your suggestion of the conditional to check if the value exist and to display nothing if somehow the user managed to save without it.

[benmartin-coforma]

Brilliant. Thank you for all of this!

[angelaco11]

Tested on cloudfront and the banner editor looks great!