Drupal 7 port of Private module

Hi

I have not installed this module yet as I am only running Drupal 7, but I did try to run your patch. I got a long list of HUNK errors on a dry run:

patch -p0 --dry-run < d7-port-private.patch
patching file b/private.info
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file b/private.info.rej
patching file b/private.install
Hunk #1 FAILED at 1.
Hunk #2 FAILED at 8.
2 out of 2 hunks FAILED -- saving rejects to file b/private.install.rej
patching file b/private.module
Hunk #1 FAILED at 13.
Hunk #2 FAILED at 23.
Hunk #3 FAILED at 46.
Hunk #4 FAILED at 82.
Hunk #5 FAILED at 101.
Hunk #6 FAILED at 113.
Hunk #7 FAILED at 123.
Hunk #8 FAILED at 159.
Hunk #9 FAILED at 171.
Hunk #10 FAILED at 278.
Hunk #11 FAILED at 299.
Hunk #12 FAILED at 311.
Hunk #13 FAILED at 326.
13 out of 13 hunks FAILED -- saving rejects to file b/private.module.rej
patching file b/private.views.inc
Hunk #1 FAILED at 1.
Hunk #2 FAILED at 34.
2 out of 2 hunks FAILED -- saving rejects to file b/private.views.inc.rej
patching file b/private_handler_filter_private.inc
Hunk #1 FAILED at 8.
1 out of 1 hunk FAILED -- saving rejects to file b/private_handler_filter_private.inc.rej

Any suggestions ( I did check the file permissions were writeable)

Thanks

Joe90

Ah, I just took -p0 out of my command and the patch works. Off to try it out!

patch < d7-port-private.patch
patching file private.info
Hunk #1 succeeded at 1 with fuzz 2.
patching file private.install
patching file private.module
patching file private.views.inc
patching file private_handler_filter_private.inc

Thanks

OK, installed, a couple of issues.

1. The patch doesn't clear out the v6 info from private.info, so you have to delete it manually, then it is happy to install properly.

2. Get this error:
Notice: Undefined property: stdClass::$private in private_node_view() (line 274 of ../modules/private/private.module).
which relates to this:

/**
 * Implements hook_node_view().
 */
function private_node_view($node, $view_mode){
    if($node->private){
        $links['private_icon']['title'] = theme('private_node_link', array('node' => $node));
        $links['private_icon']['html'] = TRUE;
        $node->content['links'][$node->type] = array(
            '#theme' => 'links__node__' . $node->type,
            '#links' => $links,
            '#attributes' => array('class' => array('links', 'inline')),
      );
    }
}

Line 274 being "if($node->private){"

So unticking for now

@ kevee

Thanks for the update but this has not resolved either of the issues in #3 above.

I have ACL / Forum Access / Menu per Role / Chain Menu Access up and running. Simple Access is disabled at present due to issues with that one!

sub

Subscribe

I've tested the patch in #9 and it working nicely on my setup. I had a notice when editing a node so I changed the line 156 in private.module from this:
$privacy = $node->private;
to this:
$privacy = isset($node->private) ? $node->private : 0;

The test setup had ~1000 nodes. I made some nodes private and then did the test in two steps.
1. see if the node is listed in a view without changing the view
2. try to access a node from his path ( node/xy ).

I did this with an anonymous user and another authenticated user. Everything is behaving as expected. The nodes are simply not shown in the view if they are private. Then if the nodes are directly accessed the "You are not authorized to access this page." is shown.

I've also ran the module through coder and fixed some spacing issues.

I've added some tests and they all pass as they should. Also some changes like the one described in #11, only in more places.

Adjusting the title to not get lost in Dashboard.

1)

The private_node_update can causes query errors cause $node->private is not necesirly always set (which may be a seperate bug); private_node_load looks all results in the database, but if there's no results ... well, no results.

Then it actually fails again with duplicate ID; instead of that awkward update, check if updated, insert code can do

/**
 * Implements hook_node_insert().
 */
function private_node_insert($node) {
  private_node_update($node);
}

/**
 * Implements hook_node_update().
 */
function private_node_update($node) {
  db_merge('private')
    ->key(array('nid' => $node->nid))
    ->fields(array(
      'nid' => $node->nid,
      'private' => isset($node->private) ? isset($node->private) : 0,
    ))
    ->execute();
}

