Re: [PR] fix(mixed-chart): tooltip displaying numbers incorrectly with secondary y-axis format [superset]

[via GitHub]

korbit-ai[bot] commented on code in PR #34298:
URL: https://github.com/apache/superset/pull/34298#discussion_r2227713823


##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -648,36 +648,40 @@ export default function transformProps(
           .filter(key => keys.includes(key))
           .forEach(key => {
             const value = forecastValues[key];
-            // if there are no dimensions, key is a verbose name of a metric,
-            // otherwise it is a comma separated string where the first part 
is metric name
+
+            const isQueryB = key.includes('(Query B)');

Review Comment:
   ### Fragile Query B Detection Logic <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The logic for determining whether a series belongs to Query B is fragile as 
it relies on a string pattern check '(Query B)' in the key name.
   
   
   ###### Why this matters
   If the naming convention for the series changes or if there are 
translations, the tooltip formatting will break and display incorrect number 
formats.
   
   ###### Suggested change ∙ *Feature Preview*
   Use the pre-existing primarySeries/secondarySeries Sets to determine which 
query a series belongs to:
   ```typescript
   const isQueryB = secondarySeries.has(key);
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d5dd34a4-2a6f-433d-b283-d2922e7c4025/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d5dd34a4-2a6f-433d-b283-d2922e7c4025?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d5dd34a4-2a6f-433d-b283-d2922e7c4025?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d5dd34a4-2a6f-433d-b283-d2922e7c4025?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d5dd34a4-2a6f-433d-b283-d2922e7c4025)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:23ebd44c-293b-498c-b5a6-c165ffb6f2c6 -->
   
   
   [](23ebd44c-293b-498c-b5a6-c165ffb6f2c6)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org