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

detect correct uid when even wrong casing in given uid for files:scan #35324

Merged
merged 1 commit into from
May 28, 2019

Conversation

karakayasemi
Copy link
Contributor

@karakayasemi karakayasemi commented May 24, 2019

Description

When doing a files:scan and entering the user id in the wrong case. The job tells, that no files are scanned. This PR fixes issue by learning correct uid from user manager. @pako81 since casing is not important in our uid policy, in my opinion this solution better than throwing an error. Do you think this PR is acceptable with this behavior?

Related Issue

Motivation and Context

Solving bugs.

How Has This Been Tested?

  • use files:scan command with wrong casing
  • it should detect correct uid and work

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:

Open tasks:

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

@pako81
Copy link
Contributor

pako81 commented May 27, 2019

@karakayasemi yes, I do agree with you. Better getting the right uid instead of throwing an exception. thank you.

@codecov
Copy link

codecov bot commented May 28, 2019

Codecov Report

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

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #35324       +/-   ##
=============================================
- Coverage     65.53%   48.87%   -16.67%     
=============================================
  Files          1218      109     -1109     
  Lines         70548    10560    -59988     
  Branches       1288     1288               
=============================================
- Hits          46235     5161    -41074     
+ Misses        23936     5022    -18914     
  Partials        377      377
Flag Coverage Δ Complexity Δ
#javascript 53.69% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 38.17% <ø> (-28.73%) 0 <ø> (-18648)
Impacted Files Coverage Δ Complexity Δ
lib/private/Files/Storage/DAV.php 59.45% <0%> (-21.64%) 0% <0%> (ø)
apps/updatenotification/templates/admin.php
lib/private/Encryption/Keys/Storage.php
lib/private/App/CodeChecker/NodeVisitor.php
lib/private/RedisFactory.php
apps/dav/lib/Avatars/AvatarNode.php
...s/dav/appinfo/Migrations/Version20170202213905.php
apps/dav/lib/Upload/ChunkLocationProvider.php
apps/files/lib/AppInfo/Application.php
apps/systemtags/list.php
... and 1100 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 7684118...d4bd6fe. Read the comment docs.

@codecov
Copy link

codecov bot commented May 28, 2019

Codecov Report

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

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #35324      +/-   ##
============================================
+ Coverage     65.53%   65.53%   +<.01%     
  Complexity    18648    18648              
============================================
  Files          1218     1218              
  Lines         70548    70549       +1     
  Branches       1288     1288              
============================================
+ Hits          46235    46236       +1     
  Misses        23936    23936              
  Partials        377      377
Flag Coverage Δ Complexity Δ
#javascript 53.69% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 66.89% <100%> (ø) 18648 <0> (ø) ⬇️
Impacted Files Coverage Δ Complexity Δ
apps/files/lib/Command/Scan.php 71.91% <100%> (+0.09%) 74 <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 7684118...d4bd6fe. Read the comment docs.

@karakayasemi karakayasemi merged commit 9882365 into master May 28, 2019
@delete-merged-branch delete-merged-branch bot deleted the fix-files-scan branch May 28, 2019 07:11
@phil-davis
Copy link
Contributor

I added backport-request because I guess this change is relevant in stable10 also.

@karakayasemi
Copy link
Contributor Author

Backport PR: #35344

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.

None yet

4 participants