OSCER-496: Enforce CE determination_data with value objects#521
Merged
jgavin-nava merged 14 commits intoMay 21, 2026
Conversation
- Add Determinations::{HoursBased,IncomeBased,ExParteCECombined}DeterminationData
(ValueObject) for validated serialization to determination_data JSONB
- CertificationCase automated CE write path uses VOs; remove ad-hoc hash builders
- Document canonical contract vs legacy rows on Determination and in income-data.md
- Add determinations_ce_determination_data_contract_spec for stable shapes per calculation_type
Made-with: Cursor
- Determination::CALCULATION_TYPE_CE_COMBINED = ce_combined; doc legacy ex_parte_ce_combined rows - Determinations::CECombinedDeterminationData + ce_combined_determination_data.rb - CertificationCase#record_ce_combined_assessment, #ce_combined_reason_codes - Update CommunityEngagementCheckService, specs, income-data.md Made-with: Cursor
…dation - Add Determination::CALCULATION_TYPE_CE_COMBINED_LEGACY; reference in comments and income-data.md - Reword Determination and HoursBasedDeterminationData comments (combined CE wording) - CECombinedDeterminationData.build validates nested VOs eagerly; YARD @raise - Spec: legacy constant + build fails on invalid nested hours aggregate - PR body draft: nava-notes/PRs/PR-OSCER-496-description.md (vault, not in repo) Made-with: Cursor
- CECombinedDeterminationData: assign nested VOs in validate_nested; reuse in to_h via accessors - HoursBasedDeterminationData: YARD on class and #to_h for service-controlled numeric aggregates Made-with: Cursor
- hours_based_determination_data_spec, income_based, ce_combined (RSpec path per class) - Move CALCULATION_TYPE_CE_COMBINED_LEGACY example to determination_spec - Update income-data.md and remove determinations_ce_determination_data_contract_spec Co-authored-by: Cursor <cursoragent@cursor.com>
jdettmannnava
left a comment
jdettmannnava
left a comment
Contributor
There was a problem hiding this comment.
Looks good! Just some tweak options and test opportunities.
…come_by_source Co-authored-by: Cursor <cursoragent@cursor.com>
…nationData#to_h Pair by_category with by_source for symmetric readability (PR review). Co-authored-by: Cursor <cursoragent@cursor.com>
- CE combined: hours-only and income-only satisfied_by; assert nested compliant on both track pass; rename stable payload example - Hours/income: compliant key when nested; omit when standalone CE Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve determination/CE conflicts with main: record_external_ce_combined_assessment, CALCULATION_TYPE_EXTERNAL_CE_COMBINED + _LEGACY, ExternalCECombinedDeterminationData VO; hours/income VOs use external_hourly_activity_ids, external_income_activity_ids, and income_by_source external/activity keys matching aggregate services. Co-authored-by: Cursor <cursoragent@cursor.com>
…rmination-data-value-objects
- HoursBasedDeterminationData: apply with_indifferent_access in from_aggregate; document string/symbol keys - IncomeBasedDeterminationData: same; ALLOWED_INCOME_BY_SOURCE_KEYS + validation for unknown keys - ExternalCECombinedDeterminationData: store indifferent-access copies in .build; YARD @raise ArgumentError on #to_h without .build - Specs: leaf negative/string-key cases, hours_by_category non-Hash, combined VO new/to_h without .build Co-authored-by: Cursor <cursoragent@cursor.com>
jgavin-nava
commented
May 11, 2026
| CALCULATION_TYPE_EXTERNAL_CE_COMBINED = "external_ce_combined" | ||
| # Stored in +determination_data["satisfied_by"]+ when +calculation_type+ is +CALCULATION_TYPE_EXTERNAL_CE_COMBINED+. | ||
| # Historical +determination_data["calculation_type"]+ before +external_ce_combined+; use for BI/read filters on old rows. | ||
| CALCULATION_TYPE_EXTERNAL_CE_COMBINED_LEGACY = "ex_parte_ce_combined" |
Contributor
Author
There was a problem hiding this comment.
I am not sure if it is better to leave so it is known but I dont think any real data would have this information or add in a migration to make sure to update any rows to the right type when needed? Or just remove it in general as I stated i dont think any real data would have this type?
…rmination-data-value-objects
…rmination-data-value-objects
jdettmannnava
approved these changes
May 21, 2026
Share one expected_calculated_at let with travel_to so frozen-time expectations stay aligned across the three value-object spec files. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket
Resolves OSCER-496
Changes
Determinations::HoursBasedDeterminationData,IncomeBasedDeterminationData, andExternalCECombinedDeterminationData(ValueObject) as the canonical, validated serialization layer for automated CEdetermination_datawritten byCertificationCase(hours, income, and combined hours + income paths).CertificationCasewith VO#to_houtput for those flows; manual activity report accept/deny now use the hours VO as well.Determinationand indocs/architecture/income-data/income-data.mdthat the value objects are the contract; other flows (exemption placeholder, eligibility JSON) remain out of scope for the VOs.determination_data["calculation_type"]is nowexternal_ce_combined(Determination::CALCULATION_TYPE_EXTERNAL_CE_COMBINED). Historical rows may still haveex_parte_ce_combined— useDetermination::CALCULATION_TYPE_EXTERNAL_CE_COMBINED_LEGACYin SQL/BI/read paths that must match both.CertificationCase#record_ex_parte_ce_combined_assessmentwas renamed torecord_external_ce_combined_assessment; any external callers or docs must follow the new name.ExternalCECombinedDeterminationData.buildeagerly validates nested hour and income aggregates (not only on#to_h) so invalid payloads fail before persist.spec/models/determinations/(hours_based_determination_data_spec.rb,income_based_determination_data_spec.rb,external_ce_combined_determination_data_spec.rb) assert stable serialized shapes forhours_based,income_based, andexternal_ce_combined, plus legacy constant and build-time validation coverage.Context for reviewers
external_ce_combined_determination_data.rb→Determinations::ExternalCECombinedDeterminationData(projectCEacronym inflection).external_ce_combined.ex_parte_ce_combinedor called the old case method.Testing
From
reporting-app/:make lint-cimake test(If the Docker image lacks gems, run
docker compose run --rm reporting-app sh -c "bundle install && bundle exec rspec …"first, per local setup.)Targeted run used during development:
bundle exec rspec spec/models/determinations/hours_based_determination_data_spec.rb spec/models/determinations/income_based_determination_data_spec.rb spec/models/determinations/external_ce_combined_determination_data_spec.rb spec/models/certification_case_spec.rb spec/services/community_engagement_check_service_spec.rb spec/services/hours_compliance_determination_service_spec.rb spec/services/income_compliance_determination_service_spec.rbFull suite before push:
make test— 1876 examples, 0 failures, 1 pending (pre-existing).Preview environment for reporting-app
♻️ Environment destroyed ♻️