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

add Api test for adding space managers #3166

Merged
merged 4 commits into from
Feb 17, 2022
Merged

Conversation

micbar
Copy link
Contributor

@micbar micbar commented Feb 14, 2022

Description

  • Add Api Tests for space managers

Problem

the currentl logic parsing the json file and using specific key -> values breaks when we have unsorted lists.

We need to refactor this @phil-davis @ScharfViktor

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:

@update-docs
Copy link

update-docs bot commented Feb 14, 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.

@ownclouders
Copy link
Contributor

ownclouders commented Feb 14, 2022

💥 Acceptance test localApiTests-apiSpaces-ocis failed. Further test are cancelled...

@ScharfViktor
Copy link
Contributor

Thanks for bringing it up. I'll try to fix it in this PR

@ScharfViktor
Copy link
Contributor

ScharfViktor commented Feb 14, 2022

We can't share space with the manager's role. It still doesn't work for me:
image

https://jira.owncloud.com/browse/OCIS-2095 still in progress

@micbar
Copy link
Contributor Author

micbar commented Feb 14, 2022

this is broken in ocis master but already fixed in reva cs3org/reva@6ac2dfa

@ScharfViktor
Copy link
Contributor

I liked it because I could see the answer structure in the test
Screenshot 2022-02-14 at 17 45 09
but I don't like 0 and 1. I could do something like this: https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiCapabilities/capabilities.feature#L86 but it wouldn't make much difference.

I added a check that user has role in response. I tried combining these checks, but it didn't work for me

@micbar
Copy link
Contributor Author

micbar commented Feb 15, 2022

LGTM, I cannot approve because I created it. Feel free to approve and merge.

@ScharfViktor
Copy link
Contributor

LGTM, I cannot approve because I created it. Feel free to approve and merge.

I cannot also because there is no "Approve button" available

@ScharfViktor
Copy link
Contributor

according to cs3org/reva#2524 the owner does not display in the space type project.
fix test

@micbar
Copy link
Contributor Author

micbar commented Feb 17, 2022

Needed a rebase.

@ScharfViktor ScharfViktor merged commit 94f3d75 into master Feb 17, 2022
@delete-merged-branch delete-merged-branch bot deleted the space-managers-api branch February 17, 2022 10:21
@ScharfViktor
Copy link
Contributor

Thank @micbar for rebase. I merged it

ownclouders pushed a commit that referenced this pull request Feb 17, 2022
Author: Michael Barz <mbarz@owncloud.com>
Date:   Thu Feb 17 11:21:07 2022 +0100

    add Api test for adding space managers (#3166)

    * add Api test for adding space managers

    * add cheking permissions in response

    * fix after rebase

    * only personal space dispayes owner

    Co-authored-by: Viktor Scharf <scharf.vi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants