Fix password reset confirmation email from occ #34137

sharidas · 2019-01-14T11:18:46Z

Fix password reset confirmation email from occ.
This was missing when user:resetpassword command
was used with options "--send-email --password-from-env".
With this change set the issue is resolved.

Signed-off-by: Sujith H [email protected]

Description

When password for user is set using command user:resetpassword, with options --send-email --password-from-env ( apart from username ) , the password successful email is not sent. This is a minor issue though. In this PR an extra condition is added, so that password setting logic is routed to LostController's setPassword, if --send-email is used along with the option password-from-env. Care has also been taken to evaluate the return from LostControllers setPassword() method.

Related Issue

Fixes Password reset confirmation email is not sent on occ command user:resetpassword #33384

Motivation and Context

An email will be triggered if the password change has been made for the user with the options ``--send-email --password-from-envpassed to commanduser:resetpassword`.

How Has This Been Tested?

Create user1 user
Execute the command below:

sujith@sujith-ownCloud  ~/test/owncloud3   master ●  ./occ user:resetpassword user1 --send-email --password-from-env
Cannot load Xdebug - it was already loaded
Successfully reset password for user1
 sujith@sujith-ownCloud  ~/test/owncloud3   master ● 

Verified that email is sent when the password is changed.

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests

Checklist:

Code changes
Unit tests added
Acceptance tests added
Documentation ticket raised:

Open tasks:

Backport (if applicable set "backport-request" label and remove when the backport was done)

codecov · 2019-01-14T11:55:27Z

Codecov Report

Merging #34137 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master   #34137      +/-   ##
============================================
+ Coverage     64.76%   64.77%   +<.01%     
- Complexity    18345    18349       +4     
============================================
  Files          1198     1198              
  Lines         69450    69460      +10     
  Branches       1281     1281              
============================================
+ Hits          44982    44992      +10     
  Misses        24095    24095              
  Partials        373      373

Flag	Coverage Δ	Complexity Δ
#javascript	`53.09% <ø> (ø)`	`0 <ø> (ø)`	⬇️
#phpunit	`66.12% <100%> (ø)`	`18349 <0> (+4)`	⬆️

Impacted Files	Coverage Δ	Complexity Δ
core/Command/User/ResetPassword.php	`70.52% <100%> (+3.46%)`	`23 <0> (+4)`	⬆️

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 7b278e1...f28fa37. Read the comment docs.

codecov · 2019-01-14T11:55:30Z

Codecov Report

Merging #34137 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master   #34137      +/-   ##
============================================
+ Coverage     64.76%   64.77%   +<.01%     
- Complexity    18345    18349       +4     
============================================
  Files          1198     1198              
  Lines         69450    69460      +10     
  Branches       1281     1281              
============================================
+ Hits          44982    44992      +10     
  Misses        24095    24095              
  Partials        373      373

Flag	Coverage Δ	Complexity Δ
#javascript	`53.09% <ø> (ø)`	`0 <ø> (ø)`	⬇️
#phpunit	`66.12% <100%> (ø)`	`18349 <0> (+4)`	⬆️

Impacted Files	Coverage Δ	Complexity Δ
core/Command/User/ResetPassword.php	`70.52% <100%> (+3.46%)`	`23 <0> (+4)`	⬆️

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 7b278e1...f28fa37. Read the comment docs.

codecov · 2019-01-14T11:55:40Z

Codecov Report

Merging #34137 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master   #34137      +/-   ##
============================================
+ Coverage     64.76%   64.77%   +<.01%     
- Complexity    18347    18351       +4     
============================================
  Files          1198     1198              
  Lines         69454    69464      +10     
  Branches       1281     1281              
============================================
+ Hits          44983    44993      +10     
  Misses        24098    24098              
  Partials        373      373

Flag	Coverage Δ	Complexity Δ
#javascript	`53.09% <ø> (ø)`	`0 <ø> (ø)`	⬇️
#phpunit	`66.12% <100%> (ø)`	`18351 <0> (+4)`	⬆️

Impacted Files	Coverage Δ	Complexity Δ
core/Command/User/ResetPassword.php	`70.52% <100%> (+3.46%)`	`23 <0> (+4)`	⬆️

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 028ba04...2ab6f12. Read the comment docs.

phil-davis · 2019-01-14T12:11:29Z

@sharidas I discovered that the acceptance test for this issue was not really demonstrating the issue.
I corrected the acceptance test in PR #34139
After that is merged, the acceptance test will then be demonstrating the wrong behavior described in the issue.
Then this PR will need to correct the acceptance test, as well as fixing the issue.
^this is resolved

phil-davis

Adjusted acceptance test passes, so that is "a good thing"

Fix password reset confirmation email from occ. This was missing when user:resetpassword command was used with options "--send-email --password-from-env". With this change set the issue is resolved. Signed-off-by: Sujith H <[email protected]>

sharidas · 2019-01-14T16:47:11Z

Current state of the PR:

Verified the issue using this PR. And it resolves the issue.
Have cherry-picked one of the commits of @phil-davis for this fix.

PVince81

👍

PVince81 · 2019-01-15T16:36:02Z

@sharidas please backport

sharidas · 2019-01-16T06:21:55Z

Backport to stable10 PR #34154

sharidas self-assigned this Jan 14, 2019

sharidas added this to the development milestone Jan 14, 2019

phil-davis mentioned this pull request Jan 14, 2019

[DO NOT MERGE] Adjust test for issue 33384 2 #34140

Closed

sharidas force-pushed the fix-reset-confirmation-mail-from-occ branch from f28fa37 to cdf7958 Compare January 14, 2019 13:02

phil-davis reviewed Jan 14, 2019

View reviewed changes

sharidas and others added 2 commits January 14, 2019 19:58

Adjust acceptance test for issue 33384

2ab6f12

sharidas force-pushed the fix-reset-confirmation-mail-from-occ branch from cdf7958 to 2ab6f12 Compare January 14, 2019 14:30

sharidas requested a review from PVince81 January 14, 2019 16:45

sharidas added the 3 - To Review label Jan 14, 2019

PVince81 approved these changes Jan 15, 2019

View reviewed changes

PVince81 merged commit 17675cb into master Jan 15, 2019

delete-merged-branch bot deleted the fix-reset-confirmation-mail-from-occ branch January 15, 2019 16:35

PVince81 added the backport-request label Jan 15, 2019

sharidas mentioned this pull request Jan 16, 2019

Fix reset confirmation mail from occ stable10 #34154

Merged

10 tasks

sharidas removed the backport-request label Jan 16, 2019

lock bot locked as resolved and limited conversation to collaborators Jan 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix password reset confirmation email from occ #34137

Fix password reset confirmation email from occ #34137

sharidas commented Jan 14, 2019

codecov bot commented Jan 14, 2019

codecov bot commented Jan 14, 2019

codecov bot commented Jan 14, 2019 •

edited

Loading

phil-davis commented Jan 14, 2019 •

edited

Loading

phil-davis left a comment

sharidas commented Jan 14, 2019

PVince81 left a comment

PVince81 commented Jan 15, 2019

sharidas commented Jan 16, 2019 •

edited

Loading

Fix password reset confirmation email from occ #34137

Fix password reset confirmation email from occ #34137

Conversation

sharidas commented Jan 14, 2019

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Open tasks:

codecov bot commented Jan 14, 2019

Codecov Report

codecov bot commented Jan 14, 2019

Codecov Report

codecov bot commented Jan 14, 2019 • edited Loading

Codecov Report

phil-davis commented Jan 14, 2019 • edited Loading

phil-davis left a comment

Choose a reason for hiding this comment

sharidas commented Jan 14, 2019

PVince81 left a comment

Choose a reason for hiding this comment

PVince81 commented Jan 15, 2019

sharidas commented Jan 16, 2019 • edited Loading

codecov bot commented Jan 14, 2019 •

edited

Loading

phil-davis commented Jan 14, 2019 •

edited

Loading

sharidas commented Jan 16, 2019 •

edited

Loading