Skip to content
This repository has been archived by the owner on Jul 12, 2024. It is now read-only.

Download Report API: Add path to response, fix ordering, and fix schema typo #1248

Merged
merged 2 commits into from
Jan 8, 2019

Conversation

justinshreve
Copy link
Collaborator

Fixes #1245.

This PR fixes a few issues with the download report endpoints.

  • It adds a download_path to the response which contains a URL to the download of the file.
  • It fixes a typo between download_count and downloads_count in the schema.
  • It fixes the ?orderby option so it actually sorts. Tests included.

Note on download_path:
This links to the file path that is stored in the product data. This can link externally or to the media library. It is different from a WC download link. Download links are tied to orders/users, so to use that we would need to generate permissions for each admin and link here otherwise.

Detailed test instructions:

  • Running phpunit should be enough to test the changes here, though you can make manual requests or test the report page as well.

@justinshreve justinshreve self-assigned this Jan 7, 2019
@justinshreve justinshreve requested review from Aljullu and a team January 7, 2019 18:11
Copy link
Contributor

@Aljullu Aljullu left a comment

Choose a reason for hiding this comment

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

This is looking good to me. :shipit:

Probably that's outside of the scope of this PR, but I couldn't see match: any behaving differently than match: all.

Using match: any with a downloadIp_includes that matches one download but a user_includes that doesn't match any, returns 0 results while I would expect it to return 1. Am I missing something or is there a bug here?

@justinshreve
Copy link
Collaborator Author

Sounds like a bug - I'll take a look in a follow-up PR!

@justinshreve justinshreve merged commit c61bdbe into master Jan 8, 2019
@justinshreve justinshreve deleted the update/download-file-response-and-schema branch January 8, 2019 14:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
focus: analytics Issues about Analytics/Reports focus: wc rest api
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Downloads report: file name should link to the file
2 participants