Skip to content

Conversation

@blizzz
Copy link
Member

@blizzz blizzz commented Jul 11, 2017

With a downstream PR came a change in the user deletion process that rendered cleanup in LDAP broker. Key issue was the getHome() method. On cleaning up, it needs to return the user folder, but otherwise should throw an Exception.

This PR fixes it in a more reliable way. (It could have been more beautiful but it ate already too much time :( cannot spend more on refactoring )

Needs to be backported for 12.

Fixes #4117

@codecov
Copy link

codecov bot commented Jul 13, 2017

Codecov Report

Merging #5689 into master will increase coverage by 23.33%.
The diff coverage is 48%.

@@              Coverage Diff              @@
##             master    #5689       +/-   ##
=============================================
+ Coverage     30.54%   53.88%   +23.33%     
- Complexity    22497    25219     +2722     
=============================================
  Files          1403     1420       +17     
  Lines         87079    96613     +9534     
  Branches       1327     1402       +75     
=============================================
+ Hits          26601    52059    +25458     
+ Misses        60478    44554    -15924
Impacted Files Coverage Δ Complexity Δ
apps/user_ldap/lib/User/OfflineUser.php 0% <0%> (ø) 17 <1> (ø) ⬇️
apps/user_ldap/lib/User/Manager.php 78.04% <50%> (+78.04%) 22 <2> (+2) ⬆️
apps/user_ldap/lib/User_LDAP.php 71.83% <50%> (+71.83%) 73 <4> (+1) ⬆️
apps/files/appinfo/app.php 43.47% <0%> (-56.53%) 0% <0%> (ø)
lib/private/Hooks/LegacyEmitter.php 50% <0%> (-50%) 2% <0%> (+1%)
apps/files_sharing/appinfo/app.php 43.9% <0%> (-46.1%) 0% <0%> (ø)
apps/federation/lib/TrustedServers.php 55.37% <0%> (-43.16%) 39% <0%> (+16%)
apps/files_trashbin/appinfo/app.php 60% <0%> (-40%) 0% <0%> (ø)
lib/public/AppFramework/Controller.php 55.93% <0%> (-38.01%) 23% <0%> (+11%)
.../federation/lib/Middleware/AddServerMiddleware.php 52% <0%> (-34.67%) 5% <0%> (+1%)
... and 562 more

@blizzz blizzz force-pushed the fix-4117 branch 2 times, most recently from b67513c to f542e80 Compare July 13, 2017 14:35
@blizzz blizzz added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 13, 2017
@blizzz
Copy link
Member Author

blizzz commented Jul 13, 2017

Please review @nextcloud/ldap

blizzz added 2 commits August 31, 2017 23:03
homesToKill was not set in runtime since some changes some place else. It
required deleteUser() to be called first. The method acts independent of it
now.

Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz
Copy link
Member Author

blizzz commented Aug 31, 2017

rebased

@blizzz
Copy link
Member Author

blizzz commented Sep 4, 2017

Still looking for reviewers :)

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Tested and LDAP setup still works 👍

@MorrisJobke MorrisJobke merged commit ba2e1c5 into master Sep 13, 2017
@MorrisJobke MorrisJobke deleted the fix-4117 branch September 13, 2017 22:23
@blizzz
Copy link
Member Author

blizzz commented Sep 13, 2017

thx :) ❤️ i do the backport tomorrow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to delete User_LDAP user files/shares

4 participants