(thanks to dekita on #drupal channel for telling me about db_merge).

/**
 * Implements hook_file_download().
 */
function private_file_download($file) {
  $file = file_prepare_directory($file);
  $result = db_query("SELECT f.* FROM {files} f WHERE filepath = :filepath", array(':filepath' => $file));

That table doesn't exist. It's files_managed now and it doesn't have an nid column (does files have it in 6? might be a pre-existing issue. I think upload keeps track of it's own download).

 if (isset($node->private) ? $node->private : FALSE) {

Why not:

 if (!empty($node->private)) {

To tired to make patchy.

Subscribe

Subscribe

@hefox, regarding your last question:

I see that those two are functionally the same and the latter is shorter, but the former is more readable IMHO. I had written several sentences about the differences and then I remembered that empty() considers a boolean FALSE to be an empty value.

Just my two cents.

I find !empty much more readable personally; I've never seen a ternary operator used in a if statement before so it was a big WTF moment.

Typo in the update function

/**
* Implements hook_node_update().
*/
function private_node_update($node) {
  db_merge('private')
    ->key(array('nid' => $node->nid))
    ->fields(array(
      'nid' => $node->nid,
      'private' => !empty($node->private) ? 1 : 0,
    ))
    ->execute();
}

yeah, actually, I totally agree with you. I was reading that line wrong before. I think !empty() is the way to go too.

Here's current patch using.

Fixes stuff above + theme('image' the alt/title got movd to width, height tags o.O

Too lazy to update patch atm, but for views ingergration 'Node' => 'content' for label

Help! I installed the dev version of the module, patched it, followed the instructions to rebuild permissions, and everything worked fine. Until I logged out - access denied for guests everywhere. I uninstalled the private module, rebuild permissions again, but guests are still blocked. I think it messed up my database when updating permissions.

What happened? How to restore to normal?

Update: Disabling the ACL module fixed the issue.

Patch from #21 applies cleanly for me to a 6.x-dev version with: patch -p0 < private_1077600_d7_port_21.patch
Not sure exactly what is meant in #22 about what's missing in the views integration?

The fields appear under 'Node', whereas all other node related stuff appears under 'Content', so it's allll lonely.

Yeah, I saw what you mean..
I think in private_views_data() , this:

  $data['private']['table']['group'] = t('Node');

Should be:

  $data['private']['table']['group'] = t('Content');

New one

the view filter also had to be updated from add_where to add_where_expression

Subscribe

Is there a 'final' beta version just for Drupal 7. 7.x-dev maybe?

Seems not. There is still a lot of work to do here :

For what i saw, we need to change the first condition in private_form_alter (first line of the function), for example : if (strpos($form_id, '_node_form')) { works fine.

Seems that hook_node_grants has changed as well : http://api.drupal.org/api/drupal/modules--node--node.api.php/function/ho...

I'm searching for an taxonomy term based access control which is controled by the user/ author.
It should work similar to access by term module, but controled by
the users not administrators

For example...

Each user can tag other users with a taxonomy term (private/ own taxonomy term, not viewed to other users). This terms act as groups/ friend lists. so not only private/ public access, but rather a friend list(s) based access.
Author tag his nodes with his terms (groups/ friend lists) to grant access.

Any chance to get such a feature build in?

I have updated the D7 patch and tested the basic functionality.

I have made fixes with the help of the node_access_example module and have run it through Coder.

Please test and report back here.

When this patch works well I plan to ask to be made a co-maintainer of the module and release a D7 version.

Lots of great work in here! Some things to consider:

     if ($default != PRIVATE_DISABLED || !empty($node->privacy)) {

This looks to be a bug in the d6 version as well, but shouldn't the above be $node->private not $node->privacy?

+    drupal_set_message(t('@type %title has private set to "%private".', array('@type' => node_type_get_name($node), '%title' => $node->title, '%private' => !empty($node->private) ? t('Yes') : t('No'))));

Can we break this out? It's very hard to read ternaries inside of other code. I would break all the t() placeholders out into a separate array variable to then pass to t().

 /**
-* Implementation of hook_file_download().
-*/
+ * Implements hook_file_download().
+ */
 function private_file_download($file) {
-  $file = file_create_path($file);
-  $result = db_query("SELECT f.* FROM {files} f WHERE filepath = '%s'", $file);
-  if ($file = db_fetch_object($result)) {
-    $node = node_load($file->nid);
-    if ($node->private == 1) {
+  $file = file_prepare_directory($file);
+  $result = db_query("SELECT f.* FROM {files} f WHERE filepath = :filepath", array(':filepath' => $file));
+  foreach ($result as $record) {
+    if ($record) {
+      $node = node_load($record->nid);
+      if (isset($node->private) && $node->private == 1) {
         if (node_access('view', $node) == FALSE) {
-            return -1;
+          return -1;
         }
       }
+    }
   }
 }
 

This whole section is confusing the heck out of me. First of all, there is no nid column in the files table in d6. Second of all, there is no files table in d7. Perhaps this is supposed to be joining to the file_usage table? I'd almost propose just removing this code altogether, because I can almost guarantee it doesn't work in d6.

+    $node->content['links'][$node->type] = array(

Shouldn't this be $node->content['links']['private']?

+  return theme('image', array('path' => drupal_get_path('module', 'private') . '/icon_key.gif', 'width' => '16', 'height' => '16', 'alt' => t('Private'), 'title' => t('This content is private.')));

For readability, let's break this out into a $variables array().

+    $this->query->add_where_expression($this->options['group'], $qualified_name . (empty($this->value) ? " = 0 OR $qualified_name IS NULL" : ' = 1'));

Again, ternaries inside code like this are hard to read. :)

This patch and a couple of other very minor fixes have been committed to the new 7.x-1.x branch. I'll be rolling a 7.x-1.1 release shortly. Thanks, everyone!