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

Update email template for lost password email #4308

Merged
merged 1 commit into from
Apr 13, 2017

Conversation

MorrisJobke
Copy link
Member

@MorrisJobke MorrisJobke commented Apr 11, 2017

Before and after:

bildschirmfoto 2017-04-11 um 17 14 23

bildschirmfoto 2017-04-11 um 17 12 49

@mention-bot
Copy link

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

@MorrisJobke MorrisJobke added 3. to review Waiting for reviews design Design, UI, UX, etc. enhancement labels Apr 11, 2017
@MorrisJobke MorrisJobke added this to the Nextcloud 12.0 milestone Apr 11, 2017
@MorrisJobke
Copy link
Member Author

requires #4307 to be merged first (only review the last commit in this PR)

64873bd

I'm here as well open for help with the texts - cc @nextcloud/designers @Espina2

@codecov-io
Copy link

Codecov Report

Merging #4308 into master will increase coverage by 0.05%.
The diff coverage is 33.33%.

@@             Coverage Diff              @@
##             master    #4308      +/-   ##
============================================
+ Coverage     54.14%   54.19%   +0.05%     
  Complexity    21409    21409              
============================================
  Files          1327     1322       -5     
  Lines         81812    81756      -56     
  Branches       1305     1305              
============================================
+ Hits          44298    44310      +12     
+ Misses        37514    37446      -68
Impacted Files Coverage Δ Complexity Δ
apps/sharebymail/lib/ShareByMailProvider.php 61.11% <0%> (+0.63%) 62 <0> (-3) ⬇️
lib/private/Mail/EMailTemplate.php 72.22% <75%> (-0.8%) 24 <3> (+3)
core/Controller/LostController.php 82.67% <90%> (+0.03%) 26 <0> (ø) ⬇️
apps/files_external/lib/Lib/Storage/SMB.php 47.22% <0%> (+0.39%) 112% <0%> (ø) ⬇️
lib/private/Files/Cache/Propagator.php 96.2% <0%> (+1.26%) 16% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6bd1c50...64873bd. Read the comment docs.

@nickvergessen
Copy link
Member

Conflicting files
lib/private/Mail/EMailTemplate.php

@Espina2
Copy link
Contributor

Espina2 commented Apr 12, 2017

Why not something like this?

new password

Copy link
Member

@schiessle schiessle left a comment

Choose a reason for hiding this comment

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

Seems like the plain text version suffers from the same issues we discussed and solved here: #4307

So I would suggest to wait for the other PR to get merged, then rebase and adjust it.

@rullzer
Copy link
Member

rullzer commented Apr 12, 2017

#4307 is in. Rebase needed

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke
Copy link
Member Author

I rebased this one and fixed the plain text email:

Password reset

Click the following link to reset your password. If you have not requested the password reset, then ignore this email.

http://192.168.99.100/server/lostpassword/reset/form/KC0Va7NeBbrnXVK11UVLm/admin

--
Nextcloud - ein sicherer Ort für all Ihre Daten
Dies ist eine automatisch generierte E-Mail, bitte nicht antworten.

bildschirmfoto 2017-04-12 um 15 17 06

Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

Whee!

@enoch85
Copy link
Member

enoch85 commented Apr 12, 2017

Would it be possible to implement @Espina2 fix and use the Sessions + add more text?

@eppfel
Copy link
Member

eppfel commented Apr 13, 2017

Yeah, @Espina2 copy would be much appreciated.

Too minor edits:

This reset password reset is only available...

Or:

This password reset link is only....

And

For security, this request...

@MorrisJobke
Copy link
Member Author

Would it be possible to implement @Espina2 fix and use the Sessions + add more text?

Not in this PR. Additional PR for sure, but please keep it small.

@MorrisJobke
Copy link
Member Author

@LukasReschke @rullzer Could one of you update the texts like @eppfel proposed? Thanks :)

@LukasReschke LukasReschke merged commit 81d3732 into master Apr 13, 2017
@LukasReschke LukasReschke deleted the lost-password-email branch April 13, 2017 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews design Design, UI, UX, etc. enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants