Skip to content

fix(storage-resize-images): publish the 1st gen context on onCompletion - #3279

Open
IzaakGough wants to merge 3 commits into
kitsfrom
fix/kits-resize-images-oncompletion-context
Open

IzaakGough wants to merge 3 commits into
kitsfrom
fix/kits-resize-images-oncompletion-context

Conversation

@IzaakGough

@IzaakGough IzaakGough commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #3266.

The kit's onCompletion event had no context.resource.name, so a subscriber couldn't tell which upload finished.

This publishes context in the extension's shape, including the object path.

  • No CloudEvent subject is added. The extension never set one, so this keeps strict parity for now.
  • eventId and timestamp match the extension's format, not its values. The extension read them from a separate delivery of the storage notification, which the kit doesn't receive.

Tested with unit tests built from a real storage event and the extension's matching onCompletion, and a live deploy.

The kit published a context built from the 2nd gen event, so resource.name was missing. It now rebuilds the context the extension published.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a helper function toEventContext to map 2nd gen StorageEvent data to the 1st gen EventContext format, ensuring compatibility for completion events. The implementation was updated in handlers.ts and corresponding tests were added and updated to verify the context reconstruction. I have included a suggestion to improve the robustness of the toEventContext function by using optional chaining and nullish coalescing to handle potentially malformed event data safely.

@IzaakGough
IzaakGough marked this pull request as ready for review October 26, 2026 11:39

@CorieW CorieW left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Problems

  • #3266 says to set subject, and this PR deliberately doesn't. A note on #3266 would stop the two contradicting each other.

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