search

FriendsOfFlarum / upload

The file upload extension with insane intelligence for your Flarum forum.
https://discuss.flarum.org/d/4154
MIT License
177 stars 96 forks source link

Fix wrong setting values #230

Closed the-turk closed 3 years ago

the-turk commented 4 years ago

Modified line was returning true everytime when the default value is set to true and stored value is set to false. This makes impossible to disable a setting which's default value is true.

I've wanted to use the unused __isset() magic method.

clarkwinkelmann commented 4 years ago

Good observation!

But do we actually pass true as a default value anywhere?

Looking at the code I see all instances where we use the default parameter are numeric settings, where the current approach isn't causing any issue.

If it's not broken, maybe we don't need to fix it :man_shrugging:

I find that Settings helper class is a bit of a monstrosity. The __get() magic methods should probably be removed as we're not using them. But again, until it breaks, I'm not sure it's really worth changing it.