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

Use of undefined constant GROCY_USER_ID (warning/notice) #1267

Closed
etam opened this issue Jan 12, 2021 · 3 comments
Closed

Use of undefined constant GROCY_USER_ID (warning/notice) #1267

etam opened this issue Jan 12, 2021 · 3 comments
Milestone

Comments

@etam
Copy link

etam commented Jan 12, 2021

Grocy: version: 3.0.1
PHP version: 7.4.6

When not logged in, this warning appears on the login page:

Warning: Use of undefined constant GROCY_USER_ID - assumed 'GROCY_USER_ID' (this will throw an Error in a future version of PHP) in /srv/www/grocy/controllers/BaseController.php on line 162

It refers to this line:
https://github.com/grocy/grocy/blob/v3.0.1/controllers/BaseController.php#L162

Obviously it's an error to call GetUserSetting, when no user is logged in.

@berrnd
Copy link
Member

berrnd commented Jan 12, 2021

It's a warning, not an error, so this is an enhancement, not a bug.

Of course, could be improved, however I don't care too much about warnings, there will probably a lot more places - simply disable them (which is IMHO the default/recommended setting for production environments).

@berrnd berrnd changed the title Bug: Use of undefined constant GROCY_USER_ID in controllers/BaseController.php on line 162 Use of undefined constant GROCY_USER_ID (warning/notice) Jan 12, 2021
@Forceu
Copy link
Contributor

Forceu commented Jan 12, 2021

I agree with etam, there should be a check if the constant is actually defined. At the moment this can potentially lead to bugs or even errors in future PHP versions.

@berrnd berrnd closed this as completed in e42f4b4 Jan 12, 2021
@berrnd berrnd added this to the vNEXT milestone Jan 12, 2021
@berrnd
Copy link
Member

berrnd commented Jan 12, 2021

This is one place were the warning is rendered to visible markup, there are (for sure) a lot of other places where warnings pop up in not visible markup - so always kind of funny that when it's visible, it's important to a lot of people, otherwise not...

So I rather have fixed the most important thing immediately. ;D

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

No branches or pull requests

3 participants