WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 9 months ago

#44083 reviewing enhancement

Add action to wp_mkdir_p() when directory is created successfully

Reported by: johnjamesjacoby Owned by: SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version: 2.0.1
Component: Filesystem API Keywords: good-first-bug has-patch has-unit-tests
Focuses: Cc:

Description (last modified by johnjamesjacoby)

It would be nice if the wp_mkdir_p() function contained a hook for plugins to interact with when a directory gets created.

The use-case I have currently is that Easy Digital Downloads (for example) creates .htaccess files and empty index.php files in its own uploads/edd directory, and each directory inside it, as one way of protecting those empty directories from being publicly browsable.

Because there is no action hook here, EDD uses a daily transient, which leaves new directories potentially open until the transient expires.

An action hook on directory creation would allow for EDD to create those files immediately.

Attachments (6)

44083.1.diff (573 bytes) - added by nx2017 2 years ago.
Before and after actions for direction creation has been implemented.
44083.2.diff (930 bytes) - added by nx2017 2 years ago.
Before and after actions for directory creation has been implemented. Target attribute is set with documentation.
44083.3.diff (871 bytes) - added by seusmaniqbal 2 years ago.
44083.diff (1.0 KB) - added by kkarpieszuk 2 years ago.
Changes suggested by @desrosj
44083.4.diff (2.7 KB) - added by donmhico 11 months ago.
Added unit tests.
44083.5.diff (3.0 KB) - added by donmhico 11 months ago.
Correct @since to adhere to WPCS.

Download all attachments as: .zip

Change History (21)

#1 @johnjamesjacoby
2 years ago

  • Description modified (diff)

@nx2017
2 years ago

Before and after actions for direction creation has been implemented.

@nx2017
2 years ago

Before and after actions for directory creation has been implemented. Target attribute is set with documentation.

@seusmaniqbal
2 years ago

#2 @seusmaniqbal
2 years ago

  • Keywords has-patch added; needs-patch removed

Following concerns about the above patch:

  • Remove extra line from the inline documentation.
  • As for the hook, I think it should be more specific with its name like before_directory_creation.

Thanks

This ticket was mentioned in Slack in #core by abdullahramzan. View the logs.


2 years ago

This ticket was mentioned in Slack in #core by seusmaniqbal. View the logs.


2 years ago

#5 @desrosj
2 years ago

  • Keywords needs-refresh added

@seusmaniqbal this is looking good. Handful of small things.

  • Can we tweak the action names a bit more. How about before_create_directory / after_create_directory.
  • The @since docblock tags don't need a description when it indicates the function is being introduced. Only when changes are made do these need notes.
  • Can we change the doclbock descriptions a bit? Thinking "Fires before the directory is created." and "Fires after the directory is created and the permissions are set." sound a bit better.
  • For the after action, the $target parameter should not say attempt because it only fires if the directory is created. How about "Full path to the created directory."?
  • Please add spaces between the opening and closing parentheses and the contents of the actions to adhere to the coding standards.
  • I think we could include the $dir_permissions as a second optional filter parameter. Might be useful for plugins to be able to match permissions or perform actions only when the directory has a certain permission level.
  • Can we move the after action below the if() statement? This way, if the permissions need to be reset because of the presence of a umask, the action can reasonably expect the permissions on the directory to be correct.
Last edited 2 years ago by desrosj (previous) (diff)

#6 @seusmaniqbal
2 years ago

@desrosj Thank you for feedback. I will update new patch soon.

@kkarpieszuk
2 years ago

Changes suggested by @desrosj

#7 @kkarpieszuk
2 years ago

Hello

I am trying to contribute to the WordPress first time and decided to help here a bit :) I have attached diff with all suggestions from @desrosj applied, I hope I helped

Additionally, I updated @since to 5.0 (the same which I found in wp-includes/version.php)

Last edited 2 years ago by kkarpieszuk (previous) (diff)

This ticket was mentioned in Slack in #core by sergey. View the logs.


19 months ago

#9 @SergeyBiryukov
19 months ago

  • Milestone changed from Awaiting Review to 5.1
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#10 @pento
18 months ago

  • Milestone changed from 5.1 to 5.2

#11 @SergeyBiryukov
16 months ago

  • Milestone changed from 5.2 to 5.3

Missed the 5.2 Beta 1 deadline, moving to 5.3.

@donmhico
11 months ago

Added unit tests.

#12 @donmhico
11 months ago

  • Keywords needs-refresh removed

@donmhico
11 months ago

Correct @since to adhere to WPCS.

#13 @donmhico
11 months ago

  • Keywords has-unit-tests added

#14 @davidbaumwald
9 months ago

@SergeyBiryukov Can this be realistically included by tomorrow for 5.3 Beta 1? If not, we can punt to 5.4.

#15 @davidbaumwald
9 months ago

  • Milestone changed from 5.3 to Future Release

With 5.3 Beta 1 shipping today, I’m punting this Future Release. If this can be committed to for 5.4, feel free to move to that milestone.

Note: See TracTickets for help on using tickets.