Closed (fixed)
Project:
Node access user reference
Version:
7.x-3.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
28 Jan 2011 at 20:52 UTC
Updated:
21 Feb 2011 at 03:00 UTC
Jump to comment: Most recent file
Comments
Comment #1
danielb commentedJust set that grant on the node with any of the modules that can already do that?
http://drupal.org/project/modules?filters=tid:13434
Like Content Access, or Node Access to set it on the specific node, or something else where you can do it to all the relevant nodes in one hit?
I don't want to keep adding functionality that doesn't fit into the idea of Node access user reference, for particular use-cases. The idea is that you can use multiple modules to get the configuration you want.
Comment #2
ezra-g commentedIf you won't accept a patch, I respect that and I'll move on. but, since this is still marked "needs review":
I encourage you to consider this feature. Adding another module like Content access (which requires ACL) means adding 2 more node access modules to the mix, along with additional points of configuration through the UI.
The present patch is relatively small and allows users to take care of what seems like a common use case without adding additional modules and UI complexity. For distributions like COD where this feature originates, it's particularly desirable to not add additional modules.
Comment #3
danielb commentedI don't understand your reasons behind not wanting to install a module that does what you want? I've never used Node access user reference by itself. I set up my site with the default grants I want using modules like Node access and TAC lite, and then I can supplement these with the user reference grants in situations where it is useful. It really isn't as complex as you make it out to be.
I'm a little bothered by redundant functionality between Node access modules. People want module maintainers to add everything they want for their site into one module, and we wind up with modules stepping on each other's toes. This means that ultimately less original functionality and new modules are created because different maintainers are all busy fixing bugs, supporting, and upgrading almost identical pieces of code rather than specializing on a feature, and dividing the work load to give people everything they want through the various modules.
However I'm still open to it because the idea here is to reinstate the original access that was there before this module interfered.
The other issue for me is that I maintain several Node access modules, and I'm trying to be consistent with features. If the reasons for the feature are valid for one module, then they are valid for all. A couple weeks ago when author grants were added to this module (which I feel slightly dirty about), I went ahead and put the same feature into the other modules. If I added view all grants I would do the same. Then the problem is that there is all this redundant code I have to maintain between these modules. So the logical thing to do would be to separate that code out and put it into it's own module... but there are already several modules that can do this - so why not just install one of them?
Comment #4
danielb commentedI might add this feature in on the basis that it is restoring the access rather than granting it.
The patch isn't exactly right, but it's a fairly simple idea so I can roll my own.
Comment #5
danielb commentedAre you familiar with the author grants? I had added them in a global config screen, but it might make more sense to move them into a field the same way you did the view all one. :/ Hmmm
Comment #6
danielb commentedI wrote this patch to remove the settings page, and put the settings on the CCK widget, as well as adding the view all grants. I've also added an update script to move the settings over. I've run out of time to test it out right now, but this is roughly what I'd like to do with it.
Comment #7
danielb commentedGot a chance to test it and there were a couple bugs, so here's a new patch.
Still need to check if the update stuff works.
Comment #8
danielb commentedI've made an rc4 for D6, will need to port these changes to Drupal 7 too.
Comment #9
danielb commentedoh the README file and project page need updating as a result of this
Comment #10
danielb commentedAll this stuff is in RC releases now.
One thing I'm still not certain on is whether the author and view all grants should fire if the user reference field is not empty (as it does now), or whether additional checks are needed to ensure referenced user grants were set.
Comment #11
danielb commentedAlso the view all settings defaults to checked, it probably shouldn't since a more likely use-case is granting view to only referenced users.
Comment #12
danielb commentedI've finished this in D6, need to update D7 accordingly.
Comment #13
danielb commented