-
Notifications
You must be signed in to change notification settings - Fork 5
EES-5962 On Release Slug changed - Event Handler Function #5750
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
...tion.ExploreEducationStatistics.Content.Search.FunctionApp/Services/EventGridEventHandler.cs
Outdated
Show resolved
Hide resolved
<ItemGroup> | ||
<Using Include="System.Threading.ExecutionContext" Alias="ExecutionContext" /> | ||
</ItemGroup> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the docs on functions running with the isolated worker model, we should add this. Just wondering what's the reason for removing it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
....Search.FunctionApp/Functions/RefreshSearchableDocument/RefreshSearchableDocumentFunction.cs
Outdated
Show resolved
Hide resolved
public required string PublicationSlug { get; init; } | ||
public required Guid ReleaseId {get;init;} | ||
public required string ReleaseSlug { get; init; } | ||
public required Guid ReleaseVersionId { get; init; } | ||
public required string BlobName { get; init; } | ||
public string? PublicationSlug { get; init; } | ||
public Guid? ReleaseId {get;init;} | ||
public string? ReleaseSlug { get; init; } | ||
public Guid? ReleaseVersionId { get; init; } | ||
public string? BlobName { get; init; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like these could stay as required and non optional, because I think the only place where we create one we are guaranteed to have all the values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These messages come from the queue so there's no guarantee of their contents. Better to code defensively and acknowledge that than to assume it will always be ok (== guarantee that it won't always be!)
...arch.FunctionApp/Functions/OnReleaseVersionPublished/Dtos/ReleaseVersionPublishedEventDto.cs
Outdated
Show resolved
Hide resolved
...ExploreEducationStatistics.Content.Search.FunctionApp/Clients/ContentApi/ContentApiClient.cs
Outdated
Show resolved
Hide resolved
…lls into the properties whether you like it or not) Added On Release Slug changed event handler function. Also made all properties for the dtos nullable since the serialiser will not respect the "required" keyword. Added event handler OnReleaseSlugChanged. Added Tests for all of the functions.
6d01098
to
f31e75f
Compare
For the final part of this ticket, added a new event handler function app for receiving and handling OnReleaseSlugChanged events.
Added On Release Slug changed event handler function.
Added event handler OnReleaseSlugChanged.
Made all properties for the dtos nullable since the serialiser will not respect the "required" keyword.
Added null guards where appropriate
Added Tests for all of the functions.