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

Validate expiration date for public links only #293

Merged
merged 4 commits into from
Mar 19, 2020
Merged

Conversation

VicDeo
Copy link
Member

@VicDeo VicDeo commented Mar 17, 2020

Depends on owncloud/core#37135
Fixes #287

TODO

  • add an unit test to cover user/group/public link shares separately

@VicDeo VicDeo added this to the development milestone Mar 17, 2020
@VicDeo VicDeo self-assigned this Mar 17, 2020
@CLAassistant
Copy link

CLAassistant commented Mar 17, 2020

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Mar 17, 2020

Codecov Report

Merging #293 into master will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #293      +/-   ##
============================================
+ Coverage     73.52%   73.57%   +0.05%     
- Complexity      230      233       +3     
============================================
  Files            25       25              
  Lines           982      984       +2     
============================================
+ Hits            722      724       +2     
  Misses          260      260
Impacted Files Coverage Δ Complexity Δ
lib/HooksHandler.php 82.16% <100%> (+0.23%) 42 <0> (+3) ⬆️

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 edb438f...18948be. Read the comment docs.

@phil-davis
Copy link
Contributor

phil-davis commented Mar 18, 2020

@VicDeo I pushed acceptance tests:

  1. test scenarios that demonstrate the bug were done and merged in PR Add test scenarios to demonstrate issue-287 #294
  2. adjustments to the test scenarios to test what is the really desired correct behaviour (those pass in this PR)

Please pull from GitHub, and add whatever unit tests you want. Then IMO we are done.

@VicDeo
Copy link
Member Author

VicDeo commented Mar 18, 2020

@phil-davis great, thanks!

@VicDeo
Copy link
Member Author

VicDeo commented Mar 18, 2020

@phil-davis unit tests have been updated

@phil-davis
Copy link
Contributor

https://drone.owncloud.com/owncloud/password_policy/1210/43/10 acceptance tests fail because in drone CI the "current" daily-master-qa tarball is used as the core "server-under-test" and the current password_policy app code from this PR is used.

daily-master-qa tarball is now stale - the related/needed fix in core was only merged a few hours ago. We need someone to build a new daily-master-qa tarball, or wait until tomorrow.

@phil-davis
Copy link
Contributor

I rebased just now, because #294 was merged.

We will need to restart CI again tomorrow after the core daily-master-qa tarball gets updated overnight.

@phil-davis phil-davis merged commit 6d26c4c into master Mar 19, 2020
@delete-merged-branch delete-merged-branch bot deleted the bugfix/287 branch March 19, 2020 08:23
@phil-davis phil-davis mentioned this pull request Mar 19, 2020
32 tasks
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.

Cannot create ordinary share with users when public link expiry is set
3 participants