WordPress.org

Make WordPress Core

Opened 6 weeks ago

Last modified 5 hours ago

#53171 assigned defect (bug)

Total number of attachments not updated when attachment uploaded

Reported by: joedolson Owned by:
Milestone: 5.8 Priority: normal
Severity: normal Version: trunk
Component: Media Keywords: has-patch needs-testing 2nd-opinion
Focuses: javascript, administration Cc:

Description

With the removal of infinite scroll in #50105, we're now displaying a value for the total number of attachments as part of the 'load more' interface. This value doesn't update if an attachment is added or removed from the visible collection.

It looks like the the total attachments are only set when there's a collection query, and this isn't run when an item is uploaded or deleted. See the parse method in media/models/attachments.js, where this.totalAttachments is defined.

Attachments (1)

53171.diff (2.1 KB) - added by joedolson 3 days ago.
First pass

Download all attachments as: .zip

Change History (8)

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


5 weeks ago

#2 @desrosj
8 days ago

  • Keywords needs-patch added

This one still needs a patch. Today is the beta 1 deadline, but I think this one can remain as it directly relates to a feature/enhancement being shipped in 5.8.

@joedolson do you think this is realistic to include in 5.8? Ideally this would be fixed before beta 3 on 22 June. But absolutely must be fixed by 29 June to be included.

#3 @joedolson
8 days ago

I think it's feasible, assuming we work out a change to how the number of attachments is passed into BackBone in #50105, which is currently in progress. But it depends on solving that problem, so there definitely won't be a patch for this before that's solved.

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


8 days ago

#5 @antpb
8 days ago

We are leaving this open through Beta 1 as this is partially merged or related to a partially merged feature. All that is needed is a review of the above patch and potentially refinement. If the patch can't be merged before Beta 3 we'll need to revert to allow time for the revert to be tested before Release Candidate.

@joedolson
3 days ago

First pass

#6 @joedolson
3 days ago

  • Keywords has-patch needs-testing 2nd-opinion added; needs-patch removed

I'm not sure this is the best way to handle this, but it does work. Patch adds two private functions attached to the observer on the attachments model, one tracking additions and one on removals. It just increments the total attachments up or down one based on behavior.

Tested when adding an attachment, deleting an attachment, bulk deleting attachments, and bulk adding items.

What I'm unsure about is whether incrementing up and down is really going to be sufficiently reliable - however, we aren't updating the query results on add/remove events, so getting that from the AJAX response doesn't seem viable.

I tested this with 50105.13.diff applied from #50105, but I don't think anything in this patch actually depends on that.

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


5 hours ago

Note: See TracTickets for help on using tickets.