-
Notifications
You must be signed in to change notification settings - Fork 11
FFS-3602: Add concept of "Reporting Month" to HR1 flow with date validation #1247
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
base: main
Are you sure you want to change the base?
Conversation
j-shilling
left a comment
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.
Looks awesome to me
| organization_name: "Scoped", | ||
| hours: 1, | ||
| date: Date.new(2000, 1, 1) | ||
| date: Date.current |
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 wonder if using FactoryBot attributes_for / create would make these tests easier to refactor in the future. That would let us only need to update test where the new column is relevant to the assertions.
|
|
||
| flow = ActivityFlow.create_from_invitation(invitation, device_id) | ||
|
|
||
| expect(flow.reporting_month).to eq(Date.new(2025, 3, 1)) |
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.
nit: I feel like make the assertion relative to the invitation is more semantically meaningful--just because we don't care about the date as much as we care about it matching what the input was.
| expect(flow.reporting_month).to eq(Date.new(2025, 3, 1)) | |
| expect(flow.reporting_month).to eq(invitation.reporting_month) |
| flow = build(:activity_flow, reporting_month: Date.new(2025, 2, 1)) | ||
|
|
||
| expect(flow.reporting_month_range).to eq(Date.new(2025, 2, 1)..Date.new(2025, 2, 28)) |
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.
ni: just like above this could use the method names to describe intention.
| flow = build(:activity_flow, reporting_month: Date.new(2025, 2, 1)) | |
| expect(flow.reporting_month_range).to eq(Date.new(2025, 2, 1)..Date.new(2025, 2, 28)) | |
| reporting_month = Date.new(2025, 2, 1) | |
| flow = build(:activity_flow, reporting_month: reporting month) | |
| expect(flow.reporting_month_range).to eq(reporting_month.beginning_of_month..reporting_month.end_of_month) |
Ticket
Resolves FFS-3602.
Changes
reporting_monthtoActivityFlow,ActivityFlowInvitationdefaulting to current monthActivityclassContext for reviewers
There was some ambiguity around how we'd like to handle the date picker UI when there are multiple months vs one month, but I say let's defer that until the future when we have more solidified designs. This is a great starting point for the UI and validations.
While I added date concepts to the job training activity, it didn't really make sense to do anything with education activity at this point, given how it's processed.
Note that tokenized flows intake a reporting month during link creation and it is stored on the flow invitation. That invitation information is then copied over to the flow when the link is used.
Acceptance testing
:alert: Deploy block! @ffs-eng I just merged PR [#123] and will be doing acceptance testing in demo - please don't deploy until I'm finished!)