feat(planning): carry the page's period and tags into the Excel export by renemadsen · Pull Request #1721 · microting/eform-angular-timeplanning-plugin

renemadsen · 2026-09-18T11:09:11Z

Mockups that this was designed from: https://claude.ai/artifact/YSrnP6Q2JFgUabKSR9S8q9

Problem

On the planning page you set a period and pick tags, then press Download Excel — and the modal opens blank. The period has to be typed a second time, and the tags cannot be carried over even by hand: neither export request model had a tag field, and neither export filtered by tags. The tags in the workbook are a display column, not a filter.

What changes for the user

The modal opens with the page's period and tags already filled in, and has a tag filter of its own. Editing there affects only that export — the page behind keeps what it had. Under the fields, a line reads 14 workers · 30 days and recomputes as the period, tags or worker change, so the size of the export is visible before it runs. Pick a single worker and the tag field is disabled with a note: that export covers one person, so tags could only narrow it to nothing.

What that required underneath

A count is only worth showing if it predicts the workbook. That drove everything else:

The all-workers export takes tagIds and filters sites by them, reusing the SiteTags → Site.MicrotingUid lookup the planning list endpoint already runs. Empty means no filter, exactly as today.
A new plannings/site-tags endpoint feeds the count, so it recomputes locally with no request per keystroke.
The count leaves out resigned workers, because the export does.
The export's tag filter excludes soft-deleted tags, deliberately stricter than the grid idiom it was modelled on, so the count and the export agree.

Behaviour changes worth a reviewer's attention

The all-workers export is now scoped to the caller. Previously any authenticated user — the endpoint is [Authorize], not admin-gated — could export every worker in the organisation. A manager would have seen "3 workers" above a workbook containing everyone. Admin output is unchanged: the site-id query is untouched and the scope only filters it, so for an admin every id survives in the original order and the workbook is byte-identical. The export now requires a resolvable caller, which is a new failure mode on a route that is already [Authorize].
A restricted worker whose site has no MicrotingUid previously fell through the site filter and saw every site on the planning board. That degenerate case now shows nothing instead of everything.
Single-site tenants auto-select their only site on the page, so the modal inherits it and offers "Download Excel" rather than "Download Excel (all workers)".
The site-scoping rule moved into SiteScopeResolver, shared by the board, the export and the count, so they cannot drift apart. The unscoped pool is no longer reachable from outside it.

Tests

The query shape is pinned. Tags travel as repeated keys, tagIds=1&tagIds=2. A comma-joined tagIds=1,2 binds to an empty list with no error — a "filtered" export quietly containing everyone — so there are tests on both sides: a C# model-binder test for the shapes that bind, and a TS spec asserting the exact URL.
C#: tag filtering (any-of, empty, multi-tag, soft-deleted tag), the site-tags endpoint (per-site tags, resigned flag, duplicate rows collapsing, manager and plain-worker scoping), manager vs admin export scope. ExportTagFilterAndSiteTagsTests is registered in the shard allowlist in both workflow files.
Four existing export fixtures gained an admin caller because they passed baseDbContext: null!; no assertion in any of them changed, which is itself the admin-unchanged check across ~8 existing call sites.
Jest: inheritance, no write-back to the page, count arithmetic, the disabled-tags state, the count hidden when its fetch fails while the export still works.
Playwright: the modal opens pre-filled, and the count changes when tags change.

Verified locally: dotnet build and tsc --noEmit. Tests run in CI.

Known and deliberately not in this PR

The single-worker export takes a caller-supplied SiteId with no scoping at all — the same exposure class as the one closed here, reachable by any authenticated user who guesses a site id. Untouched; worth its own PR.
The tag-filtered export still computes its shift columns over every site, so it can carry a shift column no included worker uses. Pre-existing and cosmetic.
The plural keys assume a two-form rule, which does not fit Polish, Czech, Lithuanian, Latvian or Ukrainian; those locales carry the English fallback today.

🤖 Generated with Claude Code

