Skip to content

Add typings to config #35581

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Add typings to config #35581

wants to merge 1 commit into from

Conversation

J0WI
Copy link
Contributor

@J0WI J0WI commented Dec 3, 2022 •
edited
Loading

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-security bot found potential problems Dec 3, 2022
View reviewed changes
apps/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
apps/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
apps/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
apps/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
apps/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
@J0WI J0WI added the 2. developing Work in progress label Dec 3, 2022
@szaimen szaimen 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
Copy link
Contributor

come-nc commented Dec 6, 2022

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

@J0WI
Copy link
Contributor Author

J0WI commented Dec 6, 2022

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.

@blizzz blizzz mentioned this pull request Feb 1, 2023
Merged
@skjnldsv skjnldsv mentioned this pull request Feb 23, 2023
Merged
@blizzz blizzz mentioned this pull request Mar 7, 2023
Merged
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 9, 2023
This was referenced May 3, 2023
Merged
Closed
Merged
@skjnldsv skjnldsv modified the milestones: Nextcloud 27, Nextcloud 28 May 9, 2023
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
Merged
This was referenced Nov 6, 2023
Merged
Merged
This was referenced Nov 14, 2023
Merged
Merged
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@Altahrim Altahrim mentioned this pull request Mar 12, 2024
Merged
This was referenced Mar 14, 2024
Merged
Merged
Merged
@skjnldsv skjnldsv modified the milestones: Nextcloud 29, Nextcloud 30 Mar 28, 2024
@skjnldsv skjnldsv modified the milestones: Nextcloud 30, Nextcloud 31 Aug 14, 2024
@skjnldsv skjnldsv closed this Aug 14, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 31 milestone Aug 14, 2024
@sorbaugh
Copy link
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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst ChristophWurst will be requested when the pull request is marked ready for review ChristophWurst is a code owner

@miaulalala miaulalala Awaiting requested review from miaulalala miaulalala will be requested when the pull request is marked ready for review miaulalala is a code owner

@nickvergessen nickvergessen Awaiting requested review from nickvergessen nickvergessen will be requested when the pull request is marked ready for review nickvergessen is a code owner

@st3iny st3iny Awaiting requested review from st3iny st3iny will be requested when the pull request is marked ready for review st3iny is a code owner

Assignees

No one assigned

Labels

2. developing Work in progress

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@J0WI @come-nc @sorbaugh @blizzz @skjnldsv @szaimen