[wp-trac] [WordPress Trac] #26992: Failing to make new yyyy/mm for uploads affects existing URLs (WP_CONTENT_URL, wp_get_attachment_url and GUID)

[WordPress Trac]

#26992: Failing to make new yyyy/mm for uploads affects existing URLs
(WP_CONTENT_URL, wp_get_attachment_url and GUID)
-------------------------------------------------+-------------------------
 Reporter:  tivnet                               |       Owner:  (none)
     Type:  defect (bug)                         |      Status:  new
 Priority:  normal                               |   Milestone:
Component:  Media                                |     Version:  2.7
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch needs-unit-tests needs-    |     Focuses:
  test-info 2nd-opinion                          |
-------------------------------------------------+-------------------------
Changes (by SirLouen):

 * keywords:  has-patch needs-testing => has-patch needs-unit-tests needs-
     test-info 2nd-opinion


Comment:

 Replying to [ticket:26992 tivnet]:
 > I believe, something has to be changed here, so '''when the upload
 directory for new attachments cannot be created, it won't affect the
 existing attachments'''.

 I have to agree with @tivnet. The fallback behaviour is too unexpected in
 this scenario, it feels like left at random, not all use cases, like these
 are covered correctly.

 For code correctness, leaving this permissions' scenario to such fallback
 is inadequate. Surely, with permission sorting this might not happen, but
 with the right fallback logic, this should be restrained.

 I'm not 100% convinced about @wpdavis patch, though. It can solve the
 problem, but it's broadening the error condition to new unexpected
 behaviour. If we could add some Unit Tests before modifying this logic
 (`needs-unit-tests`), it would feel more safe overall, looking for a
 better condition.

 The thing here is that, regardless or not, of having an error (permission
 error), doing such GUID fallback, when not desired is not ideal. In fact,
 I would like to see Test Info (`needs-test-info`), on when falling back to
 GUID is the right thing. I've been reviewing unit tests for a while and I
 can't see this in action.

 I would like to hear other experienced members on media opinions (`2nd-
 opinion`)

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