Skip to content

Beautify test email#4379

Merged
MorrisJobke merged 1 commit intomasterfrom
nicely-designed-confirmation-mail
Apr 18, 2017
Merged

Beautify test email#4379
MorrisJobke merged 1 commit intomasterfrom
nicely-designed-confirmation-mail

Conversation

@LukasReschke
Copy link
Member

screen shot 2017-04-18 at 21 31 35

vs.

screen shot 2017-04-18 at 21 31 28

@LukasReschke LukasReschke added the 3. to review Waiting for reviews label Apr 18, 2017
@LukasReschke LukasReschke added this to the Nextcloud 12.0 milestone Apr 18, 2017
@mention-bot
Copy link

@LukasReschke, thanks for your PR! By analyzing the history of the files in this pull request, we identified @nickvergessen and @MorrisJobke to be potential reviewers.

$message->setTo([$email => $this->userSession->getUser()->getDisplayName()]);
$message->setFrom([$this->defaultMailAddress]);
$message->setSubject($this->l10n->t('test email settings'));
$message->setPlainBody('If you received this email, the settings seem to be correct.');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was not even translated before 🙈

}

return array('data' =>
array('message' =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed this in the other PR today #4372


$message = $this->mailer->createMessage();
$message->setTo([$email => $this->userSession->getUser()->getDisplayName()]);
$message->setFrom([$this->defaultMailAddress]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from doesn't seem to be set now – was that intended? 😉

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also #4380 :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense 👍 thanks

@MorrisJobke
Copy link
Member

Let me rebase and fix the conflicts.

Signed-off-by: Lukas Reschke <[email protected]>
@MorrisJobke MorrisJobke force-pushed the nicely-designed-confirmation-mail branch from 77eeed6 to 0a54d5a Compare April 18, 2017 21:18
@codecov
Copy link

codecov bot commented Apr 18, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@d379ac7). Click here to learn what that means.
The diff coverage is 66.66%.

@@            Coverage Diff            @@
##             master    #4379   +/-   ##
=========================================
  Coverage          ?   54.08%           
  Complexity        ?    21589           
=========================================
  Files             ?     1327           
  Lines             ?    82311           
  Branches          ?     1305           
=========================================
  Hits              ?    44519           
  Misses            ?    37792           
  Partials          ?        0
Impacted Files Coverage Δ Complexity Δ
settings/Controller/MailSettingsController.php 58.02% <66.66%> (ø) 10 <0> (?)

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works 👍

@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Apr 18, 2017
@MorrisJobke MorrisJobke merged commit 4b2d594 into master Apr 18, 2017
@MorrisJobke MorrisJobke deleted the nicely-designed-confirmation-mail branch April 18, 2017 22:18
MorrisJobke added a commit that referenced this pull request Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants