-
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
Accept to ocm group 2 #40886
Accept to ocm group 2 #40886
Conversation
Signed-off-by: Michiel de Jong <michiel@unhosted.org>
All Broken tests are fixed Signed-off-by: Michiel de Jong <michiel@unhosted.org> Signed-off-by: navid-shokri <naviid-shokrii@gmail.com>
Signed-off-by: navid-shokri <naviid.shokrii@gmail.com>
return defauldt Provider for Share_Type_Remote_Group
Fix/review comments
…s' into accept-ocm-to-groups
Who is reviewing this PR? |
Let's still target master here and try to get it in 10.13. @jvillafanez can you maybe review this as apparently you already had a look at this? |
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.
just a tiny change from my side, but I think it's ok.
I can't test it, so I assume it works as intended.
} | ||
return new JSONResponse(); | ||
} | ||
|
||
private function initGroupManager() { |
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.
we probably should rename this function. There is a "groupManager" which is widely used (https://github.com/owncloud/core/blob/master/lib/public/IGroupManager.php) so it might be confusing.
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.
@jvillafanez Hi, I've changed the name a bit to "initGroupExternalManager", it should not be confused with simple "groupManager" anymore since it explicitly declares it is external.
If it still counts as confusing name, let me know.
@shokri-navid @michielbdejong please see above and address the change request if possible. |
@shokri-navid @michielbdejong if this is needed for 10.13 then it would need to be merged till end of this week. We want to have 10.13 code freeze after that. |
@pako81 @jvillafanez Hi, would you look at the change and see if this is good to merge? |
good to merge from my side |
Kudos, SonarCloud Quality Gate passed! |
Description
These are the hooks we need in the core repo to allow the FederatedGroups app to deal with outgoing and incoming OCM shares to group.
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: