Skip to content

feat: add mail_sender_strategy policy to send notifications as the requester - #8106

Open
maia-andre wants to merge 5 commits into
LibreSign:mainfrom
maia-andre:feat/7954-mail-sender-strategy
Open

feat: add mail_sender_strategy policy to send notifications as the requester#8106
maia-andre wants to merge 5 commits into
LibreSign:mainfrom
maia-andre:feat/7954-mail-sender-strategy

Conversation

@maia-andre

@maia-andre maia-andre commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Pull Request Description

Implements the mail_sender_strategy policy described in #7954 so LibreSign can send signature request notification emails either through the system mailer (current behavior) or through the mail account of the person who requested the signature.

Backend

  • New MailSenderStrategyPolicy (lib/Service/Policy/Provider/MailSenderStrategy/), registered in PolicyProviders. Values: system (default) and requester. System scope only in this first step (supportedScopes: ['system'], no user preference, no group delegation), since per-user choice is out of scope.
  • MailService::notifyUnsignedUser() and notifySignDataUpdated() now resolve the policy for the requester (resolveForUserId() on the file owner, so the same strategy applies to reminders sent from cron, where there is no session user).
  • Under requester, the notification is sent through OCP\Mail\Provider\IManager: the service is looked up by the requester's email address (findServiceByAddress()), falling back to the first service of the account that implements IMessageSend. The message reuses the existing IEMailTemplate (renderSubject(), renderHtml(), renderText()), so content and templating are unchanged.
  • Fallback to IMailer (identical to the current code path) when there is no mail provider, the requester account is missing, no service can send messages, or provider sending throws. Logging: info for the "not available" cases, warning (with the exception) when provider sending fails. Errors from the final IMailer path keep raising LibresignException as before.

