-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CMM-1142 stats create todays card rust #22502
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
base: trunk
Are you sure you want to change the base?
Conversation
# Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/newstats/todaysstat/TodaysStatsCard.kt # WordPress/src/main/java/org/wordpress/android/ui/newstats/todaysstat/TodaysStatsViewModel.kt # WordPress/src/test/java/org/wordpress/android/ui/newstats/todaysstat/TodaysStatsViewModelTest.kt
Generated by 🚫 Danger |
| wordpress-lint = '2.2.0' | ||
| wordpress-persistent-edittext = '1.0.2' | ||
| wordpress-rs = 'trunk-21200d63735de310067d88c7dfb6080a2200f18e' | ||
| wordpress-rs = '1106-8dad0111fb97082cc2ab4280835d4e1e3167210a' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is pointing a PR on the wp repo. But, the idea is to merge it today and point to trunk again
Project dependencies changeslist! Upgraded Dependencies
rs.wordpress.api:android:1106-8dad0111fb97082cc2ab4280835d4e1e3167210a, (changed from trunk-21200d63735de310067d88c7dfb6080a2200f18e)
rs.wordpress.api:kotlin:1106-8dad0111fb97082cc2ab4280835d4e1e3167210a, (changed from trunk-21200d63735de310067d88c7dfb6080a2200f18e)tree +--- project :libs:fluxc
-| \--- rs.wordpress.api:android:trunk-21200d63735de310067d88c7dfb6080a2200f18e
-| +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.3.2 (*)
-| +--- com.squareup.okhttp3:okhttp-tls:4.12.0
-| | +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.3.2 (*)
-| | +--- com.squareup.okio:okio:3.6.0 -> 3.16.4 (*)
-| | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.21 -> 1.9.24 (*)
-| +--- net.java.dev.jna:jna:5.18.1
-| +--- rs.wordpress.api:kotlin:trunk-21200d63735de310067d88c7dfb6080a2200f18e
-| | +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.3.2 (*)
-| | +--- com.squareup.okhttp3:okhttp-tls:4.12.0 (*)
-| | +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.2 (*)
-| | \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.2.21 (*)
-| \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.2.21 (*)
+| \--- rs.wordpress.api:android:1106-8dad0111fb97082cc2ab4280835d4e1e3167210a
+| +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.3.2 (*)
+| +--- com.squareup.okhttp3:okhttp-tls:4.12.0
+| | +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.3.2 (*)
+| | +--- com.squareup.okio:okio:3.6.0 -> 3.16.4 (*)
+| | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.21 -> 1.9.24 (*)
+| +--- net.java.dev.jna:jna:5.18.1
+| +--- rs.wordpress.api:kotlin:1106-8dad0111fb97082cc2ab4280835d4e1e3167210a
+| | +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.3.2 (*)
+| | +--- com.squareup.okhttp3:okhttp-tls:4.12.0 (*)
+| | +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.2 (*)
+| | \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.2.21 (*)
+| \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.2.21 (*)
-\--- rs.wordpress.api:android:trunk-21200d63735de310067d88c7dfb6080a2200f18e (*)
+\--- rs.wordpress.api:android:1106-8dad0111fb97082cc2ab4280835d4e1e3167210a (*) |
|
| App Name | WordPress | |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr22502-abefb77 | |
| Commit | abefb77 | |
| Direct Download | wordpress-prototype-build-pr22502-abefb77.apk |
|
| App Name | Jetpack | |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr22502-abefb77 | |
| Commit | abefb77 | |
| Direct Download | jetpack-prototype-build-pr22502-abefb77.apk |
|
Claude finished @adalpari's task in 1m 37s —— View job Code Review Complete ✅I've completed a comprehensive review of PR #22502. Overall, the implementation is solid with good test coverage. Below are my findings organized by category. Todo List
✅ Strengths
🔴 Critical Issues1. Potential Data Loss in Long-to-Int Conversion📍 TodayStatsData(
views = result.aggregates.views.toInt(),
visitors = result.aggregates.visitors.toInt(),
likes = result.aggregates.likes.toInt(),
comments = result.aggregates.comments.toInt()
)Issue: Converting Recommendation: Either keep as
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## trunk #22502 +/- ##
==========================================
+ Coverage 38.84% 38.89% +0.04%
==========================================
Files 2193 2194 +1
Lines 105921 106024 +103
Branches 14982 14995 +13
==========================================
+ Hits 41148 41240 +92
- Misses 61333 61341 +8
- Partials 3440 3443 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
nbradbury
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! ![]()





Description
Second part of #22496, this PR uses the new wordpress-rs stats endpoint to fetch today stats. NO UI CHANGES
Testing instructions
Today's Stats Card displays correctly: