Pre release development

a_c_m - June 12, 2009 - 11:30
Project:Custom reports
Version:6.x-0.x-dev
Component:Code
Category:feature request
Priority:normal
Assigned:a_c_m
Status:active
Description

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

#1

a_c_m - June 12, 2009 - 12:28

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

#2

j_ten_man - June 12, 2009 - 14:12

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!

#3

j_ten_man - June 12, 2009 - 15:15

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

AttachmentSize
customreports.module.patch 5.81 KB

#4

a_c_m - June 12, 2009 - 16:04

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

#5

a_c_m - July 6, 2009 - 16:47

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.

#6

snufkin - July 6, 2009 - 20:16

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.

 
 

Drupal is a registered trademark of Dries Buytaert.