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 "strange statistics" #14

Merged
merged 1 commit into from
Jan 5, 2021

Conversation

skmedix
Copy link
Contributor

@skmedix skmedix commented Dec 26, 2020

Fixes flarum/framework#2528

Before:
image
After:
image

Copy link
Member

@askvortsov1 askvortsov1 left a comment

Choose a reason for hiding this comment

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

Makes sense to me! One quick question before approving: how would a non-integer value get processed in the first place? In addition to validating here, we might also want to ensure that we're only dealing with integers throughout the process.

@skmedix
Copy link
Contributor Author

skmedix commented Jan 5, 2021

@askvortsov1 This is a great question, but unfortunately I don't have answer for it yet. This behavior is really inconsistent between machines, and I can't find the real cause of it, but I added an additional casting in the backend though.

From my debug session, it must happens before any data manipulation, as results has already wrong type after the first query.
When I dumped the results variable on both of my machines, one machine output'ed:
array(1) { ["2020-12-23"]=> int(1) } }
which is correct, by second machine output'ed:
array(1) { ["2019-12-23"]=> string(1) "1" } }

In the future I would like to make even more precise debug with dockerized examples, but it should be good for now. :)

Copy link
Member

@askvortsov1 askvortsov1 left a comment

Choose a reason for hiding this comment

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

Either way, I'm in favor of the additional validation / type casting.

@askvortsov1 askvortsov1 merged commit 6fcdc5f into flarum:master Jan 5, 2021
@skmedix skmedix deleted the dev/fix-numbers-sum branch January 5, 2021 21:45
askvortsov1 pushed a commit that referenced this pull request Mar 11, 2022
askvortsov1 pushed a commit that referenced this pull request May 10, 2022
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.

Statistics dashboard has irrationally large numbers
3 participants