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: add File Downloads to Core Data #12275

Merged
merged 5 commits into from Aug 6, 2019

Conversation

@ScoutHarris
Copy link
Contributor

commented Aug 6, 2019

Fixes #11963
WPKit PR: wordpress-mobile/WordPressKit-iOS#175

To test:

  • Go to Stats period.
    • Verify File Downloads now has data. (This verifies the WPKit change.)
    • FD is not live in Calypso yet. Data can be verified via the console with the file-downloads endpoint. Ex: /sites/<site_id>/stats/file-downloads?period=day&date=2019-08-05&num=1
    • The filenames are displayed as-is. I've inquired on the FD P2 exactly how filenames are going to be displayed/formatted in Calypso, and will make any adjustments with a future change if necessary.

file_downloads

  • Disable your network.
    • Return to stats.
    • Verify File Downloads has data.

Update release notes:

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

This comment has been minimized.

Copy link

commented Aug 6, 2019

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

Generated by 🚫 dangerJS

@ScoutHarris ScoutHarris requested a review from danielebogo Aug 6, 2019

@ScoutHarris ScoutHarris self-assigned this Aug 6, 2019

@ScoutHarris ScoutHarris added this to In progress in Full Stats Refresh via automation Aug 6, 2019

@ScoutHarris ScoutHarris added this to the 13.1 milestone Aug 6, 2019

@ScoutHarris ScoutHarris referenced this pull request Aug 6, 2019
1 of 1 task complete
@ScoutHarris

This comment has been minimized.

Copy link
Contributor Author

commented Aug 6, 2019

@SylvesterWilmott - screenshots:

iphone

ipad_portrait

ipad_landscape

@ScoutHarris ScoutHarris requested a review from SylvesterWilmott Aug 6, 2019

@ScoutHarris ScoutHarris referenced this pull request Aug 6, 2019
48 of 77 tasks complete

Full Stats Refresh automation moved this from In progress to Reviewer approved Aug 6, 2019

@danielebogo
Copy link
Contributor

left a comment

Hey @ScoutHarris ! Code looks good and I had no problem while testing it. I noticed only that the text is truncated if it's too long but I think it's the current behaviour for all the rows right?
:shipit:

Simulator Screen Shot - iPhone X - 2019-08-06 at 12 51 56

@SylvesterWilmott

This comment has been minimized.

Copy link

commented Aug 6, 2019

Looks good @stephenie thanks.

The filenames are displayed as-is. I've inquired on the FD P2 exactly how filenames are going to be displayed/formatted in Calypso, and will make any adjustments with a future change if necessary.

Is there a possibility that we can ditch the path and only show the actual filename for this version?

@ScoutHarris ScoutHarris changed the base branch from develop to release/13.0 Aug 6, 2019

@ScoutHarris ScoutHarris changed the base branch from release/13.0 to develop Aug 6, 2019

@ScoutHarris

This comment has been minimized.

Copy link
Contributor Author

commented Aug 6, 2019

Hey @SylvesterWilmott .

Is there a possibility that we can ditch the path and only show the actual filename for this version?

Removing the path creates a potential for duplicate filenames. I did ask on the FD P2 if improvements could be made to the filename.

@ScoutHarris

This comment has been minimized.

Copy link
Contributor Author

commented Aug 6, 2019

Hey @danielebogo .

I noticed only that the text is truncated if it's too long but I think it's the current behaviour for all the rows right?

Yep, you are correct.

@ScoutHarris ScoutHarris merged commit ab1b4a3 into develop Aug 6, 2019

3 checks passed

Hound No violations found. Woof!
Peril Found some issues. Don't worry, everything is fixable.
Details
ci/circleci: build_and_test Your tests passed on CircleCI!
Details

Full Stats Refresh automation moved this from Reviewer approved to Done Aug 6, 2019

@ScoutHarris ScoutHarris deleted the feature/11963-cache_file_downloads branch Aug 6, 2019

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