From b341b2ed5d98b95a7045c53049e7f030d406f25a Mon Sep 17 00:00:00 2001 From: Pit Friedrich Date: Tue, 28 Jul 2026 20:19:17 +0200 Subject: [PATCH] fix: pull dashboard widget data from a ChartDataService (#20) Chart and KPI numbers were literals in DashboardView, duplicated between the line and bar chart, and only existed at construction time. Add a thin data layer as the seam for later refresh/filter work: - ChartSeries / KpiData records, immutable, carrying translation keys rather than display text so the data layer stays free of a UI locale. - ChartDataService interface plus an in-memory implementation returning the previous hardcoded numbers, so nothing changes visually. - DashboardView injects the service, lays out the KPI tiles from its list (grid ids come from the data, so saved layouts still match), and resolves the keys against the bundle. Also fixes DashboardChartPlaywrightTest, red on main since the KPI tiles landed: it patched the first apex-chart, which is now a sparkline with no xaxis.categories, so the axis-chart patch was a structural change and legitimately redrew the SVG. It now targets an axis chart. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0124BiJikbhbiEsNfJxdWM69 --- .../com/example/data/ChartDataService.java | 25 ++++++ .../java/com/example/data/ChartSeries.java | 27 ++++++ .../data/InMemoryChartDataService.java | 51 +++++++++++ src/main/java/com/example/data/KpiData.java | 25 ++++++ .../java/com/example/views/DashboardView.java | 84 ++++++++++--------- .../data/InMemoryChartDataServiceTest.java | 73 ++++++++++++++++ .../e2e/DashboardChartPlaywrightTest.java | 12 ++- 7 files changed, 256 insertions(+), 41 deletions(-) create mode 100644 src/main/java/com/example/data/ChartDataService.java create mode 100644 src/main/java/com/example/data/ChartSeries.java create mode 100644 src/main/java/com/example/data/InMemoryChartDataService.java create mode 100644 src/main/java/com/example/data/KpiData.java create mode 100644 src/test/java/com/example/data/InMemoryChartDataServiceTest.java diff --git a/src/main/java/com/example/data/ChartDataService.java b/src/main/java/com/example/data/ChartDataService.java new file mode 100644 index 0000000..6e874d5 --- /dev/null +++ b/src/main/java/com/example/data/ChartDataService.java @@ -0,0 +1,25 @@ +package com.example.data; + +import java.util.List; + +/** + * The seam between the dashboard widgets and wherever their numbers come from. + * Views ask this service for data instead of holding literals, so a widget can + * be re-fed later (live refresh, a global filter, a per-widget refresh action) + * and so the data logic is testable without a Vaadin component tree. + *

+ * Deliberately thin: the current implementation + * ({@link InMemoryChartDataService}) is in-memory dummy data. A real backend + * would replace the implementation, not this interface. + */ +public interface ChartDataService { + + /** Revenue per month, for the line and bar charts. */ + ChartSeries revenueByMonth(); + + /** Revenue per sales region, for the pie chart. */ + ChartSeries revenueByRegion(); + + /** The KPI tiles, in the order the dashboard lays them out. */ + List kpis(); +} diff --git a/src/main/java/com/example/data/ChartSeries.java b/src/main/java/com/example/data/ChartSeries.java new file mode 100644 index 0000000..b29c3e1 --- /dev/null +++ b/src/main/java/com/example/data/ChartSeries.java @@ -0,0 +1,27 @@ +package com.example.data; + +import java.util.List; + +/** + * One named series of numbers plus the categories they are indexed by — the + * shape every chart on the dashboard is fed with, axis-based or not (a pie + * chart reads the categories as slice labels and ignores the series name). + *

+ * {@code nameKey} and {@code categoryKeys} are translation keys, not + * display text: the data layer must stay free of a {@code UI} and its locale, + * so resolving them against the bundle is the view's job. Values and keys are + * defensively copied, so a data set handed out by a + * {@link ChartDataService} cannot be modified by its consumer. + */ +public record ChartSeries(String nameKey, List values, List categoryKeys) { + + public ChartSeries { + values = List.copyOf(values); + categoryKeys = List.copyOf(categoryKeys); + if (values.size() != categoryKeys.size()) { + throw new IllegalArgumentException( + "each value needs a category: %d values, %d categories" + .formatted(values.size(), categoryKeys.size())); + } + } +} diff --git a/src/main/java/com/example/data/InMemoryChartDataService.java b/src/main/java/com/example/data/InMemoryChartDataService.java new file mode 100644 index 0000000..34f1703 --- /dev/null +++ b/src/main/java/com/example/data/InMemoryChartDataService.java @@ -0,0 +1,51 @@ +package com.example.data; + +import org.springframework.stereotype.Service; + +import java.util.List; + +/** + * Dummy implementation returning the hardcoded numbers the views used to carry + * inline. Everything is a constant, so each call returns the same data — the + * point of the service is the seam, not a persistence layer. + */ +@Service +public class InMemoryChartDataService implements ChartDataService { + + private static final List MONTHS = List.of( + "month.jan", "month.feb", "month.mar", "month.apr", "month.may", "month.jun"); + + private static final List REGIONS = List.of( + "region.north", "region.south", "region.east", "region.west"); + + private static final ChartSeries REVENUE_BY_MONTH = new ChartSeries( + "chart.revenueSeries", List.of(30.0, 40.0, 35.0, 50.0, 49.0, 60.0), MONTHS); + + private static final ChartSeries REVENUE_BY_REGION = new ChartSeries( + "chart.revenueSeries", List.of(30.0, 40.0, 35.0, 50.0), REGIONS); + + private static final List KPIS = List.of( + new KpiData("kpi-revenue", "kpi.revenueTotal", 12.4, + List.of(30.0, 40.0, 35.0, 50.0, 49.0, 60.0)), + new KpiData("kpi-orders", "kpi.openOrders", -3.1, + List.of(52.0, 47.0, 49.0, 44.0, 40.0, 38.0)), + new KpiData("kpi-customers", "kpi.newCustomers", 8.0, + List.of(74.0, 81.0, 79.0, 95.0, 104.0, 112.0)), + new KpiData("kpi-order-value", "kpi.averageOrderValue", 0.0, + List.of(480.0, 492.0, 478.0, 489.0, 483.0, 486.0))); + + @Override + public ChartSeries revenueByMonth() { + return REVENUE_BY_MONTH; + } + + @Override + public ChartSeries revenueByRegion() { + return REVENUE_BY_REGION; + } + + @Override + public List kpis() { + return KPIS; + } +} diff --git a/src/main/java/com/example/data/KpiData.java b/src/main/java/com/example/data/KpiData.java new file mode 100644 index 0000000..1c97ce4 --- /dev/null +++ b/src/main/java/com/example/data/KpiData.java @@ -0,0 +1,25 @@ +package com.example.data; + +import java.util.List; + +/** + * The numbers behind one KPI tile: the change versus the previous period and + * the trend the sparkline draws. + *

+ * {@code id} identifies the KPI itself (the dashboard reuses it as the grid + * item's {@code gs-id}, which must stay stable across reloads for a saved + * layout to match it again), while {@code labelKey} is a translation key — the + * displayed value is looked up as {@code labelKey + ".value"}, because it + * carries locale-specific formatting (decimal separator, currency, "Mio."). + */ +public record KpiData(String id, String labelKey, double deltaPercent, List trend) { + + public KpiData { + trend = List.copyOf(trend); + } + + /** The bundle key of the pre-formatted display value. */ + public String valueKey() { + return labelKey + ".value"; + } +} diff --git a/src/main/java/com/example/views/DashboardView.java b/src/main/java/com/example/views/DashboardView.java index 1a95826..0c1edf2 100644 --- a/src/main/java/com/example/views/DashboardView.java +++ b/src/main/java/com/example/views/DashboardView.java @@ -1,6 +1,7 @@ package com.example.views; import com.example.components.ApexChart; +import com.example.components.AxisChart; import com.example.components.BarChart; import com.example.components.Card; import com.example.components.Fa; @@ -9,6 +10,9 @@ import com.example.components.GridStackLayout; import com.example.components.KpiTile; import com.example.components.LineChart; import com.example.components.PieChart; +import com.example.data.ChartDataService; +import com.example.data.ChartSeries; +import com.example.data.KpiData; import com.vaadin.flow.component.Component; import com.vaadin.flow.component.button.Button; import com.vaadin.flow.component.button.ButtonVariant; @@ -27,17 +31,26 @@ import java.util.List; * layout is persisted per browser, plus controls to add widgets and reset the * layout at runtime. Widgets are removed by their own close button (see * {@link GridStackItem#setClosable(boolean)}), not from the toolbar. + *

+ * All widget numbers come from {@link ChartDataService}; this view only decides + * where a widget sits and resolves the data's translation keys against the + * bundle. */ @Route("") public class DashboardView extends VerticalLayout implements HasDynamicTitle { private static final String STORAGE_KEY = "dashboard"; + /** A KPI tile is a quarter row wide, so the n-th one starts at 3n. */ + private static final int KPI_WIDTH = 3; + + private final ChartDataService dataService; private final GridStackLayout grid = new GridStackLayout(); private final Span status = new Span(); private int extraWidgetCount; - public DashboardView() { + public DashboardView(ChartDataService dataService) { + this.dataService = dataService; addClassName("dialect-content"); grid.setWidthFull(); @@ -46,20 +59,15 @@ public class DashboardView extends VerticalLayout implements HasDynamicTitle { getTranslation("gridstack.status", e.getPositions().size()))); // KPI tiles first: the numbers a dashboard is read for, above the charts - // that explain them. They are 3x1 — a quarter row each, one cell high. - grid.add( - new GridStackItem("kpi-revenue", 0, 0, 3, 1, - kpiTile("kpi.revenueTotal", 12.4, - List.of(30.0, 40.0, 35.0, 50.0, 49.0, 60.0))), - new GridStackItem("kpi-orders", 3, 0, 3, 1, - kpiTile("kpi.openOrders", -3.1, - List.of(52.0, 47.0, 49.0, 44.0, 40.0, 38.0))), - new GridStackItem("kpi-customers", 6, 0, 3, 1, - kpiTile("kpi.newCustomers", 8.0, - List.of(74.0, 81.0, 79.0, 95.0, 104.0, 112.0))), - new GridStackItem("kpi-order-value", 9, 0, 3, 1, - kpiTile("kpi.averageOrderValue", 0.0, - List.of(480.0, 492.0, 478.0, 489.0, 483.0, 486.0)))); + // that explain them. They are 3x1 — a quarter row each, one cell high, + // laid out left to right in the order the service returns them. The + // grid id is the KPI's own id, so it survives reordering. + List kpis = dataService.kpis(); + for (int i = 0; i < kpis.size(); i++) { + KpiData kpi = kpis.get(i); + grid.add(new GridStackItem(kpi.id(), i * KPI_WIDTH, 0, KPI_WIDTH, 1, + kpiTile(kpi))); + } grid.add( new GridStackItem("revenue-trend", 0, 1, 6, 3, @@ -106,38 +114,37 @@ public class DashboardView extends VerticalLayout implements HasDynamicTitle { } /** The displayed value comes from the bundle alongside the label, since it - * carries locale-specific formatting (decimal separator, currency, "Mio."). - * Like the chart literals above, it moves to the data service in #20. */ - private KpiTile kpiTile(String labelKey, double delta, List trend) { - String label = getTranslation(labelKey); - return new KpiTile(label, getTranslation(labelKey + ".value")) - .setDelta(delta) - .setSparkline(label, trend); + * carries locale-specific formatting (decimal separator, currency, "Mio."); + * the service supplies the key, not the formatted text. */ + private KpiTile kpiTile(KpiData kpi) { + String label = getTranslation(kpi.labelKey()); + return new KpiTile(label, getTranslation(kpi.valueKey())) + .setDelta(kpi.deltaPercent()) + .setSparkline(label, kpi.trend()); } private Component lineChart() { - LineChart chart = new LineChart(); - chart.setData(getTranslation("chart.revenueSeries"), - List.of(30.0, 40.0, 35.0, 50.0, 49.0, 60.0), months()); - chart.addPointClickListener(e -> Notification.show(getTranslation( - "chart.pointClick", e.getSeriesIndex(), e.getDataPointIndex()))); - return sizeFull(chart); + return axisChart(new LineChart()); } private Component barChart() { - BarChart chart = new BarChart(); - chart.setData(getTranslation("chart.revenueSeries"), - List.of(30.0, 40.0, 35.0, 50.0, 49.0, 60.0), months()); + return axisChart(new BarChart()); + } + + private Component axisChart(AxisChart chart) { + ChartSeries series = dataService.revenueByMonth(); + chart.setData(getTranslation(series.nameKey()), series.values(), + translate(series.categoryKeys())); chart.addPointClickListener(e -> Notification.show(getTranslation( "chart.pointClick", e.getSeriesIndex(), e.getDataPointIndex()))); return sizeFull(chart); } private Component pieChart() { + ChartSeries series = dataService.revenueByRegion(); PieChart chart = new PieChart(); - chart.setData(List.of(30.0, 40.0, 35.0, 50.0), - List.of(getTranslation("region.north"), getTranslation("region.south"), - getTranslation("region.east"), getTranslation("region.west"))); + // A pie has no series name — its categories are the slice labels. + chart.setData(series.values(), translate(series.categoryKeys())); chart.addPointClickListener(e -> Notification.show( getTranslation("chart.sliceClick", e.getDataPointIndex()))); return sizeFull(chart); @@ -152,11 +159,10 @@ public class DashboardView extends VerticalLayout implements HasDynamicTitle { return chart; } - private List months() { - return List.of( - getTranslation("month.jan"), getTranslation("month.feb"), - getTranslation("month.mar"), getTranslation("month.apr"), - getTranslation("month.may"), getTranslation("month.jun")); + /** Data sets carry translation keys, not display text — see + * {@link ChartSeries}. */ + private List translate(List keys) { + return keys.stream().map(this::getTranslation).toList(); } @Override diff --git a/src/test/java/com/example/data/InMemoryChartDataServiceTest.java b/src/test/java/com/example/data/InMemoryChartDataServiceTest.java new file mode 100644 index 0000000..5ec95a3 --- /dev/null +++ b/src/test/java/com/example/data/InMemoryChartDataServiceTest.java @@ -0,0 +1,73 @@ +package com.example.data; + +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** The data layer is plain Java: no Spring context, no Vaadin UI. */ +class InMemoryChartDataServiceTest { + + private final ChartDataService service = new InMemoryChartDataService(); + + @Test + void revenueByMonth_hasOneValuePerMonth() { + ChartSeries series = service.revenueByMonth(); + + assertEquals(List.of(30.0, 40.0, 35.0, 50.0, 49.0, 60.0), series.values()); + assertEquals(List.of("month.jan", "month.feb", "month.mar", + "month.apr", "month.may", "month.jun"), series.categoryKeys()); + assertEquals("chart.revenueSeries", series.nameKey()); + } + + @Test + void revenueByRegion_hasOneValuePerRegion() { + ChartSeries series = service.revenueByRegion(); + + assertEquals(4, series.values().size()); + assertEquals(List.of("region.north", "region.south", + "region.east", "region.west"), series.categoryKeys()); + } + + @Test + void kpis_haveStableUniqueIdsAndATrend() { + List kpis = service.kpis(); + + assertEquals(4, kpis.size()); + Set ids = kpis.stream().map(KpiData::id).collect(Collectors.toSet()); + assertEquals(kpis.size(), ids.size(), "grid ids must be unique"); + assertTrue(ids.stream().allMatch(id -> id.startsWith("kpi-"))); + assertTrue(kpis.stream().allMatch(kpi -> !kpi.trend().isEmpty()), + "each tile draws a sparkline"); + } + + @Test + void kpi_derivesTheValueKeyFromTheLabelKey() { + KpiData revenue = service.kpis().getFirst(); + + assertEquals("kpi.revenueTotal", revenue.labelKey()); + assertEquals("kpi.revenueTotal.value", revenue.valueKey()); + assertEquals(12.4, revenue.deltaPercent()); + } + + @Test + void series_isImmutableAndBalanced() { + ChartSeries series = service.revenueByMonth(); + + assertThrows(UnsupportedOperationException.class, () -> series.values().add(1.0)); + assertThrows(IllegalArgumentException.class, + () -> new ChartSeries("x", List.of(1.0, 2.0), List.of("only.one"))); + } + + @Test + void callsAreRepeatable() { + assertEquals(service.revenueByMonth(), service.revenueByMonth()); + assertFalse(service.kpis().isEmpty()); + } +} diff --git a/src/test/java/com/example/e2e/DashboardChartPlaywrightTest.java b/src/test/java/com/example/e2e/DashboardChartPlaywrightTest.java index dc1af3a..c7641e2 100644 --- a/src/test/java/com/example/e2e/DashboardChartPlaywrightTest.java +++ b/src/test/java/com/example/e2e/DashboardChartPlaywrightTest.java @@ -26,7 +26,7 @@ class DashboardChartPlaywrightTest extends PlaywrightTestBase { @Test void dataUpdate_keepsTheRenderedSvgInPlace() { - Locator chart = page.locator("apex-chart").first(); + Locator chart = axisChart(); assertThat(chart.locator("svg").first()).isVisible(); // Tag the live SVG node: a full rebuild would replace it and drop the tag. @@ -43,7 +43,7 @@ class DashboardChartPlaywrightTest extends PlaywrightTestBase { @Test void dataUpdate_keepsTheThemeOverlay() { - Locator chart = page.locator("apex-chart").first(); + Locator chart = axisChart(); assertThat(chart.locator("svg").first()).isVisible(); chart.evaluate("(el, patch) => el.updateData(patch)", PATCH); @@ -55,4 +55,12 @@ class DashboardChartPlaywrightTest extends PlaywrightTestBase { assertTrue(String.valueOf(borderColor).startsWith("rgb"), "expected a resolved theme color, got: " + borderColor); } + + /** The first chart in the DOM is a KPI sparkline, which has no + * {@code xaxis.categories} — patching one with categories is a structural + * change and legitimately redraws. The patch above is an axis-chart patch, + * so it has to be applied to an axis chart. */ + private Locator axisChart() { + return page.locator("apex-chart:not(.dialect-sparkline)").first(); + } }