Group old versions in chart and improve colors for readability

[Gared]

No description provided.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Group old versions and enhance chart colors for readability

✨ Enhancement

Walkthroughs

Description

• Group old versions into single aggregated dataset labeled "X.x"
• Implement dynamic color scheme based on major/minor version numbers
• Add explicit colors for "Instances added" and "Instances removed" bars
• Improve chart layout with fixed height container and responsive sizing
• Update card title from "Etherpad version count" to "Etherpad versions"

Diagram

flowchart LR
  A["All Datasets"] -->|"Slice first N-8"| B["Old Versions"]
  B -->|"Group by Major.x"| C["Aggregated Dataset"]
  A -->|"Slice last 8"| D["Recent Versions"]
  C --> E["Apply Dynamic Colors"]
  D --> E
  E -->|"Add Bar Datasets"| F["Final Chart"]
  F --> G["Render with Fixed Height"]

Loading

File Changes

1. src/pages/instancesByVersion.tsx ✨ Enhancement +74/-16

Version grouping and dynamic color scheme implementation

• Added showVersions constant set to 8 to control number of displayed recent versions
• Implemented getColorForVersion() function that generates HSL colors based on major/minor version
 numbers
• Refactored dataset processing to group versions older than the last 8 into aggregated "X.x"
 datasets
• Added explicit background colors for "Instances added" (green) and "Instances removed" (red) bar
 datasets
• Wrapped Line chart in a fixed-height container div with 80vh height and disabled aspect ratio
 maintenance
• Updated card title from "Etherpad version count" to "Etherpad versions"
• Set first month's added instances to zero for data consistency

src/pages/instancesByVersion.tsx

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Null dataset pushed ✓ Resolved 🐞 Bug ≡ Correctness

Description

When allDatasets.length <= showVersions, the grouping loop never runs and tmpDataset stays
null, but it is still pushed into datasets, causing a runtime crash in datasets.forEach and
invalid Chart.js dataset data.

Code

src/pages/instancesByVersion.tsx[R91-121]

+        let datasets: ChartDataset[] = []
+        let tmpDataset: null|ChartDataset = null
+
+        allDatasets.slice(0, -showVersions).forEach((dataset: ChartDataset) => {
+            const label = dataset.label?.substring(0, 1) + '.x'
+            if (tmpDataset === null || tmpDataset.label !== label) {
+                if (tmpDataset !== null) {
+                    datasets.push(tmpDataset)
+                }
+                tmpDataset = dataset
+
+                tmpDataset.label = label
+                tmpDataset.hidden = true
+                return
+            }
+            if (tmpDataset.label === label) {
+                dataset.data.forEach((element, index) => {
+                    tmpDataset.data[index] = tmpDataset.data[index] + element
+                })
+            }
+        })
+
+        datasets.push(tmpDataset)
+        datasets.push(...allDatasets.slice(-showVersions))
+
+        datasets.forEach((dataset) => {
+            const color = getColorForVersion(dataset.label)
+
+            dataset.borderColor = color;
+            dataset.backgroundColor = color;
+        })

Evidence

tmpDataset is declared nullable and only assigned inside the forEach over `allDatasets.slice(0,
-showVersions). When that slice is empty, tmpDataset stays null`, but is still pushed and later
iterated, where dataset.label is accessed, which will throw on null.

src/pages/instancesByVersion.tsx[91-121]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`tmpDataset` is initialized as `null` and is unconditionally pushed to `datasets`. If `allDatasets.slice(0, -showVersions)` is empty (i.e., when there are `<= showVersions` versions), `tmpDataset` remains `null`, leading to a `TypeError` during `datasets.forEach` and malformed chart datasets.
## Issue Context
This code runs inside `useMemo` during chart data construction.
## Fix Focus Areas
- src/pages/instancesByVersion.tsx[91-121]
### Suggested fix
- Handle the short-list case explicitly:
- If `allDatasets.length <= showVersions`, skip grouping and set `datasets = [...allDatasets]`.
- Otherwise, only `datasets.push(tmpDataset)` when `tmpDataset !== null`.
- Optionally, avoid mutating the original dataset object when using it as an accumulator (clone `data` array) to reduce side effects.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Implicit any breaks build ✓ Resolved 🐞 Bug ≡ Correctness

Description

getColorForVersion(versionString) declares versionString without a type, which fails TypeScript
compilation under strict/noImplicitAny settings.

Code

src/pages/instancesByVersion.tsx[R73-76]

+        function getColorForVersion(versionString) {
+            // extract major and minor version
+            const [major, minor] = versionString.split('.').map(Number);
+

Evidence

The code defines getColorForVersion with an untyped parameter, and the repo TypeScript config
enables strict mode, which includes noImplicitAny, making this a build-time error.

src/pages/instancesByVersion.tsx[73-89]
tsconfig.json[22-26]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`getColorForVersion` has an untyped parameter (`versionString`), which is a compile error under TypeScript `strict` mode (`noImplicitAny`).
## Issue Context
The project uses `"strict": true` in `tsconfig.json`.
## Fix Focus Areas
- src/pages/instancesByVersion.tsx[73-89]
- tsconfig.json[22-26]
### Suggested fix
- Add an explicit type: `function getColorForVersion(versionString: string | undefined) { ... }`.
- If you choose `string`, update call sites to pass a string (e.g., `getColorForVersion(dataset.label ?? '')`) and ensure undefined labels are handled safely.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Bad major version grouping ✓ Resolved 🐞 Bug ≡ Correctness

Description

Old-version grouping uses substring(0, 1) to derive the major version, which mis-groups
multi-digit major versions (e.g., 10.x becomes 1.x) and produces incorrect aggregated series.

Code

src/pages/instancesByVersion.tsx[R94-105]

+        allDatasets.slice(0, -showVersions).forEach((dataset: ChartDataset) => {
+            const label = dataset.label?.substring(0, 1) + '.x'
+            if (tmpDataset === null || tmpDataset.label !== label) {
+                if (tmpDataset !== null) {
+                    datasets.push(tmpDataset)
+                }
+                tmpDataset = dataset
+
+                tmpDataset.label = label
+                tmpDataset.hidden = true
+                return
+            }

Evidence

The grouping label is explicitly built from the first character of the version string, so any major
version with more than one digit will be collapsed into the wrong group (e.g., '10.*' and '11.*'
both become '1.x').

src/pages/instancesByVersion.tsx[94-105]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The grouping key for aggregated old versions is computed using `dataset.label?.substring(0, 1) + '.x'`, which only takes the first character. This misrepresents versions with multi-digit majors.
## Issue Context
`dataset.label` is a version string (e.g., `value[0].version`) and is later used both for grouping and coloring.
## Fix Focus Areas
- src/pages/instancesByVersion.tsx[94-105]
### Suggested fix
- Derive the major version using parsing instead of first character, e.g.:
- `const major = String(dataset.label ?? '').split('.')[0]`
- `const label = `${major}.x``
- (Optional) Validate `major` is numeric and fall back to a safe bucket if not.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---