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

provisioning API to use only public API of IAccountManager #26923

Merged
merged 2 commits into from
May 12, 2021

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented May 7, 2021

helps with #26866

@blizzz blizzz added this to the Nextcloud 22 milestone May 7, 2021
@blizzz blizzz force-pushed the techdebt/noid/provapi-no-private-accountmanager branch from a860557 to 1e271e9 Compare May 7, 2021 23:20
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@nickvergessen
Copy link
Member

Integration tests are failing

@nickvergessen nickvergessen added 2. developing Work in progress and removed 3. to review Waiting for reviews labels May 11, 2021
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Failing integration test is related

@blizzz
Copy link
Member Author

blizzz commented May 11, 2021

Failing integration test is related

yes, addressing this after the dav stuff in the other PR.

the auto-fallback to v2-local is removed as well to react on wrong input

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
Copy link
Member Author

blizzz commented May 11, 2021

So the test failed, because the AccountProperty does not accept an invalid value, but falls back to IAccountManager::SCOPE_LOCAL in mapScopeToV2() via setScope. The test would work however, if the provided value was v2-invalid instead of invalid.

@PVince81 I removed the auto-fallback to v2-local, to react properly on invalid data (and satisfy integration test). Was there a specific reason why it was there in first place? Not that I am breaking something else.

@PVince81
Copy link
Member

So the test failed, because the AccountProperty does not accept an invalid value, but falls back to IAccountManager::SCOPE_LOCAL in mapScopeToV2() via setScope. The test would work however, if the provided value was v2-invalid instead of invalid.

@PVince81 I removed the auto-fallback to v2-local, to react properly on invalid data (and satisfy integration test). Was there a specific reason why it was there in first place? Not that I am breaking something else.

I don't remember whether the fallback from "invalid value" to "v2-local" was important.
I do remember that it's important to convert the legacy ones to the new ones, and I saw that you left this bit.
Should be fine.

@blizzz blizzz requested a review from nickvergessen May 12, 2021 08:38
@blizzz blizzz added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels May 12, 2021
@blizzz blizzz merged commit 7e1c842 into master May 12, 2021
@blizzz blizzz deleted the techdebt/noid/provapi-no-private-accountmanager branch May 12, 2021 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish feature: users and groups technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants