Protect nodes per content-type

Hello,

Did you have read the documentation before ?

What do you mean by "this does not seem to protect the node" ?

Did you have tested with another browser ? Checked the permissions ?

Ok, not a problem of documentation, a problem of code, there was nothing relative to the per content type password, it exists but was not used.

With this patch, if a content type password exists, the password is no more mandatory on the node form.

And on the password page, it checks first if there is content type password and if it matches.

I want to make refactoring on this module, but i'm waiting for my previous patches to be reviewed.

Thanks for the review.

Thanks for the patch,
The patch is working fine for me and corrects the bug.
Here is my code review :

1. +++ b/protected_node.redirect.inc
   @@ -148,45 +148,58 @@
   +  $vars = get_defined_vars();
   

   this doesn't seem to be used

2. +++ b/protected_node.redirect.inc
   @@ -148,45 +148,58 @@
      $sql = "SELECT nid FROM {protected_nodes} WHERE protected_node_passwd = '%s' AND nid = %d";
   

   can you please delete this too as it doesn't seem to be used at all

3. +++ b/protected_node.redirect.inc
   @@ -148,45 +148,58 @@
   +  $protected_node_nid = $form_state['input']['protected_node_nid'];
   

   Please don't trust what is in 'input' key, but what is in 'values' key.

   $form_state['values']['protected_node_nid'] is used in the line above to define $nid. Please define $protected_node_nid before $nid, and make use of it all over this function.

Can you please delete the Readme in the patch as it is already commited.

Can you please also warn the user that changing the password policy for content types will not have effect on existing protected nodes of that type. (Test it please and add the warning if needed in the content type setting description of protected node)

I saw some ugly code in that function logic making $nid = 1 instead of defining a flag...this is surely something to clean up in the future. i know it's not this issue concern, i'm just sharing my thoughts :)

1 : sorry, it was for the dpm

2 : that was already there, I didn't see it was not used. removed now.

3 : done

Readme : sorry, it was there for the issue of the readme (not commit in local) and it was taken in the patch.

$nid / $protected_node_nid : now it uses $protected_node_nid in the function instead of $form_state... etc, but I keep $nid and $protected_node_nid, because I saw something (I don't remember now) when making the patch. Something strange about how the feature was thought. I think it will be cleaned in a refactoring issue.

"
Can you please also warn the user that changing the password policy for content types will not have effect on existing protected nodes of that type. (Test it please and add the warning if needed in the content type setting description of protected node)
"
I test, it's ok :
you protect a node, good
you protect with content type password another node, good
you protect node 2 with it's own password, accessible with the two password
and node 1 also accessible with content type password.

So no need of warning ?

ugly code : yeah I know, and my patch is a quick and dirty patch, I know it, but I want to fix the issues so you can make a tag so I can work on the dev version to make refactoring and cleaning to provide a stable release.

This is merged now.
Thank you all !