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 crash in all time stats #9552

Merged
merged 2 commits into from
Apr 9, 2019
Merged

Fix crash in all time stats #9552

merged 2 commits into from
Apr 9, 2019

Conversation

planarvoid
Copy link
Contributor

Fixes #8886

Sometimes the viewsBestDay seems to be empty. The date parses cannot handle this use case. I'm changing the code so we only parse a non-empty date. If it's empty, we don't show the tooltip.

I've added a test to cover this scenario

To test:

  • Go to Stats/Insights on an empty Site
  • The App doesn't crash

@planarvoid planarvoid added this to the 12.2 ❄️ milestone Apr 9, 2019
@planarvoid planarvoid self-assigned this Apr 9, 2019
@planarvoid planarvoid requested review from 0nko and jtreanor April 9, 2019 12:40
fun `best day is null when it's empty`() = test {
val forced = false
val refresh = true
val model = InsightsAllTimeModel(1L, null, 1, 0, 0, "", 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Reporter: Checkstyle
Rule: no-multi-spaces
Severity: ERROR
File: /home/circleci/project/WordPress/src/test/java/org/wordpress/android/ui/stats/refresh/lists/sections/insights/usecases/AllTimeStatsUseCaseTest.kt L134
Unnecessary space(s)

@0nko 0nko self-assigned this Apr 9, 2019
Copy link
Contributor

@0nko 0nko left a comment

Choose a reason for hiding this comment

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

The code looks good and it doesn't crash on a fresh site 👍.

@0nko 0nko merged commit dcf5aa3 into release/12.2 Apr 9, 2019
@0nko 0nko deleted the fix_crashin_all_time_stats branch April 9, 2019 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants