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

Stats - File downloads stats card #10411

Merged
merged 11 commits into from Aug 22, 2019

Conversation

@planarvoid
Copy link
Contributor

commented Aug 19, 2019

Related FluxC changes - wordpress-mobile/WordPress-FluxC-Android#1352

This PR adds a new card - File downloads - to the granular stats screens. The card shows a list of files and the number of downloads in a DESC order by downloads. We only started recording the data after July 29, 2019 so we show a different empty message when the user selects a date before this date. Everything else should work like the other cards.

The file downloads is not supported on Jetpack sites so we hide it there.

I've removed the RemoteTests file because it was completely broken.

Screenshot 2019-08-19 at 10 59 22

Screenshot 2019-08-19 at 10 58 46

Screenshot 2019-08-19 at 10 58 25

To test:

  • Go to WP.com site
  • Go to DWMY Stats
  • Scroll down
  • You see the file downloads card
  • Select a date before July 29, 2019
  • You see the "unsupported" error message
  • Go to a Jetpack site
  • Go to DWMY stats
  • The File downloads card is not visible

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

@planarvoid planarvoid added this to the 13.2 milestone Aug 19, 2019

@planarvoid planarvoid requested a review from 0nko Aug 19, 2019

@planarvoid planarvoid self-assigned this Aug 19, 2019

@peril-wordpress-mobile

This comment has been minimized.

Copy link

commented Aug 19, 2019

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

@planarvoid planarvoid requested review from khaykov and 0nko and removed request for 0nko and khaykov Aug 19, 2019

@0nko 0nko self-assigned this Aug 20, 2019

@0nko
Copy link
Contributor

left a comment

I've tested the WP.com and Jetpack sites with different dates and everything works normally. Nice work, Vojta! 💯

There's just a minor issue with margin when the error message is displayed (see the screenshot):

image

@@ -1,673 +0,0 @@
package org.wordpress.android.ui.stats;

This comment has been minimized.

Copy link
@0nko

0nko Aug 20, 2019

Contributor

Why was this file deleted?

This comment has been minimized.

Copy link
@planarvoid

planarvoid Aug 22, 2019

Author Contributor

I've moved it into a separate PR. I doesn't really belong here but the code was broken because the old stats code was already removed. It's just that we never run/build the connected tests

@0nko

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2019

Also, I just noticed that when I tap on the View more button, I can see the loading animation and when it's done there's just blank screen. When I go back and open the View all stats the data's there:

view_more

@peril-wordpress-mobile

This comment has been minimized.

Copy link

commented Aug 22, 2019

You can test the changes on this Pull Request by downloading the APK here.

@wordpress-mobile wordpress-mobile deleted a comment from wpmobilebot Aug 22, 2019

@wordpress-mobile wordpress-mobile deleted a comment from wpmobilebot Aug 22, 2019

@wordpress-mobile wordpress-mobile deleted a comment from wpmobilebot Aug 22, 2019

@wordpress-mobile wordpress-mobile deleted a comment from wpmobilebot Aug 22, 2019

@planarvoid

This comment has been minimized.

Copy link
Contributor Author

commented Aug 22, 2019

@0nko I'll look into the "View all" issue separately. Everything else should be fixed and this should be ready for another check. thanks!

@0nko
0nko approved these changes Aug 22, 2019
Copy link
Contributor

left a comment

Looking good! 👍

@0nko 0nko merged commit 0faff59 into develop Aug 22, 2019

5 checks passed

Peril Found some issues. Don't worry, everything is fixable.
Details
ci/circleci: Installable Build Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: strings-check Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details

@0nko 0nko deleted the feature/file_downloads_stats_card branch Aug 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.