feat: Implement admin-only specialties management interface
Playwright Tests / test (push) Canceled after 0s
Playwright Tests / test (push) Canceled after 0s
- Added MailerAttachmentPolicy for validating email attachments against server rules. - Introduced fis.specialties.js for managing hidden maintenance actions via an admin interface. - Created fis.specialties.scss for styling the specialties management UI. - Added German localization for specialties management in fis.specialties_txt_de.js. - Developed CSS for specialties management in fis.specialties.css. - Implemented server-side stored procedure fds__admin_backupAndClearInvoiceFile for backing up and clearing invoice files. - Created fds__invoice_file_backups table to store backup records of invoice files.
This commit is contained in:
Stefan
@@ -0,0 +1,114 @@
|
||||
---
|
||||
status: Accepted
|
||||
date: 2026-09-17
|
||||
applyTo:
|
||||
- "Fuchs/Controllers/IntranetController.Admin.cs"
|
||||
- "Fuchs/Services/IInvoiceFileBackupService.cs"
|
||||
- "Fuchs/Services/InvoiceFileBackupService.cs"
|
||||
- "Fuchs/Services/IBlobStorageService.cs"
|
||||
- "Fuchs/Services/AzureBlobStorageService.cs"
|
||||
- "Fuchs_Database/dbo/Stored Procedures/fds__admin_backupAndClearInvoiceFile.sql"
|
||||
- "Fuchs_Database/dbo/Tables/fds__invoice_file_backups.sql"
|
||||
supersededBy: ""
|
||||
---
|
||||
|
||||
# 0027 — Admin invoice-file backup-and-clear action
|
||||
|
||||
## Context
|
||||
|
||||
Support occasionally needs to force a finalised invoice's PDF to be regenerated
|
||||
(e.g. it was rendered with wrong data before a fix, or is otherwise corrupt).
|
||||
Since [0025](0025-invoice-document-regenerate-on-miss.md), clearing
|
||||
`fds__invoices.[file]` on a finalised, non-external invoice is enough to make
|
||||
the next `req/idoc`/`inv/rdoc` read regenerate and re-persist it.
|
||||
|
||||
A manual maintenance proc for this already exists,
|
||||
`[dbo].[_BackupAndClearInvoiceFile]`, but its own header comment states it is
|
||||
"Manual-only maintenance utility. Never called from application code", and
|
||||
0025's "Alternatives considered" explicitly rejected relying on it from the
|
||||
app, partly because it leaves no application log entry. We still want this
|
||||
capability reachable from the app (a hidden, admin-gated action) rather than
|
||||
requiring direct SSMS access, without contradicting that standing decision.
|
||||
|
||||
Additionally, `IBlobStorageService` archives invoice PDFs to Azure Blob
|
||||
Storage under a fixed per-invoice blob name (`{invoiceId}/{fileName}`) and
|
||||
always uploads with `overwrite: true`. If a cleared invoice's document is
|
||||
later regenerated (0025), `StoreInvoiceDocumentFileAsync` re-uploads under
|
||||
that same blob name — silently overwriting the very copy this action is
|
||||
meant to preserve as a backup, unless it is moved first.
|
||||
|
||||
## Decision
|
||||
|
||||
- A **new** proc, `[dbo].[fds__admin_backupAndClearInvoiceFile]`, is added
|
||||
rather than calling `_BackupAndClearInvoiceFile`. It copies the invoice's
|
||||
current `[file]` bytes into a new table, `[dbo].[fds__invoice_file_backups]`
|
||||
(one row per backup, not a timestamp-named ad-hoc table per call), then
|
||||
clears `[file]` in the same transaction (`XACT_ABORT` + TRY/CATCH with
|
||||
rollback and re-raise, so a clear can never commit without its backup row).
|
||||
It logs through the normal
|
||||
`fds__admin_activity` insert (matching `fds__setInvoiceFile`'s pattern) and
|
||||
every call is also logged through the app's `ILogger`/DB-log-sink pipeline
|
||||
(ADR 0024) — unlike the manual proc, this path is fully auditable.
|
||||
`_BackupAndClearInvoiceFile` is left completely untouched as the manual
|
||||
break-glass tool for cases the app can't reach (e.g. a corrupt row the
|
||||
admin UI can't load).
|
||||
- The action applies 0025's regeneration precondition **before** touching
|
||||
anything: only a **finalised, non-external** invoice qualifies. Both the
|
||||
service (`IsDraft`/`IsExternal`) and the proc (`[DateFinalized] IS NOT NULL
|
||||
AND ISNULL([IsExternal],0) = 0`) enforce it. Clearing a draft's `[file]`
|
||||
would let the next read persist a draft render through
|
||||
`fds__setInvoiceFile` (`WHERE [file] IS NULL`), which then blocks the real
|
||||
final PDF forever — the exact defect 0025 calls load-bearing; an external
|
||||
(MFR) document has no template and would just become a permanent 404.
|
||||
- `IBlobStorageService` gains `CopyInvoicePdfToBackupAsync`: a
|
||||
**server-side copy** (`StartCopyFromUriAsync`, bytes never pass through
|
||||
the app) of the existing blob to a timestamped backup name in the same
|
||||
container — the original blob is **kept**, not renamed. A private source in
|
||||
the same storage account is authorised by the request's own Shared Key
|
||||
credential, so no SAS is generated. The copy runs *before* the DB clear,
|
||||
so the archived original is safe before any regenerate-on-read (ADR 0025)
|
||||
could overwrite the original name; because the original stays in place, a
|
||||
DB error or no-op afterwards leaves the canonical archive exactly as it
|
||||
was (with a harmless spare copy). Keeping the original also removes the
|
||||
half-state a copy-then-delete would have ("copied but couldn't delete").
|
||||
Best-effort: a copy failure is logged and counted
|
||||
(`fuchs.blobstorage.backupcopies.failed`), the DB backup row stays the
|
||||
authoritative copy, and the DB step still runs.
|
||||
- The action is exposed as a `Do_Process_Admin` case,
|
||||
`admin/backupclearinvoicefile/<invoiceId>`, gated by the existing
|
||||
`fds_sys > 4` check and restricted to `POST` (a GET must never trigger it,
|
||||
since GETs can be issued by crawlers/prefetchers without user intent). It
|
||||
is not part of any regular workflow UI; its only frontend is a card on the
|
||||
admin-only "Specialties" page (`/specialties`, see
|
||||
`Docs/Concepts/admin-specialties.md`).
|
||||
|
||||
## Consequences
|
||||
|
||||
- Two invoice-file "backup and clear" code paths now exist for different
|
||||
audiences: `_BackupAndClearInvoiceFile` (manual, SSMS, no app log) and
|
||||
`fds__admin_backupAndClearInvoiceFile` (app-triggered, fully logged). Keep
|
||||
them separate — do not merge them, and do not point the manual proc's
|
||||
printed recovery links at the new admin action.
|
||||
- `fds__invoice_file_backups` accumulates one row per backup with no
|
||||
automatic retention/cleanup; if this ever becomes a storage concern, that
|
||||
is a separate decision.
|
||||
- Blob backup-copy is best-effort: if it fails (or storage is
|
||||
disabled/unconfigured), the invoice's DB row is still backed up and
|
||||
cleared — the archived blob is simply left as-is (and will be overwritten
|
||||
by the next regeneration). The DB backup table is the only *guaranteed*
|
||||
copy this action makes.
|
||||
- The blob container accumulates one `backup_<timestamp>_…` copy per action
|
||||
next to the current blob; like `fds__invoice_file_backups`, no retention
|
||||
is applied.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **Call `_BackupAndClearInvoiceFile` directly from the app.** Rejected: its
|
||||
own header comment and ADR 0025 both state it must never be
|
||||
application-triggered, and it produces no application log entry — using it
|
||||
here would need updating that comment and (since decisions are immutable)
|
||||
superseding 0025's "Alternatives considered" note, for no real benefit over
|
||||
a small dedicated proc.
|
||||
- **Skip the blob rename and let the next regeneration overwrite the archived
|
||||
blob.** Rejected: the user asked for the blob copy to also survive as a
|
||||
backup, and a silent overwrite on next read would defeat that.
|
||||
@@ -0,0 +1,91 @@
|
||||
---
|
||||
status: Accepted
|
||||
date: 2026-09-17
|
||||
applyTo:
|
||||
- "Fuchs/Services/IComService.cs"
|
||||
- "Fuchs/Services/ProcessWebComService.cs"
|
||||
- "Fuchs/Services/ProcessWebComSettings.cs"
|
||||
- "Fuchs/Services/MailerAttachmentPolicy.cs"
|
||||
supersededBy: ""
|
||||
---
|
||||
|
||||
# 0028 — Mailer push_com: multipart attachments, count confirmation, contract pre-validation
|
||||
|
||||
## Context
|
||||
|
||||
The ProcessWeb Mailer API (`push_com`) was extended to accept attachments.
|
||||
Its contract — https://processweb.de/api/mailer/fc — specifies
|
||||
`multipart/form-data` with the JSON envelope in an unnamed form field and one
|
||||
file part per attachment, an optional `attachmentCount` safety check, and a
|
||||
200 response reporting `attachmentsReceived`/`attachmentsProcessed`.
|
||||
|
||||
`ProcessWebComService` predated that: it base64-inlined attachments as
|
||||
`communication.attachments[]` in a JSON body. That property is not part of
|
||||
the contract, so the Mailer accepted the message and dropped the files — a
|
||||
200 for an invoice mail whose PDF never arrived, with no error anywhere. The
|
||||
client also fabricated the audit `guid` (`Guid.NewGuid()`), treated a 409
|
||||
(idempotency duplicate) as a generic API error, and called the legacy
|
||||
`/mailer/fc?fn=verify` and `api.processweb.de` paths.
|
||||
|
||||
## Decision
|
||||
|
||||
1. **Request shape follows the contract exactly.** No attachments →
|
||||
`application/json` envelope. With attachments → `multipart/form-data`:
|
||||
envelope in an unnamed field (`name=""`, set via an explicit
|
||||
`Content-Disposition` because `MultipartFormDataContent.Add` rejects an
|
||||
empty name), then one file part per attachment with real bytes. Field
|
||||
names as in the contract (`comType`, not `comtype`). Base URL
|
||||
`https://processweb.de`, paths `/api/mailer/push_com` and
|
||||
`/api/mailer/verify`.
|
||||
2. **`attachmentCount` is always sent** so the Mailer itself rejects a
|
||||
request whose file parts don't match.
|
||||
3. **A 200 is not success when attachments were sent unless
|
||||
`attachmentsProcessed` equals the number sent.** Anything else — fewer
|
||||
processed, or a body that isn't the contract JSON — is reported to the
|
||||
caller as a failed send (`false`, reason `attachments-mismatch`, counts
|
||||
in the audit log). Callers gate "mark as sent" on that boolean, so an
|
||||
invoice whose PDF was not processed is never flagged as sent.
|
||||
4. **Contract limits are pre-validated client-side** (`MailerAttachmentPolicy`:
|
||||
count, per-file and per-message size, empty file, missing/forbidden
|
||||
extension) with the contract's defaults configurable under
|
||||
`Fuchs:Mailer:Attachments`. A violation fails before any bytes are sent,
|
||||
with the exact rule in the audit log. Empty files are a violation, not
|
||||
silently skipped as before.
|
||||
5. **`comStackID` is the audit key**, falling back to the `Message-Id`
|
||||
header; a local id only when the Mailer returned neither.
|
||||
6. **409 is a distinct failure** (`duplicate`): the Mailer already holds a
|
||||
message under this `reference` and did not queue this one.
|
||||
|
||||
## Consequences
|
||||
|
||||
- `IComService.SendEmailAsync` returning `true` now means "accepted *and*
|
||||
every attachment confirmed processed" — strictly stronger than before.
|
||||
Existing callers (`req/sconf`, reminders, admin test mail) need no change;
|
||||
they already treat `false` as not-sent.
|
||||
- A caller passing a 0-byte attachment now gets `false` instead of a mail
|
||||
without the file. That is intended: an empty PDF is an upstream failure
|
||||
that should surface, not be hidden by a successful-looking send.
|
||||
- The forbidden-extension list in the client is the contract's quoted subset;
|
||||
the server's longer list stays authoritative and still answers 400 for the
|
||||
rest.
|
||||
- The concept doc `Docs/Concepts/processweb-mailer-interface.md` describes
|
||||
the current client behaviour and must be kept in sync when the contract
|
||||
evolves (it is declared backward compatible, so additions rather than
|
||||
breaks are expected).
|
||||
- The verify response is still parsed as `{ ok, account, serverid }`; if the
|
||||
`/api/mailer/verify` endpoint's body differs from the legacy `fc?fn=verify`
|
||||
one, only `VerifyCredentialsAsync`'s DTO needs adjusting.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **Keep inline base64 and ask the Mailer to accept it too.** Rejected: the
|
||||
contract is the Mailer's; two accepted shapes would just be a second
|
||||
thing to keep compatible, and base64 inflates every PDF by a third.
|
||||
- **Treat 200 as success regardless of `attachmentsProcessed`.** Rejected —
|
||||
that is exactly the silent-drop failure mode this change exists to close.
|
||||
- **Treat 409 as success ("already sent").** Rejected: a resend under the
|
||||
same reference would then report success without anything being queued;
|
||||
the caller must see it and decide (e.g. use a new reference).
|
||||
- **Skip client-side validation and rely on the server's 400.** Rejected:
|
||||
the 400 body is an opaque string in the audit log; a named rule is what
|
||||
support needs, and it costs nothing.
|
||||
@@ -0,0 +1,101 @@
|
||||
---
|
||||
status: Accepted
|
||||
date: 2026-09-17
|
||||
applyTo:
|
||||
- "Fuchs/Services/ProcessWebComService.cs"
|
||||
- "Fuchs/Services/FuchsMailerClient.cs"
|
||||
- "Fuchs/Services/MailerAttachmentPolicy.cs"
|
||||
- "Fuchs/Services/ProcessWebComSettings.cs"
|
||||
- "Fuchs/Program.cs"
|
||||
supersededBy: ""
|
||||
---
|
||||
|
||||
# 0029 — Outbound mail goes through the OCORE_web PWMailer client
|
||||
|
||||
## Context
|
||||
|
||||
Right after [0028](0028-mailer-push-com-multipart-and-attachment-confirmation.md)
|
||||
re-implemented the Mailer API's `push_com` contract by hand in
|
||||
`ProcessWebComService`, OCORE_web gained a complete typed client for that
|
||||
API (`OCORE_web/comm/pwmailer/`, `IPWMailerClientService`): every operation,
|
||||
the JSON/multipart request shapes, `attachmentCount`, typed receipts,
|
||||
Basic auth with Key Vault/encrypted token resolution, `x-pcw-excd` error
|
||||
headers, retries and timeouts — declared to be kept in sync with the
|
||||
server-side controllers. Keeping a second implementation of the same
|
||||
contract in Fuchs would mean two things to update on every API change.
|
||||
|
||||
The client validated a message's required fields and empty/extension-less
|
||||
attachments, but had no hook for application rules (size limits, a
|
||||
file-type deny-list) — `PWMailerMessage` is sealed and `PushAsync` called
|
||||
`Validate` directly.
|
||||
|
||||
## Decision
|
||||
|
||||
1. **`ProcessWebComService` is an adapter over `IPWMailerClientService`.**
|
||||
It keeps only what is Fuchs-specific: recipient validation and the
|
||||
`Fuchs:Email:OverrideRecipient` safety net, the signature, building the
|
||||
`PWMailerMessage`, interpreting the `PWMailerResult`, the `fds__emaillog`
|
||||
audit row and telemetry. All HTTP transport lives in the OCORE client;
|
||||
the hand-rolled `PostPushComAsync`/MIME guessing from 0028 is deleted.
|
||||
2. **Pre-validation is an overridable hook in the client.** OCORE_web's
|
||||
`PWMailerClientService` gains `protected virtual ValidateMessage(PWMailerMessage)`
|
||||
(default: `PWMailerMessage.Validate`), called by `PushAsync` and
|
||||
`PushBulkAsync` before anything is sent. Fuchs overrides it in
|
||||
`FuchsMailerClient` to add the contract's attachment limits
|
||||
(`MailerAttachmentPolicy`: count, per-file/per-message size, forbidden
|
||||
extensions, configurable under `Fuchs:Mailer:Attachments`). A refusal is a
|
||||
`PWMailerResult` with `StatusCode 0` / `Attempts 0`, which the adapter
|
||||
reports as reason `validation` with one audit line per rule.
|
||||
3. **One configuration section.** `Fuchs:Mailer` (`ProcessWebComSettings`)
|
||||
remains the source; `Program.cs` maps it onto `PWMailerClientOptions`
|
||||
(`BaseUrl + /api/mailer`, `AccountId → Account`, `Token`, `ServerId`) and
|
||||
sets `TokenSecretName = "Fuchs--Mailer--Token"`, i.e. the Key Vault secret
|
||||
OCORE secret management already maps into `Fuchs:Mailer:Token`. No second
|
||||
`PWMailer` section, no second secret.
|
||||
4. **0028's behavioural rules stand**, with one correction: multipart with
|
||||
`attachmentCount`, a 200 counts as success only when
|
||||
`attachmentsProcessed` equals the number sent, `comStackID` is the audit
|
||||
key. **409 is now a success, not a `duplicate` failure** (this replaces
|
||||
0028's rule 6): the client documents 409 as "duplicate of a still-pending
|
||||
entry with the same server/reference/content — nothing was enqueued, the
|
||||
earlier entry will be sent". The mail is therefore on its way; the resend
|
||||
paths (`req/resend`, reminders) reuse `inv_<Id>` as the reference, and a
|
||||
double click must not surface as a send error. The deduplication is
|
||||
recorded in the audit row. They are enforced partly by the client (shape,
|
||||
count) and partly by the adapter (processed check, audit, 409 mapping).
|
||||
5. Two small OCORE_web fixes made in the same change: `PWMailerMessage.Validate`
|
||||
no longer demands a subject for `comType "s"` (SMS), and a call refused
|
||||
by validation reports `Attempts = 0` as its documentation already stated.
|
||||
|
||||
## Consequences
|
||||
|
||||
- Fuchs no longer asserts the wire format in its own tests; that lives in
|
||||
`OCORE_web/OCORE_webTests/comm/PWMailerClientServiceTests.cs`. Fuchs tests
|
||||
assert the `PWMailerMessage` handed to the client and the interpretation of
|
||||
each `PWMailerResult` shape, plus `FuchsMailerClient`'s refusals.
|
||||
- OCORE_web is a submodule owned by another repository: the hook, the SMS
|
||||
rule and the `Attempts` fix are a commit there and a pointer bump here.
|
||||
- The client is a singleton with a cached token; after a token rotation
|
||||
`IPWMailerClientService.ResetToken()` (on the concrete type) is the way to
|
||||
pick it up without a restart — not wired to any admin action yet.
|
||||
- Empty attachments are still refused (now by the client's base rule), so
|
||||
an upstream render failure never produces a mail without its PDF. The
|
||||
flip side: a caller attaching an *optional* file must skip an empty byte
|
||||
array itself — the reminder's invoice copy (`IntranetController.Reminder.cs`)
|
||||
does, otherwise a never-stored invoice file would fail the whole reminder
|
||||
instead of sending it without the copy.
|
||||
- The client's `getCom`/`getComStatus`/`abortCom` and inbox operations are
|
||||
available to Fuchs for free (e.g. a delivery-status column keyed by the
|
||||
audited `comStackID`) — not used yet.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **Keep the 0028 hand-rolled transport.** Rejected: duplicate contract
|
||||
implementation, and the OCORE client is the one declared to track the
|
||||
server.
|
||||
- **Pre-validate in the adapter only (before `PushAsync`).** Rejected: the
|
||||
rules belong with the client's own validation so every OCORE consumer can
|
||||
apply them the same way, and the user asked for an overridable hook.
|
||||
- **Bind a separate `PWMailer` config section.** Rejected: `Fuchs:Mailer` is
|
||||
already Key Vault-managed and read by the Admin status view; mapping in
|
||||
code avoids a second token secret and drift between two sections.
|
||||
Reference in New Issue
Block a user