Skip to content

Update class-wp-document-revisions.php - property subdir is required.#342

Merged
benbalter merged 1 commit intowp-document-revisions:mainfrom
earthlingdavey:main
Mar 8, 2024
Merged

Update class-wp-document-revisions.php - property subdir is required.#342
benbalter merged 1 commit intowp-document-revisions:mainfrom
earthlingdavey:main

Conversation

@earthlingdavey
Copy link
Copy Markdown
Contributor

Property subdir is required, so don't remove it during the filter, just set it to an empty string.

https://developer.wordpress.org/reference/functions/wp_upload_dir/

@earthlingdavey
Copy link
Copy Markdown
Contributor Author

earthlingdavey commented Mar 8, 2024

FWIW. WP Media Offload shows a notice and depreciation warning due to the missing property.

Related PR deliciousbrains/wp-amazon-s3-and-cloudfront#642

@benbalter benbalter enabled auto-merge March 8, 2024 17:12
@benbalter benbalter merged commit f8062bc into wp-document-revisions:main Mar 8, 2024
@earthlingdavey
Copy link
Copy Markdown
Contributor Author

Thanks for merging :)

NeilWJames added a commit to NeilWJames/wp-document-revisions that referenced this pull request Mar 11, 2024
…t-revisions#342)

A fix has been made available. This updates the fix to make it consistent.
[If the option "Organize my uploads into month- and year-based folders" is ticked, then the value should be set to yyyy/mm.
@NeilWJames NeilWJames mentioned this pull request Mar 11, 2024
@NeilWJames
Copy link
Copy Markdown
Collaborator

Thanks for identifying the issue.
Looking at _wp_upload_dir, I agree that the subdir component believe that it should be set to year/month if that is what the user had set it to.
This is done via setting it in the output parameter by simplying copying it from the input.
I have created a pull request for this.

@eriktorsner
Copy link
Copy Markdown

Hi,

I'm Erik from the WP Offload Media dev team. First, thank you @earthlingdavey for reporting this and also suggesting a fix in our plugin. Also thanks to @NeilWJames for looking into this and solving it so quickly.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants