wordpress-mobile / WordPress-Android

WordPress for Android
http://android.wordpress.org
GNU General Public License v2.0
2.92k stars 1.3k forks source link

Fixes dynamic dashboard cards ordering in the new MySiteViewModel Architecture #20512

Closed antonis closed 1 month ago

antonis commented 1 month ago

Fixes https://github.com/wordpress-mobile/WordPress-Android/pull/19869#pullrequestreview-1948972666

Based on: https://github.com/wordpress-mobile/WordPress-Android/pull/19869

Description

Separates the dynamic cards so that they could be displayed at the top and bottom of the dashboard


To Test:

See pc8HXX-1sr-p2)


Regression Notes

  1. Potential unintended areas of impact

    • Dashboard
  2. What I did to test those areas of impact (or what existing automated tests I relied on)

    • Manual testing
  3. What automated tests I added (or what prevented me from doing so)

    • Relied on existing DynamicCardsViewModelSliceTest and CardsViewModelSliceTest

PR Submission Checklist:


Testing Checklist (strike-out the not-applying and unnecessary ones):

wpmobilebot commented 1 month ago
Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr20512-daa26aa
Commitdaa26aa13760e421b6833fd45437187aa8300f7c
Direct Downloadjetpack-prototype-build-pr20512-daa26aa.apk
Note: Google Login is not supported on these builds.
wpmobilebot commented 1 month ago
WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr20512-daa26aa
Commitdaa26aa13760e421b6833fd45437187aa8300f7c
Direct Downloadwordpress-prototype-build-pr20512-daa26aa.apk
Note: Google Login is not supported on these builds.
codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 26.31579% with 14 lines in your changes are missing coverage. Please review.

Project coverage is 40.34%. Comparing base (f4235e3) to head (81090f6).

:exclamation: Current head 81090f6 differs from pull request most recent head daa26aa. Consider uploading reports for the commit daa26aa to get more accurate results

Files Patch % Lines
...id/ui/mysite/cards/DashboardCardsViewModelSlice.kt 0.00% 8 Missing :warning:
...id/ui/mysite/cards/dashboard/CardViewModelSlice.kt 45.45% 1 Missing and 5 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## move-site-menu-items-to-viewmodel-slice #20512 +/- ## =========================================================================== - Coverage 40.34% 40.34% -0.01% =========================================================================== Files 1459 1459 Lines 67377 67389 +12 Branches 11160 11166 +6 =========================================================================== + Hits 27183 27185 +2 - Misses 37737 37745 +8 - Partials 2457 2459 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sonarcloud[bot] commented 1 month ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

antonis commented 1 month ago

I have looked at the ordering of the cards in the dashboard and made some minor changes.

Thank you for reviewing, improving the and making the PR ready for merge @AjeshRPai 🙇

Since there are no cards between the activity card and the bottom dynamic card, @antonis, I don't think it's necessary to expose bottom dynamic cards in the state, but I felt that it's a better practice and kept the change.

I agree 👍 Maybe it would be helpful if new cards are added in the future.

Can you double check it and merge it 🤔 ?

I retested with my sandbox and confirmed the order of the dashboard cards is the expected one.

Top Bottom
Screenshot_1710999711 Screenshot_1710999716

I'll proceed with merging on the main feature branch.