Process the correct record upon record.copy events#681
Conversation
liam-lloyd
force-pushed
the
per-10488_send_correct_file_to_archivematica_upon_copy_events
branch
from
7870273 to
f5bb402
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #681 +/- ##
=======================================
Coverage 98.27% 98.27%
=======================================
Files 107 107
Lines 2673 2673
Branches 453 453
=======================================
Hits 2627 2627
Misses 42 42
Partials 4 4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
liam-lloyd
force-pushed
the
per-10488_send_correct_file_to_archivematica_upon_copy_events
branch
from
f5bb402 to
42e3406
Compare
liam-lloyd
force-pushed
the
per-10488_send_correct_file_to_archivematica_upon_copy_events
branch
from
42e3406 to
61a5a71
Compare
There was a problem hiding this comment.
Pull request overview
Fixes record.copy handling in the Archivematica trigger so that processing is initiated for the copied record rather than the original, aligning behavior with the event payload structure (newRecord).
Changes:
- Update record ID extraction to use
body.newRecord.recordIdforrecord.copyevents. - Extend unit tests to cover missing
newRecordand validate multi-record handling. - Expand SQL fixtures to include additional records/files needed for the updated test scenarios.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/trigger_archivematica/src/index.ts | Correctly selects the copied record ID on record.copy events. |
| packages/trigger_archivematica/src/index.test.ts | Adds/updates tests to exercise record.copy path and multi-message handling. |
| packages/trigger_archivematica/src/fixtures/create_test_records.sql | Adds records needed for copy/deleted-file test cases. |
| packages/trigger_archivematica/src/fixtures/create_test_record_files.sql | Updates record↔file relationships to support new test coverage. |
| packages/trigger_archivematica/src/fixtures/create_test_files.sql | Adds an additional file fixture used by new/updated tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| } else if (action === "copy") { | ||
| if (parsedMessage.body.newRecord === undefined) { | ||
| logger.error( | ||
| `record.copy event missing newRecord: ${JSON.stringify( | ||
| parsedMessage.body, | ||
| )}`, | ||
| ); | ||
| throw new Error("newRecord field missing in body of record.copy"); | ||
| } | ||
| return parsedMessage.body.newRecord.recordId; |
There was a problem hiding this comment.
For posterity, the robot is wrong about this; these fields cannot be null
| if (action === "create") { | ||
| if (parsedMessage.body.record === undefined) { |
There was a problem hiding this comment.
Bah. robots over here outperforming me.
slifty
left a comment
slifty
left a comment
There was a problem hiding this comment.
I am awkwardly waiting on copilot to notice anything but... this looks good to me!
I guess let's see if the robot finds anything useful before actually merging.
Currently when a record is copied we trigger archivematica for the original record (which has already been processed by archivematica) rather than the copy. This commit fixes that.
liam-lloyd
force-pushed
the
per-10488_send_correct_file_to_archivematica_upon_copy_events
branch
from
61a5a71 to
b373953
Compare
liam-lloyd
deleted the
per-10488_send_correct_file_to_archivematica_upon_copy_events
branch
Currently when a record is copied we trigger archivematica for the original record (which has already been processed by archivematica) rather than the copy. This commit fixes that.