Enable 'access comments' permission for anonymous users by default.

Sounds good to me. This is usually step 4 on any Drupal site I ever build.

Hmm... just noticed the Novice tag and think I read about leaving these jobs for beginners. I already patched and tested it, so I suppose it's still better to post up the code. Hopefully a novice could still review it and appreciate it. : P

fwiw, I never liked the fact that there wasn't an API function to add a permission to a role. Would that be a considerable separate patch?

This patch just adds the appropriate permission in system.install. I tested on the latest unstable release, both available profiles, and it obviously sets the permission fine. ; )

Hmm... curious... any way to see what test failed? My local test site keeps failing w/ a 500 error on "/drupal7/batch?id=3&op=do".

The test is failing because of the new comment block test that I wrote (CommentBlockFunctionalTest in comment.test), which expects that an anonymous user by default does not have access to view comments.

    // Test that a user without the 'access comments' permission can not see the block.
    $this->drupalLogout();
    $this->drupalGet('');
    $this->assertNoText($block['title'], t('Block was not found.'));

    $this->drupalLogin($this->web_user);
    $this->drupalGet('');
    $this->assertText($block['title'], t('Block was found.'));

What will work best is if you create a new user in the test that does not have the access comments permission and instead of using the anonymous user, login with the new user:

    $no_comments_user = $this->drupalCreateUser(array());
    $this->drupalLogin($no_comments_user);

BTW, the role/permission APIs are in #300993: User roles and permissions API which also moves this default permissions to default.profile so I'll need to keep an eye on this patch in case it lands before 300993.

The above patch is dead now. The code has been refactored.

The issue is still valid: http://d7p2.dev/admin/config/people/permissions/1

Rerolling patch...

- Edited both default install profiles to include "access comments" for anon user.
- Verified "comment block" test above still fails.
- Then fixed it with Dave's suggestion :-)

TO TEST PATCH:

1) download d7 from CVS.
2) FIRST apply patch (patch is to install code!)
3) THEN run install.php
4) Check permissions here: http://d7p2.dev/admin/config/people/permissions/1 Both anon and auth should have access comments. Admin doesn't need (is auth)

Hurray for new install profiles!

Let's try again. Fixed the test.

What happened? createDrupalUser(array()) does not return a user with empty permissions. It returns a user with a user with authorized user default permissions (which include "access comments").

There isn't a practical way to test the absence of the block. General block access control is outside the appropriate coverage for comments.test. So I removed the failing bit from the test. Left in the test for presence of the block.

See testing notes above. Moving on!

Looks good and access comments is enabled by default for anon. Tested with the default install profile. not with the expert profile. But the changes are identical, so should work there too.

This change makes a lot of sense. But I'd really rather not lose that test for access denied in the process.

Can we not test to make sure that they can access the comment, then call http://api.drupal.org/api/function/user_role_revoke_permissions/7 to revoke the permission and ensure that they're successfully blocked?

Rerolled patch from #11 with the following alterations:
- Only added 'access comments' permission to anonymous user in standard.install, leaving minimal.install as is;
- As per #14, preserved the access denied test with user_role_revoke_permissions().

I think this should make the test pass.

test works, code looks good

Also reviewed and agree it looks good.

oops... setting back to reviewed...

Hm, note that if we do this, we make the standard Drupal installation pretty "human-spammer friendly" (see discussion at #698144: D7: Fresh install allows visitors to create accounts). We already let anyone come along, get a free account and add comments/upload pictures - but with this patch we'd then be letting the rest of the world see whatever they uploaded, which is a big difference...

I wonder if we should consider accompanying this with taking away the 'post comments without approval' permission from authenticated users? This is probably what a lot of sites want anyway if they aren't going to have spam protection (which Drupal core doesn't have built in)... Or is that being too paranoid?

Alright, well let's try it and see what happens. I'm guessing it will break some tests.

chicago camp sprint, patch coming

summary:
patch:
1) adds access comments permission for anonymous users by default in standard install,
2) removes post comments without approval to authenticated users

#14 keep test for access denied (view comments?)
#15 only added access comments permission to anonymous in standard.install (not minimal.install)

does that need to be documented somewhere (diff between standard and minimal doc)?
and a changelog? for removing the post comments without approval permission from authenticated users?

#23: anon-access-comments-364159-23.patch queued for re-testing.

my test results on my local machine with the patch did not match the test bot... retesting.

day over for now, if someone wants to try to make a patch... when the testbot reports back. :)

Re-uploading the patch from #17.

My comment in #21 is no longer valid, since in the interim #174972: User creation setting should be "Visitors, with admin approval" was committed. This means spammers won't be able to get through in the default install (because the administrator has to approve their account first).

So, this should be RTBC again if the tests pass.

Re #26:

does that need to be documented somewhere (diff between standard and minimal doc)?

The standard and minimal profiles were already different in this regard. I don't think there is any documentation of these kinds of differences, just the general idea that the minimal profile has very little in it (e.g., it doesn't even have the comment module enabled).

Tests passed, so back to RTBC.

Totally in for this

Committed to CVS HEAD. Thanks.