Repository navigation
Edit failed messages in EF - #5721
Merged
Merged
Conversation
rbev
commented
Aug 10, 2026
johnsimons
approved these changes
Aug 10, 2026
johnsimons
left a comment
Member
There was a problem hiding this comment.
I added a few comments, but there is nothing really wrong it.
The manager pattern is interesting! Which is not introduced by this PR.
| { | ||
| if (!Guid.TryParse(failedMessageId, out var uniqueMessageId)) | ||
| { | ||
| return null!; |
Member
There was a problem hiding this comment.
This feels like we should make the interface nullable?
Comment on lines
+70
to
+73
| if (failedMessage == null) | ||
| { | ||
| throw new InvalidOperationException("No failed message loaded"); | ||
| } |
Member
There was a problem hiding this comment.
I am not the greatest fan of this design, where methods need to be called in a specific order!
Contributor
Author
There was a problem hiding this comment.
Yeah I don't love it, this interface is small enough that it probably doesn't need to do this at all. It's really just used for some reads and a TryUpsert()
rbev
force-pushed
the
rhys/edit-failures
branch
from
August 11, 2026 01:04
852ef3c to
4176754
Compare
rbev
enabled auto-merge
August 11, 2026 01:05
rbev
disabled auto-merge
August 11, 2026 01:42
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
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.
This pull request implements support for editing failed messages in the EFCore persistence layer, including both SQL Server and PostgreSQL backends. It introduces a new
FailedMessageEditEntityto track edit operations, updates the EFCore data model and migrations, and provides a concrete implementation of the edit manager. The changes also include enabling and testing the edit-and-retry feature in the test suites for both backends.EFCore Data Model and Migrations
FailedMessageEditEntityclass to represent edits of failed messages, and created corresponding migrations for both PostgreSQL (20260810031820_AddFailedMessageEdits.cs) and SQL Server (20260810031725_AddFailedMessageEdits.cs). This includes new tables, primary keys, and indexes. [1] [2] [3]ServiceControlDbContext) and model configurations to includeFailedMessageEditEntityand its configuration. [1] [2] [3]Edit Manager Implementation
EditFailedMessagesManagerand its data store, providing methods to get and set edit IDs, resolve failed messages (with correct timestamp stamping for retention), and persist changes. [1] [2]Testing and Feature Enablement
EditHandlerAuditTests.cs,EditMessageTests.cs) for both PostgreSQL and SQL Server test projects, and added focused EFCore-only tests to validate correct timestamp handling and edit ID persistence. [1] [2] [3]EditMessageTests.csto use the correct unique message ID for edit tracking.