The Download Excel modal opened blank: the period had to be typed a second time, and the page's tag filter could not be carried over at all, because neither export request model had a tag field and neither export filtered by tags. The modal now opens with the page's period and tags already filled in, and gains a tag filter of its own. Editing there changes only that one export; the page behind keeps what it had. A line under the fields reads "14 workers · 30 days" and recomputes as the period, tags or worker change, so the size of the export is visible before it runs. Tags are disabled for a single worker, whose export covers one person. The count is only worth showing if it predicts the workbook, which drove the rest: - the all-workers export takes tagIds and filters sites by them, using the lookup the planning list endpoint already runs - a new plannings/site-tags endpoint feeds the count, so it recomputes without a round trip per keystroke - the count leaves out resigned workers, because the export does - both the export and that endpoint now scope to the caller. A manager could previously export every worker in the organisation from an endpoint that is only [Authorize]; now they get their own sites, and the count matches. Admin output is unchanged: the site-id query is untouched and the scope only filters it, so every id survives in the same order and the workbook is byte-identical. - the site-scoping rule moved into SiteScopeResolver so the board, the export and the count cannot drift apart. Reading the unscoped pool is no longer possible from outside it, which is how a restricted worker whose site has no MicrotingUid could see the whole board. Tags travel as repeated query keys. A comma-joined tagIds=1,2 binds to an empty list with no error, which would mean a "filtered" export quietly containing everyone, so a test pins the query shape. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot

🔵 Needs a closer look

One or more issues must be addressed before approval.

Pull request overview

Carries planning filters into Excel export and adds caller-scoped worker counts.

Changes:

Added tag filtering and caller-based site scoping to all-worker exports.
Added site-tag lookup API and client-side export scope counting.
Updated modal inheritance, localization, tests, and CI shards.

File summaries

File	Description
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningWorkingHoursService/TimePlanningWorkingHoursService.cs	Updated as part of this pull request.
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningPlanningService/TimePlanningPlanningService.cs	Updated as part of this pull request.
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningPlanningService/ITimePlanningPlanningService.cs	Updated as part of this pull request.
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Models/WorkingHours/Index/TimePlanningWorkingHoursReportForAllWorkersRequestModel.cs	Updated as part of this pull request.
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Models/Planning/SiteTagsModel.cs	Updated as part of this pull request.
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/SiteScopeResolver.cs	Updated as part of this pull request.
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Controllers/TimePlanningPlanningController.cs	Updated as part of this pull request.
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelShiftColumnOrderTests.cs	Updated as part of this pull request.
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelHolidayColumnTests.cs	Updated as part of this pull request.
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/WorkingHoursExcelExportTagsColumnTests.cs	Updated as part of this pull request.
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/TestBaseSetup.cs	Updated as part of this pull request.
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/ExportTagFilterAndSiteTagsTests.cs	Updated as part of this pull request.
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/DagsoversigtWorksheetExportTests.cs	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/services/time-planning-pn-working-hours.service.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/services/time-planning-pn-working-hours.service.spec.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/services/time-planning-pn-plannings.service.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/models/working-hours/time-plannings-report-all-workers-download-request.model.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/models/plannings/site-tags.model.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/models/plannings/index.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/ukUA.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/svSE.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/slSL.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/skSK.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/roRO.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/ptPT.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/ptBR.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/plPL.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/noNO.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/nlNL.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/lvLV.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/ltLT.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/itIT.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/isIS.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/huHU.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/hrHR.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/frFR.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/fiFI.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/etET.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/esES.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/enUS.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/elGR.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/deDE.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/da.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/csCZ.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/i18n/bgBG.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/components/plannings/time-plannings-container/time-plannings-container.component.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/components/plannings/time-plannings-container/time-plannings-container.component.spec.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/components/plannings/time-planning-actions/download-excel/download-excel-dialog.component.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/components/plannings/time-planning-actions/download-excel/download-excel-dialog.component.spec.ts	Updated as part of this pull request.
eform-client/src/app/plugins/modules/time-planning-pn/components/plannings/time-planning-actions/download-excel/download-excel-dialog.component.html	Updated as part of this pull request.
eform-client/playwright/e2e/plugins/time-planning-pn/n/tags-download-excel-modal.spec.ts	Updated as part of this pull request.
.github/workflows/dotnet-core-pr.yml	Updated as part of this pull request.
.github/workflows/dotnet-core-master.yml	Updated as part of this pull request.

