Skip to content

fix: Preview payload when correlating numeric properties#671

Merged
e-fisher merged 1 commit intomainfrom
fix/correlate-numeric-values
Apr 17, 2025
Merged

fix: Preview payload when correlating numeric properties#671
e-fisher merged 1 commit intomainfrom
fix/correlate-numeric-values

Conversation

@e-fisher
Copy link
Collaborator

Description

Payload preview doesn't work when a numeric value is replaced by a correlation variable, for example:

{ "id": ${correlation_vars['correlation_0'] }

In Payload preview, we parse the string as JSON, but the injected variable makes in invalid.

To solve this I've added a function to wrap variable expressions in quotes, that's not ideal because it makes it look like the value injected would be a string, but at least it doesn't break the preview. We might want to revisit this in the future and remove JSON parse altogether, but we'd need a new way to prettify it.

How to Test

Use attached recording and generator.

Steps to reproduce:

  1. In the generator, inspect POST /api/ratings request
  2. Click on payload.

Expected result: payload preview with injected correlation value is displayed
Actual result: payload preview is not available

Verify payload preview works as expected in recording, generator and validator

correlate-pizza-id.zip

Checklist

  • I have performed a self-review of my code.
  • I have added tests for my changes.
  • I have run linter locally (npm run lint) and all checks pass.
  • I have run tests locally (npm test) and all tests pass.
  • I have commented on my code, particularly in hard-to-understand areas.

Screenshots (if appropriate):

screencapture 2025-04-17 at 12 59 12

Related PR(s)/Issue(s)

Copilot AI review requested due to automatic review settings April 17, 2025 10:10
@e-fisher e-fisher requested a review from a team as a code owner April 17, 2025 10:10
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

The PR addresses an issue where payload preview fails when a numeric value is replaced by a correlation variable by wrapping variable expressions in quotes before JSON parsing.

  • Updated parseParams to wrap content with variable expressions in quotes before parsing
  • Added a new function (wrapTemplateExpressionsInQuotes) to handle the string transformation
Comments suppressed due to low confidence (2)

src/components/WebLogView/RequestDetails/utils.ts:74

  • Consider adding unit tests for the wrapTemplateExpressionsInQuotes function to validate the regex against various edge cases.
function wrapTemplateExpressionsInQuotes(str: string) {

src/components/WebLogView/RequestDetails/utils.ts:40

  • Wrapping variable expressions in quotes may lead to type inconsistencies, especially if numeric types are expected downstream. Verify the transformation with comprehensive tests to ensure that all intended cases are handled correctly.
parsePythonByteString(wrapTemplateExpressionsInQuotes(contentDecoded))

Copy link
Collaborator

@Llandy3d Llandy3d left a comment

Choose a reason for hiding this comment

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

Works well 🙌

Copy link
Collaborator

@going-confetti going-confetti left a comment

Choose a reason for hiding this comment

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

Works as expected and remedies the issue 👍

that's not ideal because it makes it look like the value injected would be a string, but at least it doesn't break the preview. We might want to revisit this in the future and remove JSON parse altogether, but we'd need a new way to prettify it.

I agree, it looks pretty confusing. I think we should remove JSON parse in future. Could you please create an issue for that?

@e-fisher e-fisher merged commit ae4c428 into main Apr 17, 2025
3 checks passed
@e-fisher e-fisher deleted the fix/correlate-numeric-values branch April 17, 2025 12:41
This was referenced Apr 24, 2025
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.

3 participants