Skip to content

refactor(share): DRY up expiration date validation and fix dispatchEvent() log message#59731

Open
joshtrichards wants to merge 1 commit into
masterfrom
jtr/refactor-share-dry-exp-dat-validation
Open

refactor(share): DRY up expiration date validation and fix dispatchEvent() log message#59731
joshtrichards wants to merge 1 commit into
masterfrom
jtr/refactor-share-dry-exp-dat-validation

Conversation

@joshtrichards
Copy link
Copy Markdown
Member

@joshtrichards joshtrichards commented Apr 20, 2026

Summary

Extracts the duplicated expiration-date validation logic in Share20/Manager.php into a new validateExpirationDate() helper.

Both validateExpirationDateInternal() and validateExpirationDateLink() implemented the same algorithm for:

  • Skipping validation when no expiration date is allowed and none is enforced
  • Rejecting past expiration dates
  • Applying a default expiration date for new shares
  • Enforcing the maximum allowed expiration date
  • Running the verifyExpirationDate hook

The public-facing validateExpirationDateInternal() and validateExpirationDateLink() are preserved as thin wrappers that resolve their share-type-specific policy values and delegate to the new helper, keeping the API unchanged.

Also fixed (unrelated): A string interpolation bug in dispatchEvent() caused the error log message to render literally as Error while sending ' . $eventName . ' event instead of Error while sending 'EventName' event.

No functional change intended

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Both `validateExpirationDateInternal()` and `validateExpirationDateLink()` implemented the
same algorithm for:

- skipping validation when no expiration date is allowed
- rejecting past expiration dates
- applying a default expiration date for new shares
- enforcing the maximum allowed expiration date
- running the `verifyExpirationDate` hook

This change keeps the existing behavior but centralizes the logic so future fixes only need to be made in one place.

This commit also fixes a tiny unrelated quoting bug in `dispatchEvent()` that caused garbled error-log output in `dispatchEvent()`.

Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards added feature: sharing bug ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) labels Apr 20, 2026
@joshtrichards joshtrichards changed the title refactor(share): extract common share expiration date validation logic refactor(share): DRY up expiration date validation and fix dispatchEvent() log message Apr 20, 2026
@joshtrichards joshtrichards added the 3. to review Waiting for reviews label Apr 21, 2026
@joshtrichards joshtrichards added this to the Nextcloud 34 milestone Apr 21, 2026
@joshtrichards joshtrichards marked this pull request as ready for review April 21, 2026 01:18
@joshtrichards joshtrichards requested a review from a team as a code owner April 21, 2026 01:18
@joshtrichards joshtrichards requested review from artonge, leftybournes, provokateurin and salmart-dev and removed request for a team April 21, 2026 01:18
@artonge
Copy link
Copy Markdown
Collaborator

artonge commented Apr 21, 2026

I know @provokateurin is working on files_sharing currently, this might overlap or conflict.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews bug feature: sharing ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants