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

fix: use scoped variables in the query builder #414

Merged
merged 4 commits into from
May 24, 2023

Conversation

maartenofzo
Copy link
Contributor

When using repeated variables via the query builder, all multi-value variables were sent from each panel instead of using the scoped variables. This is the simple fix for that.

@simPod
Copy link
Owner

simPod commented Apr 26, 2023

Can you please elaborate on what problem does this solve? Add a test so I can fully understand the change.

@maartenofzo
Copy link
Contributor Author

Sure, this change makes sure that if you have repeating panels via multi-value variable that it uses the variable scope instead of sending all variables through json.

Example how it sends a scoped multi-value now:

{
  "targets": [
    {
      "datasource": { "type": "simpod-json-datasource", "uid": "qsUNIjEVz" },
      "editorMode": "builder",
      "payload": {
        "value1": "foo",
        "value2": "bar",
        "multi-value": "(foo|bar)",
      },
      "refId": "A",
      "target": "opsdata"
    }
  ],
  "scopedVars": {
    "value1": { "selected": false, "text": "foo", "value": "foo" },
    "value2": { "selected": false, "text": "bar", "value": "bar" },
    "multi-value": { "text": "bar", "value": "bar", "selected": true },
    "__interval": { "text": "500ms", "value": "500ms" },
    "__interval_ms": { "text": "500", "value": 500 }
  },
}

As you can see it sends (foo|bar) as if the multi-value was never scoped at all. In the scopedVars parameter you can see that the multi-value field actual has a scope where the value is only bar.

This code changes the payload so it only sends the scoped variable so it will look like this:

{
  "targets": [
    {
      "datasource": { "type": "simpod-json-datasource", "uid": "qsUNIjEVz" },
      "editorMode": "builder",
      "payload": {
        "value1": "foo",
        "value2": "bar",
        "multi-value": "bar",
      },
      "refId": "A",
      "target": "opsdata"
    }
  ],
  "scopedVars": {
    "value1": { "selected": false, "text": "foo", "value": "foo" },
    "value2": { "selected": false, "text": "bar", "value": "bar" },
    "multi-value": { "text": "bar", "value": "bar", "selected": true },
    "__interval": { "text": "500ms", "value": "500ms" },
    "__interval_ms": { "text": "500", "value": 500 }
  },
}

If no repeat setting is on, the variables will not be scoped, because scopedVars in that case is "undefined" already.

@simPod
Copy link
Owner

simPod commented May 3, 2023

Overall, it makes sense.

But we need to adapt tests so it's not broken later. There's TemplateSrvStub you can use I think https://github.com/simPod/GrafanaJsonDatasource/blob/66da5e8b7365e959266c51d8536441d8fa8be93f/spec/lib/TemplateSrvStub.ts

I'll test this today on my G install.

@maartenofzo
Copy link
Contributor Author

Wrote a test for it, used original variable types that Grafana uses. Let me know if this is alright.

Copy link
Owner

@simPod simPod left a comment

Choose a reason for hiding this comment

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

Overall LGTM, the test documents the change sufficiently

spec/lib/TemplateSrvStub.ts Outdated Show resolved Hide resolved
spec/lib/TemplateSrvStub.ts Outdated Show resolved Hide resolved
spec/lib/TemplateSrvStub.ts Outdated Show resolved Hide resolved
@maartenofzo
Copy link
Contributor Author

Updated based on your comments.

Copy link
Owner

@simPod simPod left a comment

Choose a reason for hiding this comment

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

LGTM, let's try it. Thanks

@simPod simPod merged commit 4489e33 into simPod:0.6.x May 24, 2023
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