Closed (fixed)
Project:
File Entity (fieldable files)
Version:
7.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
25 Sep 2011 at 17:29 UTC
Updated:
11 Oct 2011 at 16:41 UTC
Jump to comment: Most recent file
Comments
Comment #1
JacobSingh commentedComment #2
JacobSingh commentedComment #3
JacobSingh commentedoops, bad diff.
Comment #4
dave reidWe should probably add a file_get_operation_info() function that runs the module_invoke_all() and then a drupal_alter() so we only have to call that rather than invoke/alter each time. This function should go in file_entity.module.
Default value is approve but we don't have an approve operation.
Don't think we can use media functions here.
Should be 'No files available.' and use 'file' in CSS classes.
Rather than re-invoking a hook, let's store the operations in $form['#operations'].
Let's use $files = file_load_mulitple(array_keys($files)) since entities can be cached.
Again let's use file_load_multiple() here.
We should probably provide a valid example, even if it's something silly.
Is the permission 'administer file' or 'administer files'?
Comment #5
JacobSingh commentedComment #6
JacobSingh commentedhere's the diff 'o' diffs
Comment #7
ParisLiakos commentedApplied #5
getting this when trying to delete a file in file/%/delete
Comment #8
JacobSingh commentedargh... okay, this isn't too pretty, but it works.
Comment #9
ParisLiakos commentedWorks!
but a side note:
When you go to file/% and hit delete you are being sent to file/%/delete with no destination.So if you hit cancel,u stay on the same page,if you hit delete you get 404 not found:)
Comment #10
JacobSingh commentedokay, another try :)
Comment #11
dave reidWe're using a media variable here. We should just have a default of 50.
Still no alter hook here.
For links of operations, we should really use the standard in node_admin_nodes with theme_links:
$options[$file->fid]['operations'] = array(
'data' => array(
'#theme' => 'links__file_operations',
'#links' => $operations,
'#attributes' => array('class' => array('links', 'inline')),
),
);
Documentation isn't wrapper properly at 80 characters.
Comment #12
JacobSingh commentedComment #13
ParisLiakos commentedTested again.Cancel link sends you to homepage,delete button deletes and just reloads the page resulting to 404
Comment #14
JacobSingh commentedokay, dave's fixes and delete redirect flow + a require_once...
Comment #15
JacobSingh commentedComment #16
ParisLiakos commentedcool,works now:)
but maybe get the cancel link back to file/%/view instead of the homepage?(sorry for this,didnt think that befrore)
but yeah this is working
Comment #17
JacobSingh commentedCool, thanks! I won't post another patch for just that one change, but I agree. as soon as @davereid signs off I'm sending it in!
Comment #18
dave reidtheme('username') needs a user account object, so really we should be passing user_load($file->uid) here for 'account'. This can be done as a follow-up.
Default local tasks do not need descriptions - this information is not displayed anywhere.
There is also trailing whitespace in the patch but otherwise if this has been tested when applied and people can delete normally, then this should be good to go. We can have some follow-up cleanups.
Comment #19
JacobSingh commentedComment #20
dave reidReviewed this manually and everything looks good in the UI. I have some follow-up issues to address in code but let's get this in.
Comment #21
JacobSingh commentedlast try :)
Comment #22
JacobSingh commentedInterdiff
Comment #23
dave reidComment #24
JacobSingh commentedFinally...
Comment #25
JacobSingh commentedFollow-up for media:
#1292474: Make media browser at admin/content/file enhance the default file display with thumbnails.