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

Add appointment to Claim Status summary #345

Merged
merged 30 commits into from
Jul 30, 2021
Merged

Conversation

rocketnova
Copy link
Contributor

Ticket

Resolves #341

Changes

  • Create <Appointment> component (with story and tests)
  • Create <ClaimSummary> component (with story and tests)
  • Add business logic for when to display an appointment as part of Scenario 2
  • Add translation strings for appointments
  • Add missing Scenario 2 and Scenario 3 snapshot tests for <ClaimStatus> component
  • Add test helpers (broken out of getScenarioContent.test.tsx)
  • Fix bugs in formatDate
  • Fix bugs in timeSlot
  • Add a util file for handling Sentence case in strings

Context

This PR is big, but it is only actually handling one very narrow case: when to display the appointment date (pendingDetermination.scheduleDate) and time (pendingDetermination.timeSlotDesc) and how to format it.

Testing

yarn test

ALSO

  1. While Installing pino-applicationinsights has broken storybook builds #299 is not resolved, comment out the following lines:
    if (isAzureEnv) {
    const appInsightsStream = await appInsights.createWriteStream()
    logger = pino(appInsightsStream)
    }
  2. Run yarn storybook
  3. Navigate to Page > Default
  4. Select scenario "Determination interview: scheduled"
  5. Verify it looks right
  6. Click the globe in the top menu bar and select "Español"
  7. Verify it looks right
  8. Navigate to Atoms > Appointment
  9. Select a date (ignore the time picker) and verify in both English and Spanish
  10. Enter a start and end time (must be between 1–12) and verify in both English and Spanish

Note: that some Spanish translation strings are still missing from main at the moment, but will be added with #329.

Comment on lines +41 to +46
timeSlot: {
table: {
disable: true,
},
},
},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This hides the timeSlot argument from the Storybook Controls. Let me know if you disagree with this approach.

Comment on lines +17 to +18
import enUS from 'date-fns/locale/en-US'
import es from 'date-fns/locale/es'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*/
export function getDateWithOffset(daysOffset = 1): Date {
export function formatFromApiGateway(daysOffset = 1): string {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I eliminated getDateWithOffset() as a standalone function because it tempted me into falling into the trap where running tests on different machines (with different server timezones) caused tests to fail. We should always be testing using the timezone-free datetime strings that the API gateway will give us.

Copy link
Contributor

@lomky lomky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple small questions!

@@ -0,0 +1,49 @@
import { useTranslation } from 'next-i18next'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:chef-kiss: component!

Comment on lines +23 to +29
const testCases = [
['with no time slot, then match the snapshot', null, null],
['with a morning time slot, then match the snapshot', 8, 10],
['with an afternoon time slot, then match the snapshot', 1, 3],
['with a time slot that starts in the morning and ends in the afternoon, then match the snapshot', 8, 3],
['with a time slot that has a nonsense time range, then match the snapshot', 3, 9],
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love this setup!

tests/components/Appointment.test.tsx Outdated Show resolved Hide resolved
['with a morning time slot, then match the snapshot', 8, 10],
['with an afternoon time slot, then match the snapshot', 1, 3],
['with a time slot that starts in the morning and ends in the afternoon, then match the snapshot', 8, 3],
['with a time slot that has a nonsense time range, then match the snapshot', 3, 9],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we just wanna allow this? 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good question. I'm not positive? I'll open a new ticket for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ticket is #351

Comment on lines +1 to +3
/**
* Common test helpers to create shared mock data.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

tests/utils/timeSlot.test.ts Outdated Show resolved Hide resolved
Comment on lines +20 to +21
// Test buildAppointment()
describe('An appointment is', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really appreciate the nice layering of these tests to scope to each level's concerns

Comment on lines +24 to +38
min: 1,
max: 12,
},
},
end: {
name: 'end time',
table: {
type: {
summary: 'number',
},
},
control: {
type: 'number',
min: 1,
max: 12,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these don't seem to stop me:

Friday, July 30, 2021, between 12–15 p.m. Pacific time

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmm. I'm going to go ahead and merge this without resolving it. I'll open a follow up (low priority) ticket.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ticket is #350

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.

Format and display appointment date and time for Scenario 2
2 participants