Configuration vs. runtime (follow-up of the review in #7954)

  • The requester strategy can only be saved while a mail provider is registered (OCP\Mail\Provider\IManager::has(), independent of any specific app). This uses a new persistence-only hook in the policy framework, IPolicyDefinition::validateValueForPersistence(), called from the PolicyService save paths. Runtime resolution keeps using validateValue(), so an already stored requester value is not silently discarded by DefaultPolicyResolver when the environment changes later; instead MailService falls back at sending time.
  • The availability is exposed as meta.mailProviderAvailable (OpenAPI spec and generated types updated) and the Policy Workbench editor disables the requester option with a hint when no provider is available.
  • When the strategy is requester and the notification falls back to the system mailer, Reply-To is set to the requester's address, so replies still reach the person who requested the signature. The system strategy is unchanged.

Frontend

  • Policy Workbench definition under src/views/Settings/PolicyWorkbench/settings/mail-sender-strategy/ (radio editor with the two strategies), added to the system-behavior category in realDefinitions.ts.

Tests

  • MailSenderStrategyPolicyTest: definition, scope restriction, normalization and validation, provider required on save (and stored value still valid at runtime), resolved meta.
  • PolicySpecTest: the persistence-only validation hook.
  • MailServiceTest: system strategy, successful requester sending (from/to/subject/html/plain), recipient label handling, every fallback path (no provider, unknown requester, missing account, no sendable service, provider failure, fallback failure) and the Reply-To behavior on fallback.
  • realDefinitions.spec.ts, mail-sender-strategy/*.spec.ts: workbench card, definition callbacks, model and editor (including the disabled state).

Checks run locally: PHPUnit unit suite, psalm and php-cs-fixer on the changed files, npm run lint, vue-tsc --noEmit, vitest for the Policy Workbench suites.

Manual verification

Devcontainer with Mailpit as the system mailer and no mail provider app installed, so the requester strategy exercises the fallback path end to end.

  1. New card in the Policy Workbench (System behavior), default System mailer:

    Workbench card with the default value

  2. Rule editor with the two strategies:

    Rule editor options

  3. Selecting Requester mail account and saving the system rule:

    Requester selected

    Policy dialog after saving

  4. Card after saving (occ config:app:get libresign mail_sender_strategyrequester):

    Workbench card with requester

  5. Signature request sent through the API with the policy set to requester: the notification is still delivered, through the system mailer, and the fallback is logged at info level:

    Mailpit delivery

    {"level": 1, "app": "libresign", "message": "No mail provider is available to send the notification as the requester, falling back to the system mailer.", "requester": "admin"}
  6. Follow-up of the review: on the same instance (no mail provider), the requester option can no longer be configured. The editor disables it with a hint, and the API rejects it with 400:

    Requester option disabled without a mail provider

    POST /ocs/v2.php/apps/libresign/api/v1/policies/system/mail_sender_strategy  {"value":"requester"}
    HTTP 400  {"ocs":{"meta":{"status":"failure","statuscode":400},"data":{"error":"The requester strategy requires an available mail provider"}}}
    

    The screenshots in steps 3–5 were taken before this change; they show the rule saved while the check did not exist yet, which is exactly the "environment changed later" case that the runtime fallback (and now Reply-To) covers.

Related Issue

Fixes #7954

Pull Request Type

  • Feature

Pull request checklist

  • The policy supports system and requester
  • The policy is available in the backend and Policy Workbench
  • system preserves the current behavior
  • requester attempts provider-based sending
  • Falls back to IMailer when no provider, account or sending service is available, or when sending fails
  • requester cannot be configured while no mail provider is available (persistence-time check)
  • Reply-To set to the requester when falling back to the system mailer
  • Applied to notifyUnsignedUser and notifySignDataUpdated
  • Tests cover system, successful requester and fallback
  • AI-assisted: yes (see commit trailer)

…quester

Add the mail_sender_strategy policy (system scope only) with two values:
system, the current behavior through OCP\Mail\IMailer, and requester,
which tries to send the signature request notification through the mail
account of the person who requested the signature using
OCP\Mail\Provider\IManager.

MailService::notifyUnsignedUser() and notifySignDataUpdated() resolve the
policy for the requester (the file owner) so reminders sent from cron use
the same strategy. Under requester, the service is looked up by the
requester email address, falling back to the first account service that
implements IMessageSend, and the existing IEMailTemplate is reused for the
subject and bodies. When there is no provider, no account, no sendable
service, or provider sending fails, the notification falls back to the
system mailer (logged at info/warning level), keeping the current error
handling for that path.

Expose the policy in the Policy Workbench under "System behavior" with a
radio editor for the two strategies.

Fixes LibreSign#7954

Assisted-by: Claude Code:claude-fable-5
Signed-off-by: André Maia <andrefnkmm@gmail.com>
@codecov-commenter

codecov-commenter commented Aug 28, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Files with missing lines Coverage Δ
...l-sender-strategy/MailSenderStrategyRuleEditor.vue 100.00% <100.00%> (ø)
...cyWorkbench/settings/mail-sender-strategy/model.ts 100.00% <100.00%> (ø)
...ch/settings/mail-sender-strategy/realDefinition.ts 100.00% <100.00%> (ø)
...ttings/PolicyWorkbench/settings/realDefinitions.ts 100.00% <ø> (ø)

... and 208 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…editor

Codecov reported 14 uncovered lines in the new frontend files. Add specs
mirroring the existing tsa/signing-mode ones: model normalization,
every callback of the real definition (draft value, fallback default,
summary, override message) and the rule editor (rendering, selection
state, emitted strategy and ignored deselection).

Ref LibreSign#7954

Assisted-by: Claude Code:claude-fable-5
Signed-off-by: André Maia <andrefnkmm@gmail.com>
@maia-andre
maia-andre force-pushed the feat/7954-mail-sender-strategy branch from 238d872 to 3b00b4e Compare August 28, 2026 21:02
…tions

Some constraints only make sense when an administrator saves a value,
for example requiring an external capability to be available. Running
them during runtime resolution would make DefaultPolicyResolver discard
an already stored value as soon as the environment changes, instead of
letting the consumer fall back gracefully.

Add IPolicyDefinition::validateValueForPersistence(), implemented by
PolicySpec as validateValue() plus an optional persistenceValidator
closure, and call it from the PolicyService save paths (system, group,
user preference and user policy). Runtime resolution keeps using
validateValue().

Ref LibreSign#7954

Assisted-by: Claude Code:claude-fable-5
Signed-off-by: André Maia <andrefnkmm@gmail.com>
…trategy

Configuration and runtime behavior are now separated, as requested in
the issue: the requester strategy can only be saved while a mail
provider is registered (OCP\Mail\Provider\IManager::has()), because we
already know it cannot work otherwise. The check uses the persistence
hook, so an already stored value keeps resolving at runtime and
MailService falls back to the system mailer when the environment
changes later. The check looks for any mail provider instead of a
specific app.

Expose the availability as mailProviderAvailable in the resolved policy
meta and use it in the Policy Workbench editor to disable the requester
option with a hint. The OpenAPI spec and generated types are updated
for the new meta field.

Ref LibreSign#7954

Assisted-by: Claude Code:claude-fable-5
Signed-off-by: André Maia <andrefnkmm@gmail.com>
…ailer

When the requester strategy is configured but the notification cannot
leave the requester mail account (no provider anymore, account removed,
no sendable service or a sending failure), the system mailer now sets
Reply-To to the requester address, so replies still reach the person
who requested the signature. The system strategy is unchanged.

Ref LibreSign#7954

Assisted-by: Claude Code:claude-fable-5
Signed-off-by: André Maia <andrefnkmm@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 0. Backlog

Development

Successfully merging this pull request may close these issues.

[FEATURE] When sending a sign request send from the users email rather than the system email.

2 participants