Review details

Suppressed comments (2)

eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/SiteScopeResolver.cs:139

GetCurrentUserAsync() can return an identity whose row has been deleted or is not present in this BaseDbContext; in that case Single(...) throws and the newly scoped export/site-tags requests return a 500 instead of the operation result used for an unresolvable caller. Use SingleOrDefault and return SiteScope.Failed("UserNotFound") when the base user is missing, before reading roles or the email.

        var currentUser = baseDbContext.Users
            .Include(x => x.UserRoles)
            .ThenInclude(x => x.Role)
            .Single(x => x.Id == currentUserAsync.Id);

eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningPlanningService/TimePlanningPlanningService.cs:2603

This lookup returns every non-removed AssignedSite group, but the export's siteIds list is later resolved through sdkContext.Sites and silently skips IDs with no live SDK Site (lines 3441-3443). Therefore an orphaned/stale AssignedSite is counted in the dialog while producing no workbook sheet, violating the stated count/export parity. Filter the endpoint's rows to the same live SDK-site set (or otherwise exclude orphaned site IDs) before constructing SiteTagsModel results.

            var result = assignedSites
                .GroupBy(x => x.SiteId)
                .Select(g => new SiteTagsModel
                {
                    SiteId = g.Key,
                    TagIds = tagIdsBySiteUid.TryGetValue(g.Key, out var tagIds) ? tagIds : new List<int>(),

Files reviewed: 53/53 changed files
Comments generated: 0
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

The two soft-deleted-tag tests failed in CI against production code that was right. PnBase.Delete sets WorkflowState on the object and then saves only `if (dbContext.ChangeTracker.HasChanges())`, and the SDK hands out a fresh DbContext per call — so deleting an entity that was created on a different context writes nothing and throws nothing. The tag stayed live, the queries reported it, and the assertions failed. Deletion now goes through helpers that re-read the row on the context they delete on, then verify from a separate no-tracking context that it really is removed, asserted as a seeding precondition. The failure can no longer masquerade as a behaviour bug. TagSiteRaw hands back ids instead of entities so a caller cannot hold a detached one. One seeding case was vacuous: a removed SiteTag on a site whose AssignedSite was itself removed, so the site never reached the result and the case proved nothing. Replaced with site 9646 — live Tag, removed SiteTag — which does exercise that predicate. Each site now carries one rule, so a failure names the rule that broke. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot

🔵 Needs a closer look

The broad backend scoping and export behavior changes warrant final human review.

Review details

Files reviewed: 53/53 changed files
Comments generated: 0 new
Review effort level: Lite

Copilot AI lite review requested due to automatic review settings September 18, 2026 11:09

Copilot started reviewing on behalf of renemadsen September 18, 2026 11:09 View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 18, 2026 11:36

Copilot started reviewing on behalf of renemadsen September 18, 2026 11:37 View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

renemadsen merged commit a0f03f9 into stable Sep 18, 2026
41 checks passed

renemadsen mentioned this pull request Sep 18, 2026

fix(security): scope the working-hours export and grid to the caller #1722

Merged

Navigation Menu

feat(planning): carry the page's period and tags into the Excel export - #1721

feat(planning): carry the page's period and tags into the Excel export#1721
renemadsen merged 2 commits into
stablefrom
feat/export-modal-inherits-filters

renemadsen commented Sep 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

renemadsen commented Sep 18, 2026

Problem

What changes for the user

What that required underneath

Behaviour changes worth a reviewer's attention

Tests

Known and deliberately not in this PR

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (2)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants