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

[full-ci] Initial implementation of archiver v2 #5832

Merged
merged 22 commits into from
Oct 5, 2021
Merged

Conversation

kulmann
Copy link
Member

@kulmann kulmann commented Sep 23, 2021

Initial implementation of archiver v2 (REVA) for authenticated contexts. Public links are not supported, yet.

Description

Related Issue

Motivation and Context

Being able to download folders

How Has This Been Tested?

  • not yet

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

Checklist:

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

Open tasks:

@update-docs
Copy link

update-docs bot commented Sep 23, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@kulmann kulmann marked this pull request as ready for review September 29, 2021 12:09
@kulmann kulmann changed the title Initial implementation of archiver v2 [full-ci] Initial implementation of archiver v2 Sep 29, 2021
@kulmann kulmann force-pushed the archive-download branch 2 times, most recently from 8d5faf0 to 9a115d9 Compare October 1, 2021 12:07
@ownclouders
Copy link
Contributor

Results for oC10SharingInternalUsers https://drone.owncloud.com/owncloud/web/19381/25/1
The following scenarios passed on retry:

  • webUISharingInternalUsersShareWithPage/shareWithUsers.feature:91

@@ -19,6 +19,7 @@ Background: prepare user and files
And only the following items with default items should be visible in the actions menu on the webUI
| items |
| open folder |
| download folder |
Copy link
Member

Choose a reason for hiding this comment

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

this test fails AssertionError [ERR_ASSERTION]: only 'open folder,download folder' actions menu item(s) was expected to be visible but found 'open folder' instead.

copyButton = wrapper.find(elSelector.copyButton)
moveButton = wrapper.find(elSelector.moveButton)
deleteButton = wrapper.find(elSelector.deleteButton)
})

it('should display the action buttons', () => {
expect(actionButtons.length).toEqual(3)
expect(downloadButton.exists()).toBeTruthy()
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't checking for the text of an element be enough (test fails if it doesn't exist since it can't find the text)?

it('is announcing itself as available', () => {
expect(service.available).toBe(true)
})
it('uses the highest major version', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
it('uses the highest major version', () => {
it('uses the highest major version that is enabled', () => {

Optional clarification

@@ -19,6 +19,7 @@ Background: prepare user and files
And only the following items with default items should be visible in the actions menu on the webUI
| items |
| open folder |
| download folder |
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems to fail in a OC10 pipeline which makes sense (since the download action doesn't get rendered into those actions based on capabilities?)

Copy link
Member

Choose a reason for hiding this comment

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

will that be added in oc10? If yes, then the test need to he added to the expected to fail list, if not, need to split the scenario and one tagged with @skipOnOC10 and one with @notToImplementOnOCIS

Copy link
Contributor

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

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

LGTM 🗃️ if CI agrees

…nd and boot logic needs to be improved. oids-callback page is in vue so the mounted hook was already called.
@sonarcloud
Copy link

sonarcloud bot commented Oct 5, 2021

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell A 0 Code Smells

40.4% 40.4% Coverage
0.0% 0.0% Duplication

@ownclouders
Copy link
Contributor

Results for oC10Files1 https://drone.owncloud.com/owncloud/web/19427/12/1
The following scenarios passed on retry:

  • webUIFilesActionMenu/versions.feature:36

@fschade fschade merged commit 1032246 into master Oct 5, 2021
@delete-merged-branch delete-merged-branch bot deleted the archive-download branch October 5, 2021 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants