Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Any volunteer to port this great module to Drupal 7? I would be happy to contribute testing and an updated README.txt file.
Comment | File | Size | Author |
---|---|---|---|
#165 | 690610_165.patch | 77.23 KB | good_man |
#164 | 690610-164.patch | 60.36 KB | good_man |
#142 | 690610-142.patch | 32.48 KB | good_man |
#142 | 690610_142.tar_.gz | 16.72 KB | good_man |
#112 | 690610-112.patch | 31.68 KB | good_man |
Comments
Comment #1
BenK CreditAttribution: BenK commentedSubscribing... very interested in D7 port as well.
Comment #2
beedaddy CreditAttribution: beedaddy commentedsubscribe
Comment #3
adeb CreditAttribution: adeb commentedthis is a must have
Comment #4
Dave ReidSubscribing...maybe be able to help with this.
Comment #5
spacereactor CreditAttribution: spacereactor commentedsubscribe
Comment #6
BenK CreditAttribution: BenK commentedNow that the Drupal 7 beta release is right around the corner (scheduled for May 21), I thought I'd check back in to see if anyone had made any progress on this D7 port....
--Ben
Comment #7
BenK CreditAttribution: BenK commented@Dave Reid: I'm familiar with your great work on a bunch of other modules. Are you still able to help with this?
Also, just a note that the ACL module seems to have a mostly working Drupal 7 port... so for those of us who use Content Access with ACL, we're at least part way there. :-)
Here's a link to the D7 release of ACL:
http://drupal.org/node/694210
--Ben
Comment #8
atchijov CreditAttribution: atchijov commentedIf anyone interested, I have done quick and dirty port (module attached) It works well enough for me on alpha-5.
One caveat, I am not 100% sure that it is related to this module, but once I got this module running I start getting "1052 Column 'nid' in where clause is ambiguous" message (line 1285 of /Library/WebServer/Documents/touchNOC/includes/menu.inc). I was able to get rid of this message by changing line
$select->condition('nid', $nids, 'IN');
to
$select->condition('node.nid', $nids, 'IN');
As you probably can imagine, I do not provide any guaranties, but I do not mind to help to get this module running if you hit any snags.
Comment #9
atchijov CreditAttribution: atchijov commentedMuch improved version. Pretty much everything works.
Comment #10
BenK CreditAttribution: BenK commentedHey atchijov,
Thanks for getting the ball rolling on this! I just tried installing the version of the module you posted in #9 and I got the following error message:
DatabaseSchemaObjectExistsException: Table content_access already exists. in DatabaseSchema->createTable() (line 605 of /xxx/xxx/xxx/public_html/includes/database/schema.inc).
Any ideas?
It does look like the module installed, however, and I'll continue testing. But I thought I would post this first.
--Ben
Comment #11
FrancewhoaSame here. I tested #9 and Drupal display the same error message as #10. Find attached screenshot to clarify. Hope this helps.
Using 7.0-alpha5 2010-May-23
Fresh install
Steps to reproduce issue
*Copy module to
sites/all/modules
folder.*Go to
#overlay=admin/modules
Activate module.*Click on
Save configuration
button.*Drupal display error message
Comment #12
BenK CreditAttribution: BenK commentedHi atchijov and everyone,
I've just completed some initial testing of atchijov's D7 port attached to #9. First, the basic functionality of the port works pretty well. I was able to control view, edit, and delete access by role for a given node. Nice work.
I did find, however, some bugs... some major and some minor ones. I've described them below:
1. CONTENT TYPE DEFAULT SETTINGS IN WRONG LOCATION: At first, I didn't see any way to set the access control defaults for a given content type. For the "article" content type, for instance, when I went to edit page for the article content type, I didn't see a tab named "Access" as I would have in the D6 version. Likewise, I tried to manually go to "admin/structure/types/manage/article/access", but that didn't work either.
Then I remembered that in the D6 version, the path would have been different. So I manually went to admin/content/node-type/article/access and I was able to find the content type access settings. So we need to get this settings page to appear on the right path for D7 with the appropriate tab on the content type edit page.
2. WARNING MESSAGE: On the content type access control defaults page, when trying to save changes to "view any content" or "view own content," I'm getting the following warning message:
"Warning: htmlspecialchars() expects parameter 1 to be string, array given in check_plain() (line 1473 of /home/sandboxes/sandbox10/public_html/includes/bootstrap.inc)."
This doesn't happen every time, but if you fiddle with the settings a few times (saving each time), you'll eventually see it.
3. EDIT AND DELETE DEFAULT PERMISSIONS WON'T SAVE:
On the content type default access control page, when changing the "edit any content," "edit own content," "delete any content," and "delete own content" permissions, it won't save for default Drupal roles (anonymous, authenticated, and administrator). For roles created by the site admin (I created a role called "editor"), I'm getting the following error message:
"PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '4-4' for key 1: INSERT INTO {role_permission} (rid, permission) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1); Array ( [:db_insert_placeholder_0] => 4 [:db_insert_placeholder_1] => 4 ) in content_access_save_permissions() (line 282 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/content_access/content_access.admin.inc)."
4. PER CONTENT NODE CHECKBOX NOT CHECKED BY DEFAULT BUT STILL IS ACTIVATED: On the content type access control defaults page, the "Enable per content node access control settings" checkbox is not checked by default even though this functionality is being enabled by default. The checkbox, however, does appear to function properly once you manually check it and uncheck it. The way its supposed to work (based on the D6 version) is that it is unchecked by default and the per content node access control is disabled by default.
5. OVERLAY NOT WORKING ON CONTENT TYPE ACCESS CONTROL DEFAULTS PAGE: Even though the "Seven" administration theme is being called, the page is not being displayed in the D7 admin overlay.
6. OVERLAY AND ADMIN THEME NOT WORKING ON PER NODE ACCESS CONTROL PAGE: On the "Access" page for a given node, not only is the overlay not working, but the page is being displayed in the Garland theme instead on in the Seven default admin theme. As a result, some of the usability improvements aren't present (making it easy, for instance, to click the "Reset to defaults" button when you meant to click the "Submit" button).
7. RESET TO DEFAULTS ERROR: When controlling the access for a given node, I clicked the "Reset to defaults" button and received the following error message:
"Recoverable fatal error: Argument 2 passed to db_query() must be an array, string given, called in /xxx/xxx/xxx/public_html/sites/all/modules/contrib/content_access/content_access.module on line 415 and defined in db_query() (line 2226 of /xxx/xxx/xxx/public_html/includes/database/database.inc)."
Checking the page after this error message, the defaults are not being re-set.
Well, that's it for now. Thoughts on the causes or potential solutions for any of these issues? atchijov got the ball rolling and let's keep up the momentum on this!
--Ben
Comment #13
BenK CreditAttribution: BenK commentedComment #14
FrancewhoaThanks for testing Ben :)
Comment #15
BenK CreditAttribution: BenK commentedOne more minor bug I just noticed: On both the default content type settings page and the access control page per node, "View any Content" and "View own Content" are not being lined up vertically (unlike the D6 version). The same holds true for "Edit any content" and "Edit own Content," as well as "Delete any Content" and "Delete own Content." This is a usability issue that makes it a bit too easy to select an unintended permission.
To show what I mean, I've attached a screenshot to this comment.
--Ben
Comment #16
atchijov CreditAttribution: atchijov commentedHi Ben,
I have seen this error message. It looks like it does not prevent module from functioning. When I have few sec free, I will try to see what is going on
Andrei
Comment #17
atchijov CreditAttribution: atchijov commentedBen,
Do you mean that it supposed to be 3 columns (I have not worked with D6)? I can see that it will make total sense.
Andrei
Comment #18
BenK CreditAttribution: BenK commentedHey Andrei,
Thanks for checking back in. Yes, it's supposed to be 3 columns as follows:
Left Column:
"View any content" (top)
"View own content" (bottom)
Middle Column:
"Edit any content" (top)
"Edit own content" (bottom)
Right Column:
"Delete any content" (top)
"Delete own content" (bottom)
Did the rest of my comments and bug reports in #12 make sense?
Thanks again,
Ben
Comment #19
atchijov CreditAttribution: atchijov commentedBen,
Yes, all of your comments make total sense. I can not promise that I will address all of them soon, but we do use this module extensively and I will try my best to fix it.
Andrei
PS Am I missing something or there is no way to to have drupal.org e-mail you when particular issue was updated?
Comment #20
Francewhoa@atchijov: I don't know if drupal.org can e-mail you. I wish ;) What I do to keep track of updated issues is going to the following page. Hope this helps. Thanks for your contributions :)
http://drupal.org/project/issues/user
Then under the column
Summary
search for the redUpdated
word(s). There are also handy shortcuts under the columnReplies
. Read more at http://drupal.org/node/317Comment #21
atchijov CreditAttribution: atchijov commentedSome more bug fixed. Most notably: #3 and #7 from comment #12.
Comment #22
BenK CreditAttribution: BenK commentedHey Andrei,
Thanks for posting the new version in #21. I did some testing on it (focusing on #3 and #7 from comment #12 since you said those were fixed). Here's what I noticed:
#3: Thanks for the improvements on this, but it doesn't appear to be totally fixed. I'm no longer getting the error message (that's great!), but there is a bug when saving changes to the "edit any content," "edit own content," "delete any content," and "delete own content" permissions. Basically, when ADDING these permissions to any role, it saves fine. But when trying to REMOVE any of these permissions, saving doesn't work. Thus, once you grant those permissions, you're stuck with them. Additionally, when viewing a node's access control page, those permissions that have been granted as default for the content type are grayed out... thus you can't remove them at the node level either. Note that these problems appear to be confined to "edit" and "delete" permissions... "view any content" and "view own content" seem to be working properly.
#7: This appears to be fixed and is functioning properly. Thanks!
Comment #15: The permissions now seem to be lined up vertically. Thanks for fixing this.
Also, one quick question: Did you fix the error message mentioned in comment #10? I didn't have time to check this since it would involve re-installing the module on a fresh D7 site.
Thanks,
Ben
Comment #23
BenK CreditAttribution: BenK commentedAndrei,
We also discovered another bug when using your version of Content Access with the ACL module.
Basically, in content_access.admin.inc, there needs to be a change on Line 63 (to prevent the error message reported at http://drupal.org/node/824642):
Change Line 63 to read:
Note the inclusion of "(int)" in several places. This is because the grants should be ints (i.e. 0 or 1). For more details on this, see comment #3 at http://drupal.org/node/824642.
Andrei, can you make this change in the next version of Content Access that you post? I really appreciate all of your hard work!
Thanks again,
Ben
Comment #24
BenK CreditAttribution: BenK commentedHey Andrei,
Just wanted to check in to see if you had made any more progress on the remaining issues... I'm happy to do extensive testing.
--Ben
Comment #25
BenK CreditAttribution: BenK commentedHey everyone,
I've tried to use this D7 version of Content Access in combination with the ACL module (see http://drupal.org/node/798106, Comment #12 for a fix to ACL needed to make this work). But when I've tried to grant a user view access, update access, or delete access to a node by clicking the "Add user" button on the "Access Control" tab, the page reloads, but nothing happens (the user is not added).
After trying this for a few times, I eventually get the following error message:
PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '2' for key 1: INSERT INTO {content_access} (nid, settings) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1); Array ( [:db_insert_placeholder_0] => 2 [:db_insert_placeholder_1] => a:6:{s:4:"view";a:3:{i:0;i:1;i:1;i:2;i:2;i:3;}s:8:"view_own";a:3:{i:0;i:1;i:1;i:2;i:2;i:3;}s:6:"update";a:1:{i:0;i:3;}s:10:"update_own";a:1:{i:0;i:3;}s:6:"delete";a:1:{i:0;i:3;}s:10:"delete_own";a:1:{i:0;i:3;}} ) in content_access_save_per_node_settings() (line 398 of /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/content_access/content_access.module).
Any ideas? What I'm not clear about is whether this is a bug with the ACL module or with Content Access itself.
Thanks,
Ben
Comment #26
atchijov CreditAttribution: atchijov commentedI have fixed few problems and I have submitted patch for ACL module to make it work (new version attached).
I can not reproduce problem #3 (failure to un-grant permissions once you grant them). Could you please confirm that this is still a problem?
Comment #27
BenK CreditAttribution: BenK commentedHey Andrei,
Thanks so much for posting your updated module! Very exciting. I've been testing the version of the module you posted in #26 and I'll be posting a full bug review shortly.
But first, I did want to let you know that problem #3 (failure to un-grant permissions once you grant them) seems to have gone away. So whatever you did, it appears to have fixed it. So we can cross #3 off the list.
And problem #7 (reset to default error) has been fixed, too. So we can definitely cross that off the list as well.
I'll be posting more of my testing results in a subsequent comment....
--Ben
Comment #28
BenK CreditAttribution: BenK commentedAndrei,
Here's a full roundup of the bugs I found in the most recent version of the module (comment #26). Thanks again for all of your hard work. The only thing I haven't yet tested is the ACL integration, but I'll post about that in a subsequent comment:
NEW BUGS:
A. ERROR WHEN ADDING NEW CONTENT: After installing the module, I created a new content type ("Test Type") and tried creating some new Test Type content. But when I tried to save the new content, I got a white screen of death and the following error appeared in my apache log:
"PHP Fatal error: Unsupported operand types in /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/content_access/content_access.module on line 97, referer: http://sandbox10.example.org/node/add/test-type"
The only way to get rid of this error (and WSOD) was by checking "Enable per content node access control settings" on the Test Type access control default page. When I did this, I then received the following error:
"Recoverable fatal error: Argument 2 passed to db_query() must be an array, string given, called in /home/sandboxes/sandbox10/public_html/sites/all/modules/contrib/content_access/content_access.admin.inc on line 468 and defined in db_query() (line 2226 of /home/sandboxes/sandbox10/public_html/includes/database/database.inc)."
But once I did this, the original error went away and I could create new Test Type content. Any ideas?
B. ARRAY DATA DISPLAYED WHEN SAVING ACCESS CONTROL DEFAULTS PAGE: When visiting the access control default page for a given content type and clicking "Submit," I'm getting a huge amount of array data displayed as a confirmation message. Here's a little excerpt of the voluminous code that's being printed to the page:
"array ( 'build_info' => array ( 'args' => array ( 0 => 'test_type', ), 'file' => 'sites/all/modules/contrib/content_access/content_access.admin.inc', ), 'rebuild' => false, 'redirect' => NULL, 'temporary' => array ( ), 'submitted' => ........"
Did you add some debug code to print messages and forget to remove it? ;-)
C. EXTRA "SUBMIT" WHEN SAVING: This is very minor. When saving access control settings for a node, in addition to the "Your changes have been saved" confirmation message, I'm also getting a 2nd confirmation message that says "submit".
D. ERROR WHEN INSTALLING MODULE: When installing the module, I'm getting the following error message:
"DatabaseSchemaObjectExistsException: Table content_access already exists. in DatabaseSchema->createTable() (line 605 of /xxx/xxx/xxx/public_html/includes/database/schema.inc)."
However, the module does seem to install properly.
PRIOR BUGS (AS FIRST REPORTED IN COMMENT #12):
1. Content Type Access Defaults in Wrong Location: Still a problem. Default settings should be located at "admin/structure/types/manage/test-type/access" instead of "admin/content/test-type/access" ('test-type' is the machine name of the content type).
2. Warning Message on 'View Any' or 'View Own': Still a problem.
3.
Failure to Un-grant Permissions:FIXED!4. Per Node Checkbox is Being Activated: Still a problem. May be related to NEW BUG #A (above).
5. Overlay Not Working on Defaults Page: Still a problem.
6. Overlay and Admin Theme Not Working on Node Access Page: Still a problem.
7.
Reset to Defaults Error:FIXED!As always, I'm interested to hear your thoughts....
Best,
Ben
Comment #29
BenK CreditAttribution: BenK commentedOkay, I just finished testing the ACL module fixes and posted my comments on that thread:
http://drupal.org/node/836822#comment-3142272
Basically, most of the integration with ACL appears to be working now except for when viewing/editing the node itself. More specifically, everything works fine if the node's title and teaser is displayed in a list of nodes (such as on the default front page). The node title and teaser do appear to the user granted view access (and disappear if the user's view access is revoked). But if the user then tries to click through to view the node itself, he receives an "access denied" message.
Thanks again, Andrei, for your help with the ACL issue. It seems like we're almost there!
Cheers,
Ben
Comment #30
atchijov CreditAttribution: atchijov commentedMost of problems (except overlay related) should be fixed with this version.
Comment #31
BenK CreditAttribution: BenK commentedAndrei,
Wow, terrific update! You've fixed the vast majority of bugs. So here's a roundup of all of the previously reported bugs (as described in Comment #28) and their current status:
A. ERROR WHEN ADDING NEW CONTENTFixed!B. ARRAY DATA DISPLAYED WHEN SAVING ACCESS CONTROL DEFAULTS PAGEFixed!C. EXTRA "SUBMIT" WHEN SAVINGFixed!D. ERROR WHEN INSTALLING MODULE: This problem still exists. Looks like something needs to be updated in content_access.install
1. Content Type Access Defaults in Wrong LocationFixed!2. Warning Message on 'View Any' or 'View Own'Fixed!3. Failure to Un-grant PermissionsFixed!4. Per Node Checkbox is Being ActivatedFixed!5. Overlay Not Working on Defaults PageFixed! (Even though you said it was still a bug!)6. Overlay and Admin Theme Not Working on Node Access Page: Still a problem. But I've got a potential fix for this (will post to a subsequent comment.)
7. Reset to Defaults ErrorFixed!I did, however, notice three new (small) bugs:
i. Not Respecting Drupal Core Permissions By Default on Access Control Defaults Page: When a new content type is created, Content Access is supposed to respect Drupal core permissions by default. (This is how it works in the D6 version of the module.) So if authenticated and anonymous users have been granted the "View published content" permission in Drupal core (admin/people/permissions), then when a new content type is created, the access control defaults for that content type should give both anonymous and authenticated users the "View any content" and "View own content" permissions by default. However, in the current module, "View any content" and "View own content" permissions are unchecked by default (except for roles with the "administer content" permission) regardless of the current core permission settings.
Note that everything seems to work properly for Edit Any, Edit Own, Delete Any, and Delete Own permissions. It also works properly if you make subsequent changes to core "View published content" permissions (removing view permission for anonymous users, for instance, will remove it from the access control defaults page for each content type). It just doesn't work when you create a new content type (view permissions are always unchecked by default). This could cause confusion to a site administrator who doesn't understand why users can't view content of the newly created content type. (By the way, I think this worked properly in a prior version of your port, so a recent change may have caused this to regress.)
ii. Incorrect Link on Access Control Defaults Page: Very minor. At the access control defaults page for a content type (admin/structure/types/manage/[test-type]/access), under the "Per content node access control settings" header, there is an incorrect link to the "permissions" page. The link currently goes to "admin/user/permissions" but in D7 is should go to "admin/people/permissions".
iii. Consistent Menu Tab Language: The sub-menu tab of the access control defaults page (for each content type) should probably say "ACCESS CONTROL" instead of just "ACCESS". Obviously not a big deal, but probably should stay consistent with other mentions within the module.
Great work,
Ben
Comment #32
BenK CreditAttribution: BenK commentedAndrei,
In regard to Bug 6 (Overlay and Admin Theme Not Working on Node Access Page) in comment #31, I think this could caused by a corresponding bug in the D6 version of the module that was just recently fixed (it's a short, simple fix). To read about this, go to this thread:
http://drupal.org/node/811338
Also, the D6 patch that was committed (which fixes the bug) can be found here:
http://drupal.org/files/issues/811338-use_admin_theme_6.patch
Basically, the admin theme wasn't being called properly for that page. I think all of the overlay stuff will work once the admin theme is called.
So can the code from the patch be copied (or adapted) to fix Bug 6 in our D7 port? Could you add that to the next version of the module that you post?
Thanks again for all your help,
Ben
Comment #33
atchijov CreditAttribution: atchijov commentedI think that patch did solve the problem with overlay. See new version attached...
Personally, I keep my overlay module off. Not sure what is the value of it (beside esthetics)
Comment #34
BenK CreditAttribution: BenK commentedThanks, atchijov. Will test the new module in #33 later today. Quick question: In addition to the overlay patch, does the module include fixes to D., i, ii, and iii above (in comment #31)?
Cheers,
Ben
Comment #35
atchijov CreditAttribution: atchijov commented@BenK #33 only overlay fix.
Comment #36
BenK CreditAttribution: BenK commentedAndrei,
I tried out your updated module in #33, but unfortunately, the overlay issue doesn't seem to be fixed. Perhaps a small tweak is necessary to make the patch code function in D7? Any ideas?
Thanks,
Ben
P.S. Once we get this working, I think it will also benefit those who turn off the overlay. Since the code allows the node access control page to use the admin theme (rather than the site default theme), it would benefit anyone who uses a different theme for administration tasks.
Comment #37
BenK CreditAttribution: BenK commentedHey Andrei,
I don't know if you had a chance to take a look at salvis' comments on the ACL module thread (http://drupal.org/node/836822, comments #17 and #20), but he pointed out that there may be an issue with the node access grants currently generated by Content Access.
Currently, if you use the Devel Node Access module to see the "Access Permissions by User" for a given node, it will report access denied as "No: by content_access". But salvis said it shouldn't do this (based on how D7 is supposed to work). Here's what he wrote:
So I'm not exactly sure about the implications of this, but until it's fixed, I'm wondering if this could cause Content Access to have difficulty playing well with other node access modules (like ACL or anything else). Does this mean that Content Access is currently overruling node access set by other modules?
Thoughts?
--Ben
Comment #38
BenK CreditAttribution: BenK commentedHey Andrei,
Okay, here's the latest update: salvis and I have confirmed that the reason we're having problems with integrating Content Access with ACL is that Content Access is overruling node access set by ACL on node pages. How this works changed in D7.
I did some more research on this (see http://api.drupal.org/api/group/node_access/7). Basically, it appears the root cause of the problem is that Content Access is returning NODE_ACCESS_DENY (which blocks access to the node no matter what ACL says) if someone unchecks a box (for view, edit, or delete permissions) on the access control page. Instead, we should probably be returning NODE_ACCESS_IGNORE to allow other modules or the node_access table to control access (so Content Access doesn't override all other modules).
Here's some more specific information taken from the D7 node access API page (http://api.drupal.org/api/group/node_access/7):
So, Andrei, can you make the needed changes and post a new version of the module as soon as you have a chance?
We'll probably need to fix both the content type access control defaults page as well as the per-node access control settings page. As always, I'll be an eager tester. It will be great to get all of this wrapped up!
Best,
Ben
Comment #39
BenK CreditAttribution: BenK commentedJust a quick note: The D7 port of ACL is pretty much done and working well. So we only have the remaining bugs on this thread and then we're done! :-)
Comment #40
BenK CreditAttribution: BenK commentedIn doing a review of what's left to be done on this D7 port, I noticed that the content_access.rules.inc file may not have been ported yet. Since the Rules 7.x-2.x-dev branch is working well these days, this is something that we should do ASAP. A lot of people (including myself) use the Rules integration along with the ACL module to grant access to a node on specific event triggers.
Fortunately, there are not too many lines of code in that file. I did some research on upgrading the integration to Rules 7.x-2.x-dev and here's a link to a recent thread where Rules API changes (from 6.x-1.x-dev to 7.x-2.x-dev) are discussed:
http://drupal.org/node/832046
As you'll see from fago's comments on the thread, he jotted down some of the biggest changes in comment #2.
Also, the Rules 7.x-2.x-dev module itself appears to have some helpful documentation in the rules.api.php file. I've attached the file to this comment (renamed it with a .txt extension) so that it's handy for reference.
--Ben
Comment #41
BenK CreditAttribution: BenK commentedHey everyone,
Given all of the comment on this thread, I thought it might be helpful to post a summary of the bugs/issues remaining to be fixed on the D7 port:
a) ERROR WHEN INSTALLING MODULE: Looks like something needs to be updated in content_access.install. See comment #10 for more details on the error message.
b) Not Respecting Drupal Core Permissions By Default on Access Control Defaults Page: When a new content type is created, Content Access doesn't respect Drupal core permissions by default. See comment #31 for more details.
c) Change NODE_ACCESS_DENY to NODE_ACCESS_IGNORE: This change will prevent Content Access from inadvertently overriding other access control modules like ACL. See comments #37 and #38 for more details.
d) Port content_access.rules.inc to D7: We need this file updated so that it can work with Rules 7.x-2.x-dev. See comment #40 for more details.
e) Overlay and Admin Theme Not Working on Node Access Page: We tried a fix for this, but it's still a problem. The patch used may need to be updated to D7. See comment #32 for more details.
f) Incorrect Link on Access Control Defaults Page: Very minor. At the access control defaults page for a content type, there is an incorrect link to the "permissions" page. See comment #31 for more details.
g) Consistent Menu Tab Language: Very minor. The sub-menu tab of the access control defaults page (for each content type) should say "ACCESS CONTROL". See comment #31 for more details.
Well, that's it! Not too much left to do...
Hope this helps,
Ben
Comment #42
atchijov CreditAttribution: atchijov commentedI am having problems with this. Either I do not understand how it is supposed to work or there is an issue wit the whole scheme of things.
Let assume that we want to configure access in such a way that anonymous users can access most of published articles, but not some. In current form I can do it, by enabling "View published content" for role "anonymous" (via "standard" user interface), then going to "Content/Access Control" and disabling "view any content" for anonymous user on particular nodes. In order to be able disable access, I have to return "deny".
If we do not want to use "deny", then the only way to do it is to disable "View published content" at "role" level, then enable "view any content" at content type level and then disable "view any content" for particular nodes.
I have 2 problems with this:
1) disabling "View published content" at "role" level - looks too drastic
2) "Access Control" in current form, will disable particular checkbox if role lucking given permission (so if we deny "View published content" for "anonymous", then in Access Control, "View any content" checkbox for role anonymous will be disabled).
Bottom line: it seems to me that at some point someone should be able to say "deny". If you have multiple modules which participate in node_access - then I do not understand how it is supposed to work.
Comment #43
BenK CreditAttribution: BenK commentedHey Andrei,
I'm trying to do more research into the issues you brought up about NODE_ACCESS_DENY (what you are saying makes sense), but in the meantime, have you had a chance to work on the other issues/bugs?
Thanks!
--ben
Comment #44
atchijov CreditAttribution: atchijov commentedfrom #41, I think I fixed a, c, e, f, g. Maybe b as well. Will deal with Rules over weekend.
Comment #45
BenK CreditAttribution: BenK commentedHey Andrei,
I tested out your patch in #44 and things are looking good. I can confirm that a, c, e, f, and g are fixed. In particular, Content Access and ACL are now working together well!
So here's a summary of what's left to be fixed (b & d), plus two new bugs that recently surfaced (h & i):
b) Not Respecting Drupal Core Permissions By Default on Access Control Defaults Page:
I asked salvis about this on another thread, just to make sure my understanding of the proper defaults were correct. (It was.) Here's what he said:
The only modification to these defaults is if the 'access content' core permission is unchecked for a role. If this is the case, here's what salvis says should be done:
d) Port content_access.rules.inc to D7: That's awesome you're working on this over the weekend. I'll be ready to test.
New Items:
h) Error Message - Duplicate records returned from hook_node_access_records(): There's one new error message that surfaced (reported by Devel Node Access). When visiting a node with per-node access control settings enabled--but no individual users granted view, update, or delete access--I'm getting the following error message reported by Devel Node Access:
Interestingly, if I grant a user one permission (either view, update, or delete), one of the "4/acl/0/000 by acl" will go away. If I grant a user two or more permissions, then the entire error will go away completely.
i) Error When Saving Site Permissions: I'm not sure this is a Content Access error, but it surfaced right after I changed some of the Content Access permissions located at admin/people/permissions. (I gave a role the "Grant Own Content Access" permission.) The permission initially saved fine, but then when returning to the permissions page, making other changes, and trying to save the page, I started getting the following error message:
Once I get this error message, changes to permissions are no longer being saved.
I'm not quite sure how to debug this. Any ideas as to what could be causing it?
Thanks for all of your awesome work! :-)
--Ben
Comment #46
atchijov CreditAttribution: atchijov commentedwill look into these new things over weekend.
Comment #47
BenK CreditAttribution: BenK commentedHey Andrei,
It seems that h (Error Message - Duplicate records returned from hook_node_access_records) has been fixed in the latest -dev version of the ACL module. salvis found the issue with ACL and committed the fix late yesterday. So make sure you have the latest version of ACL and we don't have to worry about h) :-)
So all we have left is b, d, and i...
--Ben
Comment #48
BenK CreditAttribution: BenK commentedAndrei,
I did some more testing of i (Error When Saving Site Permissions) to try to figure out what was causing the error. I was able to re-create the error on a clean install of the latest -dev version of D7 core (with Content Access and ACL installed)... and I didn't have this error before installing Content Access.
The error seemed to surface after I granted "Edit Any" and "Edit Own" permission to my "editor" role for the Article content type (on the content type access control defaults page). This seemed to cause a corresponding change on the D7 core permission page (as it should), which then caused the "Edit Any" and "Edit Own" permission to be disabled (and checked) on an individual Article node's access control page. But perhaps there is an issue with how Content Access is changing the core permission, thereby causing some type of duplicate entry.
Any thoughts on why this error might be occurring? It's obviously a pretty major issue if it's causing the core permission page to no longer work...
Thanks,
Ben
Comment #49
BenK CreditAttribution: BenK commentedIn regard to bug i) Error When Saving Site Permissions, I also asked salvis what he thought. Here's what he said:
Just thought this might help....
--Ben
Comment #50
fagoI think there some new API functions too: http://api.drupal.org/api/search/7/user_role_*
I'd be fine committing this once it is basically working, so we can roll patches for the remaining issues afterwards.
Comment #51
atchijov CreditAttribution: atchijov commentedRE: #45. Last issue.
I have run into it as well. I do think that there is a bug in user.module. I was able to fix this issue by apply following fix to user.module
replace this: (~line 2863)
with this
Basically in definition of role_permission only rid and permision are parts of primary key. module is not.
Comment #52
BenK CreditAttribution: BenK commentedHey atchijov,
Thanks for the suggestion in #51. It worked for me and the error went away. Yup, it's a core bug. There's a core patch to fix it submitted here: http://drupal.org/node/607238#comment-3218818. Perhaps you can give the patch a +1 over on that thread? This way, we have more people saying it's been tested and can get it committed ASAP.
So that leaves only b) and d) left to fix... How are things coming on your end?
Cheers,
Ben
Comment #53
BenK CreditAttribution: BenK commented@fago: Hey fago, great to see you chiming in on this thread. Yes, we're very close to a fully functional D7 port and once we fix the last two issues (atchijov is currently working on them and I am ready to test his fixes), it will be awesome to open up an official D7 branch. Also, we'd love to have your help with updating the Rules integration since you are the ultimate resource! :-)
Cheers,
Ben
Comment #54
BenK CreditAttribution: BenK commentedIn follow up to #52 (related to the 'Error When Saving Site Permissions' bug), the patch to the core bug was just committed here:
http://drupal.org/cvs?commit=404470
So that issue is now totally resolved!
--Ben
P.S. The only things we have left to fix in the D7 port are:
b) Not Respecting Drupal Core Permissions By Default on Access Control Defaults Page
d) Port content_access.rules.inc to D7
Can anyone assist with either of these? I'm happy to review/test any patches as soon as they are posted.
Comment #55
mertskeli CreditAttribution: mertskeli commentedsubscribing
Comment #56
fagoI've not tested or looked at the code yet, but I'm willing to commit it to CVS so we can start rolling separate patches for the remaining issues. To proceed, please roll a patch as usual against the latest 6.x-dev.
Comment #57
fagoI had a short look at it, however the code has some serious flaws. I decided to better postpone committing it until at least the serious stuff is fixed.
I rolled it into a proper patch.
Issues:
Serious:
+function expand_checkboxes($element) {
What's that? Looks like a weird copy/paste fix. Solve it properly and don't violate namespaces.
* Code style. Changes introduce code style violations.
- acl_node_add_acl($node->nid, $acl_id, $op == 'view', $op == 'update', $op == 'delete', content_access_get_settings('priority', $node->type));
+ acl_node_add_acl($node->nid, $acl_id, (int)($op == 'view'), (int)($op == 'update'), (int)($op == '
!?!?
+// $form['per_role']['#after_build'] = array('content_access_force_permissions');
Why that!?
- drupal_install_schema('content_access');
+ // happen automatically in D7
+ // drupal_install_schema('content_access');
}
/**
@@ -15,7 +16,8 @@
function content_access_uninstall() {
variable_del('content_access_settings');
// Remove tables.
- drupal_uninstall_schema('content_access');
+ // happen automatically in D7
+ // drupal_uninstall_schema('content_access');
}
What about removing lines? ;)
Non serious:
-// $Id: content_access.admin.inc,v 1.1.2.24 2010/07/19 11:40:11 fago Exp $
+// $Id: content_access.admin.inc,v 1.1.2.23 2009/07/31 10:03:22 fago Exp $
The port is outdated. We need to integrate recent changes.
* Code needs to update to d7 coding standars. Coder module?
-function content_access_admin_settings(&$form_state, $type) {
+function content_access_admin_settings( $form, &$form_state, $typeObject ) {
+ $type = $typeObject->type;
+
we do not use camelcasing outside of classes
Comment #58
BenK CreditAttribution: BenK commentedHey fago,
Thanks for rolling the patch file and your helpful comments. I'll try to research each of the points you brought up, but I wanted to first address one bit of code you mentioned:
This code change was actually suggested by salvis, the ACL module maintainer, at the following thread:
http://drupal.org/node/824642#comment-3080112
Basically, without this change we were getting an error message when viewing a node's "Access Control" tab.
Because my own coding skills aren't great, I asked Berdir about the purpose of this in IRC. He said that:
So is this code change OK with you? Or is there some other way you'd rather deal with it?
Thanks,
Ben
Comment #59
BenK CreditAttribution: BenK commentedHey fago,
I was able to reach atchijov and ask him your question about:
atchijov said that he added this because expand_checkboxes is a function from D6 which is missing in D7. Because Content Access is using this function, he just copied it "AS-IS" from D6 (just to keep the port moving along).
I asked Berdir in IRC if he knew of a D7 equivalent to expand_checkboxes. He said that expand_checkboxes was renamed form_process_checkboxes in D7:
http://api.drupal.org/api/function/form_process_checkboxes/7
Because form_process_checkboxes looks virtually identical to expand_checkboxes (see http://api.drupal.org/api/function/expand_checkboxes/6), replacing expand_checkboxes with form_process_checkboxes should be enough to make everything work properly. atchijov concurs with this.
--Ben
Comment #60
mertskeli CreditAttribution: mertskeli commented@fago #57
I can confirm that #44 had been working.
What do you mean by "serious stuff"?
Comment #61
BenK CreditAttribution: BenK commentedHey fago,
Thanks for the patch file and your code review in #57. I've gone through all of the issues you raised (all excellent points!) and have fixed them one by one. A new patch file is attached. If it meets your approval, can we create the new 7.x branch?
Here's is a summary of how I fixed each of the issues you raised:
>> I got rid of this code because it was a temporary hack. expand_checkboxes was renamed form_process_checkboxes in D7, so once I implemented that change, this code was not necessary.
>> This code change was necessary because of a change in the D7 version of the ACL module. The change was actually suggested by salvis, the ACL module maintainer, at the following thread: http://drupal.org/node/824642#comment-3080112
>> This was a code fragment left over from the debugging process. I deleted it. I also found several other ones like this and I deleted those too.
>> Yes, I've deleted these lines.
>> I've updated the port to all the latest commits. It's now current. Thanks for pointing this out.
>> I ran the Coder module and made a bunch of coding standard improvements suggested by the module.
>> I fixed this (including both the example you mentioned and a couple other examples found elsewhere).
So I think we should be all set to open the new D7 branch. We also did another round of manual module testing on our end and things are working great.
Cheers,
Ben
Comment #62
Scott J CreditAttribution: Scott J commentedThanks to everyone for this effort.
#44 is working OK for me with ACL on Drupal 7 Beta 1.
Comment #63
fagoThanks for working on this - you made good improvements :)
That's a wrong usage of t(). This could be converted to be set via hook_menu() or just tell drupal_set_title() to not check plain (have a look at its docs).
There is leading space in front of $form.
I see your argument, makes sense. But please add a space after (int) to be consistent with the coding standards.
You are introducing whitespaces.
The same here.
Again the leading space and bad variable naming. Better just stay with $type and then use $type_name.
Lots of too many spaces. It should be
Again spaces.
forgotten debug statement? :)
You also introduce several empty lines containing spaces ins the .module - watch out for those or configure your editor to kill them automatically.
You like that extra spaces? :) It should always be
content_access_node_type('delete', $info);
- please watch out for that too.Comment #64
dawehnerA short review of the patch:
Shouldn't db_delete always be used?
I'm not sure whether ? is a good placeholder here. I think :nid is a better one
Some other things
hook_nodeapi -> hook_node_delete
I guess also db_update should be used when possible.
There is a api for this user_role_permissions(user_roles()). I'm not sure whether this is the right usage of the function.
Comment #65
good_man CreditAttribution: good_man commentedsubscribing, thanks for all the efforts!
Comment #66
good_man CreditAttribution: good_man commentedLet me know how I can help in making a stable version, I enabled it with the latest ACL dev version and gettings WSOD when adding individual users to nodes, got the following error:
I'll try to dig into this error and see how I can fix it.
Comment #67
good_man CreditAttribution: good_man commentedLet me know how I can help in making a stable version, I enabled it with the latest ACL dev version and gettings WSOD when adding individual users to nodes, got the following error:
I'll try to dig into this error and see how I can fix it.
Comment #68
BenK CreditAttribution: BenK commented@good_man: Did you use the patch in #61? Last I checked it worked fine with ACL. What version of D7 core are you using?
--Ben
Comment #69
good_man CreditAttribution: good_man commentedI'm sorry for duplicating the reply, it's varnish issue in d.o infrastructure, anyhow I filled an issue in ACL issue queue
#946636: "Call to undefined function _acl_edit_form_after_build()" — acl_edit_form() signature changed!
Comment #70
FiNeX CreditAttribution: FiNeX commentedsubscribing too
Comment #71
Anonymous (not verified) CreditAttribution: Anonymous commentedwhere I can find content_access module to test in a D7 fresh installation?
Comment #72
Scott J CreditAttribution: Scott J commentedgiorez,
It's still being built! But there's a version to get started with up above at #44. Then follow the discussion after that to see where it is up to.
Comment #73
BenK CreditAttribution: BenK commentedYou should use the patch in #61. That is the latest version. (The zip file in #44 is an older version.)
--Ben
Comment #74
thekevinday CreditAttribution: thekevinday commentedsubscribe
Comment #75
gorillaz.f CreditAttribution: gorillaz.f commentedsubscribe
Comment #76
BenK CreditAttribution: BenK commentedFor anyone who can help with this, here's a quick summary of where things stand:
1. The patch in comment #61 is the latest version. That should be the basis of future work.
2. We need to incorporate the code cleanup specified in comment #63.
3. We need to review and possibly incorporate the code suggestions in comment #64.
4. ACL module has changed the signature of acl_edit_form(). We need to make corresponding changes for Content Access to work with ACL. This is a recent change as ACL was previously working well with CA. See http://drupal.org/node/946636#comment-3665690 for more info.
5. Rules integration still needs work to be functional.
--Ben
Comment #77
jhedstromSubscribing.
Comment #78
andypostsubscribe
Comment #79
smls CreditAttribution: smls commentedsubscribe
Comment #80
amanaplan CreditAttribution: amanaplan commentedsubscribe
Comment #81
cnolle CreditAttribution: cnolle commentedSubcribe
Comment #82
trekoid CreditAttribution: trekoid commentedsubscribe
Comment #83
harriska2 CreditAttribution: harriska2 commentedsubscribe
Comment #84
emmeade CreditAttribution: emmeade commentedsubscribe
Comment #85
thekevinday CreditAttribution: thekevinday commentedI have encountered the following when renaming a content type machine name:
The code in question:
Would it be more appropriate to add an if (isset()), such as:
Comment #86
Finn Lewis CreditAttribution: Finn Lewis commented@BenK - thanks for this.
I too am keen that this module be ready for D7 production soon.
Following your list,
I took the patch in #61 and applied it to the 6.x-1.x-dev branch from CVS.
The limited testing I did gave me the results I was expecting, so it works for me.
I reviewed the comments in #63 as suggested, and cleaned up what I could.
Coder module suggested a who lot more cleanup - mostly related to spaces, so I went through these until coder module was almost happy.
Interestingly, it still had two complaints, which didn't seem to make sense to me:
Is that just coder being strange?
That's as far as I got tonight. I looked at the comments in #64, but wasn't sure which should be applied.
I attach a patch of the clean-ups made. Hope this helps and look forward to helping further with the other items you mention.
Comment #87
good_man CreditAttribution: good_man commentedWe did something very similar to Content Access for D7, the main reason was that we needed a similar simpler approach ASAP with a simple API methods to call, please checkout Flexi Access, and I'm keen to hear thoughts & how to-not duplicate any efforts.
Comment #88
cnolle CreditAttribution: cnolle commentedThe patch in #61 seems to work quite nicely. A couple of issues.
- Nodes that are supposed to be hidden from certain roles still show up on taxonomy/term pages.
- If I enable individual access control on nodes and click on access control for a particular node I get a white screen (as if page not found, BUT not a drupal rendered 404).
Comment #89
alayham CreditAttribution: alayham commentedsub
Comment #90
cewolcott CreditAttribution: cewolcott commentedBenK,
Are overlays still an issue?
Comment #91
BenK CreditAttribution: BenK commented@cewolcott: As of the patch in #61, overlay stuff seemed to work fine.
Comment #92
cewolcott CreditAttribution: cewolcott commentedBenK,
The UI looks great when I perform:
Admin -> Structure -> Content Type -> ContentType Edit -> Access Control
I can see the Content Access Form (Role Based Access Control Settings, Per Content ...., Advanced).
I then select "Enable per content node access control settings" and submit.
I then navigate to Admin -> Content and select an instance of the content type that I allowed node access control settings.
When I select an instance I now have the View, Edit and Access Control tabs. But when I select the Access Control tab the screen goes grey (IE) or white (Firefox, Chrome, Safari).
Any thoughts.
Comment #93
BenK CreditAttribution: BenK commented@cewolcott: That sounds like a fatal error rather than anything to do with the overlay. Do you have ACL module installed to grant access to individual users?
Comment #94
cewolcott CreditAttribution: cewolcott commented@BenK (#93) Yes I do. I dowloaded acl-7.x-1.0-beta1.tar.gz from Nov 6, 2010.
I have received this issue on both my home laptop running Windows 7 and IIS 7.5 and at work running Windows XP and IIS 6.0.
Both sites are Drupal 7 RC2 and then just the latest ACL module (above) and Content Access 6.x-1.x-dev with patch from #61. I also tried the patch from #86.
Any insight on were to look?
Comment #95
BenK CreditAttribution: BenK commented@cewolcott: Can you confirm that the problem goes away if you disable the ACL module?
The cause of the fatal error is probably this:
--Ben
Comment #96
cewolcott CreditAttribution: cewolcott commented@BenK,
My apologies. It does go away when I diable the ACL module. I saw the comment you highlight when resding the thread, but when I finally had a chance to test everything out I forgot about that statement. I thought Content Access required ACL?
Thanks this allows be to continue with the testing.
Comment #97
BenK CreditAttribution: BenK commentedNow we just need someone to help implement fixes related to http://drupal.org/node/946636#comment-3665690 so that ACL can work with Content Access again. Any takers?
--Ben
P.S. I can't work on this myself now because I'm swamped with stuff for the 7.x ports of Private Message and Userpoints modules.
Comment #98
good_man CreditAttribution: good_man commentedNote: The attached port contains a small bug, better see comment #100.
This D7 port is based on comment #44, current D6 dev (2010-Sep-03), code reviews, patches in previous comments, and some hard work while I was eating sweet potatos!
A list of changes since initial port in comment #44:
content_access.module
- no need for hook_init() to set the admin theme, D7 handles this.
- removed old content types commented code from hook_menu().
- adding some explanation comments.
- changed D6 hook_nodeapi() to the new node api hooks.
- changing D6 DB queries to D7 new DB abstraction layer.
- removed many trailing tabs, extra spaces, and modified some code to match Drupal's coding standards & coder module reviews.
content_access.install
- no need for hook_install() and hook_uninstall() to install/uninstall the schema, since D7 automatically do that.
content_access.admin.inc
- using the new permissions & roles api in D7 mentioned in comment #64, removed unused function as a result and changed the permissions array a little to match D7 permissions array.
- removed many extra white spaces
- removed expand_checkboxes() and used the D7 form_process_checkboxes() instead.
- t() in D7 takes vars as @varname
- changing D6 DB queries to D7 new DB abstraction layer.
- removed many trailing tabs, extra spaces, and modified some code to match Drupal's coding standards & coder module reviews.
content_access.rules.inc
- NOT YET
tests
- NOT YET
Questions:
- why in content_access_save_per_node_settings() you are using a foreach approach instead of the old approach (update then insert if no records found)?
- why using old and new node type hooks? why not just using the new ones?
Also,
I didn't understand it very well, or how to reproduce, any how maybe this issue is now gone since I used D7 permissions & roles API.
Comment #99
BenK CreditAttribution: BenK commented@good_man: Thanks for getting involved! :-) Can you post your changes as a patch? It's much easier to review (and make sure that all prior patches were included). Also, if we want fago to review and commit the final version, he'll need to see it as a patch, too.
And did you address the ACL integration issue mentioned in comment #95 and #97?
As for your questions:
a. Not sure.
b. Probably just an oversight. The developed who did the original D7 port on this thread hasn't been working on this issue for quite awhile.
c. As for "Not Respecting Drupal Core Permissions By Default," here is an explanation:
So I'm not sure if your changes fixed this... When you create a new content type, is "View any content" and "View own content" checked by default for the anonymous and authenticated user? If so, then it is fixed.
--Ben
Comment #100
good_man CreditAttribution: good_man commentedHere is a patch against the current dev version (sep 3), and to see the changes go to comment #98.
re. point b) yes it's working as expected, only manual testing though, and #95, #97, yes they are compatible with the latest changes of ACL, as I opened that issue in ACL :)
Comment #101
cewolcott CreditAttribution: cewolcott commented@good_man - Thanks for the patch. It fixes my issue in #92. The ACL and Content_Access modules work well together. On the further testing.
Comment #102
AdrianC-1 CreditAttribution: AdrianC-1 commentedSubscribing
Comment #103
good_man CreditAttribution: good_man commented@cewolcott: thanks for testing, anyhow want to work on tests today, and release a new patch with tests.
re. rules: is the new rules 7.x API stable?
Comment #104
Scott J CreditAttribution: Scott J commentedI know that Rules is being successfully relied on by Commerce module. Just be sure to use dev versions of both Rules & Entity as both modules have recently had their structure reorganised to prevent a problem with Update manager.
http://drupal.org/node/660108
Comment #105
BenK CreditAttribution: BenK commented@good_man: Yes, rules 7.x API is quite stable. Private Message module and Userpoints module have successfully integrated with it for a few months now. So any work you can do on porting content_access.rules.inc would be great! :-)
--Ben
Comment #106
cewolcott CreditAttribution: cewolcott commented@good_man & BenK,
I have applied the latest patch (690610-100.patch) and found the following issue.
I enabled "Per content node access control settings" for the Article content type. I then surfed to a article node and selected the Access Control tab. I then de-selected a checkbox and pressed the Submit button and received an http 500. After adding echo statements (because I do not know any other way to debug) I found the following issue.
File: content_access.module
Function: content_access_save_per_node_settings
Issue: I believe the db_result() needs to be removed from around the db_query and a -->fetchfield() added to the end. This will set the $result variable to the number of rows found (which will be 0 or 1).
The http error went away and a row was actually added to the content_access table.
Comment #107
good_man CreditAttribution: good_man commented@cewolcott, there is an error in the count query (based on old count query from D6), I changed it to be based on the new D7 DB layer, it should be fixed now.
@Scott J & @BenK, good news, working on it now.
expect a new patch soon!
Edit: @cewolcott will be in the next patch not the one in #100.
Comment #108
binford2k CreditAttribution: binford2k commentedsubscribe
Comment #109
good_man CreditAttribution: good_man commentedUpdates:
content access was using Rules 1.x way to attach it's form to the action UI, this is changed now, and Rules 2.x should handle your data by providing them using metadata & entity API.
from 8. Updating module integration
The main reason behind this big change for content access 7.x is that there is no way in Rules 2.x to save your form's data on submit (i.e. in Rulex 1.x, content access uses action_name_form() and action_name_submit() with the form & form_state as a param, now you can't pass form or form_state to the new action_name_process() only rules' settings array is passed). Also, Rules 2.x docs are still lacking many examples & howto. Any thoughts?
Comment #110
fagohttp://drupal.org/node/878940 might help you, in Rules 2 you usually don't have to write forms at all. Just specify your parameters. If really necessary, you can use the form_alter callback though.
Comment #111
good_man CreditAttribution: good_man commentedThanks you are so fast! was talking with Berdir on IRC and he suggested to open an issue in Rules queue, but it seems you were faster :)
Anyhow to summarize the problem:
in Rules 1.x there was a callback for configuration form submit:
The nice thing in this method, is it takes 3 params. which is important if you want to save back the settings to $settings & take values from $form_state
In Rules 2.x there is no easy way to do so. Tried the form_alter with $form['#submit'], but it doesn't take $settings, only $form and $form_state, so no way to save the config. form.
Hope the problem is more clear now, thoughts?
Comment #112
good_man CreditAttribution: good_man commentedJust a patch to solve #106 before the Rules integration patch.
Comment #113
fagoThere is nothing like that in Rules 2.x, because you are not supposed to add your own settings via a form. Use parameters.
Comment #114
good_man CreditAttribution: good_man commentedIn the begining I thought it's an overhead to use entity for a simple case like this, but after looking at entity & metadata for a while, all I can say *it must be in D8* :)
Comment #115
wasare CreditAttribution: wasare commented+1
Comment #116
cestmoi CreditAttribution: cestmoi commentedsubscribing
Comment #117
BenK CreditAttribution: BenK commented@good_man: How goes the Rules integration? Anything you want me to test?
--Ben
Comment #118
good_man CreditAttribution: good_man commentedharddisk crashed last week, still in the process of recovery to get my work on rules back, if anyone can help please jump in, otherwise I'll need at least 4-5 days.
Comment #119
danmcewan CreditAttribution: danmcewan commentedsubscribing
Comment #120
Hawkwala CreditAttribution: Hawkwala commentedI installed content_access from #98 and get following error when trying to set permissions:
* Notice: Undefined index: access comments in user_role_grant_permissions() (line 2966 of /../../../../modules/user/user.module).
* PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'module' cannot be null: UPDATE {role_permission} SET module=:db_update_placeholder_0 WHERE ( (rid = :db_condition_placeholder_0) AND (permission = :db_condition_placeholder_1) ); Array ( [:db_update_placeholder_0] => [:db_condition_placeholder_0] => 1 [:db_condition_placeholder_1] => access comments ) in user_role_grant_permissions() (line 2968 of /../../../../modules/user/user.module).
Following are installed:
ACL 7.x-1.0-beta2
Content Access No information appears in modules - installed from #98
Any ideas or help will be appreciated.
Comment #121
jarush CreditAttribution: jarush commentedI'm looking for this module for a long time......
Comment #122
Yannakis CreditAttribution: Yannakis commentedsubscribing
Comment #123
daviesap CreditAttribution: daviesap commentedSubscribing
Comment #124
spykerc8 CreditAttribution: spykerc8 commentedSubscribing
Comment #125
Jiri Volf CreditAttribution: Jiri Volf commentedSubscribing
Comment #126
skov CreditAttribution: skov commentedSubscribe
Comment #127
snupy CreditAttribution: snupy commentedSubscribe
Comment #128
freddan CreditAttribution: freddan commentedsubscribe
Comment #129
gadams CreditAttribution: gadams commentedsubscribing
Comment #130
Csabbencs CreditAttribution: Csabbencs commentedI'm facing the same issue as #120.
Comment #131
BenK CreditAttribution: BenK commented@Csabbencs: Did you try the patch in #112? That is the latest version. Can you see if you get the same problem and report back?
The post in #120, unfortunately, tested an older version of the patch.
--Ben
Comment #132
Csabbencs CreditAttribution: Csabbencs commented@BenK: I applied the patch #112, but got the same result. :(
Comment #133
cloudbull CreditAttribution: cloudbull commentedSubscribe
Comment #134
ianm CreditAttribution: ianm commentedSubscribe
Comment #135
gcastagneti CreditAttribution: gcastagneti commented@good_man: Could you create a new .tar.gz file with all of patches applied? It'll be really useful
Thank you very much!
With kind regards,
Gabriele Castagneti
Comment #136
ropaolle CreditAttribution: ropaolle commentedSubscribe
Comment #137
emptyvoid CreditAttribution: emptyvoid commentedVery cool, I am the maintainer for the node_access module and I plan to not release a D7 version in favor of directing users to use your module instead. I'd like to review the code and contribute if I can, there are a few differences between node_access and content_access and when I have time, I'll list the business logic differences. I'd love to hear your input on what would be useful features to incorporate (or not).
http://drupal.org/project/node_access
Comment #138
good_man CreditAttribution: good_man commentedokay back to work again, will try to provide the patch ASAP. Thanks all for testing, I'll try to fix that too.
Comment #139
sludwig CreditAttribution: sludwig commentedSubscribe
Comment #140
good_man CreditAttribution: good_man commented@BenK, the case in #120 and #130 is true, it appears when you disable a module, it's permissions still (won't be removed unless you uninstall it), and granting this disabled module's permission will raise that exception, a check for activated modules' permissions should do the job. The next patch should fix it.
I want a D7 branch to commit changes so I won't lose my changes if any accedent happens like #118 which I lost long working hours. In the mean time I'm mainting the project using github.
Comment #141
good_man CreditAttribution: good_man commented@emptyvoid: can you open another issue? I want to discuss flexiaccess too.
Comment #142
good_man CreditAttribution: good_man commentedOkay a new patch to fix #120 and #130. NOTE: make sure to use the dev version of ACL as it fixes #1024114: Fatal error in function acl_get_id_by_name.
A version of the module in tar format for easy testing.
Can anyone confirm if upgrade from Drupal 6 to 7 with Content Access enabled works? (please take a backup first)?
Comment #143
BenK CreditAttribution: BenK commented@good_man: Great to have you back! :-) Yeah, working on github sounds great. Makes it easy for me to clone your repository and pull in changes as you have them. I did a quick search and I'm assuming this is the repository you're using:
https://github.com/khaledalhourani/Content-Access
Let me know when you want me to start another round of testing (or any other help you need) and we can get this done! It shouldn't be much more work (mostly just the rules stuff and notices) to get something good enough for fago to start the official D7 branch.
--Ben
Comment #144
BenK CreditAttribution: BenK commented@good_man: I think we just cross posted. Do you ever hang out in IRC? I can usually be found in #drupal-games and I'm there right now. So if you ever want to chat directly and coordinate, you should stop by.
--Ben
Comment #145
good_man CreditAttribution: good_man commentedyou are super fast! I was just updating the git repo..
I'm working on rules now, it won't take long, after that we should do the testing.
Next steps IMO should be:
- Make sure the upgrade path is working.
- Update the tests if necessary.
After that we should release the D7 version.
Comment #146
xlyz CreditAttribution: xlyz commentedsubscribing
Comment #147
sludwig CreditAttribution: sludwig commentedi used the tar.gz from #142 with the lastest dev version of ACL and everything seems to be fine on a fresh installation of D7. Really good work
Comment #148
venkatd CreditAttribution: venkatd commentedI want to thank you all for the hard work on the upgrade! I also used the .tar.gz from #142 and so far it has worked flawlessly.
This came in the nick of time for the project I am working on.
Comment #149
justin.maclean CreditAttribution: justin.maclean commentedThank you for this great module, and for the Port to Drupal 7! I'm looking to implement this on a site that I started creating in Drupal 6, and have upgraded to Drupal 7.
During my testing of this module, I, have encountered the following issue:
1) Steps begin Prior to Content Access / ACL Being Installed
2) Anonymous users are set up with no permissions (they do not have even the "View published content" permission).
3) Using Drupal 7's private files to manage attached documents, create a node and upload a file associated with it (content type = Page).
4) Attempt to access the uploaded file using an anonymous user, access is denied as expected.
5) Install the content_access module, and set the Content Type of Page to allow per node access control.
6) Attempt to access the node created in step 3 as an anonymous user. Access is denied as expected.
7) Attempt to download the file uploaded in association with the node, and the file is downloaded to an anonymous user. This should not be allowed.
Thinking this could be related to the conversion from D6 to D7, I ran this same test on a "fresh" D7 install, and was able to duplicate.
This same scenario works as I would expect in the D6 version of the module (i.e. access denied message is issued to an anonymous user when attempting to access an attachment.)
Thanks in advance for any guidance you can provide on this!
Comment #150
justin.maclean CreditAttribution: justin.maclean commentedI should have mentioned, I'm using the tar.gz file from #142 and the development version of ACL.
Comment #151
BenK CreditAttribution: BenK commentedComment #152
ropaolle CreditAttribution: ropaolle commentedHi justin.maclean. Can't help you but I can confirm that I reproduced the same issue.
Comment #153
yngens CreditAttribution: yngens commentedsubscribe
Comment #154
mindgame CreditAttribution: mindgame commentedsubscribe
Comment #155
fagoPatch already looks good! Thanks!
I think we can care about the rules integration in a follow-up issue. Let's concentrate on the main functionality first, and make sure the tests run. Then we'll can commit this and move on with follow-up issues.
In regard to node-access the module update-docs mention:
- http://drupal.org/update/modules/6/7#hook_node_access_records
So I think, best we take care of unpublished nodes too?
Comment #156
Jej CreditAttribution: Jej commentedsubscribe
Comment #157
eddin CreditAttribution: eddin commentedSubscribe!
Comment #158
emptyvoid CreditAttribution: emptyvoid commentedI wonder if integrating with the entity access module would provide the most coverage for security?
http://drupal.org/project/accessapi
Mainly because D7 is based on the entity system and this module provides the CRUD API for almost every object.
http://drupal.org/project/entity
I'm curious if just adding a GUI would allow for the ability to not only define role and user level access to actions (view, edit, delete) of content types but also users, files, whatever is an entity?
Comment #159
good_man CreditAttribution: good_man commented@fago: yeah it won't hurt since D7 is doing so with the unpublished nodes.
@emptyvoid: the plan now is to provide the required tests and release a "port" release, there are many new features to be added but not now, one if them is abstracting the module to provide access for different entities.
Comment #160
Cyberwolf CreditAttribution: Cyberwolf commentedSubscribing.
Comment #161
fago#158 sounds interesting, but is certainly out-of-scope for content access as its name say, it focusses on content == nodes.
Also, yep we really should concentrate on porting here, but not on adding new stuff. That can be done in follow-ups.
Comment #162
emptyvoid CreditAttribution: emptyvoid commentedSounds like a plan to me, I could see adding small sub-modules to enable a GUI for each type of entity later on.
Comment #163
threewestwinds CreditAttribution: threewestwinds commentedSubscribe. I'm also very keen on Rules integration - I'd love to use content access for a new site I'm setting up, which basically runs on a combination of rules and userpoints.
Comment #164
good_man CreditAttribution: good_man commentedThe current patch includes the updated test cases with all tests passing.
Slightly changed from the previous patch to include better form elements labels.
@BenK: the case you told me about is true in D6 version, you can't override type's settings in node's settings, it's a module design issue, maybe we can discuss later if you found that it's better to be able to override it.
Comment #165
good_man CreditAttribution: good_man commentedA patch with the current work of Rules integration.
Note: rules integration port is still in progress and should only be used for testing (not for production).
kudos to Benk for testing and feedback.
Comment #166
BenK CreditAttribution: BenK commentedI've just finished testing the upgrade path from D6 to D7 and everything is working great. I started with a clean D6 install with both Content Access and ACL enabled. I created some nodes and configured node access at both the content type and node levels. I also granted specific users view, update, and delete access via ACL.
Then I upgraded the entire site to D7 and checked all of my previously created nodes. All of the node access settings were retained (including the user ACL settings) and access to the nodes functioned just like in the D6 version. In addition to logging in and out as different users, I also used Devel module's Devel Node Access and Devel Node Access by User blocks to double-check that all of the node access tables looked good. Everything functioned terrifically. So the upgrade path is good to go.
Great work by good_man on this! :-)
Additionally, I manually tested good_man's latest commits via github (including commits that aren't included in the patch in #165) and everything worked very well. He even fixed a few little form bugs. We've now got cleaned-up code, functioning tests, functioning basic rules integration (although more work is needed), and solid integration with ACL.
So good_man, can you upload a final patch with all of your latest commits?
Once we do that (I can test the new patch one final time), then I think we're ready to ask fago to open up the official 7.x branch! :-) And then we can finish up the remaining issues (like the missing rules integration features) on separate follow-up issue threads.
Cheers,
Ben
Comment #167
good_man CreditAttribution: good_man commentedHi BenK, thanks a lot for testing and for this summary, actually the last patch #165 is already containing my latest commits and the commits that came after it to git repo. are just a sync commits because I forgot to update the repo. after the patch :)
I think we are ready to ask for a D7 branch now @fago.
Comment #168
BenK CreditAttribution: BenK commented@good_man: Yes, I now see that the patch in #165 does contain all of your latest github commits.
So I agree that this is RTBC and the patch in #165 should be the one used to create the official D7 branch. Yay! :-)
@fago: Can you create the D7 branch as soon as you have a chance? We can then do follow-up issues in separate threads.
Cheers,
Ben
Comment #169
bryancasler CreditAttribution: bryancasler commentedsubscribe
Comment #170
nuttaphat CreditAttribution: nuttaphat commentedsubscribe
Comment #171
Breakerandi CreditAttribution: Breakerandi commentedsubscribe
Comment #172
anselm.marie CreditAttribution: anselm.marie commentedSubscribe. I wish I could help with the code but I'm not at that level yet.
Comment #173
sw3b CreditAttribution: sw3b commentedSubscribe !!!
Comment #174
giullina CreditAttribution: giullina commentedsubscribing to this... Content Access is a life saver!
Comment #175
tmm360 CreditAttribution: tmm360 commentedsubscribe
Comment #176
fagoWow, nearly 200 comments!
Yes, we need to clear the page and block caches and maybe other caches where the node might appear now. So I removed that comment.
As in core and in d7, we should just capitalize the first character, so I've changed that back.
Analogously I've fixed the permission labels.
Indeed, that looks confusing, in particular as it looks like you have moved content_access_get_acl_id() to .admin.inc ? We should not put API-like functions like this in an include, but we can clean that up in follow-ups.
No, that's needed. I've re-added it and added a comment to explain it:
I've also replaced all "Implementation of" strings with "Implements", as it is used in d7.
Maybe caused by the roles change, I still got a test-case fail. Anyway, I decided to commit it right now, so we can move on. Let's handle the rest in follow-up issues. Thanks for the great work!
Comment #177
threewestwinds CreditAttribution: threewestwinds commentedAwesome. Testing out the CVS version right now.
I don't see an option to open issues against a 7.x version yet - is there a setting you have to toggle somewhere on d.o for that?
Comment #178
nitrospectide CreditAttribution: nitrospectide commentedI was researching access solutions today, and this module seemed to be *the* one, but the apparent lack of a D7 version in the works seriously concerned me - that and the "Maintenance fixes only" on the status. Someone on IRC then pointed me to this thread. Was a checkbox missed somewhere that makes the D7 dev version not show on the project page?
Comment #179
Scott J CreditAttribution: Scott J commentedStruth mate! Are you joking? It's been "in the works" since #8 posted by atchijov on June 3, 2010 at 6:11am.
Here, it was only committed yesterday:
http://ftp.drupal.org/files/projects/content_access-7.x-1.x-dev.tar.gz
Comment #180
nitrospectide CreditAttribution: nitrospectide commentedScott J: I saw that as soon as I was linked to this thread. My point was that there is no evidence of it on the actual project page - down where recommended/development releases show up. That, combined with no mentions in the description made it look like this module had stalled out.
Comment #181
AdrianC-1 CreditAttribution: AdrianC-1 commentedAgreed - let's get it on the project page, if the dev version's mature.
(Renamed for thread tracking)
Comment #182
good_man CreditAttribution: good_man commented@fago, the end thing in test, is because in D7 test it's default to give a user an auth. role + the new passed role when created, so the new passed role as param will go first in array, and then goes the auth. role, so when using end() you'll get the auth. role not the new one, anyhow it's hack y way, that's why I've commented it instead of removing, we should find better way to do so.
Comment #183
fagohm, in my tests end gave me the right role, but anyway I agree that we should find a more reliable way instead.
The dev-snapshot appears on the project page now.
Comment #184
good_man CreditAttribution: good_man commentedok will close this issue now and open #1058526: Rules 2.x integration, don't reopen this issue if you find a bug instead create a new issue.