-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Command to troubleshoot invalid shares generated by transfer ownership #37860
Conversation
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. |
bb1dfdf
to
e8034c2
Compare
@micbar Please check and let's discuss then. |
Please use a table (default) and json (pretty) print (optional) output format for better readbility of the result, like we have starting with in eg
If this progresses, you MUST file a docs issue (and a docs PR) |
e8034c2
to
8e392d3
Compare
@karakayasemi @pmaier1 @pako81 this new command took me significant amount of time, and due to very low usage I suggest to not invest much time into adding unit tests here (it is used only by customer impacted by past bug or heavily using transfer ownership). We could however release it and add documentation (or add tests in the future if this command stops working). |
870124c
to
0efb20c
Compare
@mrow4a Feel free to add basic integration tests. |
Codecov Report
@@ Coverage Diff @@
## master #37860 +/- ##
============================================
- Coverage 64.75% 64.71% -0.04%
- Complexity 19408 19442 +34
============================================
Files 1285 1286 +1
Lines 75830 76020 +190
Branches 1336 1336
============================================
+ Hits 49101 49196 +95
- Misses 26335 26430 +95
Partials 394 394
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
0efb20c
to
5030c5d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks okay and harmless. I am aware of the problems for adding tests from the last time. It is up to you to whether add some simple tests.
5030c5d
to
85792f8
Compare
@karakayasemi it was agreed to add tests and doc to this command , and so did |
awesome! THX! Will approve after CI is green. |
add integration tests add troubleshoot transfer ownership unit
85792f8
to
0405d40
Compare
Please add a docs issue (and PR if possible) because this is a new occ command that needs to be documented. |
@mmattei will create a PR with doc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests look good - 🥇
I raised docs issue owncloud/docs#2794 |
@micbar please force-merge if you are OK with the codecov differences. |
Adds new commands that detect and fix invalid shares generated by transfer ownership.
1. Invalid initiator
This has been triggered by past bug, that has been fixed in #37791
2. Invalid owner_uid (detached shares from their file storages, these would not show in UI to the user unless adjusted)
This could have been created for three reasons I can think of:
Share\IManager->transferShares
is not wrapped with transaction, and thus any termination in the middle could have resulted in mismatch betweenshare. uid_owner
andstorages.id
Related: https://github.com/owncloud/enterprise/issues/4121