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

Fix issue 36925 #36926

Merged
merged 2 commits into from
Feb 17, 2020
Merged

Fix issue 36925 #36926

merged 2 commits into from
Feb 17, 2020

Conversation

phil-davis
Copy link
Contributor

@phil-davis phil-davis commented Feb 10, 2020

Description

1st commit: Add test scenario to demonstrate the problem with:

php occ user:resetpassword phil --send-email --password-from-env

(using both these command options together)

2nd commit: fix the problem. Add code to check that the user does have an email address (similar to the checks done higher up in the case when generating/sending a password-reset link)

Related Issue

How Has This Been Tested?

Local run of test scenario

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

Sorry, something went wrong.

@phil-davis phil-davis self-assigned this Feb 10, 2020
@phil-davis phil-davis changed the title [Tests-Only] Add test scenario for issue 36925 Fix issue 36925 Feb 10, 2020
@phil-davis phil-davis requested a review from micbar February 10, 2020 11:37
@phil-davis
Copy link
Contributor Author

@micbar I am investigating owncloud/user_ldap#509 which is about some test fails with user_ldap+encryption. While looking at that, I found this bug with the unusual combination described in issue #36925

It would be useful if you or some developer can review my code fix. If I can get it merged, then the nightly tests will run with this bit fixed. Then I can see the nightly results and think what to touch next.

@codecov
Copy link

codecov bot commented Feb 10, 2020

Codecov Report

Merging #36926 into master will decrease coverage by 1.54%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #36926      +/-   ##
============================================
- Coverage     65.53%   63.98%   -1.55%     
- Complexity    19135    19136       +1     
============================================
  Files          1207     1270      +63     
  Lines         67370    74875    +7505     
  Branches          0     1329    +1329     
============================================
+ Hits          44150    47908    +3758     
- Misses        23220    26576    +3356     
- Partials          0      391     +391     
Flag Coverage Δ Complexity Δ
#javascript 54.17% <ø> (?) 0.00 <ø> (?)
#phpunit 65.07% <100.00%> (-0.46%) 19136.00 <0.00> (+1.00)
Impacted Files Coverage Δ Complexity Δ
lib/private/DB/SQLiteMigrator.php 0.00% <0.00%> (-100.00%) 7.00% <0.00%> (ø%)
lib/private/DB/SQLiteSessionInit.php 0.00% <0.00%> (-100.00%) 4.00% <0.00%> (ø%)
lib/private/DB/AdapterOCI8.php 0.00% <0.00%> (-86.67%) 4.00% <0.00%> (ø%)
lib/private/Repair/SqliteAutoincrement.php 0.00% <0.00%> (-85.19%) 9.00% <0.00%> (ø%)
...Builder/ExpressionBuilder/OCIExpressionBuilder.php 0.00% <0.00%> (-85.19%) 18.00% <0.00%> (ø%)
lib/private/DB/AdapterSqlite.php 0.00% <0.00%> (-83.34%) 7.00% <0.00%> (ø%)
lib/private/DB/OracleMigrator.php 0.00% <0.00%> (-76.85%) 10.00% <0.00%> (ø%)
lib/private/DB/OracleConnection.php 0.00% <0.00%> (-66.67%) 12.00% <0.00%> (ø%)
lib/private/DB/ConnectionFactory.php 73.97% <0.00%> (-16.44%) 22.00% <0.00%> (ø%)
core/Command/Db/ConvertType.php 26.85% <0.00%> (-11.16%) 55.00% <0.00%> (ø%)
... and 75 more

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 7dcf52a...268440d. Read the comment docs.

@phil-davis phil-davis requested a review from sharidas February 11, 2020 03:18
Copy link
Member

@individual-it individual-it left a comment

Choose a reason for hiding this comment

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

Tests look good!

@phil-davis
Copy link
Contributor Author

Ping @micbar @sharidas or anyone to review the command code changes for this?

@phil-davis phil-davis merged commit b77b49d into master Feb 17, 2020
@delete-merged-branch delete-merged-branch bot deleted the issue-36925 branch February 17, 2020 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

occ:resetpassword problem with send-email and password-from-env together
3 participants