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

[Tests-Only] Added API test for public link Mtime #37589

Merged
merged 1 commit into from
Jul 2, 2020

Conversation

Talank
Copy link
Contributor

@Talank Talank commented Jun 24, 2020

Description

API Acceptance test for public link mtime is added

Related Issue

Fixes owncloud/ocis-reva#296

How Has This Been Tested?

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

@Talank Talank self-assigned this Jun 24, 2020
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.

please add tests:

  • check the mtime if the share is a share of a folder and not a file
  • upload as public with a historic mtime (share is a file or folder, different permissions (e.g. upload only, upload and read) - check that the mtime is reported correctly through the webdav interface of the user
  • upload as public overwriting an existing file and setting the mtime

@codecov
Copy link

codecov bot commented Jun 24, 2020

Codecov Report

Merging #37589 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #37589   +/-   ##
=========================================
  Coverage     64.71%   64.71%           
  Complexity    19359    19359           
=========================================
  Files          1281     1281           
  Lines         75637    75637           
  Branches       1333     1333           
=========================================
  Hits          48949    48949           
  Misses        26296    26296           
  Partials        392      392           
Flag Coverage Δ Complexity Δ
#javascript 54.03% <ø> (ø) 0.00 <ø> (ø)
#phpunit 65.89% <ø> (ø) 19359.00 <ø> (ø)

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 4a1f3b3...4594b63. Read the comment docs.

@PVince81
Copy link
Contributor

fix was added in owncloud/web#3686 (review)

@Talank Talank requested a review from phil-davis June 29, 2020 16:40
@individual-it
Copy link
Member

code-style problem
image

always good to run make test-php-style before committing, e.g. in pre-commit hook

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.

don't implement tests using the old public link webdav API, nobody cares about it and it will never be implemented in OCIS

And the mtime of file "file.txt" in the last shared public link using the WebDAV API should be "Thu, 08 Aug 2019 04:18:13 GMT"

@issue-ocis-reva-49 @issue-37605
Scenario: Get the mtime of a file inside a folder shared by public link using old dav version
Copy link
Member

Choose a reason for hiding this comment

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

old or new? the test says "new"

Copy link
Contributor

@kiranparajuli589 kiranparajuli589 left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@Talank Talank force-pushed the public-link-mtime branch 4 times, most recently from e88d8a6 to e394902 Compare June 30, 2020 11:36
$mtime = \implode(" ", $mtime);
Assert::assertContains(
$mtime,
(new TestHelpers\WebDavHelper)->getMtimeOfFileinPublicLinkShare($baseUrl, $fileName, $token)
Copy link
Member

Choose a reason for hiding this comment

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

why do we need that construct?
shouldn't \TestHelpers\WebDavHelper::getMtimeOfFileinPublicLinkShare($baseUrl, $fileName, $token) work? The functions in the TestHelpers should all be static

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.

please try the tests also on ocis/ocis-reva repository.
If they work its great, if not we need to open issues, skip the existing tests and make tests that show the issues in ocis

* @return string
* @throws Exception
*/
public function getMtimeOfFileinPublicLinkShare(
Copy link
Member

Choose a reason for hiding this comment

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

make the function static like all others

| permissions | read,update,create,delete |
When the public uploads file "file.txt" to the last shared folder with mtime "Thu, 08 Aug 2019 04:18:13 GMT" using the new public WebDAV API
Then as "Alice" file "testFolder/file.txt" should exist
And the mtime of file "file.txt" in the last shared public link using the WebDAV API should not be "Thu, 08 Aug 2019 04:18:13 GMT"
Copy link
Member

Choose a reason for hiding this comment

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

check the mtime also through the "normal" webDAV api

@Talank Talank force-pushed the public-link-mtime branch from 21e4638 to 8b1412c Compare July 1, 2020 11:19
@Talank Talank requested a review from individual-it July 1, 2020 11:20
@Talank Talank force-pushed the public-link-mtime branch from 8b1412c to 4594b63 Compare July 1, 2020 12:01
@individual-it individual-it merged commit 3bb0d7b into master Jul 2, 2020
@delete-merged-branch delete-merged-branch bot deleted the public-link-mtime branch July 2, 2020 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API test for mtime preservation on public links
5 participants