Statistics: evaluate responses and attendance across all appointments - #158
Merged
Conversation
A new "Statistics" view answers, per person and across every appointment, how often someone replied and how often they actually turned up — filtered by period and category, grouped by Nextcloud groups or teams. Two denominators, deliberately different: - the response rate counts every appointment a person was addressed to, upcoming ones included; - the attendance rate counts only appointments that are over *and* had at least one check-in recorded. Counting the rest would make each person's number depend on how diligently somebody else worked the check-in list, and turn missing data into an accusation. Cancelled and inactive appointments are out of both. People in several whitelisted groups appear in each of their sections, exactly like the per-appointment summary; the totals row counts every person once. Responses of deleted accounts are ignored — without a group membership there is no audience to measure them against. Guests are in, badged, under "Others". Access is a new see_statistics permission, defaulting to nobody. Without it the page still renders, reduced to the viewer's own row plus the group and overall averages — the harmless, motivating part — with no other names, no charts and no export. Organizers get nothing extra: an evaluation spanning months is a different thing from administering one appointment. Version000021 seeds the permission from manage_appointments, but only when that is actually restricted to groups. Version000018 gave every install an explicit mode, so "manage_appointments is stored" no longer distinguishes a configured install from an untouched one — and an untouched one says "all", which would have opened a personal evaluation to every user on the server. Server-side the evaluation runs as one grouped query plus a per-appointment audience expansion with memoised group memberships, and refuses ranges over 1000 appointments with a "narrow the period" message rather than a timeout. Dependencies: adds chart.js ^4.5.1 and vue-chartjs ^5.3.4 for the three charts. Only the controllers those charts need are registered instead of chart.js' full `registerables`, and the chart component is loaded on demand — it lands in its own 170 kB (59.8 kB gzipped) chunk that no other view pulls in. Colours are read from the Nextcloud CSS variables at runtime and re-read on theme changes, which chart.js cannot do by itself. Deliberately held back: no chart plugins (datalabels, zoom), no date adapter — the timeline uses a category axis of pre-formatted labels instead. psalm-baseline.xml gains 54 lines for the new files. Every entry is a category the repo already carries for each comparable file: ClassMustBeFinal (final services break PHPUnit mocking), UnusedClass and PossiblyUnusedMethod / PossiblyUnusedParam (DI and routing hide the call sites), MissingDependency (the OCP stubs lack oc\hooks\emitter behind IRootFolder), MissingOverrideAttribute (#[\Override] needs PHP 8.3, the app floor is 8.1) and one DeprecatedMethod for IConfig::setAppValue, which the migration uses precisely because PermissionService writes that key the same way. No existing entry was touched and no new category was introduced. The ODS container (zip, manifest, styles, meta) moves into a shared OdsWriter; the appointment export keeps its own cell rendering unchanged. Mobile: new statisticsAvailable capability. The API change is additive only — three new endpoints, eight new schemas, two new response fields, nothing removed or altered — so older clients keep working untouched.
Quality pass over the statistics feature. No behaviour changes except the first item, which is a gap the review found. **see_statistics was unreachable in the admin settings.** The catalogue in `src/utils/permissions.js` is shared, but which rows the admin screen renders came from a second hand-maintained list in AdminSettings.vue — so the permission existed on the server, in the API, and in the setup wizard, while the one screen that grants it never showed it. The grouping now lives on the catalogue entry and the screen derives its sections from it, which makes that class of half-wiring impossible. **Reuse.** The person sidebar formatted dates and answer labels itself; it now uses `formatDate()` from utils/datetime.js and `getResponseText()` from utils/response.js, so this view stops being the one place that renders dates in the browser default locale instead of the Nextcloud one. ExportService dropped its byte-identical copy of `escapeXml()` and its raw 'ce1'/'ce-yes' style literals in favour of OdsWriter's escape() and STYLE_* constants, so the style vocabulary the extraction created has one owner. **Wasted work.** The evaluation used to hydrate a full 15-column response entity per row to read four fields; it now selects those four columns directly, which at the 1000-appointment cap is the difference between ~2.4 s and ~0.1 s. "Did anyone check in here" moved from a per-appointment scan of that matrix to one DISTINCT query. The drill-down no longer loads every person's responses to render one person's rows. Section membership is resolved per group/team instead of asking each person for their groups — one query per section rather than one per person. The over-large-range guard pushes its limit into the query instead of hydrating everything and then refusing. The audience id list is memoised rather than rebuilt per appointment, and the visibility JSON is decoded once instead of twice. **Simplification.** `attendedDespiteNo` was accumulated, merged, serialized and typed in six places while nothing read it — the sidebar derives "said no but attended" per appointment, which is where it was asked for. Same for `categoryId` on the drill-down entries. The per-category counters were a second hand-rolled tally beside StatisticsTally and now reuse it. The controller's repeated filter construction and range-error response became two private helpers. Both watchers keyed on freshly allocated objects with `deep: true`, which disables Vue's change check entirely — they now key on the serialized filter, so a spurious fire cannot trigger a full evaluation. Dropped an unused chart colour, an unused export and a duplicated chart options computed. Deliberately not done, with reasons: - `VisibilityService::getRelevantUsersForAppointment()` silently drops `visible_teams` in its restricted branch, which is why the statistics resolve that audience themselves. Fixing it there is the right depth, but it would change what six existing callers see — check-in lists, response summaries, reminders — and that belongs in its own change, not inside a statistics feature. - The ODS destination code (get folder, replace file) is written twice. Unifying it means touching the shipped appointment export's failure behaviour, whose only coverage is the e2e suite. - Folding the two export sheet builders into one, and importing the psalm shapes from ResponseDefinitions rather than declaring them locally: both fought psalm's exact-shape matching for less than they were worth, and `@psalm-import-type` does not resolve for these services in this setup.
`usePermissions` copies each field out of the API response by hand, and the two new flags were only added to the state shape, never to that copy. Both stayed false forever, so the navigation entry never appeared and the page always rendered in its reduced form — the e2e run caught it as "no person rows", because those only exist in the full view. The same key list existed in four places (the shape, the success copy, the error reset, resetPermissions). The two reset paths now derive from one defaults object, so only the success copy — where the per-field fallbacks genuinely differ — is still written out by hand. The statistics spec also depended on global admin settings it never established: omitted settings stay untouched on save, so a whitelist left by an earlier sequential spec would narrow the audience under it. It now resets the settings up front and sends the whitelists with every permission change. Added a test that reaches the view through the navigation entry, which is what would have caught the capability half-wiring, and an assertion that the group sections exist — without it an empty section list looks exactly like an empty audience.
luflow
force-pushed
the
claude/event-attendance-statistics-p0uxo7
branch
from
August 9, 2026 11:58
5b3ca28 to
de90c60
Compare
reset-dev-data.sh clears each permission by name from a fixed list, and the new one was not in it — a reset would have left see_statistics configured while claiming to restore a fresh install.
…it with it Without see_statistics the page now answers with the viewer's own row and nothing else — no group sections, no totals row. The group and overall averages were meant as harmless context, but an average is still a statement about colleagues, and a reader who cannot see the rows behind it cannot judge what it means. The server stops computing them for that case too, so nothing about other people leaves it. With the permission the viewer's own row is tinted in the Nextcloud primary colour and stays that way on hover, so it can be found in a table of a few hundred people. The sidebar pills were painting --color-success / --color-error as fills. shared.scss says plainly that those are surface tokens: in the dark theme they resolve to near-black tints, which turned the three states into three shades of dark grey. They are NcChips now, with the variant coming from the same getResponseVariant() the appointment detail page feeds its response dots and count chips — one source for the yes/no/maybe scale rather than a second palette that drifts. Also replaces the empty own-row case, which rendered a blank tbody, with a line saying there were no appointments to be invited to.
…theme The attendance series read --color-success, which is a *surface* token and near-black in the dark theme — the same trap the sidebar pills had. It now comes from $color-yes through a custom property on the chart container. chart.js' autoSkip drops x-axis labels that do not fit, so the group and category charts showed five bars under three names. The bar charts shorten their labels with an ellipsis instead; the tooltip still carries the full one. Also gives the name column a floor so the numeric columns stop squeezing it, and keeps each sort arrow on the line of its label.
The sidebar spoke its own dialect: text pills for answers, "Not recorded" where the detail page says nothing, and no comments at all. It now uses the same ResponseDot glyphs, the same "Checked in?" line and the same comment row, so a person's history reads like the appointment it came from. ResponseDot gains the case only this list has — somebody who never answered. The glyph its own helper already names (ProgressQuestion) was missing from its icon map, so that fell back to the amber "maybe" circle. Comments follow see_comments, as they do in the response overview; your own are always yours to read. And the drill-down as a whole now needs manage_appointments or see_response_overview: naming what one person answered per appointment is that permission's disclosure, not the aggregate row's.
"No sidebar appeared" is also true of a sidebar that has not rendered yet, so the missing tabindex carries the assertion.
Two radio groups over the table: grouped or flat, and compact or full. Compact keeps the columns that answer the question at a glance — yes, no, maybe, present and the two rates — while full keeps every column there was. Grouped and compact is the default. Both only pick what is drawn from an answer the server already sent, so neither is part of the query and flipping one costs no evaluation. They do go into the URL, so a view stays linkable. Ungrouped also drops the by-group chart and the footnote about people in several groups: without sections neither has anything to say.
The drill-down rule was spelled out twice, once per side of the wire. PermissionService::canSeeIndividualResponses() names it, getPermissions() carries it, and the view reads the flag instead of re-ORing two others — the shape canSeeResponseCounts already uses. A check-in dot said "Yes" on the appointment page and "Present" in the statistics sidebar, because the sidebar passed a label the older call site never got. getCheckinText() settles the wording next to getResponseText(), and `kind="checkin"` picks it at both call sites. The chart's signal colour moves to `:root` in an unscoped block, which is what the scoped-style workaround was dancing around — useChartTheme() goes back to reading the document root for every colour, not just three of four. Also: getStatistics had two idioms for "only this user, when limited" — now one loop; and findStatisticsRows only carries a comment key when the column was actually selected, keeping the bulk path as lean as its docblock claims.
Grouping, level of detail and the people search only redraw rows that are already on the screen; the filter bar above them re-queries the server. They now sit in a strip directly over the table, so the two kinds of control are no longer mixed. Group by (groups or teams) stays with the filters — it decides what the server sends, not what is drawn.
…dings The attendance series was a fixed green while the acceptance series follows the instance's primary colour — on a green-themed instance the two charts became one. It now takes the accent's opposite hue at a fixed, moderate saturation and lightness, so it stays apart from any primary without shouting. The custom property that carried the fixed colour is gone with it. The page headings were styled per view: 1.4em in the appointment list, the browser default everywhere else, and on the statistics page no top margin at all — which put the heading straight under the navigation toggle, since that page runs full width and has no left gutter to hide behind. One rule in App.vue now sets size and top clearance for all of them; the views keep only the column they centre their heading in.
Nested flex rows squeeze each radio button down to its longest word, so "Ungrouped" broke in two while its neighbours stayed single-line.
The compact view answered "how many said yes" in absolute numbers but only gave the rate for responses and attendance, so the one column that puts the yes count in proportion was the one missing.
Moving the top spacing onto the page heading was the wrong place for it: the appointment list opens with the unanswered banner, above the heading, so the banner ended up flush against the top edge. The clearance belongs to the content area — it now sits on the app content wrapper, where a heading, a banner and a row of buttons alike start below the toggle. The views carry no top padding of their own any more, so nothing doubles up. The banner also stops hiding on the all-appointments list; there is nothing about that list that makes an open response less worth showing. It stays out of the unanswered view, which has its own, and out of past appointments. Clickable rows in the statistics table set the cursor on their cells as well as on the row, so a `th`/`td` rule from the server theme cannot outrank the value the cells only inherit.
The colour maths hand-rolled a hex parser that only understood #rrggbb, so a theme handing back rgb(…) or #08c would silently fall through to the default hue. chart.js ships @kurkle/color and exports it from chart.js/helpers, which this file already had as a dependency — parsing and the alpha variant go through it now, and the result is emitted as hsl() rather than converted back to hex by hand. Only the hue extraction is still ours: chart.js can rotate a colour, but its saturation and lightness adjustments are relative, so a vivid primary would keep a vivid opposite — the fixed, muted pair is the point. The table's sort is derived rather than reset by a watcher: between a column disappearing and the watcher firing, one render sorted by a column that was no longer there. Also: complementOf stops being exported (no caller outside the file), the detail prop validates its two values, and writeUrlState marks where the linkable-but-not-queried parameters start.
`.app-content-wrapper` is only rendered when NcAppContent gets a `list` slot, which this app never passes — the rule matched nothing, so the spacing was gone entirely rather than moved. The content now sits in a wrapper of our own, which also lets the check-in view opt out: it runs without the navigation, so there is no toggle to leave room for. The statistics table gains a groups column when grouping is off, carrying the membership the section headings used to show, and the Name header is aligned with the names below it — the generic `th` rule is a class plus an element, so the bare class it was written as never won.
It only decides how the table lists people; the by-group chart says something the table cannot show at all and has no reason to disappear with it. The groups/teams selector stays put for the same reason — it picks the chart's axis too.
chart.js centres the legend over the whole canvas, and the y-axis labels are part of that — centred therefore reads as shifted left of the plot it belongs to. Aligned to the end instead, which lines up with the plot's right edge.
Comments where the English is ambiguous out of context: the three rates, which differ in what they count and over what; "Not recorded", which is not absence; "No-show", a noun most languages need a phrase for; "Period" and "Range", which sit next to each other and mean the kind and the instance; "Compact"/"Full", about how many columns, not about the data; the Q and H abbreviations, which are not universal; and "People" in the export, which labels a sheet and a person count with one word.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A new Statistics view answers, per person and across every appointment, how often someone replied and how often they actually turned up — filtered by period and category, grouped by Nextcloud groups or teams.
The two denominators
This is the part that decides whether the numbers mean anything, so it is worth stating plainly:
Counting the rest would make each person's number depend on how diligently somebody else worked the check-in list, and would quietly turn missing data into an accusation. The view says how many appointments the attendance figure actually rests on.
Cancelled and inactive appointments are out of both. People in several whitelisted groups appear in each of their sections, exactly like the per-appointment summary; the totals row counts every person once. Responses of deleted accounts are ignored — without a group membership there is no audience to measure them against. Guests are included, badged, under "Others".
What you get
Access
New
see_statisticspermission, defaulting to nobody. Without it the page still renders, reduced to the viewer's own row plus the group and overall averages — the harmless, motivating part — with no other names, no charts and no export. Organizers get nothing extra: an evaluation spanning months is a different thing from administering one appointment.Version000021seeds the permission frommanage_appointments, but only when that is actually restricted to groups.Version000018gave every install an explicit mode, so "manage_appointments is stored" no longer distinguishes a configured install from an untouched one — and an untouched one says "all", which would have opened a personal evaluation to every user on the server.Performance
The evaluation is one grouped query for the four response columns it needs, one DISTINCT query for "was anyone checked in here", and a per-appointment audience expansion with memoised group memberships. Section membership resolves per group rather than per person. Ranges over 1000 appointments are refused with a "narrow the period" message, and the limit is pushed into the query so an over-wide range is not hydrated just to be rejected.
Dependencies
Adds
chart.js ^4.5.1andvue-chartjs ^5.3.4. Only the controllers the three charts need are registered instead of chart.js' fullregisterables, and the chart component is loaded on demand — it lands in its own 170 kB (59.8 kB gzipped) chunk that no other view pulls in. Colours come from the Nextcloud CSS variables at runtime and are re-read on theme changes, which chart.js cannot do by itself. Deliberately held back: no chart plugins (datalabels, zoom) and no date adapter — the timeline uses a category axis of pre-formatted labels.Mobile compatibility
New
statisticsAvailablecapability. The API change is additive only — three new endpoints, eight new schemas, two new response fields, nothing removed or altered — so older clients keep working untouched. The matching model change is in luflow/attendance-flutter#(paired PR); the mobile view follows separately, gated on the flag.Notes for review
psalm-baseline.xmlgrows by 54 lines. Every entry is a category the repo already carries for each comparable file:ClassMustBeFinal(final services break PHPUnit mocking),UnusedClass/PossiblyUnusedMethod/PossiblyUnusedParam(DI and routing hide the call sites),MissingDependency(the OCP stubs lackoc\hooks\emitterbehindIRootFolder),MissingOverrideAttribute(#[\Override]needs PHP 8.3, the app floor is 8.1) and oneDeprecatedMethodforIConfig::setAppValue, which the migration uses precisely becausePermissionServicewrites that key the same way. Every genuine type error was fixed in the code instead. No existing entry was touched and no new category introduced.VisibilityService::getRelevantUsersForAppointment()silently dropsvisible_teamsin its restricted branch, which is why the statistics resolve that audience themselves. Fixing it at the source is the right depth, but it would change what six existing callers see — check-in lists, response summaries, reminders — so it belongs in its own change rather than inside a statistics feature. Worth filing.see_statisticswas never rendered in the admin settings, because the screen kept its own list of which permission rows to show. The grouping now lives on the shared catalogue.Testing
./scripts/check.shpasses in full: eslint, stylelint, php-cs-fixer, psalm, PHPUnit (317 tests), the vite build, both l10n checks and the OpenAPI drift check. New coverage: 10 unit tests for the denominator logic (no-show, exclusion of appointments without check-ins, multi-group membership, guests, deleted users, the range guard, the reduced shape), 5 for the migration's seeding rule, and 4 Playwright specs. The e2e suite itself needs Docker and was not run here.Generated by Claude Code