Skip to content

UI workaround for the inconsistent formatting of listVirtualMachinesUsageHistory#10824

Merged
DaanHoogland merged 1 commit intoapache:4.19from
winterhazel:fix-metrics-formatting-ui
May 9, 2025
Merged

UI workaround for the inconsistent formatting of listVirtualMachinesUsageHistory#10824
DaanHoogland merged 1 commit intoapache:4.19from
winterhazel:fix-metrics-formatting-ui

Conversation

@winterhazel
Copy link
Copy Markdown
Member

@winterhazel winterhazel commented May 6, 2025 •
edited
Loading

Description

Depending on the Management Server's Locale, listVirtualMachinesUsageHistory may return decimal values separated by a comma in the cpuused field; however, the UI currently expects that this decimal is separated by a dot. In this situation, the metrics page will not show the CPU utilization graph properly (see #10672).

This PR makes the UI work around the issue by replacing commas with a dot in order to properly display the metrics page. The response formatting issue will be addressed by #10699 for the next major.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI
  • test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

Without the patch:

Screenshot from 2025-05-06 20-23-26

With the patch:

Screenshot from 2025-05-06 20-26-58

How Has This Been Tested?

  1. Initially, my Locale was set to English
  2. I left a VM running for some time, and left the MS collecting utilization metrics
  3. I accessed the VM's metrics page, and verified that the graphs were shown properly
  4. I changed my MS's Locale to Italian
  5. I accessed the VM's metrics page

Before the patch, the CPU utilization graph would not be displayed properly in step 4. After the patch, the graph is displayed properly.

@winterhazel winterhazel changed the base branch from main to 4.19 May 6, 2025 23:42
@codecov
Copy link
Copy Markdown

codecov bot commented May 6, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 16.40%. Comparing base (f13cf59) to head (f78daee).
Report is 26 commits behind head on 4.19.

Additional details and impacted files 
@@             Coverage Diff              @@
##               4.19   #10824      +/-   ##
============================================
+ Coverage     15.17%   16.40%   +1.23%     
- Complexity    11332    13589    +2257     
============================================
  Files          5415     5692     +277     
  Lines        474893   501976   +27083     
  Branches      57920    60795    +2875     
============================================
+ Hits          72046    82365   +10319     
- Misses       394792   410453   +15661     
- Partials       8055     9158    +1103
Flag Coverage Δ
uitests 4.00% <ø> (-0.29%) ⬇️
unittests 17.26% <ø> (+1.37%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@DaanHoogland DaanHoogland requested a review from Copilot May 7, 2025 06:31
@DaanHoogland DaanHoogland added this to the 4.19.3 milestone May 7, 2025
Copilot AI reviewed May 7, 2025
View reviewed changes
Copy link
Copy Markdown
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

This PR fixes the UI issue of incorrect CPU utilization graph display by addressing decimal formatting discrepancies in the response data. Key changes include:

  • Replacing commas with dots in the cpuused field value to ensure proper number formatting.
  • Updating the data extraction line in the StatsTab.vue component to support the required transformation.

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented May 7, 2025

@blueorangutan ui

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented May 7, 2025

@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented May 7, 2025

UI build: ✔️
Live QA URL: https://qa.cloudstack.cloud/simulator/pr/10824 (QA-JID-613)

DaanHoogland
DaanHoogland approved these changes May 7, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland left a comment •
edited
Loading

Choose a reason for hiding this comment

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

clgtm and the ui looks ok in Dutch and Italian. I am failing to reproduce the original error condition though (in four day old data) so more testing is needed?!

harikrishna-patnala
harikrishna-patnala approved these changes May 9, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@harikrishna-patnala harikrishna-patnala left a comment

Choose a reason for hiding this comment

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

code LGTM

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented May 9, 2025

I’m going to (half) trust the author as I could verify the end result but not reproduce the original issue and three reviews say the code is good.

@DaanHoogland DaanHoogland merged commit 751a0ad into apache:4.19 May 9, 2025
18 of 25 checks passed
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Jun 19, 2025
@winterhazel @dhslove
UI workaround for the inconsistent formatting of listVirtualMachinesU…
4f45e35 
…sageHistory (apache#10824)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@yadvr yadvr yadvr approved these changes

@DaanHoogland DaanHoogland DaanHoogland approved these changes

@harikrishna-patnala harikrishna-patnala harikrishna-patnala approved these changes

Assignees

No one assigned

Labels

component:UI

Projects

None yet

Milestone

4.19.3

Development

Successfully merging this pull request may close these issues.

6 participants

@winterhazel @DaanHoogland @blueorangutan @yadvr @harikrishna-patnala