Act on user feedback: calendar notification storm, statistics table and scheduling rate - #176
Merged
Merged
Conversation
Every updateCalendarObject() raises the DAV activity "X updated event Y in calendar Z" for the calendar owner and everybody the calendar is shared with. It fires unconditionally — the dav app never compares content — so a shared organization calendar produced one such notification per answer, attributed to the person who answered, who has never seen the calendar. Two changes, both keeping the write real-time: - Skip the write when the generated document says the same thing as the stored one, ignoring DTSTAMP and LAST-MODIFIED. Those are derived from updatedAt and change whenever the row is touched, so a comment edit or a repeated identical answer produced a write that changed not one visible character. - Add an admin switch for the response summary block. Nextcloud's own counter-setting is per activity type, not per calendar, so switching it off there silences every calendar the person has; this one is narrow. Default on, and flipping it backfills, so switching off also takes the block out of the events that already carry it. Unfolding an iCal document now lives in IcalService next to the folding it inverts, rather than being written out a second time in the comparison.
Clicking a name opens that person's appointments in the order the timeline needs them — oldest first. But the drill-down answers "what has this person been doing lately", not "how did the season run", so it should open at the recent end. Reversed in getPersonDetail() rather than in the query: the same findForStatistics() call feeds the chronological timeline chart, which has to stay ascending. Doing it server-side keeps web and mobile in step, and changes no field, so no client breaks on it.
Managers who use the planning mode want to know how often somebody who said
yes was then actually scheduled — the counterpart to the acceptance rate,
which only says how often they offered.
The denominator is the careful part. A yes counts only once the inquiry is
closed AND somebody was actually scheduled for it, mirroring
BookingService::isScheduledOut(): before closing nothing is decided, so a
person who accepted yesterday would otherwise read as "not scheduled", and an
inquiry closed without scheduling anyone is one where the feature was not
used — counting it would measure the manager rather than the person. This is
the same shape the attendance rate already uses ("over AND check-in list
worked"), now the third denominator in the evaluation.
The response carries schedulingEnabled so every consumer — table, export,
mobile — reads the rule from the one place that applied it, instead of asking
the config again in its own layer. The columns and the ODS export follow it.
The effective-status precedence (booking_status wins, booking_notified_status
is the fallback) moves into a static on BookingService, so the bulk query can
apply it without hydrating the entity it exists to avoid, and without
restating the rule.
Fully expanded the table shows fifteen columns of nowrap numbers and needs roughly 1800px, while the page was capped at the 1200px every other view uses. The overflow was therefore guaranteed on every screen, and the scroll container's bar sits below the last row — out of sight on any list long enough to be worth reading. Reported as "no horizontal scrollbar in Firefox"; it was neither Firefox nor the reporter's install. Three changes: - The statistics page gets the width it actually needs. It is the one view in the app with a real case for it. - Columns can be ticked individually, including the groups column, which is the widest one and the one that prompted the report. Compact/Full stay as the two presets; ticking refines whichever was picked last. - The name column stays put while scrolling right, so the numbers under the cursor still belong to somebody. Column descriptors move into src/utils/statisticsColumns.js because the table and the picker have to agree on what exists and what it is called — including the scheduling columns from the previous commit, which appear only where the evaluation says the planning mode is on. Row highlighting now travels through a custom property. The sticky name cell paints its own background, which would otherwise out-specify the row states and leave the frozen cell of every highlighted row unpainted.
German is owned by this repo and never comes back from Transifex, so a string added with a fresh t() call stays English until it is filled in here. Wording follows the neighbours: "Einplanungsquote" beside the existing "Zusagequote" and "Anwesenheitsquote", and "Kalendereintrag" as the organization calendar section already calls a synced event.
A git pull does not update vendor-bin/*; only composer install (via its post-install-cmd) or composer bin all install does. Pull a bump and the old binary stays behind — which is how psalm sat at 5.26.1 while the lock said 6.16.1, crashed on a PHP its version predates, and read as a broken local environment rather than an install that was simply out of date. The gate had not run for a while as a result. Also record how the baseline counts, since it makes a self-inflicted error look like somebody else's: occurrences are tallied per file and issue type, so one more call of an already-baselined kind reports against an unrelated line.
The column picker sat in a filter-bar wrapper rather than beside the switches it belongs to, so its label came out small, grey and left-aligned next to the bold centred labels of Grouping and Detail — the button read as floating free. It uses NcFormGroup now, the same component NcRadioGroup builds its label from, so the three line up by construction instead of by a restated style. The response-summary switch in the admin settings sat 8px below the target calendar's hint paragraphs, which is the spacing between those paragraphs, so it read as another remark about the calendar above it rather than as its own decision. It gets the 24px the section otherwise uses between subsections.
Small cards naming who turned up most reliably, who accepts most often, who answers "maybe" most, and — factually rather than as an award — who has not been to an appointment the longest. The shape is what keeps it from being a pillory. Rows are values, not people, so everybody tied at a value shares one line and no rank order is invented between equals: with seven people all on 90.9 % the card says so instead of picking three of them alphabetically. Award cards additionally stop at the totals average, so nobody is ever rendered as the tail of a ranking — on a small team five rows would otherwise be the whole roster, last place included. "Longest not attended" cuts the other way, because there the tail is the point. Rate cards carry their denominator behind each name and ignore anyone below half the widest basis in the period: somebody invited to their first appointment and present at it holds 100 % and would otherwise lead every card without having done anything. Which cards show is a per-user choice like the columns, four on by default. The scheduling cards appear only where the evaluation says the planning mode is on. Backend side, evaluate() now records when each person was last present. Kept beside the tallies rather than inside them: a date has no additive merge, and "the group's most recent attendance" is a number nobody asked for. Hoisting startOf() out of the per-person loop while adding it also removes a pre-existing duplicate call — it serializes the whole entity to read one field.
The skip introduced with the no-op change also changed what syncAppointment() reports. It returned false when the event was already current, and syncAllUpcoming() counts those returns — so the admin's sync button answered "0 appointments synced" on an instance where everything was in order, which reads as a failure rather than as nothing to do. Caught by the e2e suite, which is not part of the local gate. The return now means "the appointment is in the calendar afterwards", which is what the button's own promise is: create or update the events for all upcoming appointments. The write is still skipped; only the report changed.
"Longest not attended" answered when somebody was last there. What a manager plans against is how often somebody drops out, so the card now reads the absence rate and is titled plainly: "Absent more often". Not 1 - attendanceRate. An appointment nobody ticked off counts in the base but says nothing about whether the person was there, so only recorded absences count — absent over attendanceBase, sitting beside the other rates in the tally where it belongs. That retires lastPresentAt, added two commits ago for the date card: the field, its tests and its API type are gone again. With it goes the whole date apparatus in the card module — the "never present" sentinel, the scoring indirection it needed, and the two-way threshold. Every card now simply wants its highest values, and the module is a good deal shorter for it. The rate cards take Andreas' own wording, "Top …". The two counting cards keep theirs: "Top" suits a rate, not a tally.
Three things the first cut got wrong, all visible the moment real data went through it. Every name sat on its own line with a leading comma. The button reset took the colours and the border but not the box model, and Nextcloud styles every button as a full-width block with its own min-height — the same trap as the row backgrounds in the table. The value sat beside the names as a nowrap column. Carrying the basis it ran to half the card width and left the names a strip so narrow that "Christina Vogel" broke in two. It heads its group now, and the names run on beneath it. The separator leaned on the whitespace between the buttons, which rendered as a space in front of every comma and stranded one at the start of a wrapped line. It is a pseudo-element plus a column gap now, so nothing depends on invisible text nodes. Also: the basis is stated once per row instead of after every name — four repetitions of "(4 appointments)" drowned out the names it was meant to qualify — and rows are further apart than the lines within them, so it is visible which names belong to which value.
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
1 participant
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.
Acts on a round of user feedback. Four of the reported items are fixed here; the
rest are filed as issues, two of them waiting on the reporter.
Organization calendar no longer spams the activity stream
Answering an appointment produced two notifications: the app's own, and
Nextcloud's "X updated event Y in calendar Z" — attributed to the person who
answered, who has never seen the calendar.
The cause is that
updateCalendarObject()raises that DAV activityunconditionally, for the calendar owner and everybody the calendar is shared
with; the dav app never compares content. Carrying the response summary
therefore cost one such notification per answer, and would hit the whole share
circle the moment the calendar is shared.
Real-time writes stay. What changed:
stored one, ignoring
DTSTAMP/LAST-MODIFIED. Both derive fromupdatedAt, so a comment edit or a repeated identical answer used to producea write that changed not one visible character.
counter-setting is per activity type rather than per calendar, so switching it
off there silences every calendar the person has; this one is narrow. Flipping
it backfills, so switching off also removes the block from events that already
carry it.
Statistics
closed and somebody was actually scheduled, mirroring
BookingService::isScheduledOut()— before closing nothing is decided, and aninquiry closed without scheduling anyone would measure the manager rather than
the person. Same shape as the existing attendance rate. The response now
carries
schedulingEnabledso table, export and mobile read the rule from theone place that applied it.
was capped at 1200, so the overflow was guaranteed on every screen and the
scroll bar sat below the last row. Reported as "no horizontal scrollbar in
Firefox"; it was neither. Columns are now individually selectable, the name
column stays put while scrolling right.
Filed, not fixed
one of them a real bug independent of configuration (disabled accounts are
never filtered out). Waiting on the reporter's configuration before changing
behaviour.
the three candidates lead to entirely different changes.
IBrokerAPI cannot add ordinary participants, only moderators.Notes
vendor-bin/psalmwas stuck at 5.26.1 while the lock said 6.16.1, so thepsalm gate had not actually run locally for a while. Updated, five findings
fixed,
CLAUDE.mdnow says agit pulldoes not updatevendor-bin/*.Transifex.
unaffected; the scheduling columns gate on the existing
bookingEnabledcapability.
🤖 Generated with Claude Code