The array of files returned by upload_load is in no particular order. This patch adds an ORDER BY to the query so that array_pop on the result array will give you the file with the lowest fid. It then becomes possible to do things with the files based on their order.

CommentFileSizeAuthor
#8 upload.patch.txt806 byteskilles@www.drop.org
57307.patch759 bytesrobertDouglass
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dopry’s picture

Status: Active » Reviewed & tested by the community

code looks good... idea is sound.

moshe weitzman’s picture

+1 from me too. code looks proper. minimal change.

Gerhard Killesreiter’s picture

Status: Reviewed & tested by the community » Fixed

applied

Anonymous’s picture

Status: Fixed » Closed (fixed)
jhriggs’s picture

Version: » 4.7.2
Priority: Normal » Critical
Status: Closed (fixed) » Active

OK, although the idea may be sound, this broke a lot of feeds. Upload.module encloses the first attached file in feeds. By ordering with fid DESC rather than ASC, this changed the enclosed file on any feed items with more than one attachment. So, for instance, all of my podcasts that have a PPT slideshow attached also are now completely borked, because they enclose the pdf file rather than the mp3. While I agree the attachments should be ordered, they should be ordered ascending rather than descending (or it should be an option).

jhriggs’s picture

(I should have mentioned, of course, that it breaks things when upgrading from a previous version.)

Gerhard Killesreiter’s picture

We chould probably change DESC to ASC (or simply remove it).

killes@www.drop.org’s picture

Status: Active » Needs review
FileSize
806 bytes

Solution: Remove DESC

Dries’s picture

Looks OK to me.

killes@www.drop.org’s picture

Version: 4.7.2 » x.y.z
Status: Needs review » Reviewed & tested by the community

applied to 4.7.

robertDouglass’s picture

Does that mean it is closed? Or are we waiting to have it forward-ported to HEAD?

killes@www.drop.org’s picture

we are just waiting for Dries to commit to HEAD.

drumm’s picture

Status: Reviewed & tested by the community » Fixed

Committed to HEAD.

Anonymous’s picture

Status: Fixed » Closed (fixed)