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

Download reporting #17679

Merged
merged 38 commits into from Nov 14, 2017

Conversation

Projects
None yet
3 participants
@mikejolley
Member

mikejolley commented Nov 13, 2017

This is a continuation of #16418

TODO:

  • PHPCS

procifer and others added some commits Jul 30, 2017

[#12517] Replace download_id on customer download and product downloa…
…d with static UUID instead of filename based hash, to preserve download links, logs, and permissions across filename changes
[#12517] Order download permissions: Use i18n number formatting on do…
…wnload count and proper escaping on report links
[#12517] Move download log table to const and helper, output download…
… log report titles in standalone function, reorganize download log data store read for code readability
[#12517] When tracking downloads, increment/decrement download count …
…and downloads remaining in SQL to avoid race conditions with updating in PHP
[#12517] Deprecate process_product_file_download_paths function since…
… download log ids should no longer change based on file paths
[#12517] Remove calls to action woocommerce_process_product_file_down…
…load_paths since download ids should no longer change. They are now static UUIDs.

mikejolley added some commits Nov 13, 2017

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Nov 13, 2017

Codecov Report

Merging #17679 into master will increase coverage by <.01%.
The diff coverage is 18%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #17679      +/-   ##
============================================
+ Coverage     34.89%    34.9%   +<.01%     
- Complexity    12197    12286      +89     
============================================
  Files           332      336       +4     
  Lines         50016    50400     +384     
============================================
+ Hits          17455    17591     +136     
- Misses        32561    32809     +248
Impacted Files Coverage Δ Complexity Δ
includes/class-wc-data-store.php 75.55% <ø> (ø) 16 <0> (ø) ⬇️
...a-stores/class-wc-customer-download-data-store.php 58.45% <0%> (+11.64%) 28 <0> (ø) ⬇️
includes/class-wc-download-handler.php 0% <0%> (ø) 66 <0> (+1) ⬆️
...dmin/meta-boxes/class-wc-meta-box-product-data.php 0% <0%> (ø) 78 <0> (ø) ⬇️
includes/admin/class-wc-admin-post-types.php 0% <0%> (ø) 357 <7> (+6) ⬆️
includes/class-wc-post-data.php 59.68% <0%> (-2.63%) 78 <0> (-5)
...-wc-customer-download-log-data-store-interface.php 0% <0%> (ø) 0 <0> (?)
includes/class-woocommerce.php 7.34% <0%> (-0.05%) 74 <0> (ø)
...cludes/admin/reports/class-wc-report-downloads.php 0% <0%> (ø) 48 <48> (?)
includes/class-wc-product-download.php 47.94% <0%> (-8.4%) 31 <0> (ø)
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 800877c...a50f30f. Read the comment docs.

codecov bot commented Nov 13, 2017

Codecov Report

Merging #17679 into master will increase coverage by <.01%.
The diff coverage is 18%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #17679      +/-   ##
============================================
+ Coverage     34.89%    34.9%   +<.01%     
- Complexity    12197    12286      +89     
============================================
  Files           332      336       +4     
  Lines         50016    50400     +384     
============================================
+ Hits          17455    17591     +136     
- Misses        32561    32809     +248
Impacted Files Coverage Δ Complexity Δ
includes/class-wc-data-store.php 75.55% <ø> (ø) 16 <0> (ø) ⬇️
...a-stores/class-wc-customer-download-data-store.php 58.45% <0%> (+11.64%) 28 <0> (ø) ⬇️
includes/class-wc-download-handler.php 0% <0%> (ø) 66 <0> (+1) ⬆️
...dmin/meta-boxes/class-wc-meta-box-product-data.php 0% <0%> (ø) 78 <0> (ø) ⬇️
includes/admin/class-wc-admin-post-types.php 0% <0%> (ø) 357 <7> (+6) ⬆️
includes/class-wc-post-data.php 59.68% <0%> (-2.63%) 78 <0> (-5)
...-wc-customer-download-log-data-store-interface.php 0% <0%> (ø) 0 <0> (?)
includes/class-woocommerce.php 7.34% <0%> (-0.05%) 74 <0> (ø)
...cludes/admin/reports/class-wc-report-downloads.php 0% <0%> (ø) 48 <48> (?)
includes/class-wc-product-download.php 47.94% <0%> (-8.4%) 31 <0> (ø)
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 800877c...a50f30f. Read the comment docs.

@claudiulodro

This comment has been minimized.

Show comment
Hide comment
@claudiulodro

claudiulodro Nov 13, 2017

Contributor

Deprecation notices are breaking a bunch of the tests: https://travis-ci.org/woocommerce/woocommerce/jobs/301563517

Contributor

claudiulodro commented Nov 13, 2017

Deprecation notices are breaking a bunch of the tests: https://travis-ci.org/woocommerce/woocommerce/jobs/301563517

@@ -560,7 +560,7 @@ private static function get_schema() {
) $collate;
CREATE TABLE {$wpdb->prefix}woocommerce_downloadable_product_permissions (
permission_id BIGINT UNSIGNED NOT NULL auto_increment,
download_id varchar(32) NOT NULL,
download_id varchar(36) NOT NULL,

This comment has been minimized.

@claudiulodro

claudiulodro Nov 13, 2017

Contributor

Do we need an update script for this, too? Otherwise old DBs won't properly handle the new download IDs?

@claudiulodro

claudiulodro Nov 13, 2017

Contributor

Do we need an update script for this, too? Otherwise old DBs won't properly handle the new download IDs?

This comment has been minimized.

@mikejolley

mikejolley Nov 14, 2017

Member

dbdelta handles it

@mikejolley

mikejolley Nov 14, 2017

Member

dbdelta handles it

@claudiulodro

This comment has been minimized.

Show comment
Hide comment
@claudiulodro

claudiulodro Nov 13, 2017

Contributor

Looks good and works well. Just a question and the tests need fixing.

Contributor

claudiulodro commented Nov 13, 2017

Looks good and works well. Just a question and the tests need fixing.

@claudiulodro claudiulodro merged commit cc5c182 into master Nov 14, 2017

2 of 5 checks passed

codecov/patch 18% of diff hit (target 34.89%)
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
Scrutinizer 34 new issues, 34 updated code elements
Details
codecov/project 34.9% (+<.01%) compared to 800877c
Details

@claudiulodro claudiulodro deleted the update/12517-download-reporting branch Nov 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment