CHI-3585: Add Deactivated -> deletedAt mapping for USCH resources#992
CHI-3585: Add Deactivated -> deletedAt mapping for USCH resources#992stephenhand merged 7 commits intomasterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for deactivating USCH resources by mapping a CSV Deactivated column to the deletedAt field in the resource model.
Changes:
- Added
Deactivatedfield (boolean) to the resource type definitions - Implemented CSV parsing logic to convert string 'true'/'false' values to boolean
- Added mapping from
Deactivatedboolean todeletedAttimestamp field - Updated test fixtures and added test coverage for the new field
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| resources-domain/lambdas/s3-importer/src/uschMappings.ts | Added Deactivated field to type definitions, implemented CSV-to-boolean parsing logic, and added mapping to deletedAt field |
| resources-domain/lambdas/s3-importer/tests/fixtures/sampleResources.ts | Added Deactivated: '' to the empty CSV line fixture |
| resources-domain/lambdas/s3-importer/tests/unit/uschMappings.test.ts | Added comprehensive tests for parsing Deactivated field and updated existing tests to include the new field |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
resources-domain/lambdas/s3-importer/tests/unit/uschMappings.test.ts
Outdated
Show resolved
Hide resolved
| Deactivated: resourceFieldMapping('deletedAt', context => | ||
| context.currentValue ? new Date().toISOString() : '', | ||
| ), |
There was a problem hiding this comment.
Using new Date().toISOString() for the deletedAt timestamp means that every time a deactivated resource is re-imported from the CSV, it will get a new deletedAt timestamp. This could be problematic if the system expects deletedAt to represent the original deactivation time. Consider whether the mapping should preserve an existing deletedAt value, or if the CSV should include an actual DeactivatedDate field that can be mapped similar to how UpdatedOn is mapped from the CSV.
There was a problem hiding this comment.
The deletedOn date is purely informational and doesn't drive any internal logic. The last time the resource was explicitly specified as deleted is fine for now
…est.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…sch_flag' into CHI-3672-resources_deactivated_usch_flag
| Inactive: resourceFieldMapping('deletedAt', context => | ||
| context.currentValue ? new Date().toISOString() : '', | ||
| ), |
There was a problem hiding this comment.
I don't get why if Inactive is a boolean above, here's is a "current timestamp"?
There was a problem hiding this comment.
Because the target field is deletedAt - but USCH want a simple flag, so we set deletedAt to the current time if inactive = true, or blank it out if false
stephenhand
changed the title
Description
Add Deactivated -> deletedAt mapping for USCH resources
This should be all that's required to support deactivating resources for USCH so long as the input CSVs are updated to have a Deactivated column with a true / false value
Checklist
Other Related Issues
None
Verification steps
AFTER YOU MERGE
You are responsible for ensuring the above steps are completed. If you move a ticket into QA without advising what version to test, the QA team will assume the latest tag has the changes. If it does not, the following confusion is on you! :-P