Enhanced file administration

Sounds like a good idea from your description. I haven't tested, but a quick glance at the patch shows an odd comment:

+/*
+ * Menu callback:
+ * Pages where users add and delete their subscriptions to nodes
+ */

Looks useful to me. If I had more uploads on my dev site I would try it out.

Some code comments:

- don't reuse upload_page() for both user and admin page. split off admin stuff into a new menu callback which points to a new upload_admin_page() function.
- in the query for admin page, don't directly use the UID when building the $sql. instead, pass that as an argument at end of pager_query(). this protects against SQL injection attack
- do we really want to show file locations to regular users? i think not.
- perhaps show node title in the file listing tables

your proposed enhancements look swell to me.

related question: do attachments on unpublished nodes acount against user quota? if so, user is powerless to delete them? this module could fix that I think. when unpublished, i suggest showing node title without hyperlink.

yeah, i think we are ok on file location .... you might consider releasing this as a contrib module if core is uninterested.

Hey Junyor, awesome patch!

I've recently wrote my own PHP based file management script that does very similar (but not in a Drupal enivorment). I've attached a screen shot of the interface.

As for operations, I'd love to see the basics like what I have. Copy (file or folder), Move (file or folder), Zip (file or folder) for download, and obviously delete, upload, and create folder :) Also, I have a function that cleans file names too, replacing spaces and weird punctucation with a certain character (right now just a _) to avoid conflicts on Windows based machines. This feature could obviously be enabled or turned off.

I researched this quite a bit and I've found some top notch code for all of these operations, all open source as well. Additionally, all of the icons are from this set that was released as open source as well so we could include those too.

I'd love to contribute and work with you to further extend this module. Having solid file management features would be great in Drupal.

Junyor, just drop me a line through the Drupal contact and I'd be glad to discuss things and aid in the development of this!!

Looking good but needs some work.

The link to each file that has been upload is malformed: http://localhost/drupal/?q=http://localhost/drupal/files/upload.module Some reason that is absolute and it shouldn't be, looks like line 280 is the problem there. 'Page not found' with dirty URLs. However, with clean URLS, I get 403 forbidden "access denied" to this link: http://localhost/drupal/files/upload.module

Is there a way to set a limit on total file size per user? When I goto "my uploads" it says limit per file and per directory is 1mb yet I don't see any way to set these in Settings > Uploads. Also, shouldn't you be able to set different limits for each user as well?

Additionally, under "my uploads" it says allowable extensions but I don't see anywhere to define those as well?

Under latest HEAD with Drupal running on PHP 5.0.3 I get the following warnings:

warning: array_merge() [<a href='function.array-merge'>function.array-merge</a>]: Argument #1 is not an array in \modules\upload.module on line 124.warning: array_flip() [<a href='function.array-flip'>function.array-flip</a>]: The argument should be an array in \modules\upload.module on line 133.warning: implode() [<a href='function.implode'>function.implode</a>]: Bad arguments. in \modules\upload.module on line 139.

Seems all of these warnings are being caused by the extensions variable I pointed out... not defined correctly, and this *should* be configurable option in Settings > Uploads ... not hardcoded like it is in the module.

Please let me know if I can help, I think we can get this into core, it is definetly needed!

Ok turns out if file dowloads are set to "Private" the links work fine. If they are "Public" over HTTP, they break, as noted above.

Now going along with this, it would be great if an Admin could set permissions for each of these files for viewing... e.g., if files are set up to be "Private" to assign which roles can actually view or download these files or to keep them anonymous. I think this would be bring the file management features full circle and finally be up to par with this :)

Ok looks like the links for public files are formed correctly now. However, I am getting a "Forbidden" access when trying to view the file like: http://localhost/drupal/files/somefile.txt ... is that a problem with Drupal or the module? I checked .htaccess and don't see it restricting files, hmmm.

All of the warnings have been fixed and are working correctly now with PHP 5.

Now you say "Give a specific role the 'upload files' permission and you'll be able to configure max file size and extensions." What if I don't assign this permission to anyone? In that case, UID=1 logged in can only upload files up to 1MB in size and has a total of 1MB of space. Shouldn't Upload > Settings have a default setting then that takes precedence? For instance, go in there and set defaults to 25MB for each. Then logged in as UID=1, I can upload files within those limits. If I then assign another role this permission, they should inherit "this default" instead of having it hard coded as 1MB and having to change again. Does this make sense? From a usability perspective this was how I *expected* it to work but didn't see what was going on till you clarified :)

For the extensions list, I would make the code more robust. I can throw in as many spaces as I want and it is saved with them intact. I would think taking a more standard route of defining extensions like: html,php,txt,doc with a common character seperating extensions, easier to read too especially on a long list :). Then you can just explode(',') and trim() each term, preventing managled extensions from being entered in the database.

Any link or updates with that overhaul of the filesystem? I'd be interested in helping out, who is spearheading that?

Oh had one more idea... shouldn't there be a global total size limit? As logged in as UID=1 and I'm setting up my website, I should be able to specify a total size for all files with this Drupal install... either 0 for unlimited (which I believe it is now) or enter some number.

Then when I assign different roles the ability to adminster uploads, they can set file size limits too but all of these limits come under one giant umbrella limit that that main admin sets up. Otherwise I could see some problems with authenticated users assigning limits *too* big. Need some sort of check there :)

Sorry for the mass amount of critiques, just want to get this patch squared away as perfect as possible. It's really important to get into core so gotta make sure we cover all of our bases!

Well I shouldn't get 'file forbidden' logged in as UID=1 but I do. However, I have 'authenticated user' set with all of the upload permissions and I still get that error when I try to download a file with Drupal set to 'public' files as a setting.

As for cleaning up the settings, I do believe that is part of this patch and definetly within the scope :) In terms of usability, these settings should be moved around a bit. However, in terms of the code freeze, I believe this patch is ready but it does need a little bit of tweaking before release.

Patch ready to be committed! Tested and works great. Forbidden file problem and settings problem are do to inadequacies wit the current Drupal file system, not this patch! More patches needed :)

Settings > Uploads shows the same thing as Uploads ... looks like the settings from the other patch weren't moved over correctly?

Patch works great! Tested out all sorts of circumstances, patch works flawlessly. This is *much* needed functionality in core. This patch doesn't actually add anything that isn't in core, rather it just makes this information readily available which is *very* helpful. Much needed and I think because of that this should go in 4.7.

Rerolled patch to fix a usability issue on Settings > Upload ... using form_group_collapasible() cleans up page when you have more than roll with file permissions.

+1. Yes yes. patch worked well. Really like the statistics feature, and the planned extensibility with the drop-down action. hope this makes it in.

Souvent22, m3avrck: please review patches more carefully!

This patch needs a lot of work:
1. Many strings can't be translated.
2. Why do we sort the data in PHP and not in MySQL.
3. Why do we retrieve all records in MySQL but not show them all. Why do we show 10 users only?

Problem (2) and (3) should be fixed by creating a table that can be (i) sorted and (ii) paged.

Dries didn't really look through the code too much, my fault! Just tested to make sure it *actually* works. Will do that tomorrow morning, unless Junyor can patch it before me :)

In upload_total_space_used(), it looks like your query will give right results if you JOIN to the node table instead of node_revisions. same for your "too many items in list" problem. I didn't test these theories, just perused the patch.

Dries, just a heads up that Junyor, Souvent, and myself have been discussing this issue all day in #upload ... many hours went into this patch, all of your concerns have been addressed, along with many others we have discovered. This patch is rock solid and we'll be posting one final patch very soon that completes this. Better administrative options is all this patch introduces, no changes to how files are handled. Also, this patch uncovered 2 bugs inherent to revisions that were overlooked so this patch has been very worthwhile :)

Ok, fixed a few things. From the top now:

- Fixed the quries that were returning incorrect information. Namley, disk usage per user, and total disk usage.
- Added a new function to calcuate the number of files a user has.
- Formatting issues.

Now, there is a serperate issue, which should be addressed in a seperate bug, which is the current layout of the file table. I believe a second table (associatve table) should be used to link revisions to files, instead of lots of redundant data in the files table. This is a normalization issue. However, I am using the system as is, and have adjusted the quries, etc. to fit.

Hi guys!

Sorry for jumping in so lately on the threat, but I reviewed the code and I must say that I like what I see..
Though, some problems code-wise imo:

+      form_set_error('', t('Please select some items to perform the update on.'));

Why is the first argument not set? IMO this should always be set, otherwise use drupal_set_message('error', $message).

Secondly you guys are using:

+  $output .= theme('box', t('Statistics'), $content); 

By the 'introduce theme wrapper function' threath, comment #37 Dries encourages people to remove the usage of theme('box').. So, looking at the future you guys can probably do the same? Try not using the theme('box'), but try something else...

For what's left, I love the functionality!!! Very, very good work..

Junyor,

Souvent and I spent many hours last night after you posted your patch fixing many many bugs.

1. Yes we have another count query but this one actually works ;-) count(*) has the same aggregate problems as sum() and because of the nature of the files table, will *only* work this way without subselects. either the tables get changed around a bit or subselects be allowed. since the later assumes everyone is using MySQL 4.1 seesm like the former is the answer. We have already created an issue for this: http://drupal.org/node/31354

In addition, this patch *won't* get in if it assumes that another patch will to fix the table, this is not good. Souvent and myself have already committed ourselves to this patch and the other patch and will update things accordingly. If our propsed patch gets in with the correct queries, we'll post a patch later to fix the files table. Once that patch goes in, we'll post a patch to "re-fix" the uploads module to get it up to speed. Sure this is double work, but this is the best method and only way we can get this patch in now. We'll take care of it :)

3. We can change these back to 50, was just a bit too many in our opinions but consisently rules out.

4. We didn't introduce any "workarounds". Rather, we got it working as is :) Once the updated revisions patches go in: http://drupal.org/node/31355 we'll accordingly refactor this patch. Again, this patch can't wait for others, otherwise it won't go in.

5. Again, same problem with subselects.

6. We already fixed it.

Again, the biggest problem is the fact that Drupal does *not* support subselects, hence we have to be creative in our queries :) Sure a better files table would help, *however* this won't necessarily fix all of the queries magically. I still see the subselects coming into play. We rigorously tested all of the queries last night and they do indeed your work. Your patch was broken in many cases because of this.

We can roll another patch addressing Stefan's concerns.

Stefan: Souvent, Junyor and myself all discussed your ideas. The first, form_error('') is indeed valid. The content tab in admin uses the same interface setup and uses the function in the exact same way. Check node.module to see this.

Second, while we agree with the theme('box') issue, the rest of the Drupal still uses theme('box') in the same type of ways. Since there is no "better" way to do this, we went with consisentancy since we can't come up with a better alternative.

Also, we have all reached an agreement, this patch does indeed work and works *very* well. The issues with files and revisions is completely seperate. This patch works 100% and if later on their are core changes to files and revisions, we'll update upload.module accordingly (which we hope to do too :)).

New patch in a bit to fix the other outstanding problems.

Updated patch, addressed remaining issues and cleaned up a few small things. Ready to go from this side!

Small typo in query just discovered, fixed now.

Looks solid, and works solid. +1. This is a big help the the uplaods arena. I also like how this patch has been planned; regarding known issues (file table, etc.), and def. going to address those, but for now, it would be nice to have this functionality at a minnimum.

Hello, I've scanned the patch and you need to change
$result = db_query('SELECT fid,filesize FROM {files} f INNER JOIN {node} n ON f.nid = n.nid WHERE n.uid = %d GROUP BY fid', $uid);
in upload_space_used() to
$result = db_query('SELECT fid,filesize FROM {files} f INNER JOIN {node} n ON f.nid = n.nid WHERE n.uid = %d GROUP BY fid, filesize', $uid);

(i.e. add filesize to GROUP statement. Otherwise it won't work in postgres. This should not change anything, as all rows with the same fid have the same filesize (if I understand the meaning of fid correctly)).

The same goes to $result = db_query('SELECT fid,filesize FROM {files} GROUP BY fid'); in upload_total_space_used() - needs filesize in GROUP BY.

Updated SQL as per Cvbge's comments.

Sounds useful to me too. Not having read all the comments, would it be possible to get some nice table of how much space users use? I have a site with a small 100 MB of uploads, and sometimes I want to know who uploaded the most.

praseodym, already built in this patch, there is a tab with basic file upload statistics right now :-) hope to add more statistical information later but this patch sets the stage for everything.

See the above comment as a +1 and some future ideas :).

This patch would add a lot of very usefull features to drupal. I hope you guys are going to work a little more on this, maybe it'll even get committed before the release of 4.7?

Steef

Moving to 6.x-dev queue.

IMHO, this should all be in a contrib module if possible. Most users just want basic uploading functionality, they don't care about advanced administration and reporting on uploads.

Feature requests go to 7.x.

Subscribing +1

Subscribing.

Alright, so I've done a little bit of working rerolling this for HEAD. Not everything is there from the original patch, but it's a start. I just want to get some more feedback on it before I continue and because I'm not positive on how to correctly finish this up.

Issues

• Right now file deletion doesn't actually work, as I'm not positive of the right way to do that right now.
• Files are also listed by FID instead of filnames that are links. I'm still getting used to FAPI, so I'm not positive on the right way to pass the necessary data to the theme_* function.
• Tablesorting is also not setup fully, as I'm not positive on how to do so in 6.x.

Work to do:

• Display files nicely (owner, filenames+link to file, filesize, ...)
• Remove users from list who don't have any uploaded files (useless clutter and it makes sorting through remaining users easier)
• Be able to filter all information listed similar to URL Aliases at admin/build/path

Nice!

Any further progress on this?

This feature request could be related/of interest:

Up(load)API module:
"Support for zip uploads (and extracts) and multiple uploads of single local files"
http://drupal.org/node/236299

@stefan nagtegaal: I'm actually looking for more input on the patch I submitted in #55, as I'm not sure how best to proceed regarding some of the problems I've run into. I'm not familiar enough with FAPI as well as some other things that higher-ups probably have an opinion on. If someone wants to pitch in with some code or suggestions on how to code some of the other parts of this patch, I'd like to get this all wrapped up.

Alright, so I've made a little more progress. Everything has been resolved except for the following:

• Tablesorting is still an issue as I'm not familiar with how this is done on 7.x-dev
• File deletion is still not working properly and I haven't spent the time to fully debug this.

The code probably needs better documentation and some general cleanup, but the gist of it is here now. If anyone wants to pitch in on the tablesorting, that'd be great.

The tablesorting is now fixed and the only issue that needs to be resolved now is file deletion.

Please test and let me know what you think.

I'm curious about what anyone else thinks about this patch, especially someone with commit rights. I'm not sure if I should continue to work on this patch as I'm not sure if it should be included in core. Using the modules within core, it seems like only upload.module will deal with files of any kind. Supposedly the new files table organization means that files are tied to users and not nodes, so I would think file administration would make sense, though files that exist without a node doesn't ever seem to happen. Can anyone offer some input into this?

Feature Request: Per File Download Permissions. Have a situation right now that I have a single file that I only want certain groups to have access to and a Sysadmin whose only permissions are to keep this file up to date.

+1 Excellent. Must make available for 7.x

Reroll.

Files don't ever seem to exist without a node, though. I think this functionality is not as important if this could all be done through the node administration pages, though that wouldn't be as clean.

Also, was file deletion added, it wasn't part of my last patch?

Per coding standards, please follow comments with full-stops (periods).

+  $output .= t('<p>Total disk space used: %size</p>', array('%size' => format_size($total_size)));
+
+  $output .= t('<p>If users aren\'t listed below, then they have no files uploaded to the site.</p>');

Also, where possible, try to reduce the amount of HTML in strings that must be managed by translators. Something like:

+  $output .= '<p>' . t('Total disk space used: %size', array('%size' => format_size($total_size))) . '</p>';
+
+  $output .= '<p>' . t('If users aren\'t listed below, then they have no files uploaded to the site.') . '</p>';

This is going to need a full overhaul now that hook_file's been committed. Should be a lot cleaner now though.

Reroll, not tested ...

This shouldn't be limited to just files created by the upload module. It should work for any file that the site knows about so I moved it into the system.module. I'd toyed with adding a new file.module but decided I'd hold off on that until I can see how the core maintainers feel about it.

I clean up quite a few things and tried to bring this in line with the node module but broke a few things since I can't figure out how the whole delete confirmation is supposed to work in the same form where you're selecting the files. Perhaps when I've had some sleep it'll be apparent to me.

that last patch had some unrelated changes rolled in.

one thing i'd like to do with this is provide a /system/content/files/%file page that lists the file's properties and which modules are referencing it.

i also think we need to give the deleting some thought since there are now "hard" and "soft" deletes. the hard ones override the any module that claim to be using the files where the soft deletes only remove files that aren't in use.

i forked this code to start with a clean issue (#322287: Add file.module to provide UI for managing files) since we're really at a different point when this issue started back in 2005