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

[full-ci] Allow 425 status-code in propfinds #8146

Merged
merged 5 commits into from
Dec 22, 2022
Merged

[full-ci] Allow 425 status-code in propfinds #8146

merged 5 commits into from
Dec 22, 2022

Conversation

kulmann
Copy link
Contributor

@kulmann kulmann commented Dec 22, 2022

Description

Splitting out the propfind 425 status code handling from #8069 so that async upload e2e tests get unblocked.

Related Issue

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:

cc @individual-it

@update-docs
Copy link

update-docs bot commented Dec 22, 2022

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 self-assigned this Dec 22, 2022
@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 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

fschade
fschade reviewed Dec 22, 2022
View reviewed changes
packages/web-client/src/helpers/resource/functions.ts
@@ -99,6 +99,7 @@ export function buildResource(resource): Resource {
webDavPath: resource.name,
type: isFolder ? 'folder' : resource.type,
isFolder,
processing: resource.processing || false,
Copy link
Contributor

@fschade fschade Dec 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

!!resource.processing, optional i just prefer to have !! and be able to search for it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why? It's a boolean in the response. The owncloud-sdk only ever sets true or false as values.

@fschade
Copy link
Contributor

fschade commented Dec 22, 2022

i have to setup office to test it in person now, one thing i would like to see is a 225 error-code test packages/web-app-external/tests/unit/app.spec.ts

fschade
fschade approved these changes Dec 22, 2022
View reviewed changes
Copy link
Contributor

@fschade fschade left a comment

Choose a reason for hiding this comment

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

not able to run office hier at the moment, code LGTM

@kulmann
Copy link
Contributor Author

kulmann commented Dec 22, 2022

i have to setup office to test it in person now, one thing i would like to see is a 225 error-code test packages/web-app-external/tests/unit/app.spec.ts

I'll do that in #8069 because we need the same functionality for all other apps as well.

@kulmann kulmann merged commit b870786 into master Dec 22, 2022
@delete-merged-branch delete-merged-branch bot deleted the allow-425-propfind branch December 22, 2022 11:51
ownclouders pushed a commit that referenced this pull request Dec 22, 2022
@kulmann
commit b870786
996cc2e 
Merge: 81d78e5 85437d2
Author: Benedikt Kulmann <benedikt@kulmann.biz>
Date:   Thu Dec 22 12:51:01 2022 +0100

    Merge pull request #8146 from owncloud/allow-425-propfind

    [full-ci] Allow 425 status-code in propfinds
This was referenced Dec 22, 2022
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dschmidt dschmidt dschmidt approved these changes

@fschade fschade fschade approved these changes

@JammingBen JammingBen Awaiting requested review from JammingBen

Assignees

@kulmann kulmann

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kulmann @fschade @dschmidt