fix: Analytics - Excel date export and period picker review EXO-88863

[srenault-meeds]

Follow-up fixes on the Threshold-on-aggregate delivery (#422 / #448), from review of
commit 763a51c and from reviewer feedback on the resulting UI.

Excel export

• Date buckets and date columns are written as real date cells instead of the
  localized label or the raw epoch. "1 sept. 2026" in a text cell is a
  picture of a date: it sorts lexicographically, cannot be filtered by period
  and no cell formatting recovers it; a raw 1.75941E+12 is worse. Any column
  the table renders as a date (its dataType, the same signal
  AnalyticsTableCellValue uses) is now exported as a date, in the time zone
  the buckets were aligned on.
• Quarter, ISO-week and hour-of-day keep their textual label on purpose: no
  faithful spreadsheet format token exists for them, and the hour bucket key
  is an hour of day, not an instant.
• A row with no value no longer exports the literal string null.
• The response declared application/vnd.ms-excel for an OOXML .xlsx, which
  is what made Excel open the download with a "file format and extension don't
  match" warning. Corrected.

Period picker

• One row per bound (From / To), each with an editable date and hour, on a
  grid so both rows align. Laid out on a single centered line the six items
  overflowed the menu's fixed 420px and the leading label was clipped.
• The samples drawer used four bare date/time inputs. It now uses the same
  picker as the chart header, with a Select period header and a separator
  before the samples list.
• The panel is extracted (AnalyticsPeriodPickerPanel) so the chart's
  activator+menu and the drawer share one definition instead of a second copy.
  The chart's period-shortcut arithmetic moves to $analyticsUtils.

Review finding

loadAll(), waitUntilLoaded() and getDisplayedGrid() were added to
AnalyticsTable.vue by 763a51c and called by nobody: remains of a
DOM-scraping client export superseded by the server-side one, in the Table
portlet that commit's own description puts out of scope. Removed.

Build

analytics-webapps had no test directory, so its JaCoCo check passed
vacuously. Adding the first test turns the gate on, and the parent POM
defaults exo.test.coverage.ratio to 1.0 — the module now sets it to 0 like
analytics-api and analytics-services. Reproduced with CI's own profile
before and after.

Tests

AnalyticsExcelDateCellTest reads the workbook back through POI rather than
asserting on what was passed in, and was mutation-verified: reverting the fix
fails it on the cell type. mvn -P coverage verify is green (11 tests),
eslint clean.

There is no JS test harness in this repo, so the picker changes are verified
by hand only, on a local instance.

Knowledge: to be added before merge — the corpus PR is drafted but not yet
opened (dev-lifecycle §3b step 5).

Classification: N2 — touches commonAnalyticsVueComponents, a shared
module content, gamification and notes each declare a <depends> on,
and the public AbstractAnalyticsPortlet. Its approver must be an
Architect/Senior Developer who knows it is N2, not an approval on AI review
alone.

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate failed

Failed conditions
50.0% Coverage on New Code (required ≥ 60%)

See analysis details on SonarQube Cloud

[ahamdi]

AI review — Round #1

Reviewed at head 03a1d0e0, against the code at that commit — not against develop, since this branch's whole server-side export (a6bdc3dc, #420) is not on develop yet. Six findings inline, most severe first.

The Excel work is the sound part of this PR. The gap is that "which columns hold dates" is answered in three different places, and one of the three — the space-field branch — was never wired up at all.

Verified conform

• The date-cell mechanism. timestamp is mapped "format": "epoch_millis" (analytics-es-template.json:23), so the key_as_string reaching writeDateCell really is epoch millis and Long.parseLong holds. setCellValue(LocalDateTime) is the right overload for honouring filter.zoneId(), and the style cache is keyed by format string and scoped per workbook — no cross-workbook leak.
• The hour exclusion is correct, and for exactly the reason the javadoc gives. I set out to report the opposite and was wrong. hour + DATE never reaches appendIntervalTypeQuery: ElasticsearchAnalyticsService:779-801 intercepts it and emits a terms over a painless getHour() script with "size": 24, and AnalyticsAggregation.getLabel:166-169 formats the key as "%02d:00". The bucket key really is 0-23. Adding hour to EXCEL_DATE_FORMATS would put 1 January 1970 in every hourly export.
• Chart row alignment after the getLabels() → getAggregationLabels() switch is exact. ChartDataList.getLabels() is itself a map over the same LinkedHashSet, so the new list is index-for-index identical to the one the old code iterated; the label-only @EqualsAndHashCode already governed both, and the retained instance carries the raw bucket key for that label. writeCategoryCell's size() == 1 guard correctly leaves a composite multi-x-axis label as text.
• Content type. application/vnd.openxmlformats-officedocument.spreadsheetml.sheet is right for the XSSFWorkbook both exports write.
• retrieveMapping(false) reads the in-memory cache refreshed on a 2-minute schedule — one HashSet copy per export, not an ES round-trip per row.
• i18n. analytics.from / analytics.toDate have exactly six consumers at head, all inside the new panel, so from→From and du→Du regress nothing. The {0: …, 1: …} object form for analytics.period.range matches the call style already used in AnalyticsTableCellValue.vue.
• The less rework keeps all three portlet headers' width budget — the > combinator matches in the chart, rate and table portlets alike; the samples drawer deliberately matches neither (but see the drawer-styling finding, which is a different rule).
• The Vue lifecycle is sound. initPanel's $nextTick resolves after Vuetify's bootable renders the content (isActive flips synchronously in the activator's own handler, ahead of @click), so $refs.panel is populated on first open; two mounted instances do not interfere (handleOutsideClick returns on !this.menu); beforeDestroy removes the listener with the same capture flag; and the extracted $refs.activator.$el || $refs.activator fixes a real pre-existing bug — the old version used .$el only, which is undefined for the non-compact <button>.
• ViewSamplesDrawer still filters correctly. The emitted {period, min, max} is a superset of the old shape, the parent's selectedPeriod prop is never mutated, and attach on the drawer's picker is honoured (Vuetify's initDetach short-circuits on attach === true).
• exo.test.coverage.ratio=0 matches analytics-api and analytics-services (both at pom line 31), and the comment's claim is true — the parent pom sets the jacoco minimum to 1.0.
• The dead-code removal in AnalyticsTable.vue is genuinely dead — loadAll, waitUntilLoaded and getDisplayedGrid have no callers at head.

What the PR does well

The date-cell decisions are argued from the reader's spreadsheet rather than from the code, and each exclusion is justified by a property of the format or the bucket key rather than by convenience — the hour one turning out correct for a non-obvious reason three layers away in the ES query builder. Both test classes read the workbook back through POI (getLocalDateTimeCellValue, DateUtil.isCellDateFormatted, getCellType) instead of asserting on inputs, which is the only way this class of defect is catchable; testBucketIsWrittenInTheQueriedTimeZone with Honolulu is a genuinely discriminating case, and the style-cache tests pin a real ceiling rather than a tautology.

---

Classification: N2, as the author proposed, and I would not lower it. The diff touches commonAnalyticsVueComponents — a shared module content, gamification and notes each declare a <depends> on — and the public AbstractAnalyticsPortlet. No socle trigger fires: no ACL, no Liquibase or JPA, no Kernel↔Spring bridge, no MCP tool, no new trust boundary. Per ai-review-and-merge.md §5 its approver must be an Architect/Senior Developer who knows it is N2, not an approval on AI review alone.

Two scope calls are left to a human, not to this review: whether the space-field and createdDate export gaps belong to this PR or a follow-up, and whether the two surviving inline picker copies block merge.

🤖 Generated with Claude Code

[ahamdi]

🟠 High — every space-field column exports the space display name, whatever field was chosen

} else if (StringUtils.isNotBlank(columnFilter.getSpaceField())) {
  cell.setCellValue(spaceFieldValue(rowSpace, columnFilter.getSpaceField()));  // never reaches writeValue
  return;
}

private String spaceFieldValue(Space space, String field) {
  if (space == null) { return ""; }
  return switch (field) {
  case "description" -> space.getDescription();
  case "groupId"     -> space.getGroupId();
  case "prettyName"  -> space.getPrettyName();
  case "shortName"   -> space.getShortName();
  case "url"         -> space.getUrl();
  default            -> space.getDisplayName();   // <- every field the UI offers lands here
  };
}

The settings UI offers exactly seven space fields — createdTime, managersCount, membersCount, redactorsCount, template, subscription, visibility (AnalyticsTableApplication.vue:315-352) — and not one of them is a case in that switch, so all seven export the space name. AnalyticsUtils.convertFieldName only rewrites to _alt/.keyword variants, so it cannot rescue them either. This is also the one branch that bypasses the new writeValue, discarding the dateColumn flag already computed one line above — a createdTime column carries dataType == "date", exactly what this PR set out to export as a real date.

Pre-existing (introduced by a6bdc3dc, #420) and untouched by this PR — but a6bdc3dc is not on develop, so the whole server-side export is new code that reaches develop with this feature branch, which makes this the moment to fix it.

Fix: route the branch through writeValue(cell, spaceFieldValue(...), dateColumn, formatting) and give spaceFieldValue a case per offered field: createdTime -> String.valueOf(space.getCreatedTime()), visibility -> space.getVisibility(), subscription -> space.getRegistration(), template -> String.valueOf(space.getTemplateId()), and the three counts from space.getManagers()/getMembers()/getRedactors() .length (there is no getMembersCount() on Space — the REST SpaceEntity has one, the core model does not). Keep default -> "" rather than the display name, so the next unmapped field is visibly empty instead of silently wrong.

[ahamdi]

🟡 Medium — an integer-valued aggregation over a date field now exports as 1 January 1970

boolean isDateColumn(AnalyticsTableColumnFilter columnFilter, ExportFormatting formatting) {
  if (StringUtils.equalsIgnoreCase(columnFilter.getDataType(), "date")) {
    return true;                                     // no look at the aggregation type
  }
  AnalyticsTableColumnAggregation valueAggregation = columnFilter.getValueAggregation();
  return valueAggregation != null
      && valueAggregation.getAggregation() != null
      && formatting.isDateField(valueAggregation.getAggregation().getField());   // second trigger
}

isDateColumn asks which field a column aggregates, never how. A CARDINALITY or COUNT over a date field is a date column by both triggers and its value is a count — so writeValue hands "28" to writeTimestampCell, which reads it as an instant and writes 28 ms after the epoch. Before this PR the same cell exported the number 28.

Reachable two ways: AnalyticsTableColumnAggregationSetting.vue offers CARDINALITY with a field selector, and FieldSelection.vue:142 (!this.numeric || field.numeric || field.date) lets date fields through for every aggregation type. For COUNT the selector is only v-shown away, so a column switched from MAX(timestamp) to COUNT keeps aggregation.field = "timestamp", and AnalyticsTableColumnSetting.vue:290-295 then takes dataType from the field mapping — the dataType = 'long' shortcut fires only when the field is empty.

Scoped to those two: ES serialises sum/avg/min/max as floating-point literals (8.0, 3.51882E12), which Long.parseLong rejects, so they still fall through to the numeric path. cardinality and value_count are serialised as integral longs, which is why only they trip.

Nothing pins it: AnalyticsTableExportCellTest.java:86-98 hardcodes AnalyticsAggregationType.MAX in its column(...) helper, and the one small-integer case (line 151) passes dateColumn = false.

Fix: require an aggregation whose result is an instant — return false from isDateColumn for CARDINALITY, COUNT and TERMS whatever the field's mapping — and add the test: a CARDINALITY-over-lastLoginTime column whose "28" must come back as the number 28. The live table has the same defect via dataType === 'date' in AnalyticsTableCellValue.vue, so it is worth fixing on both sides.

[ahamdi]

🟡 Medium — the only user-field column the UI offers exports an empty cell

Object property = rowIdentity == null || rowIdentity.getProfile() == null ? null
                                                                         : rowIdentity.getProfile()
                                                                                      .getProperty(columnFilter.getUserField());
writeValue(cell, property == null ? null : String.valueOf(property), dateColumn, formatting);

AnalyticsTableApplication.vue:307-314 offers exactly one user field, {name: 'createdDate', type: 'date'} — and createdDate is never a Profile property. Profile.getProperty(name) is a plain properties.get(name); the creation instant lives in the separate createdTime field with its own getter, and the only createdDate key in social belongs to the REST DTO (ProfileEntity.CREATED_DATE, fed by EntityBuilder from user.getCreatedDate().getTime()). That REST map is what AnalyticsTableCell.vue:157-158 reads client-side, which is why the live table shows a date and the export does not.

To be clear, the writeValue routing this PR introduces is not a regression — it is an improvement: the cell held the literal string "null" before and holds "" now. But it also makes the gap silent, and an empty column is indistinguishable from "this user has no value".

Fix (hypothesis to confirm): special-case createdDate to rowIdentity.getProfile().getCreatedTime() before falling back to getProperty(...), so the export matches what the table draws. If it is deferred, say so — otherwise the next reader will assume the column works.

[ahamdi]

🟡 Medium — the extraction is one-third done, and the two surviving copies do not get this PR's fix

The PR extracts the picker and moves the shortcut arithmetic to $analyticsUtils, then migrates only the generic chart portlet. The Table and Rate portlets keep the whole widget inline — v-menu + activator pair + v-date-picker + shortcut buttons, plus periodRangeLabel, initCompactPeriodForm, onCompactDatesInput, selectCompactPeriodItem, applyCompactPeriod, handlePeriodSelectorOutsideClick, the document.addEventListener('click', …, true) / removeEventListener pair, and a byte-identical private computePeriodDateRange:

• table-portlet/components/AnalyticsTableApplication.vue:26-90, 540-641
• rate-portlet/components/AnalyticsRateApplication.vue:51-118, 541-642

That is not only duplication: the half-picked-range fix exists in the new panel alone. AnalyticsPeriodPickerPanel.onDatesInput skips apply() for a one-element array, while AnalyticsTableApplication.vue:546-549 and AnalyticsRateApplication.vue:547-550 still call applyCompactPeriod() unconditionally — so the exact defect the PR body describes ("the second click never landed and no range could be picked") stays live in two of the three portlets. AnalyticsRateApplication.vue:247-248 also keeps compactFromTime/compactToTime in data with no markup bound to them.

frontend-vue.md (Centralized skin) treats a component duplicating a catalogued shared one as a finding naming the module that should have been used — here analytics-period-picker, now registered in commonAnalyticsVueComponents, which both portlets already depend on.

Fix: replace both inline copies with <analytics-period-picker :period="selectedPeriod" :compact="periodSelectorCompact" @change="selectedPeriod = $event" /> and delete the dead methods. If the two portlets are deliberately deferred, say so in the PR body and note that they still carry the half-picked-range bug.

[ahamdi]

🟡 Medium — in the samples drawer the shared picker loses its own styling

.analytics-chart-period-selector (line 221) and .analytics-period-selector-full (line 249) both live inside the .VuetifyApp .analytics-application block opened at line 104 — the next top-level block is at 376. But ViewSamplesDrawer uses <exo-drawer> without attached, and social's ExoDrawer.mountOnParent() does document.querySelector('#vuetify-apps').appendChild(this.$el) on mount and on every open(). #vuetify-apps sits under .VuetifyApp but not under .analytics-application.

So the very component this PR extracted to share with the drawer renders there without position: relative / display: flex / min-width: 40px on its root, and without width: 100%, cursor: pointer or the border-bottom underline on its activator — falling back to Vuetify's generic button reset, i.e. an unstyled borderless label. The PR's two new blocks (.analytics-compact-period-popup at 502, .analytics-compact-period-times at 526) are correctly top-level and do apply, which is what makes the gap easy to miss: the popup looks right and only the activator is bare.

Positioning itself is fine — Vuetify's attach path measures against the same offsetParent either way.

Fix: move .analytics-chart-period-selector and .analytics-period-selector-full to the top level of analytics.less, next to .analytics-compact-period-popup, now that the component is shared with a re-parented drawer. Passing attached to the exo-drawer would also work but changes the drawer's own behaviour, so the stylesheet move is the smaller change.

[ahamdi]

🟢 Nit — a few small ones

• Orphaned javadoc. AbstractAnalyticsPortlet.java:442-479 is a full javadoc for writeDateCell (@param aggregation, @param key, @param styles) immediately followed by a second javadoc for writeTimestampCell. Java keeps only the last, so the first documents nothing and writeDateCell (line 496) ends up with none. Move it down.
• Dead computed. AnalyticsPeriodPickerPanel.vue:102-104 defines uid() { return this._uid; }, referenced nowhere in the template or the script.
• Two controls, one accessible name. In the same file, lines 50-75, each bound's date input and time input share an :aria-label (analytics.from, then analytics.toDate), so a screen reader announces "From" for both with no way to tell the date from the hour.
• toDate setter on an unseeded panel. $set(this.dates, 1, value) on an empty array yields [undefined, "…"] (Vue pads the length first), so apply() bails on !this.dates[0] and typing a To date first silently does nothing. Only reachable when period is null, which the chart header and the drawer both prevent — hardening, not a bug. this.$set(this.dates, this.dates.length ? 1 : 0, value) closes it.
• Duplicated guard comment. The blank/"null" check and its three-line comment appear verbatim in writeCell (487-491) and writeValue (515-520). Both guards are genuinely needed on their own paths; the comment only needs to live once.
• PR body is stale on tests. It says "green (11 tests)"; head has 16 (6 + 10), AnalyticsTableExportCellTest having arrived in the last commit. It also states mutation-verification for AnalyticsExcelDateCellTest only — per backend-spring.md §7, either mutation-verify the new class too (revert writeValue's date branch; testDateColumnValueIsWrittenAsARealDate and testZeroIsNotExportedAsFirstOfJanuary1970 must fail) or say it was not done.