[wp-trac] [WordPress Trac] #63206: WP_Filesystem and request_filesystem_credentials

[WordPress Trac]

#63206: WP_Filesystem and request_filesystem_credentials
------------------------------------------+------------------------------
 Reporter:  SirLouen                      |       Owner:  (none)
     Type:  enhancement                   |      Status:  new
 Priority:  normal                        |   Milestone:  Awaiting Review
Component:  General                       |     Version:
 Severity:  normal                        |  Resolution:
 Keywords:  needs-patch needs-unit-tests  |     Focuses:
------------------------------------------+------------------------------

Comment (by brianfo):

 Hello SirLouen,

 I believe it would be great but is there any further step about this?

 Replying to [ticket:63206 SirLouen]:
 > I want to open this ticket as a follow-up for #62718. I was trying to
 build some Unit Tests for this, and I was banging my head trying to figure
 out a test that made sense because all the edge cases, felt too obvious.
 >
 > But suddenly, I thought: "What if maybe `WP_Filesystem()` is not
 mutually exclusive with `request_filesystem_credentials`?"
 >
 > So after doing a little research, I found out, that there are a total of
 10 coincidences
 >
 > 1. https://github.com/WordPress/wordpress-
 develop/blob/3998c85e880321877abb8105a5b2a97021745fbf/src/wp-admin/update-
 core.php#L881
 > 2. https://github.com/WordPress/wordpress-
 develop/blob/3998c85e880321877abb8105a5b2a97021745fbf/src/wp-
 includes/update.php#L1131
 > 3. https://github.com/WordPress/wordpress-
 develop/blob/3998c85e880321877abb8105a5b2a97021745fbf/src/wp-
 admin/includes/theme.php#L46
 > 4. https://github.com/WordPress/wordpress-
 develop/blob/3998c85e880321877abb8105a5b2a97021745fbf/src/wp-
 admin/includes/class-wp-upgrader.php#L244
 > 5. https://github.com/WordPress/wordpress-
 develop/blob/3998c85e880321877abb8105a5b2a97021745fbf/src/wp-
 admin/includes/class-wp-upgrader.php#L1019
 > 6. https://github.com/WordPress/wordpress-
 develop/blob/3998c85e880321877abb8105a5b2a97021745fbf/src/wp-
 admin/includes/class-wp-site-health.php#L1896
 > 7. https://github.com/WordPress/wordpress-
 develop/blob/3998c85e880321877abb8105a5b2a97021745fbf/src/wp-
 admin/includes/ajax-actions.php#L4411
 > 8. https://github.com/WordPress/wordpress-
 develop/blob/3998c85e880321877abb8105a5b2a97021745fbf/src/wp-
 admin/includes/ajax-actions.php#L4757
 > 9. https://github.com/WordPress/wordpress-
 develop/blob/3998c85e880321877abb8105a5b2a97021745fbf/src/wp-
 admin/includes/plugin.php#L932
 >
 > 9 of which actually use `request_filesystem_credentials` to grab and
 pass the credentials.
 >
 > And just one that actually grab the credentials, but doesn't pass them.
 > https://github.com/WordPress/wordpress-
 develop/blob/3998c85e880321877abb8105a5b2a97021745fbf/src/wp-
 admin/includes/class-wp-site-health-auto-updates.php#L340
 > And something says that this code is only accounting for `direct`
 because right after the `WP_Filesystem()` call it will return a `false` if
 it's not `direct`
 >
 > So my question here is: Why not introducing
 `request_filesystem_credentials` straight on `WP_Filesystem` function, and
 definitely and future-proof it, removing all errors like the one in
 #62718?
 >
 > I think it's not intuitive the fact that `WP_Filesystem` is not actually
 adding `request_filesystem_credentials` by default, so unless you deal
 with this regularly, it's easy to miss.
 >
 > The problem with this is that this brings a major code refactor touching
 a couple of files at once. I may first write some unit tests to cover this
 scenario and then add a patch for this.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/63206#comment:1>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform