Remove $authorized param to UpdateMailTest::testUpdateEmail()

Created on 29 July 2025, 5 days ago

Problem/Motivation

I noticed this while reviewing πŸ› Correct & improve update status email subject Postponed . We missed a detail during πŸ“Œ Deprecate authorize.php and the FileTransfer system Active and friends. UpdateMailTest::testUpdateEmail() takes an $authorized bool parameter from its data provider. However, we've deprecated the entire authorize.php world, and there's no longer any difference in the emails sent by Update Status regardless of the permissions of the target user.

Steps to reproduce

Proposed resolution

Remove the param and adjust provider accordingly.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

πŸ“Œ Task
Status

Active

Version

11.0 πŸ”₯

Component

update.module

Created by

πŸ‡ΊπŸ‡ΈUnited States dww

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @dww
  • πŸ‡ΊπŸ‡ΈUnited States dww

    This is going to conflict with πŸ› Correct & improve update status email subject Postponed , let's land that, first.

  • πŸ‡ΊπŸ‡ΈUnited States dww

    Per Slack with @xjm, expanding scope here to actually fix UpdateMailTest on its own, before trying to add more coverage to it at πŸ› Correct & improve update status email subject Postponed .

  • Merge request !12876Task #3538637: Fix problems with UpdateMailTest β†’ (Open) created by dww
  • Pipeline finished with Failed
    4 days ago
    Total: 726s
    #560986
  • Pipeline finished with Canceled
    4 days ago
    #561014
  • πŸ‡ΊπŸ‡ΈUnited States dww

    I believe this is ready. The test is passing locally, so if the pipeline isn't green, it's due to a random fail. πŸ˜…

  • Pipeline finished with Success
    4 days ago
    Total: 483s
    #561015
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    For this one I actually did the side-by-side review as it was easier then inline.

    The matrix of core and contrib hitting every combo of current, not current, not secure, secure appear to be hit.

    Question do we need coverage for REVOKED and NOT_SUPPORTED?

  • πŸ‡ΊπŸ‡ΈUnited States dww

    Thanks for the review!

    Good question about the other statuses. "need" is a strong word. πŸ˜‚ I'm slightly worried that every possible combination of all possible status values for core vs. contrib will explode the size of this data provider. It's not like the logic in _update_message_text() is all that complicated. Seems plausible that we don't need to test every possible code path in there. This MR is already larger than the "combined" MR that prompted splitting this off to a separate issue. πŸ˜… I don't want to explode the scope here more than I already have.

    I vote we get this in ASAP as a significant improvement to what we have now (and to unblock the bugfix that prompted this). If we decide we need more cases later, we can always add them easily enough.

    Thanks,
    -Derek

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Works for me, thanks for the quick reply!

Production build 0.71.5 2024