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 app config to allow groups of users to be lock breakers #38222

Merged
merged 3 commits into from
Aug 16, 2021

Conversation

DeepDiver1975
Copy link
Member

@DeepDiver1975 DeepDiver1975 commented Dec 16, 2020

Description

Members of the lock breaker groups can break locks of other users
The groups are setup in the admin's general settings

Related Issue

How Has This Been Tested?

  • define a group for lock breakers
  • add user A to this groups
  • user B shares folder with user A
  • B locks a file within that folder
  • A will see the lock
  • A can unlock the file aka break the lock of user B

Screenshots (if appropriate):

Screenshot from 2020-12-18 13-26-59

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

@DeepDiver1975 DeepDiver1975 force-pushed the feature/lock-breaker-group branch 2 times, most recently from cc1f096 to ca97f3d Compare December 17, 2020 14:56
@sonarcloud
Copy link

sonarcloud bot commented Dec 17, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@DeepDiver1975 DeepDiver1975 self-assigned this Dec 18, 2020
@DeepDiver1975 DeepDiver1975 marked this pull request as ready for review December 18, 2020 14:56
@mmattel
Copy link
Contributor

mmattel commented Jan 14, 2021

Reminder: docs relevant if merged

@pmaier1
Copy link
Contributor

pmaier1 commented May 21, 2021

@micbar go for 10.8?

@mmattel
Copy link
Contributor

mmattel commented May 21, 2021

Reminder: docs relevant if merged, please file an issue

@DeepDiver1975 DeepDiver1975 force-pushed the feature/lock-breaker-group branch 2 times, most recently from 94e0fe7 to 50007f4 Compare May 21, 2021 13:06
@sonarcloud
Copy link

sonarcloud bot commented May 21, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@micbar micbar requested review from phil-davis and jvillafanez and removed request for phil-davis May 25, 2021 08:29
@phil-davis
Copy link
Contributor

There are already test scenarios in https://github.com/owncloud/core/tree/master/tests/acceptance/features/apiWebdavLocksUnlock that check who can unlock whose files/folders.

Should we add some scenarios that cover this new functionality of putting people in lock-breakers groups?

@owncloud owncloud deleted a comment from update-docs bot Jul 5, 2021
@micbar
Copy link
Contributor

micbar commented Jul 5, 2021

@DeepDiver1975 In which scope was this developed?

@DeepDiver1975
Copy link
Member Author

I'll have a look and fix the different config keys .....

@DeepDiver1975
Copy link
Member Author

rebased and issue fixed ...

Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

This looks OK. I did some manual testing with the webUI, and it works.

I raised issue #38851 to add some test scenarios. This PR can be merged, if needed, and the tests added later.

@DeepDiver1975
Copy link
Member Author

rebased and git history cleandup ....

@JammingBen
Copy link
Contributor

@DeepDiver1975 Can we move it to "Ready for review"? Or do you plan on adding more to it?

@DeepDiver1975
Copy link
Member Author

Or do you plan on adding more to it?

no - I am done! THX

@phil-davis
Copy link
Contributor

I squashed all the API and UI acceptance tests into 1 commit, and rebased. This is ready for review from my PoV.

@cdamken
Copy link
Contributor

cdamken commented Jul 23, 2021

@pmaier1 @micbar @jnweiger Can we test this and put in the 10.8.1?

@phil-davis
Copy link
Contributor

This is a "new feature", so according to semver, a release that adds this would be numbered 10.9.0

@JammingBen
Copy link
Contributor

Merge, or any objections? Got 2 reviews already

Copy link
Contributor

@mrow4a mrow4a left a comment

Choose a reason for hiding this comment

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

Looks good, needs rebase and merging

@phil-davis
Copy link
Contributor

Rebased and conflict resolved.

@mrow4a
Copy link
Contributor

mrow4a commented Aug 16, 2021

@phil-davis with tests passing shall we merge?

@phil-davis
Copy link
Contributor

@phil-davis with tests passing shall we merge?

yes - there are acceptance tests that cover the behavior of the new option.

@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline apiFederationToShares2-latest-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/31880/119/1

@phil-davis
Copy link
Contributor

https://drone.owncloud.com/owncloud/core/31880/119/10
drone having a bad day

@sonarcloud
Copy link

sonarcloud bot commented Aug 16, 2021

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 0 Code Smells

69.2% 69.2% Coverage
0.0% 0.0% Duplication

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.