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

Do not set the password again if it hasn't changed #31367

Merged
merged 2 commits into from
May 29, 2018

Conversation

jvillafanez
Copy link
Member

@jvillafanez jvillafanez commented May 7, 2018

Description

Do not set the password again in case the permissions has changed.

Steps (copied from central)

  1. share a folder with owncloud
  2. choose public links
  3. create new link setting a password for read only
  4. Now check the modify permission

Expected

Password-protected link still works with the password set in step 3

Actual

Password-protected link can't be access due to weird password. Password set in step 3 doesn't work.

Related Issue

Reported for 10.0.8 in https://central.owncloud.org/t/password-request-for-link-share/13342

Motivation and Context

How Has This Been Tested?

Manually tested for 10.0.8
Added unittest

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)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@codecov
Copy link

codecov bot commented May 7, 2018

Codecov Report

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

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #31367      +/-   ##
============================================
+ Coverage     62.61%   62.61%   +<.01%     
- Complexity    18273    18274       +1     
============================================
  Files          1147     1147              
  Lines         68627    68627              
  Branches       1234     1234              
============================================
+ Hits          42971    42972       +1     
+ Misses        25295    25294       -1     
  Partials        361      361
Flag Coverage Δ Complexity Δ
#javascript 52.05% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 63.81% <100%> (ø) 18274 <0> (+1) ⬆️
Impacted Files Coverage Δ Complexity Δ
lib/private/Share20/Manager.php 97.25% <100%> (+0.17%) 214 <0> (+1) ⬆️

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 b082ab6...f489e87. Read the comment docs.

@jvillafanez
Copy link
Member Author

Backport in #31370

@patrickjahns
Copy link
Contributor

@jvillafanez
please provide a digest for the test information either in a github ticket or in the PR - when testing the changelog it is a mess if you have to read through a central post to determine if it works as desired

@patrickjahns patrickjahns self-requested a review May 7, 2018 15:31
Copy link
Contributor

@patrickjahns patrickjahns left a comment

Choose a reason for hiding this comment

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

Missing test steps

@jvillafanez
Copy link
Member Author

Added unittest and include steps in the PR.

@jvillafanez
Copy link
Member Author

Rebased to fix conflicts

@PVince81
Copy link
Contributor

@ownclouders rebase

@ownclouders
Copy link
Contributor

Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently ⚠️

Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

Code looks fine 👍

@ownclouders
Copy link
Contributor

Automated rebase with GitMate.io was successful! 🎉

@PVince81 PVince81 merged commit 4137b91 into master May 29, 2018
@PVince81 PVince81 deleted the unneeded_password_set branch May 29, 2018 10:23
@phil-davis
Copy link
Contributor

Backport happened - removing backport-request label

@lock
Copy link

lock bot commented Jul 30, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants