Inconsistency between SiteService.allowAccessSite() and SiteService.getSiteVisit()
GENERAL
TESTING
GENERAL
TESTING
Description
When looking at something else I noticed that the two methods that check if the current user can access a site don't work identically. Specifically the allowAccessSite() method doesn't take account of the roleswap fix that was put in to allow someone who has switched to a role that can't access the unpublished site to still see the site so they can preview it. getSiteVisit() does work allow this.
The logic for checking if a user can access a site is duplicated which one throwing an exception in one place and return true/false in the other.
If this isn't a bug and is the correct behaviour it should be clearly documented as at the moment there isn't any mention of it:
allowAccessSite: * check permissions for accessing (i.e. visiting) a site getSiteVisit: * Access a site object for purposes of having the user visit the site - visitation permissions are in effect.
Integrated in kernel-trunk #314 (See http://builds.sakaiproject.org:8080/job/kernel-trunk/314/) Make the permission checks for accessing a site common across the methods that use them. This fixes the underlying bug of allowAccess and getSiteVisit returning different results when roleswap on an unpublished site was in effect. It also fixes the inconsistency for deleted sites between allowAccess() and getSiteVisit().
Matthew Buckett May 25, 2011 at 1:17 PM
Fixed by re-working patches(due to soft delete) and applying.
Matthew Buckett March 22, 2011 at 3:40 AM
Test which shows the bug and fix which removes the code duplication and fixes the issue.
When looking at something else I noticed that the two methods that check if the current user can access a site don't work identically.
Specifically the allowAccessSite() method doesn't take account of the roleswap fix that was put in to allow someone who has switched to a role that can't access the unpublished site to still see the site so they can preview it. getSiteVisit() does work allow this.
The logic for checking if a user can access a site is duplicated which one throwing an exception in one place and return true/false in the other.
If this isn't a bug and is the correct behaviour it should be clearly documented as at the moment there isn't any mention of it:
allowAccessSite: * check permissions for accessing (i.e. visiting) a site
getSiteVisit: * Access a site object for purposes of having the user visit the site - visitation permissions are in effect.