Skip to content
Snippets Groups Projects

improve update status email subject

Closes #1818764

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Jess
  • Jess
  • Jess
  • Jess
  • added 1 commit

    • 7c880d4b - Issue #1818764 by xurizaemon, quietone, haydeniv, amitgoyal, smustgrave, xjm,...

    Compare with previous version

  • Dhruv Mittal added 1521 commits

    added 1521 commits

    Compare with previous version

  • Daniel Rodriguez added 211 commits

    added 211 commits

    • 05379fea...10a80a91 - 210 commits from branch project:11.x
    • 8d611611 - Issue # 1818764: Fixed conflicts after rebasing at file:...

    Compare with previous version

  • added 6 commits

    Compare with previous version

  • added 1 commit

    • f308ee46 - Issue # 1818764: Fixed some PHPCS and PHPSTAN issues reported by the pipeline jobs

    Compare with previous version

  • added 1 commit

    • d2c43240 - Issue # 1818764: Added missing argument () in the testUpdateEmail() test method

    Compare with previous version

  • quietone added 5 commits

    added 5 commits

    • 1d8d0b03 - Update message and add tests
    • c32e0dd0 - Rework the logic for the subject line
    • 57fe4048 - Issue #1818764 by xurizaemon, quietone, haydeniv, amitgoyal, smustgrave, xjm,...
    • d8782a3a - Issue # 1818764: Fixed some PHPCS and PHPSTAN issues reported by the pipeline jobs
    • daf7306f - fix rebase

    Compare with previous version

  • Tyler Staples added 1 commit

    added 1 commit

    • d945ccc5 - Issue #1818764: Update subject lines to be 80 characters long to appease the review bot.

    Compare with previous version

  • quietone added 170 commits

    added 170 commits

    • d945ccc5...a2db325e - 164 commits from branch project:11.x
    • 74cea444 - Update message and add tests
    • ce5b582c - Rework the logic for the subject line
    • 52e82b95 - Issue #1818764 by xurizaemon, quietone, haydeniv, amitgoyal, smustgrave, xjm,...
    • ae964b6a - Issue # 1818764: Fixed some PHPCS and PHPSTAN issues reported by the pipeline jobs
    • 00540182 - fix rebase
    • 41d150d6 - Issue #1818764: Update subject lines to be 80 characters long to appease the review bot.

    Compare with previous version

  • quietone added 1 commit
  • Stephen Mustgrave resolved all threads

    resolved all threads

  • 200 200 $langcode = $message['langcode'];
    201 201 $language = \Drupal::languageManager()->getLanguage($langcode);
    202 $message['subject'] .= $this->t('New release(s) available for @site_name', ['@site_name' => \Drupal::config('system.site')->get('name')], ['langcode' => $langcode]);
    202
    203 // Set the message subject.
    204 $subject = '';
    205 $all_reasons = array_unique(array_values($params));
    206 // If any of the update reasons in $params are security-related, then use
    207 // the security subject.
    208 $security = array_filter($all_reasons, function ($item) {
    209 return in_array($item, [UpdateManagerInterface::NOT_SECURE, UpdateManagerInterface::REVOKED]);
    210 });
    211 if ($security) {
    212 $subject = $this->t('Security release(s) available for @site_name', [
    213 '@site_name' => \Drupal::config('system.site')
    214 ->get('name'),
  • 213 '@site_name' => \Drupal::config('system.site')
    214 ->get('name'),
    215 'langcode' => $langcode,
    216 ]);
    217 }
    218
    219 // If security updates are not included, check next for fetch failures. If
    220 // all the reasons are fetch failures, then use that as the subject.
    221 if (empty($subject)) {
    222 $fetch = array_filter($all_reasons, function ($item) {
    223 return $item > 0;
    224 });
    225 if (empty($fetch) && !empty($all_reasons)) {
    226 $subject = $this->t('Failed to get release information for @site_name', [
    227 '@site_name' => \Drupal::config('system.site')
    228 ->get('name'),
  • 224 });
    225 if (empty($fetch) && !empty($all_reasons)) {
    226 $subject = $this->t('Failed to get release information for @site_name', [
    227 '@site_name' => \Drupal::config('system.site')
    228 ->get('name'),
    229 'langcode' => $langcode,
    230 ]);
    231 }
    232 }
    233
    234 // If the subject still has not been set, then there are only non-security
    235 // updates. Use the subject for non-security updates.
    236 if (empty($subject)) {
    237 $subject = $this->t('New release(s) available for @site_name', [
    238 '@site_name' => \Drupal::config('system.site')
    239 ->get('name'),
  • Jess
    Jess @xjm started a thread on the diff
  • 158 for ($i = 0; $i < count($expected_body); $i++) {
    159 is_string($message['body'][$i]) ? $this->assertSame($expected_body[$i], $message['body'][$i]) : $this->assertSame($expected_body[$i], $message['body'][$i]->render());
    149 160 }
    150 else {
    151 if (empty($params)) {
    152 $this->assertSame($expected_body[0], $message['body'][0]);
    153 $this->assertSame($expected_body[1], $message['body'][1]->render());
    154 }
    155 else {
    156 $this->assertSame($expected_body[0], $message['body'][0]->render());
    157 $this->assertSame($expected_body[1], $message['body'][1]);
    158 $this->assertSame($expected_body[2], $message['body'][2]);
    159 $this->assertSame($expected_body[3], $message['body'][3]->render());
    160 }
    161 for ($i = 0; $i < count($expected_body); $i++) {
    162 is_string($message['body'][$i]) ? $this->assertSame($expected_body[$i], $message['body'][$i]) : $this->assertSame($expected_body[$i], $message['body'][$i]->render());
    • Comment on lines -143 to +162
      Maintainer

      These ternaries are extremely dense to have on one line and not very readable. It would be better to refactor this somehow. Possibly the ternaries could instead be grouped on earlier lines for local variables, something like:

      $message_subject = is_string($message['subject']) ? $message['subject'] : $message['subject']->render();

      (and etc. for the other assertions).

      Edited by Jess
    • Not sure about that too, I'll try to come up with a solution later today maybe.

    • Please register or sign in to reply
  • Jess
    Jess @xjm started a thread on the diff
  • 182 ],
    182 "New release(s) available for ",
    183 [
    184 "See the available updates page for more information:\nhttps://example.com/admin/reports/updates/settings",
    185 'Your site is currently configured to send these emails when any updates are available. To get notified only for security updates, https://example.com/admin/reports/updates.',
    186 ],
    183 187 ],
    184 188 'security' => [
    185 189 'security',
    186 190 [],
    187 191 FALSE,
    192 "New release(s) available for ",
    188 193 [
    189 194 "See the available updates page for more information:\nhttps://example.com/admin/reports/updates/settings",
    190 'Your site is currently configured to send these emails only when security updates are available. To get notified for any available updates, https://example.com/admin/reports/updates.',
    195 "Your site is currently configured to send these emails only when security updates are available. To get notified for any available updates, https://example.com/admin/reports/updates.",
  • Jess
    Jess @xjm started a thread on the diff
  • 310 ],
    311 'all fetch fail' => [
    312 'all',
    313 [
    314 'core' => UpdateFetcherInterface::NOT_FETCHED,
    315 'contrib' => UpdateFetcherInterface::NOT_FETCHED,
    316 ],
    317 FALSE,
    318 "Failed to get release information for ",
    319 [
    320 'There was a problem checking <a href="https://example.com/admin/reports/updates/settings">available updates</a> for Drupal.',
    321 'There was a problem checking <a href="https://example.com/admin/reports/updates/settings">available updates</a> for your modules or themes.',
    322 "See the available updates page for more information:\nhttps://example.com/admin/reports/updates/settings",
    323 "Your site is currently configured to send these emails when any updates are available. To get notified only for security updates, https://example.com/admin/reports/updates.",
    324 ],
    325 ],
    • Comment on lines +223 to +325
      Maintainer

      It would be good to have an inline comment describing the purpose of each provider scenario here and what we expect for each.

      Edited by Jess
    • Please register or sign in to reply
  • 228 ->get('name'),
    229 'langcode' => $langcode,
    230 ]);
    231 }
    232 }
    233
    234 // If the subject still has not been set, then there are only non-security
    235 // updates. Use the subject for non-security updates.
    236 if (empty($subject)) {
    237 $subject = $this->t('New release(s) available for @site_name', [
    238 '@site_name' => \Drupal::config('system.site')
    239 ->get('name'),
    240 'langcode' => $langcode,
    241 ]);
    242 }
    243 // Update the existing message.
  • Daniel Rodriguez added 246 commits

    added 246 commits

    • b68873fb...6439ca10 - 239 commits from branch project:11.x
    • 1d8d0b03 - Update message and add tests
    • c32e0dd0 - Rework the logic for the subject line
    • 57fe4048 - Issue #1818764 by xurizaemon, quietone, haydeniv, amitgoyal, smustgrave, xjm,...
    • d8782a3a - Issue # 1818764: Fixed some PHPCS and PHPSTAN issues reported by the pipeline jobs
    • daf7306f - fix rebase
    • 189cbbf6 - Merge branch '11.x' into 1818764-correct--improve
    • 06d291b5 - Issue # 1818764 by danrod: Fixed some conflicts

    Compare with previous version

  • added 1 commit

    • 56131b45 - Issue # 1818764 by danrod: Applied some suggested changes by xjm

    Compare with previous version

  • Please register or sign in to reply
    Loading