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

Fix Textarea when providing empty line, and prevent broken settings page #20070

Merged
merged 1 commit into from Dec 5, 2022

Conversation

alcalyn
Copy link
Contributor

@alcalyn alcalyn commented Dec 1, 2022

Description:

When addind an empty line in a TextArea in "General settings" page, i.e in ips whitelist:

1.1.1.1

2.2.2.2

The array value is stored as {"0": "1.1.1.1", "2": "2.2.2.2"} because of the array_filter.

Also the validate is called before the transform, which seems to be expected, but makes the error uncached.

The impact is the "General settings" page throw an error and stop rendering, so all plugin settings after the error are not rendered, then the page does not display all plugins settings. This PR also catch this js error in case other plugins have the same error, so that all plugins settings are displayed.

Review

Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @alcalyn for contributing this PR. That's very appreciated.
I was able to reproduce the problem you described. Your fixes should indeed solved that 💯
I left one suggestion, that needs to be applied. In addition we have commited the built vue files to git. So to finish the PR it would be needed to built the files.
For that you may need to run an npm install to fetch the related dependencies. Afterwards it should be possible to run ./console vue:build to build the vue files.
If you aren't able to build the files, let me know. I can also do that once the PR was merged.

} catch (e) {
// Prevent page breaking on unexpected modelValue type
console.error(e);
return [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method is supposed to return a string.

Suggested change
return [];
return '';

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@sgiehl sgiehl added the Bug For errors / faults / flaws / inconsistencies etc. label Dec 2, 2022
@alcalyn
Copy link
Contributor Author

alcalyn commented Dec 5, 2022

Thanks for the help, fixed it and vue files rebuilt.

Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating the PR. Works as expected now.

@sgiehl sgiehl added this to the 4.13.1 milestone Dec 5, 2022
@sgiehl sgiehl merged commit 84ec613 into matomo-org:4.x-dev Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants