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

Migrate post_groupDelete hook to share manager #23841

Merged
merged 1 commit into from
Apr 12, 2016

Conversation

rullzer
Copy link
Contributor

@rullzer rullzer commented Apr 7, 2016

For #22209

The hook now calls the share manager that will call the responsible
shareProvider to do the proper cleanup.

  • Unit tests added

Again nothing should change it is just to cleanup old code

CC: @schiesbn @nickvergessen @PVince81 @MorrisJobke @DeepDiver1975

@rullzer rullzer added this to the 9.1-current milestone Apr 7, 2016
@rullzer rullzer mentioned this pull request Apr 7, 2016
19 tasks
@rullzer rullzer force-pushed the share_move_delete_group_hook branch from b62c7d2 to d067fa8 Compare April 7, 2016 13:37
@rullzer rullzer changed the title Migrate post_userDelete hook to share manager Migrate post_groupDelete hook to share manager Apr 7, 2016
@rullzer rullzer force-pushed the share_move_delete_group_hook branch from d067fa8 to 3c17669 Compare April 7, 2016 17:49
@rullzer
Copy link
Contributor Author

rullzer commented Apr 7, 2016

Apparently I do something mysql does not like... I'll look into it

@rullzer rullzer force-pushed the share_move_delete_group_hook branch from 3c17669 to fe747c1 Compare April 7, 2016 18:13
@rullzer
Copy link
Contributor Author

rullzer commented Apr 7, 2016

Ok, mysql is happy now as well.
Review time.

$cursor->closeCursor();

$offset = 0;
$slice = array_slice($ids, $offset, 100);
Copy link
Contributor

Choose a reason for hiding this comment

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

array_chunk?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes... lets use that indeed..

@rullzer rullzer force-pushed the share_move_delete_group_hook branch from fe747c1 to 68953ab Compare April 8, 2016 09:40
}
$cursor->closeCursor();

if (count($ids) > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

!empty($ids)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@rullzer rullzer force-pushed the share_move_delete_group_hook branch from 68953ab to 1ec0865 Compare April 8, 2016 11:22
The hook now calls the share manager that will call the responsible
shareProvider to do the proper cleanup.

* Unit tests added

Again nothing should change it is just to cleanup old code
@MorrisJobke MorrisJobke force-pushed the share_move_delete_group_hook branch from 1ec0865 to 5899dda Compare April 11, 2016 08:57
@MorrisJobke
Copy link
Contributor

Rebased to retrigger CI

@MorrisJobke
Copy link
Contributor

Tested and works 👍

$sql = 'SELECT `id` FROM `*PREFIX*share` WHERE `share_type` = ? AND `share_with` = ?';
$result = \OC_DB::executeAudited($sql, array(self::SHARE_TYPE_GROUP, $arguments['gid']));
while ($item = $result->fetchRow()) {
Helper::delete($item['id']);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we delete Helper::delete() as well, or is it still used from elsewhere?

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay seems to be still used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah not yet... but 🔜

@nickvergessen
Copy link
Contributor

👍

@DeepDiver1975 DeepDiver1975 merged commit 495a964 into master Apr 12, 2016
@DeepDiver1975 DeepDiver1975 deleted the share_move_delete_group_hook branch April 12, 2016 07:46
@lock
Copy link

lock bot commented Aug 6, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants