Add inline file delivery to WC_Download_Handler #28412
Conversation
…recalculate Update the tax rate meta when recalculating and updating order tax items
Create a new setting option for inline file delivery for downloadable products. In the Download Handler create a new method for the new setting as well as a method for returning the proper headers to deliver files inline. Resolves: woocommerce#28410
CaldwellYSR
force-pushed
the
allow-inline-downloads
branch
from
1e11fdc to
814fc8b
Compare
|
Not sure what happened with the 4 commits. I only authored two commits. Any progress on this would be greatly appreciated. It's been working on our site for a while now but it's technical debt to keep having to apply this update every time we need to update the plugin. |
vedanshujain
requested review from
a team and
vedanshujain
and removed request for
a team
vedanshujain
left a comment
vedanshujain
left a comment
There was a problem hiding this comment.
Let's add this as a setting actually instead of a separate download method, since it can be used with X-Accel-Redirect as well, and not just with force downloads.
We can then pass the value of this setting to download_headers and set the Content-Disposition header appropriately based on the params. That way, we won't have to create (and maintain) another download method, and another download header function. What do you think?
Since inline-delivery works with multiple download methods
@vedanshujain I agree with your suggestion and have made that change with the commit 386d75b |
|
@vedanshujain Any more suggestions for moving this forward? |
|
@vedanshujain I have been using this change for a while now and would love to get it merged into master so that I don't have to keep making the change every woocommerce update. Anything I can do or change to move this forward? |
vedanshujain
left a comment
vedanshujain
left a comment
There was a problem hiding this comment.
Looks like I accidentally unfollowed this thread and missed pings, sorry. I have left couple more comments, but I can take care of them myself as well, given that it has been a lot of time since the first round of review.
| 'desc' => __( 'Deliver downloadable files inline', 'woocommerce' ), | ||
| 'id' => 'woocommerce_downloads_deliver_inline', | ||
| 'type' => 'checkbox', | ||
| 'default' => 'yes', |
There was a problem hiding this comment.
this should be disabled to false to keep the current behavior as it.
| } | ||
| } | ||
|
|
||
| private static function get_content_disposition() { |
There was a problem hiding this comment.
this function is missing doc comments.
| continue; | ||
| } | ||
| $saved_rate_ids[] = $tax->get_rate_id(); | ||
| $tax->set_rate( $tax->get_rate_id() ); |
There was a problem hiding this comment.
Definitely unintended. Not sure where it came from apologies.
|
@vedanshujain I don't believe my fork is rebased to the upstream but I made the commented changes. |
No worries, I can take care of rebasing. Thanks for the patience, code looks fine, I am planning to do some more tests tomorrow before merging. |
Happy to hear it. Thanks again. |
|
Closing in favor of #31145 (rebased from trunk). |
4 participants
Create a new setting option for inline file delivery for downloadable
products. In the Download Handler create a new method for the new
setting as well as a method for returning the proper headers to deliver
files inline.
Resolves: #28410
All Submissions:
Changes proposed in this Pull Request:
Creates a new setting for allowing downloadable product files to be delivered inline so they would open directly in the browser. This would be useful for stores selling HTML or PDF files so customers can access them directly without the need to download.
Only other change was aligning
=>signs in an array to match WordPress indentation coding standards.Closes #28410 .
How to test the changes in this Pull Request:
Other information:
Changelog entry