[Bar Graph Plugin] Accepts types that it should not

[jvigliotta]

Summary

I had accidentally created a condition set inside of a bar graph. Tried creating some other types, but looks like only condition set is the issue.

Expected vs Current Behavior

Impact Check List

• Data loss or misrepresented data?
• Regression? Did this used to work or has it always been broken?
• Is there a workaround available?
• Does this impact a critical component?
• Is this just a visual bug?

Steps to Reproduce

1. Have a bar graph selected
2. Create Menu -> Condition Set
3. Oh noes!

[akhenry]

Bar charts are probably accepting objects that produce telemetry, which condition sets do, but without considering whether the telemetry is actually plottable in a bar chart (ie. does it produce numbers?).

[scottbell]

I can take this one

[scottbell]

We are checking range values:
https://github.com/nasa/openmct/blob/master/src/plugins/charts/BarGraphCompositionPolicy.js#L27

const rangeValues = metadata.valuesForHints(['range']);
return rangeValues.length > 0;

but this isn't enough clearly. Should I be checking for a formatString too within the rangeValues?

[akhenry]

Good question. I think we want to use similar logic to regular plots? Of course I'm assuming they are working correctly in this scenario :)

[scottbell]

Regular plots behave the same in that you can drop condition sets upon them. E.g.:

In fact, the composition policy is mostly just a copy/paste from OverlayPlots, except we loosened it a bit to only check the range for values so we can accept state generators (string/numbers).

[scottbell]

Though after talking about this at the tagup, should we trying to plot ConditionSets in BarGraphs, or forbidding them? One can plot them in Overlay plots:

Though in BarGraphs, they look rather screwy with the current value being assigned to the y-axis:

What do you think @shefalijoshi @akhenry @charlesh88?

[scottbell]

As I'm not sure there's a good way to look for numbers per se, and if we don't want to modify BarCharts to support enums, we could just block telemetry of the format that is equal to enum in the composition policy.

[scottbell]

Per discussion in tagup, I'm going to check if the metadata is an enum, and block objects in the composition policy that have it.

[scottbell]

Fixed in:
#4434

[jvigliotta]

Testathon 12/7/2021 - Verified Fixed