Skip to content
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

INN-2911: Fix empty event payloads in UI due to resolver bug #1258

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

djfarrelly
Copy link
Member

Description

The change introduced in #1238 changed how IDs are used (diff) within the runner service. This invalidated the previous fix in #1201.

This issue happens as sometimes two different event ids are created for a single event which is another, separate issue. This is reproducible by using the Invoke or Sent event button via the UI, but doesn't happen when sending an event via curl. The fix in this PR makes this issue separate and not a problem.

Tested:

  • Invoking via UI
  • Send event via UI
  • Send event via UI w/ id
  • Send event via curl
  • Send event via curl w/ id

Motivation

This looks like there was a valid bug fix with #1201 then #1238 caused a regression due to a change in how IDs are handled.

Type of change (choose one)

  • Chore (refactors, upgrades, etc.)
  • Bug fix (non-breaking change that fixes an issue)
  • Security fix (non-breaking change that fixes a potential vulnerability)
  • Docs
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality not to work as expected)

Checklist

  • I've linked any associated issues to this PR.
  • I've tested my own changes.

Check our Pull Request Guidelines

Tested:
* Invoking via UI
* Send event via UI
* Send event via UI w/ id
* Send event via curl
* Send event via curl w/ id

This looks like there was a valid bug fix with #1201 then #1238 caused a regression due to a change in how IDs are handled.
Copy link

linear bot commented Apr 3, 2024

Copy link

vercel bot commented Apr 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated (UTC)
ui ⬜️ Ignored (Inspect) Apr 3, 2024 9:11pm

Comment on lines -93 to -95
// Runner.Events maps events by an event ID that could be either the
// internal or external ID. It'll be the internal ID if the event didn't
// have an external ID
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

@darwin67 darwin67 left a comment

Choose a reason for hiding this comment

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

confirmed locally.

@djfarrelly djfarrelly merged commit 51ec78b into main Apr 3, 2024
12 checks passed
@djfarrelly djfarrelly deleted the fix/use-internal-event branch April 3, 2024 21:22
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.

None yet

2 participants