-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Added support for transferring incoming file shares. #28118
Conversation
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.
Had a deeper look at the logic and added some comments
|
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.
Thanks for the changes so far.
I had a few more comments after doing some tests.
See test results above.
I also took the liberty to push an integration test to your branch for the reported cases 😄
Originally the customer only required the share transferring to the users root directory, |
94974f8
to
a97a11c
Compare
@ipasanec I have rebased the PR. You might need to delete your local branch and check it out again. I've also pushed a fix for the path matching, but for some reason the new integration test fails and I'm trying to debug it. |
@ipasanec seems we had a collision and pushed something at the same time I've resolved the conflicts. I think for now it looks good, I'll check why the integration test isn't passing despite representing the scenarios I manually tested. |
I've fixed the tests, there was a regexp mismatch and I used the wrong wording. |
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.
👍 I hereby approve the code from @ipasanec
second reviewer should also check my commits
- new option --transfer-incoming-shares=1 | 0 - new config.php option 'transfer-incoming-shares' => true | false The command line option overrules the config.php option. Signed-off-by: Vincent Petry <vincent@nextcloud.com>
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
ed81309
to
5b664e0
Compare
I've rebased and squashed to reduce the number of commits and remove the merge commits. |
Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22 |
/backport to stable22 |
Shouldn't be this documented too? 🤔 |
I'll take care:
|
docs done! |
I've been looking for documentation about how to use this flag and it's not clear to me what the values mean. I've seen I've got backups, but I'm wary about running commands on real-world data with flags that I don't understand. Short term request: please explain to me what they do. |
Please open a ticket at https://github.com/nextcloud/documentation/issues/new For any further questions please use https://help.nextcloud.com/c/support/7. Github is not a support forum. |
The basic usage is: The default value of '2' for an unset option is necessary to detect a missing value in the command line and give an error message to notify the user. btw. there is no --include-incoming-shares option. (It was changed in the original project, hence some variables use 'include' in the name) |
Fix #21856
@PVince81
--transfer-incoming-shares=1 | 0
'transfer-incoming-shares' => true | false
'transferIncomingShares' => true | false
The command line option overrules the config.php option.