Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Beautify test email #4379

Merged
merged 1 commit into from
Apr 18, 2017
Merged

Beautify test email #4379

merged 1 commit into from
Apr 18, 2017

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 <lukas@statuscode.ch>
@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
Was removed in #4379 (0a54d5a) and #4380 (bae64e8)

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
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