fix: pull dashboard widget data from a ChartDataService (#20)
CI / build-and-test (pull_request) Successful in 2m32s
CI / build-and-test (pull_request) Successful in 2m32s
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0124BiJikbhbiEsNfJxdWM69
This commit is contained in:
@@ -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.
|
||||
* <p>
|
||||
* 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<KpiData> kpis();
|
||||
}
|
||||
@@ -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).
|
||||
* <p>
|
||||
* {@code nameKey} and {@code categoryKeys} are <em>translation keys</em>, 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<Double> values, List<String> 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()));
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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<String> MONTHS = List.of(
|
||||
"month.jan", "month.feb", "month.mar", "month.apr", "month.may", "month.jun");
|
||||
|
||||
private static final List<String> 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<KpiData> 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<KpiData> kpis() {
|
||||
return KPIS;
|
||||
}
|
||||
}
|
||||
@@ -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.
|
||||
* <p>
|
||||
* {@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<Double> trend) {
|
||||
|
||||
public KpiData {
|
||||
trend = List.copyOf(trend);
|
||||
}
|
||||
|
||||
/** The bundle key of the pre-formatted display value. */
|
||||
public String valueKey() {
|
||||
return labelKey + ".value";
|
||||
}
|
||||
}
|
||||
@@ -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.
|
||||
* <p>
|
||||
* 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<KpiData> 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<Double> 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<String> 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<String> translate(List<String> keys) {
|
||||
return keys.stream().map(this::getTranslation).toList();
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
@@ -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<KpiData> kpis = service.kpis();
|
||||
|
||||
assertEquals(4, kpis.size());
|
||||
Set<String> 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());
|
||||
}
|
||||
}
|
||||
@@ -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();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user