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

Fixed issue with search API optional parameters #308

Merged
merged 3 commits into from
Dec 30, 2023
Merged

Fixed issue with search API optional parameters #308

merged 3 commits into from
Dec 30, 2023

Conversation

artshade
Copy link
Contributor

Dear Developers,

Thank you for the project! 🚀

Please consider checking out this light fix regarding the issue which results in the following exception if no file nor any severity is selected:

Cannot assign string to property Opcodes\LogViewer\Readers\MultipleLogReader::$exceptLevels of type ?array {"exception":"[object] (TypeError(code: 0): Cannot assign string to property Opcodes\\LogViewer\\Readers\\MultipleLogReader::$exceptLevels of type ?array at /var/www/html/vendor/opcodesio/log-viewer/src/Readers/MultipleLogReader.php:39)
[stacktrace]
#0 /var/www/html/vendor/opcodesio/log-viewer/src/Http/Controllers/LogsController.php(66): Opcodes\\LogViewer\\Readers\\MultipleLogReader->exceptLevels('')
#1 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/ControllerDispatcher.php(46): Opcodes\\LogViewer\\Http\\Controllers\\LogsController->index(Object(Illuminate\\Http\\Request))
#2 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/Route.php(260): Illuminate\\Routing\\ControllerDispatcher->dispatch(Object(Illuminate\\Routing\\Route), Object(Opcodes\\LogViewer\\Http\\Controllers\\LogsController), 'index')

Possibly, the issue is related to #273.

Best and kind regards ✨

@arukompas
Copy link
Contributor

arukompas commented Dec 29, 2023

hey @artshade , thanks for the PR!

I took it for a spin and realised why this issue was happening for some but not all users.

Most Laravel setups use the \Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull middleware for all routes (defined in the HTTP kernel), and so the Log Viewer's $request->query('exclude_levels', []) would default to the empty array instead of an empty string.

But, when the middleware above is not used, Laravel passes the empty string as it is (it's not null, therefore does not default to the empty array).

I found that the only fix needed is the one you have done in the frontend logViewer store! 😄 The change makes sure not to submit the parameter at all if the array is empty, which is what we want. Then the controller can safely default to the empty array.

If you could revert the changes done on the LogsController, then I could merge the PR and tag a release.

@artshade
Copy link
Contributor Author

artshade commented Dec 29, 2023

hey @artshade , thanks for the PR!

I took it for a spin and realised why this issue was happening for some but not all users.

Most Laravel setups use the \Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull middleware for all routes (defined in the HTTP kernel), and so the Log Viewer's $request->query('exclude_levels', []) would default to the empty array instead of an empty string.

But, when the middleware above is not used, Laravel passes the empty string as it is (it's not null, therefore does not default to the empty array).

Dear @arukompas ,

Thank you very much for the reply!

Exactly this it seems! The local project does have Laravel 10.38 active, and the mentioned class ConvertEmptyStringsToNull exists and is even added into autoload classmaps but is not used in any actual functional code.

Considering the official defaults of the current Laravel project, the middleware is implemented but not used, too.

Applied the requested changes. Are you sure these are the only ones you want in this PR?

Best and kind regards! ✨

@arukompas arukompas merged commit 7cfc9d7 into opcodesio:main Dec 30, 2023
13 checks passed
@arukompas
Copy link
Contributor

Thanks a lot @artshade , tagged a new release v3.1.11 💪

Happy holidays! 🎉

@artshade artshade deleted the bugfix/log-viewer branch December 31, 2023 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants