WordPress.org

Make WordPress Core

Opened 18 months ago

Last modified 8 weeks 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:
PR Number:

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 18 months ago.
Before and after actions for direction creation has been implemented.
44083.2.diff (930 bytes) - added by nx2017 18 months 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 18 months ago.
44083.diff (1.0 KB) - added by kkarpieszuk 16 months ago.
Changes suggested by @desrosj
44083.4.diff (2.7 KB) - added by donmhico 3 months ago.
Added unit tests.
44083.5.diff (3.0 KB) - added by donmhico 3 months ago.
Correct @since to adhere to WPCS.

Download all attachments as: .zip

Change History (21)

#1 @johnjamesjacoby
18 months ago

  • Description modified (diff)

@nx2017
18 months ago

Before and after actions for direction creation has been implemented.

@nx2017
18 months ago

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

#2 @seusmaniqbal
18 months 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.


18 months ago

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


17 months ago

#5 @desrosj
17 months 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 17 months ago by desrosj (previous) (diff)

#6 @seusmaniqbal
17 months ago

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

@kkarpieszuk
16 months ago

Changes suggested by @desrosj

#7 @kkarpieszuk
16 months 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 16 months ago by kkarpieszuk (previous) (diff)

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


12 months ago

#9 @SergeyBiryukov
12 months ago

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

#10 @pento
10 months ago

  • Milestone changed from 5.1 to 5.2

#11 @SergeyBiryukov
8 months ago

  • Milestone changed from 5.2 to 5.3

Missed the 5.2 Beta 1 deadline, moving to 5.3.

@donmhico
3 months ago

Added unit tests.

#12 @donmhico
3 months ago

  • Keywords needs-refresh removed

@donmhico
3 months ago

Correct @since to adhere to WPCS.

#13 @donmhico
3 months ago

  • Keywords has-unit-tests added

#14 @davidbaumwald
8 weeks ago

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

#15 @davidbaumwald
8 weeks 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.