Pdfview customization (rewrite)

This feature is here endly.....

I implemented a per node customizable PDF setting, which enables a user to choose if a node shows a "download pdf" link.

Hope you can enjoy it!

and also a .install file.

sorry, the file above is a wrong version. Here comes the right one.

Can you provide a patch? See http://drupal.org/patch

Thanks for the tip!

My patch for pdfview.module is in the attatchment.

I like the idea, but we have to work a bit more on it before I'll commit it.

I think there should be some rework of the choices in the content type settings. It should look this way:
Pdf dowload:
- Disabled
- Enabled for all nodes
- Enabled per node: defaults to disabled
- Enabled per node: defaults to enabled

The new setting should take care of the path too (node/x/pdf), not just of the link, or it would be possible to get pdf output for nodes even if the link is disabled.

So the term "showlink" should be dropped in favor of "pdf output" or something like that. The table "pdfview" should become "pdfview_nid" or "node_pdfview".

The function pdfview_show_per_node($node) should be in hook_nodeapi, case 'load'; it should set a field like $node->pdfview_link instead of 'showlink';

You must leave the // $Id header as it is ... CVS needs it

What's the reason for the new headers? Maybe it deserves it's own issue + patch

Why not using just an update query in nodeapi case 'update'?

Why the first update in the install file?

That's it, thanks! :-)

Egon Bianchet, your suggestion is great. I made the patch in that way 'cause in the mambo/joomla one can enable a link on a page in that way. :) In the fact, because the feature is not implemented for a while, but one of my projects really needs it. So I made the patch.

I think there should be some rework of the choices in the content type settings. It should look this way:
Pdf dowload:
- Disabled
- Enabled for all nodes
- Enabled per node: defaults to disabled
- Enabled per node: defaults to enabled

In my opinion, the second choice seems not necessary. Maybe you mean, there is no need to show pdf setting while editing a node with that setting?

The new setting should take care of the path too (node/x/pdf), not just of the link, or it would be possible to get pdf output for nodes even if the link is disabled.

So the term "showlink" should be dropped in favor of "pdf output" or something like that. The table "pdfview" should become "pdfview_nid" or "node_pdfview".

You're right. It's an important secrity problem too. Problem is, what should Drupal show if an user types "node/x/pdf" when the pdf output of that node is disabled? A "Page not found" page?

The function pdfview_show_per_node($node) should be in hook_nodeapi, case 'load'; it should set a field like $node->pdfview_link instead of 'showlink';

I am not so familiar with the function hook_nodeapi. but thanks for the tip.

The using of new header is just because my bad habits of file organisation. I have different versions of pdfview.module, if i don't change the header, i don't know which file is the file I edited.

Why not using just an update query in nodeapi case 'update'?

what is that for? I didn't know it before. But if it makes the module beautiful-looking. I will try it.

Why the first update in the install file?

For the per node setting, one needs a DB table. Since i think one can use the .install file to add some tables for the normal user, i did it. That was also the only autonomy way I knew.

In my opinion, the second choice seems not necessary. Maybe you mean, there is no need to show pdf setting while editing a node with that setting?

Exactly. Perhaps it could have just three options ('No pdf output', 'pdf output disabled by default', 'pdf output enabled by default') and use an access permission to discriminate between users who can change the pdf setting while editing the node and users who can't.

You're right. It's an important secrity problem too. Problem is, what should Drupal show if an user types "node/x/pdf" when the pdf output of that node is disabled? A "Page not found" page.

If the access restriction is placed the menu hook using the menu api, it should automatically show a "403 Forbidden" page.

http://api.drupal.org/api/4.7/function/hook_menu
http://api.drupal.org/api/4.7/function/hook_nodeapi

About the install file, I think that the first update isn't needed ...

Perhaps it could have just three options ('No pdf output', 'pdf output disabled by default', 'pdf output enabled by default')

suddenly i find it a little bit strange, when people need the option "No PDF output", why do they install this module? :)

They might need pdf output for stories but not for images ;-)

OK. This time i have changed some code in the module, and also the table. I hope that makes egon happier. :)

changes:
* hook_nodeapi() with case 'load':
* access denied is done in pdfview_node_controller()
* table now is called "node_pdfview"

not changed:
* the three options. I still think it may make no sense if we add an option "totally disabled".