Skip to content

VED-169-Delta-Multi-Rec #413

Merged
nhsdevws merged 4 commits intomasterfrom
VED-169-Delta-Multi-Rec
May 15, 2025
Merged

VED-169-Delta-Multi-Rec #413
nhsdevws merged 4 commits intomasterfrom
VED-169-Delta-Multi-Rec

Conversation

@nhsdevws
Copy link
Contributor

@nhsdevws nhsdevws commented May 14, 2025

Summary

  • Routine Change
  • ❗ Breaking Change
  • 🤖 Operational or Infrastructure Change
  • ✨ New Feature
  • ⚠️ Potential issues that might be caused by this change

JIRA: https://nhsd-jira.digital.nhs.uk/browse/VED-169
Changes:

  • Process all (multiple) records in an event
  • Standardise Event, Operation & ActionFlag names
  • Clean Lambda Response - API is triggered by stream, so HTTP-style response (ie statusCode, body) not required.
  • Unit tests for multiple records
  • Unit tests for Logical & Physical Delete

Reviews Required

  • Dev
  • Test
  • Tech Author
  • Product Owner

Review Checklist

ℹ️ This section is to be filled in by the reviewer.

  • I have reviewed the changes in this PR and they fill all or part of the acceptance criteria of the ticket, and the code is in a mergeable state.
  • If there were infrastructure, operational, or build changes, I have made sure there is sufficient evidence that the changes will work.
  • I have ensured the changelog has been updated by the submitter, if necessary.

@nhsdevws nhsdevws force-pushed the VED-169-Delta-Multi-Rec branch from 788408a to 344b7b7 Compare May 14, 2025 15:48
@nhsdevws nhsdevws force-pushed the VED-169-Delta-Multi-Rec branch from 58505d0 to 1580a4b Compare May 14, 2025 15:49
@nhsdevws nhsdevws changed the title DRAFT - VED-169-Delta-Multi-Rec VED-169-Delta-Multi-Rec May 14, 2025
@nhsdevws nhsdevws self-assigned this May 14, 2025
self.assertTrue(result)

@patch("boto3.resource")
def test_handler_success_remove(self, mock_boto_resource):
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this is meant to be testing the REMOVE case and the extra asserts should be added to test_handler_success_update instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mfjarvis yep, this needs addressing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mfjarvis this has been addressed along with new test for PHYSICAL DELETE / REMOVE

@nhsdevws nhsdevws enabled auto-merge (squash) May 15, 2025 08:05
@nhsdevws nhsdevws force-pushed the VED-169-Delta-Multi-Rec branch from 8a011d8 to aa8035f Compare May 15, 2025 10:35
@nhsdevws nhsdevws requested a review from mfjarvis May 15, 2025 10:55
@nhsdevws nhsdevws force-pushed the VED-169-Delta-Multi-Rec branch from baaefb2 to 5caa193 Compare May 15, 2025 11:40
@sonarqubecloud
Copy link

@nhsdevws nhsdevws merged commit f605643 into master May 15, 2025
8 checks passed
@nhsdevws nhsdevws deleted the VED-169-Delta-Multi-Rec branch May 15, 2025 12:45
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.

2 participants