Skip to content

Strict settings - #8743

Merged
rullzer merged 3 commits into
masterfrom
strict_settings
Mar 12, 2018
Merged

Strict settings#8743
rullzer merged 3 commits into
masterfrom
strict_settings

Conversation

@rullzer

Copy link
Copy Markdown
Member

For #8375

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzerrullzer added 2. developing Work in progress technical debt 🧱 🤔🚀 labels Mar 9, 2018
@rullzerrullzer added this to the Nextcloud 14 milestone Mar 9, 2018
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@codecov

codecovBot commented Mar 9, 2018

Copy link
Copy Markdown

Codecov Report

Merging #8743 into master will decrease coverage by 20.35%.
The diff coverage is 4.25%.

@@ Coverage Diff @@## master #8743 +/- ##
=============================================
- Coverage 51.79% 31.44% -20.36% 
Complexity 25377 25377 =============================================
Files 1608 1608 Lines 95173 95172 -1 Branches 1377 1377 =============================================
- Hits 49297 29927 -19370 - Misses 45876 65245 +19369
Impacted FilesCoverage ΔComplexity Δ
settings/Controller/UsersController.php0% <0%> (-68.9%)121 <114> (ø)
settings/Controller/ChangePasswordController.php16.36% <40%> (ø)26 <26> (ø)⬇️
lib/private/TagManager.php0% <0%> (-100%)4% <0%> (ø)
...public/AppFramework/OCS/OCSBadRequestException.php0% <0%> (-100%)1% <0%> (ø)
apps/user_ldap/lib/Migration/UUIDFixGroup.php0% <0%> (-100%)1% <0%> (ø)
...ivate/Files/Cache/Wrapper/CachePermissionsMask.php0% <0%> (-100%)3% <0%> (ø)
apps/user_ldap/lib/LDAPProviderFactory.php0% <0%> (-100%)2% <0%> (ø)
apps/files_trashbin/lib/AppInfo/Application.php0% <0%> (-100%)2% <0%> (ø)
apps/files_trashbin/lib/Hooks.php0% <0%> (-100%)3% <0%> (ø)
lib/private/Accounts/Hooks.php0% <0%> (-100%)14% <0%> (ø)
... and 374 more

@rullzerrullzer added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 9, 2018
private $l;

/** @var IGroupManager */
/** @var GroupManager */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just adding both here should work and we can still only request IGroupManager ?
Or why is it needed?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Else it can't resolve

$this->groupManager->getSubAdmin()->isUserAccessible($currentUser, $targetUser))
and phan keeps complaining.

I guess for Settings and Core it is fine to inject the non public ones. As we don't always want to expose everything.

@MorrisJobkeMorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tested and still works 👍

@rullzer
rullzer merged commit af89db3 into masterMar 12, 2018
@rullzer
rullzer deleted the strict_settings branch March 12, 2018 11:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviewstechnical debt🧱 🤔🚀

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@rullzer@nickvergessen@MorrisJobke@ChristophWurst