Hi !
while setting up Nodereview in our site, we discovered that the "Reviews" tab was showing for every content type (instead of only those configured in admin/config/nodereview). As we started searching for the cause of this behaviour we found out that the path node/%/reviews in hook_menu is also used in the view review_list (we don't know if this clash causes other problems but it didn't look good). The problem is that the function used to filter permissions for the Reviews tab ( read_reviews_access($arg) ) is called from the hook's access callback, but not from the view access validator (so it's never called and the tab is shown everywhere).
Since we are already using the module views_php in our site, we fixed it by adding php code in the access settings of the view. We used
return read_reviews_access(arg(1)); . We can confirm this works as expected, but as a solution, it would add another module dependency to nodereview.
Maybe a solution that doesn't involve views_php would be to use different paths for the hook and the view, and/or just embedding the view from the hook's callback (once the access function has been called).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

gargsuchi’s picture

This problem exists in the 6.x version as well.

agupta’s picture

The view should be disabled by default. Attaching patch.

agupta’s picture

h3rj4n’s picture

Status: Active » Needs review
FileSize
2.99 KB

I've fixed this by creating a custom access plugin for the view. The plugin is provided by the module and default selected when you apply the patch. The tab wont be showing up anymore if the review functionality isn't selected for that content type.

jay.lee.bio’s picture

Status: Needs review » Reviewed & tested by the community

#4 worked! :D

h3rj4n’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
3.82 KB

It doesn't work quite yet. The review tab wasn't visible for content types that should have them. Changed this in the following patch.

Problem was in the callback from the views plugin. It contained the wrong function name. After changing this the value the view provides is wrong. Changed it to NULL and get the value from the URL in the access callback.

jay.lee.bio’s picture

Status: Needs review » Reviewed & tested by the community

Yeah sorry, #4 made the "Reviews" tab disappear from everywhere. #5 works perfectly though, and this time I double-checked! :D

h3rj4n’s picture

I wasn't able to apply this patch on the current HEAD so I recreated it.

h3rj4n’s picture

Forgot a file in the patch.

h3rj4n’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
2.05 KB
5.79 KB

Found some code in the contextual filter of the view that's not needed anymore. This is handled by the access views plugin created for this module.

Somebody should check this again see that I'm right ;)

h3rj4n’s picture

Status: Needs review » Reviewed & tested by the community

Ignore comment #10, it's wrong!

Patch #9 is the way to go!

yaworsk’s picture

Status: Reviewed & tested by the community » Needs review

I was looking at the patch and I'm not sure we need it... it seems like the easiest way to do this is the validate the argument by checking the content type. If the content types validate to those which have node reviews, i think everything works.

If that's the case, we just need to change the default view and i would think we could use some logic to check which content types are enabled for reviews and pass that in.