Initialize the user before transfer-ownership

[jvillafanez]

Description

Initialize the user's FS if he hasn't logged in yet before transferring files to him.

Related Issue

https://github.com/owncloud/enterprise/issues/3819

Motivation and Context

Transfer was failing silently. The command output didn't show any error but the transfer failed

How Has This Been Tested?

2. Create 2 users, user1 and user2 (local users)
3. Upload some files to user1
4. Without having logged in once with user2, execute occ files:transfer-ownership user1 user2
5. Files are transferred correctly to the user2

Checked also with master-key encryption but not user-key encryption

Screenshots (if appropriate):

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:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:
• Changelog item, see TEMPLATE

[update-docs]

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.

[jvillafanez]

@phil-davis could you check the failing test? It seems to be caused by https://github.com/owncloud/core/blob/master/tests/lib/UtilTest.php#L388 and https://github.com/owncloud/core/blob/master/tests/lib/UtilTest.php#L411

I think the $config->deleteSystemValue('skeletondirectory'); isn't being called due to the expected exception being thrown in those tests. This causes the system value not being properly reset, which affects this PR's tests.

[codecov]

Codecov Report

Merging #37038 into master will increase coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #37038   +/-   ##
=========================================
  Coverage     64.75%   64.76%           
+ Complexity    19138    19136    -2     
=========================================
  Files          1270     1270           
  Lines         74912    74913    +1     
  Branches       1329     1328    -1     
=========================================
+ Hits          48513    48517    +4     
+ Misses        26008    26006    -2     
+ Partials        391      390    -1     

Flag	Coverage Δ	Complexity Δ
#javascript	54.18% <ø> (+0.01%)	0.00 <ø> (ø)
#phpunit	65.94% <100.00%> (+<0.01%)	19136.00 <1.00> (-2.00)	⬆️

Impacted Files	Coverage Δ	Complexity Δ
lib/private/Files/Storage/Wrapper/Checksum.php	97.05% <0.00%> (-2.95%)	26.00% <0.00%> (+1.00%)	⬇️
lib/private/Files/View.php	84.83% <0.00%> (ø)	389.00% <0.00%> (ø%)
lib/private/Share20/Manager.php	96.30% <0.00%> (ø)	273.00% <0.00%> (ø%)
lib/private/Console/Application.php	0.00% <0.00%> (ø)	23.00% <0.00%> (ø%)
lib/private/Setup/AbstractDatabase.php	0.00% <0.00%> (ø)	11.00% <0.00%> (-1.00%)
lib/private/Share/MailNotifications.php	96.19% <0.00%> (ø)	33.00% <0.00%> (ø%)
apps/dav/lib/DAV/FileCustomPropertiesBackend.php	89.32% <0.00%> (ø)	33.00% <0.00%> (ø%)
core/js/shareitemmodel.js	80.51% <0.00%> (+0.41%)	0.00% <0.00%> (ø%)
core/Command/App/Enable.php	100.00% <0.00%> (+12.12%)	5.00% <0.00%> (-3.00%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f055592...da6ac38. Read the comment docs.

[phil-davis]

@jvillafanez I pushed a commit that should fix that. Let's see how CI goes!

I also put the fix into a separate PR #37041 because it would be good to fix that UtilTest straight away, rather than waiting until this PR gets finished.

I wonder how many other places in the unit tests have "cleanup" code in a test case after an expected exception? Cleanup code should all go in the tearDown() method.

[phil-davis]

@jvillafanez If you pull this branch locally, rebase and force push, it should get rid of that 2nd commit - the UtilTest fixes are now merged in master

[jvillafanez]

I'll add the changelog entry after the review.

I have my doubts about including and using the --force-user-init. The only use case I see for the option is that you want to verify the destination user has logged in at least once. However, I think that normally you just want to transfer to that user.
I mean, we can remove the option and initialize the user without user confirmation.

[micbar]

@carlos @pako81 Can you confirm?

[pako81]

Having the --force-user-init input option would be maybe more transparent for the admin, to at least give him/her the information that the destination user has not log in yet.

On the other hand, final admin's aim is to just transfer files so, yes, I do agree with @jvillafanez in that we could probably get rid of it. My2c.

[micbar]

@jvillafanez Please update the title and the top post with the new approach.

Rest looks fine IMO

[jvillafanez]

Done