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

Fixed an issue when the collaboration service registers apps also for… #10107

Merged
merged 3 commits into from
Sep 24, 2024

Conversation

2403905
Copy link
Contributor

@2403905 2403905 commented Sep 19, 2024

… binary and unknown mime types.

Description

Related Issue

Motivation and Context

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

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:

@jvillafanez
Copy link
Member

Code looks fine, although I'm not sure why CI is failing.

.drone.star Outdated
@@ -1079,7 +1079,7 @@ def wopiValidatorTests(ctx, storage, wopiServerType, accounts_hash_difficulty =
"image": OC_CI_ALPINE,
"environment": {},
"commands": [
"curl -v -X PUT 'https://ocis-server:9200/remote.php/webdav/test.wopitest' -k --fail --retry-connrefused --retry 7 --retry-all-errors -u admin:admin -D headers.txt",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@individual-it @SagarGi Is test.wopitest name can be changed across all tests or this is a predefined file name for the wopi API validator?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's only used here, so your change should everything that needs to be changed

Copy link
Contributor

Choose a reason for hiding this comment

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

The .wopitest is an extension for the validator, specially on ms 365

@individual-it
Copy link
Member

Ci failure is unrelated and under investigation, see #10127

@individual-it
Copy link
Member

I've added a QA-team label here, because I think it would be good to add some API test coverage for that to make sure the service only registers for the correct mime-types.
But feel free to merge before the tests are written, we can add them later

@2403905 2403905 force-pushed the issue-10087 branch 3 times, most recently from d159e40 to c077249 Compare September 24, 2024 06:38
@micbar
Copy link
Contributor

micbar commented Sep 24, 2024

I fixed some return codes on the app/open endpoint.

We need to adapt the test expectations.

@ScharfViktor FYI

@ScharfViktor
Copy link
Contributor

@ScharfViktor FYI

some tests return dowble response error:
"{ "code": "PERMISSION_DENIED", "message": "permission denied to create the file"}{ "code": "SERVER_ERROR", "message": "failed to create the file"}"

@2403905 will make fix it in reva

@micbar
Copy link
Contributor

micbar commented Sep 24, 2024

@2403905 Sorry, i missed a return 🙈

@2403905 2403905 force-pushed the issue-10087 branch 2 times, most recently from 01f3855 to 2dd79a4 Compare September 24, 2024 10:37
@micbar
Copy link
Contributor

micbar commented Sep 24, 2024

Please rebase

@saw-jan
Copy link
Member

saw-jan commented Sep 24, 2024

Activity tests were failing because, there're full name in displayName for some activities but not for some.
https://drone.owncloud.com/owncloud/ocis/39303/38/6
I have tried to fix the test expectation

Examples:

{
  "value": [
    {
      "template": {
        "message": "{user} added {resource} to {folder}",
        "variables": {
          "folder": {
            "name": "Alice Hansen"
          },
          "resource": {
            "name": "FOLDER"
          },
          "space": {
            "name": "Alice Hansen"
          },
          "user": {
            "displayName": "Alice"
          }
        }
      }
    },
    {
      "template": {
        "message": "{user} shared {resource} with {sharee}",
        "variables": {
          "folder": {
            "name": "Alice Hansen"
          },
          "resource": {
            "name": "FOLDER"
          },
          "sharee": {
            "displayName": "Brian"
          },
          "user": {
            "displayName": "Alice"
          }
        }
      }
    },
    {
      "template": {
        "message": "{user} added {resource} to {folder}",
        "variables": {
          "resource": {
            "name": "newfile.txt"
          },
          "space": {
            "name": "Alice Hansen"
          },
          "user": {
            "displayName": "Brian Murphy"
          }
        }
      }
    },
    {
      "template": {
        "message": "{user} updated {resource} in {folder}",
        "variables": {
          "resource": {
            "name": "newfile.txt"
          },
          "space": {
            "name": "Alice Hansen"
          },
          "user": {
            "displayName": "Brian Murphy"
          }
        }
      }
    },
    {
      "template": {
        "message": "{user} deleted {resource} from {folder}",
        "variables": {
          "resource": {
            "name": "newfile.txt"
          },
          "space": {
            "name": "Alice Hansen"
          },
          "user": {
            "displayName": "Brian"
          }
        }
      }
    }
  ]
}
{
      "template": {
        "message": "{user} added {resource} to {folder}",
        "variables": {
          "folder": {
            "name": "Alice Hansen"
          },
          "resource": {
            "name": "textfile.txt"
          },
          "space": {
            "name": "Alice Hansen"
          },
          "user": {
            "displayName": "Alice Hansen"
          }
        }
      }
    }

Copy link

sonarcloud bot commented Sep 24, 2024

@2403905 2403905 merged commit 6611c55 into owncloud:master Sep 24, 2024
3 checks passed
ownclouders pushed a commit that referenced this pull request Sep 24, 2024
Fixed an issue when the collaboration service registers apps also for…
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.

collaboration service registers apps also for unknown mime types
7 participants