Feat/noid/exclude guests and timeline fixes - #35
Merged
Conversation
Guest accounts — users created by the Guests app — are external people invited to collaborate on files. They have no entitlement and take no leave, but the app treated every account as staff, so each guest sat in the balances report and the who's-off calendar forever with an empty allowance and nothing to show. Four services each walked IUserManager with their own copy of that loop, so a rule about who is *not* an employee had to be repeated four times to hold — and would silently not hold wherever it was forgotten. EmployeeDirectory is now the one place that answers "who are the employees", and ReportService, EntitlementService, CoverageService and ManagerResolver all ask it. Detection reads the user backend: a guest lives in the Guests app's own backend, which is what OCA\Guests\GuestManager::isGuest() checks too. Read that way the app needs no dependency on the Guests app — where it is absent or disabled no account has that backend and the rule is simply never true, so instances without guests are unaffected. Enforced, not merely hidden. Filtering only the lists would leave the rule one crafted request away from being bypassed, so the API rejects recording leave for a guest (including by HR, who may otherwise record for anyone), nominating one as a replacement, and setting one's entitlement. Guests also cannot be resolved as a line manager: that is more than tidiness, because a request routed to a guest approver could never be approved. The people pickers move off core's autocomplete to a new /api/employees/search, because only the server can tell a guest from a colleague — a client-side filter would be no filter at all. It wraps the same collaborator search core uses, so the admin's user-enumeration settings keep applying exactly as before; guests are removed from whatever that search returns. Existing records for someone who later becomes a guest are left untouched in the database; they simply stop being listed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Selecting a request in a list navigated away instead of opening the sidebar. NcListItem always renders an anchor and defaults its href to "#", and it only calls preventDefault() itself when given a `to` (router-link) prop. Left alone the browser followed that "#", which under this app's hash history is the route "/", redirecting to My leave. So an HR user opening somebody's absence was thrown back to their own overview — the record they clicked was in the sidebar, behind a page that had already moved. Approvals had it too; it was invisible on My leave only because that is already where you land. Having stayed on the page, the record then identified people by user id. Managers and HR spend most of their time in this app looking at other people's leave, and "jdoe" is not who they are looking for, so requests are now serialized with an employeeName — resolved the same way the replacement's name already was, falling back to the uid for a deleted account. The list names the employee, the sidebar's Employee row shows an avatar beside the name, and viewing somebody else's leave names them under the sidebar title. Only somebody else's: on your own there is nobody to disambiguate and it would just crowd out the dates. No regression test for the click: this repo has vitest but no @vue/test-utils and no DOM environment, and importing the component pulls in NcListItem's stylesheet, which vitest externalises and fails to load. Setting that up is worth doing on its own rather than as a rider on a two-line fix, so the reasoning lives in a comment at the call site instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The bottom row of the who's-off / team timeline was cut in half: the pill and its selection ring were sliced flat along the container's lower edge. Not a sizing bug — the rows were always the right height. Overlay scrollbars, the macOS default, do not reserve space of their own the way classic ones do; they are painted over the content. The pills sit at top: 8px in a 42px track, leaving only 8px of clearance, and an overlay scrollbar occupies roughly the bottom 11px of the scroll box. That is why only the last row could ever show it, and why the pill looked chopped flat rather than shortened. The scroll container now keeps a strip clear below the last row for the scrollbar to live in; with no scrollbar visible it reads as ordinary bottom padding. Also pins the container's flex sizing. It is a scroll container, so its automatic minimum size in a flex column is zero and a height-constrained ancestor could shrink it and clip rows outright. Nothing constrains it today — this keeps a future layout change from quietly reintroducing the same symptom by another route. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
3 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.
🤖 AI (if applicable)