Skip to content

Preserve password expiry countdowns in notification history - #5456

Merged
nilsteampassnet merged 1 commit into
nilsteampassnet:developfrom
guerricv:fix_pwd_notification
Nov 9, 2026
Merged

nilsteampassnet merged 1 commit into
nilsteampassnet:developfrom
guerricv:fix_pwd_notification

Conversation

@guerricv

@guerricv guerricv commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Description

Opening the notification inbox a week after a password's D-14 warning made both the D-14 and D-7 entries say "expires in 7 days". Each entry recomputed its countdown from the absolute expiry and the browser's current time, making separate warning milestones look like duplicate notifications.

Use the days remaining recorded at delivery so each message stays consistent with its displayed creation date. This also preserves the original tomorrow/expired messages and warnings delivered between milestones. Existing stored notifications already contain this value, so they benefit immediately without a migration. Password policy enforcement and notification scheduling are unchanged.

Published commit: 8c2c0c6fc — Preserve password expiry notification countdowns.

Related issue

Reported from a personal instance; no linked issue.
image

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (existing behaviour changes)
  • Documentation
  • Translation
  • Refactor / maintenance

How has this been tested?

  • Four JavaScript regression tests execute the production renderers with a controlled clock: distinct weekly milestones, later reads (including after expiry), delivery between milestones, and tomorrow/expired messages with or without an expiry timestamp. All four fail before the fix and pass afterwards. Locally run with the VS Code Node runtime and --test --test-isolation=none because Windows sandboxing prevents child-process creation.
  • php _tools/phpunit.phar tests/Unit/NotificationCenterLogicTest.php: 16 tests, 61 assertions passed with PHP 8.2.34.
  • PHP syntax check, table-prefix guard (86 table names) and Git diff checks passed.
  • Local full PHPStan cannot start because the installation-only app/config/settings.php is absent. On commit 8c2c0c6fc, GitHub Actions passed the complete PHP 8.2 and PHP 8.3 suites (3,564 tests / 84,111 assertions each), JavaScript tests, PHPStan level 4, MariaDB integration tests, CodeQL, and both repository guards. Scrutinizer also passed. All CI checks are green on this final commit.
  • No manual role or browser testing was performed. The shared renderer applies to administrators and other local accounts; personal-folder settings do not affect it.

Checklist

  • PHPStan level 4 passes (php app/vendor/bin/phpstan analyse --memory-limit=2G)
  • The test suite passes (php _tools/phpunit.phar — see CONTRIBUTING.md for the one-time setup)
  • Every new public function has a docblock (no new public application function)
  • Variable names and comments are in English
  • No var_dump() or console.log() left in the code
  • New app/sources/*.queries.php files have a matching public/sources/ proxy shim (none added)
  • Changes to teampassclasses were applied to both copies (app/includes/libraries/ and app/vendor/) (none changed)
  • app/vendor/composer/ is in its production form (unchanged)

Impact on install / upgrade

  • No schema change
  • Schema change — an install/upgrade_run_X.X.X.php script is included and the fresh install path was tested

Screenshots

No visual layout changes; the regression tests cover the notification text.

@guerricv
guerricv marked this pull request as ready for review November 9, 2026 15:47
@nilsteampassnet
nilsteampassnet merged commit b0c0c0b into nilsteampassnet:develop Nov 9, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants