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.

Sign up for GitHub

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

Jump to bottom

[tests-only][full-ci]Add api tests public link folder download #5232

Merged
merged 5 commits into from
Dec 20, 2022
Merged

[tests-only][full-ci]Add api tests public link folder download #5232

merged 5 commits into from
Dec 20, 2022

Conversation

SwikritiT
Copy link
Contributor

@SwikritiT SwikritiT commented Dec 15, 2022 •
edited
Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

  • locally
  • CI

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)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@SwikritiT SwikritiT force-pushed the add-api-tests-public-link-folder-download branch from e24c807 to 7db41d4 Compare December 15, 2022 08:44
@SwikritiT SwikritiT self-assigned this Dec 15, 2022
SwikritiT
SwikritiT commented Dec 15, 2022
View reviewed changes
tests/acceptance/features/apiSpacesShares/publicLinkDownload.feature Outdated
| name | someName |
And user "Alice" has created a folder "NewFolder" in space "new-space"
And user "Alice" has uploaded a file inside space "new-space" with content "some content" to "NewFolder/test.txt"
When public downloads the folder "NewFolder" of space "new-space" from the last created public link of "Alice" using the resource id
Copy link
Contributor Author

@SwikritiT SwikritiT Dec 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Need suggestions on if this is an appropriate way to do this. Realistically, the public would never know the resource-id of the file
But the archiver endpoint used to download the folders takes with query params public-token and fileid

If this is not appropriate we might need to add WebUI tests or any suggestions on better way to do this is welcomed

Copy link
Member

Choose a reason for hiding this comment

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

Or maybe the step can be only When public downloads the folder "NewFolder" of space "new-space" from the last created public link of "Alice"
And in step def, we can get the resource id. Also, I think the UI part may also does the same internally (but not sure - should be the same)

Copy link
Member

Choose a reason for hiding this comment

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

@phil-davis @individual-it

Copy link
Contributor

Choose a reason for hiding this comment

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

In a real situation, "the public" has the link token. "The public" does a PROPFIND using the token (or some similar "get a list of resources" request), and a list of the resources (file and folders) in the public link share is returned. The data returned in the PROPFIND should have resource names and ids. If it does not have the id, then "the public" has to do another PROPFIND to the resource that they want, and get the id from that.

Does something like this work for "the public"?

If it does, then there is a way for "the public" to get the resource id. The test step can do the same sequence of requests. That will demonstrate that "the public" can really find out the file id, and then use it to access the archiver endpoint.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@phil-davis Thanks I'll try that and see if it works

Copy link
Contributor Author

@SwikritiT SwikritiT Dec 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@phil-davis turns out we can propfind as a public with public files api https://doc.owncloud.com/server/next/developer_manual/webdav_api/public_files.html
I've updated the tests as well

@SwikritiT SwikritiT force-pushed the add-api-tests-public-link-folder-download branch 3 times, most recently from bf253b5 to 574e447 Compare December 15, 2022 11:41
@SwikritiT SwikritiT marked this pull request as ready for review December 16, 2022 10:15
@SwikritiT SwikritiT force-pushed the add-api-tests-public-link-folder-download branch from 6f167c5 to 9e93267 Compare December 19, 2022 04:15
grgprarup
grgprarup reviewed Dec 19, 2022
View reviewed changes
Copy link
Contributor

@grgprarup grgprarup left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@SwikritiT SwikritiT requested a review from saw-jan December 19, 2022 06:25
@SwikritiT SwikritiT force-pushed the add-api-tests-public-link-folder-download branch from e7dcd95 to d1a2054 Compare December 19, 2022 10:17
@SwikritiT SwikritiT requested a review from grgprarup December 19, 2022 10:18
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

ScharfViktor
ScharfViktor approved these changes Dec 19, 2022
View reviewed changes
Copy link
Contributor

@ScharfViktor ScharfViktor left a comment

Choose a reason for hiding this comment

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

LGTM

@phil-davis phil-davis merged commit 5c8c9cf into stable-2.0 Dec 20, 2022
@delete-merged-branch delete-merged-branch bot deleted the add-api-tests-public-link-folder-download branch December 20, 2022 07:04
ownclouders pushed a commit that referenced this pull request Dec 20, 2022
@phil-davis
commit 5c8c9cf
fe501da 
Merge: 883ed8a d1a2054
Author: Phil Davis <[email protected]>
Date:   Tue Dec 20 12:49:04 2022 +0545

    Merge pull request #5232 from owncloud/add-api-tests-public-link-folder-download

    [tests-only][full-ci]Add api tests public link folder download
@SwikritiT SwikritiT mentioned this pull request Dec 20, 2022
Closed
@phil-davis phil-davis mentioned this pull request Jan 5, 2023
Merged
9 tasks
@phil-davis
Copy link
Contributor

Port to master is #5332

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ScharfViktor ScharfViktor ScharfViktor approved these changes

@phil-davis phil-davis phil-davis approved these changes

@amrita-shrestha amrita-shrestha Awaiting requested review from amrita-shrestha

@kiranparajuli589 kiranparajuli589 Awaiting requested review from kiranparajuli589

@SagarGi SagarGi Awaiting requested review from SagarGi

@saw-jan saw-jan Awaiting requested review from saw-jan

@grgprarup grgprarup Awaiting requested review from grgprarup

Assignees

@SwikritiT SwikritiT

Labels
QA:team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@SwikritiT @phil-davis @grgprarup @SagarGi @saw-jan @ScharfViktor