Skip to content

Add typings to config - #35581

Closed
J0WI wants to merge 1 commit into
nextcloud:masterfrom
J0WI:config-types
Closed

Add typings to config#35581
J0WI wants to merge 1 commit into
nextcloud:masterfrom
J0WI:config-types

Conversation

@J0WI

@J0WIJ0WI commented Dec 3, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Add some typing interfaces to config classes.

TODO (not in this PR)

  • get rid of the mixed '' vs null usage in default vaules.
  • replace getSystemValue with type save alternatives
  • maybe also implement type safe getUserValue and getAppValue

Checklist

github-advanced-security[bot]
github-advanced-securityAI found potential problems Dec 3, 2022
View reviewed changes
Comment threadapps/user_ldap/lib/User/User.php Fixed
Comment threadapps/user_ldap/lib/User/User.php Fixed
Comment threadlib/private/AppFramework/Services/AppConfig.php Fixed
Comment threadapps/files/list.php
$publicUploadEnabled = $shareManager->shareApiLinkAllowPublicUpload() ? 'yes' : 'no';

$showgridview = $config->getUserValue($userSession->getUser()->getUID(), 'files', 'show_grid', false);
$showgridview = $config->getUserValue($userSession->getUser()->getUID(), 'files', 'show_grid', 'false');

Check notice

Code scanning / Psalm

PossiblyNullReference

Cannot call method getUID on possibly null value
Comment threadapps/files/recentlist.php
$userSession = \OC::$server->getUserSession();

$showgridview = $config->getUserValue($userSession->getUser()->getUID(), 'files', 'show_grid', false);
$showgridview = $config->getUserValue($userSession->getUser()->getUID(), 'files', 'show_grid', 'false');

Check notice

Code scanning / Psalm

PossiblyNullReference

Cannot call method getUID on possibly null value
Comment threadapps/files/simplelist.php
$userSession = \OC::$server->getUserSession();

$showgridview = $config->getUserValue($userSession->getUser()->getUID(), 'files', 'show_grid', false);
$showgridview = $config->getUserValue($userSession->getUser()->getUID(), 'files', 'show_grid', 'false');

Check notice

Code scanning / Psalm

PossiblyNullReference

Cannot call method getUID on possibly null value
Comment threadapps/files_sharing/list.php
$userSession = Server::get(IUserSession::class);

$showgridview = $config->getUserValue($userSession->getUser()->getUID(), 'files', 'show_grid', false);
$showgridview = $config->getUserValue($userSession->getUser()->getUID(), 'files', 'show_grid', 'false');

Check notice

Code scanning / Psalm

PossiblyNullReference

Cannot call method getUID on possibly null value
Comment threadapps/files_trashbin/list.php
$userSession = \OC::$server->getUserSession();

$showgridview = $config->getUserValue($userSession->getUser()->getUID(), 'files', 'show_grid', false);
$showgridview = $config->getUserValue($userSession->getUser()->getUID(), 'files', 'show_grid', 'false');

Check notice

Code scanning / Psalm

PossiblyNullReference

Cannot call method getUID on possibly null value
@J0WIJ0WI added the 2. developing Work in progress label Dec 3, 2022
@szaimenszaimen added this to the Nextcloud 26 milestone Dec 4, 2022
@J0WI
Add typings to config
d22e50c
Signed-off-by: J0WI <J0WI@users.noreply.github.com>
This was referenced Dec 4, 2022
Merged
Merged
Closed
Merged
@come-nc

come-nc commented Dec 6, 2022

Copy link
Copy Markdown
Contributor

I would vote for adding String/Bool/Int variant to getUserValue and getAppValue as well.

@J0WI

J0WI commented Dec 6, 2022

Copy link
Copy Markdown
ContributorAuthor

I would vote for adding String/Bool/Int variant to getUserValue and getAppValue as well.

agreed, but it's way too much work to solve this at once.

@blizzzblizzz mentioned this pull request Feb 1, 2023
Merged
@skjnldsvskjnldsv mentioned this pull request Feb 23, 2023
Merged
@blizzzblizzz mentioned this pull request Mar 7, 2023
Merged
@blizzzblizzz modified the milestones: Nextcloud 26, Nextcloud 27Mar 9, 2023
This was referenced May 3, 2023
Merged
Closed
Merged
@skjnldsvskjnldsv mentioned this pull request Nov 1, 2023
Merged
This was referenced Nov 6, 2023
Merged
Merged
This was referenced Nov 14, 2023
Merged
Merged
@blizzzblizzz modified the milestones: Nextcloud 28, Nextcloud 29Nov 23, 2023
@AltahrimAltahrim mentioned this pull request Mar 12, 2024
Merged
This was referenced Mar 14, 2024
Merged
Merged
Merged
@skjnldsvskjnldsv removed this from the Nextcloud 31 milestone Aug 14, 2024
@sorbaugh

sorbaugh commented Aug 16, 2024

Copy link
Copy Markdown
Contributor

Hello @J0WI, it seems this PR has not had much activity for a while now. Please let us know if you'd like us to reopen this!

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

Reviewers

@ChristophWurstChristophWurstAwaiting requested review from ChristophWurstChristophWurst will be requested when the pull request is marked ready for reviewChristophWurst is a code owner
@miaulalalamiaulalalaAwaiting requested review from miaulalalamiaulalala will be requested when the pull request is marked ready for reviewmiaulalala is a code owner
@nickvergessennickvergessenAwaiting requested review from nickvergessennickvergessen will be requested when the pull request is marked ready for reviewnickvergessen is a code owner

Assignees

No one assigned

Labels

2. developingWork in progress

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@J0WI@come-nc@sorbaugh@github-advanced-security@blizzz@skjnldsv@szaimen