It would be very nice to call hook_perm in filefield_sources.module and invoke 'perm' of all sources/*.inc. We will use a custom *.inc (thanks to attach.inc) to enable FTP uploaded files - but would not like all editors to access this functionality. I think, this would be useful for others as well ... so I attach a patch.
I hope you can include it in your dev version as well.
Thanks for the creation of this module, by the way :)
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | filefield_sources_access.tar_.gz | 8 KB | rv0 |
| #14 | alter_sources-661198-14.patch | 1.42 KB | rv0 |
| #1 | 661198-filefield_sources_perms.patch | 3.23 KB | greg.harvey |
| hook_perm.patch | 446 bytes | anschinsan |
Comments
Comment #1
greg.harveyI've gone a step further and added the necessary hooks and logic to some of the sources. New patch attached. Note, no hook required for the IMCE include, because it already uses IMCE permissions correctly, so access to the module can be controlled there.
Comment #2
quicksketchI'm not sure I want to include this. Generally speaking, FileField Sources is just a short cut. A user could easily find a file, download it, then re-upload it no matter what their permissions. Why would you want to make things difficult for some of your users? Now that you can disable/enable certain sources per-field, this seems unnecessary. Unless convinced otherwise, this will not be added.
Comment #3
greg.harveyActually, you misinterpret what we're trying to achieve here: it's not a case of "making it difficult for some users" - in my case it's responding to a client request to make it *easier* for some users. I can't find the actual email from my client that spawned this patch, but it was something along the lines of:
Trouble is, this client wanted users to be able to reference existing CCK filefield uploads but wasn't interested in the other options. So, as you can tell, it's not about access control and making things difficult - it's about tidying up the UI and the ability to hide elements some users a) don't need and b) were finding confusing.
It so happens hook_perm() was the quickest win to achieve this, but if you'd prefer it as a separate admin interface instead, because you think going the "permissions" route might give site admins the wrong idea/isn't the right way, then I take your point.
Now you understand the use case, can we develop this patch and include the functionality, even if it's not done with hook_perm()? I think the feature is valid and it can't be an unusual situation I'm in? Not everyone wants/understands the Remote URL option and I just need a way to hide it (and any other additional UI elements that may be added in the future).
Comment #4
quicksketchYou can now disable certain features globally now that we have the Configuration options patch in (in the 1.0 version), so you can disable the options you don't want. While I see your point, it still seems a little strange that a user could figure out the reference existing but not the remote URL. The file browser I admit needs some work, but I would recommend just disabling it globally if it's too confusing.
Comment #5
greg.harveyFile browser you can disable by role with IMCE, so I didn't worry about that. We didn't want the jouno users using IMCE anyway.
But it wasn't so much that they "got" one option more than the other. To be honest, they didn't get either! I had to make a nice View of referenceable files with some help text like "copy and paste this line, EXACTLY as it is, in to the online photo reference box", and perfectly formatted the field output so it was what your module expects. They needed leading by the hand anyway, as many users would.
Explaining Remote URL was a waste of time, as some of them would never remember what it's for and none of them would ever use it either, so easier to hide it so you don't have to worry about explaining to them they can just ignore it. Also the client got fed up of saying to his journalists "don't worry about that, you don't need it..." over and over.
I suspect a large part of this is because a lot of these journalists are not young people - many are in their 60s or older. Some don't get on that well with technology anyway and would rather have their typewriters back! So any additional piece of UI is an additional "problem" for them, even if it's just a case of remembering they should ignore it!
On the other hand, the editorial team want all the options, because they're technology literate and trained and it might be useful for them to be able to use a Remote URL (albeit very rarely, it's a feature the client would like to keep as an option for them). We just don't want to show the option to journalists.
So, in short... globally is ok and an improvement, by role is even better. ;-)
If you like, I can do a new patch expanding the global features to have role selectors?
Comment #6
joelstein commentedIf your feature never gets into the module, you could at least hide the options via some custom Javascript or CSS.
Comment #7
quicksketchRetitling. Closed duplicate #757132: Role-based access to filefield_sources elements
Comment #8
Remon commentedAny chances of this going into the module?
Comment #9
Remon commented+1
I find this one extremely useful. In my case, I want to enable extra sources just to staff users, since they are irrelevant to normal users. Frankly, it is disturbing to reroll/apply this patch every time we got a new release of filefield_sources.
Please apply this patch to the module :D. Thanks
Comment #10
quicksketchThis patch needs to accommodate for the new "attach" source.
Comment #11
troybthompson commentedThis would also be great for my Drupal 7 installation. I have community photo project site where the public can upload photos of themselves, but I also take a lot of photos of people myself at events, and want my staff to be able to populate nodes with them. So it'd be great if I could FTP all the files to the server for them to process and select them using your module, but I wouldn't want the public to be able to see that option. Has anyone made a Drupal 7 patch for this?
Comment #12
Morn commentedI don't know if it is the right way but it works for me:
added to filefield_sources.module
and modified filefield_sources_element_info():
Comment #13
nublaii commentedI believe this should go in the module by default, as controlling the sources from an editorial point of view is very important.
On our site we allow users to upload videos. We also do some automatic video uploading via ftp/rsync/whatever. I want the editors to be able to attach those files, but I don't want my regular users to be able to see what is being uploaded via ftp/rsync/whatever...
BTW, #12 your code works fine for me ;)
Comment #14
rv0 commentedI think the best approach would be to provide an alter hook. That way any module can alter the available sources however they like.
This also keeps the module code clean.
Attached you find a patch that does this and updates the api file with an example implementation. Both the $sources and the $enabled_sources arrays can be modified.
I also include a proof of concept module (rename the .tar.gz), which I might turn into a sandbox or real module later on if more people are interested..
The proof of concept module automatically adds a permission for every filefield source.
In any case it would not harm adding an alter hook for this..
btw, @module maintainers
The default branch is set to "master", should be 7.x-1.x
Comment #15
rv0 commentedCreated sandbox module for my filefield_sources_access module https://www.drupal.org/sandbox/rv0/2426027
requires the above patch in #14 alter_sources-661198-14.patch
Comment #16
ludo.rThank you for that contribution rv0.
It works smoothely (both filefield_sources patch + filefield_sources_access module), and the code is simple as hell! :)
Comment #17
damienmckennaComment #19
profak commentedHello @rv0!
According with documentation to drupal_alter:
So, i put required variables into single context like that:
and tested attached module - everything is working. Documentation updated.
Pushed to dev - thank you!
Comment #20
rv0 commentedGreat, thanks for the head up there, I'll have a look at the sandbox again.
Comment #21
profak commentedIn 7.x-1.10.