General ticket for talking about how development is going and submitting of patches in the leadup to 0.1

CommentFileSizeAuthor
#3 customreports.module.patch5.81 KBj_ten_man

Comments

a_c_m’s picture

Furthering j_ten_man and my email discussion, we need to be able to add an additional check when trying to view / edit the custom report nodes. We didnt want to force using a specific node - so we have a problem :

So it looks like the hook_access() is only for modules that actually create the node. We have a couple of options on getting around this. 

Option 1: Do what I am doing which is to implement hook_node_grants() and hook_node_access_records(). I am not quite clear on how this works, but I was using content_access.module to try and figure it out. It is pretty involved.

Option 2: Pretend like whichever node type the user selects as the custom report is managed by our module through hook_node_info(). This could be kind of questionable if they select the node type. Maybe we should force them to create a new node type (or we just create it on install).

Option 3: In hook_nodeapi() we check the permission and based off of that, we either output the report or not. This seems the most hacky and I would prefer not to do this. However, probably the easiest solution.

I've gone down the road of option 2, as using hook_access() seems most logical / nice. However were not using hook_node_info(), instead still using your node_type creation code in the settings submit which i've added to.

I *think* in order for us to be able to use hook_access we need to override the node_type's ->module when selecting a exsisting type - which the code now does. It will also set it back to what it was before if we change away from that type. Additionally, i think in the hook_access (and perhaps other hooks like that) we should pass the data onto the original parent module, if it exists. Code to almost do that is in our hook_access.

While i think this might work, its starting to feel quite hacky, perhaps your option 1 is better.

a_c_m

j_ten_man’s picture

I figured out how to work with hook_node_access_records() and hook_node_grants() to restrict access to reports. It adds overhead to the drupal system by using the node_access table but usually most sites won't have a large number of custom reports...

By the way, I found the answer to this in the Pro Drupal Development book!

j_ten_man’s picture

StatusFileSize
new5.81 KB

Here's the patch with the newest code for reports permissions :). Still waiting on CVS access.

a_c_m’s picture

committed, also now works with http://drupal.org/node/273893#comment-1694938 if you enable tokenSTARTER module

a_c_m’s picture

Welcome snufkin to the dev team.

snufkin has suggested we use a hook/theme functions instead of trying to use tpl files, hes going to be back porting the module to D5 and getting a working version together.

snufkin’s picture

Hey there.

I committed some stuff to the Drupal-5 branch, for now I removed the need for the input filter and the PHP code support, but I think the rendering is done in a way that it will be easy to plug in extra input/output.

snufkin’s picture

Status: Active » Closed (fixed)

Closing the issue as we have worked out the releases by now.