ENHANCEMENT: Groups can now be associated to multiple subsites. (from r89791)

code/GroupSubsites.php

@@ -11,39 +11,80 @@ class GroupSubsites extends DataObjectDecorator implements PermissionProvider {
 			if($this->owner->class != 'Group') return null;
 		}
 		return array(
-			'has_one' => array(
+			'db' => array(
-				'Subsite' => 'Subsite',
+				'AccessAllSubsites' => 'Boolean',
+			),
+			'many_many' => array(
+				'Subsites' => 'Subsite',
+			),
+			'defaults' => array(
+				'AccessAllSubsites' => 1,
 			),
 		);
 	}
 	
-	function updateCMSFields(&$fields) {
-		if($this->owner->canEdit() ){
-			$subsites = Subsite::accessible_sites(array('ADMIN', 'SECURITY_SUBSITE_GROUP'), true,
-				"All sites");
-			$subsiteMap = $subsites->toDropdownMap();
 	
-			$tab = $fields->findOrMakeTab(
+	/**
-				'Root.Subsites',
+	 * Migrations for GroupSubsites data.
-				_t('GroupSubsites.SECURITYTABTITLE', 'Subsites')
+	 */
-			);
+	function requireDefaultRecords() {
+		// Migration for Group.SubsiteID data from when Groups only had a single subsite
+		$groupFields = DB::getConn()->fieldList('Group');
 		
-			// This will trick the $dropdown code below to displaying the correct human val,
+		// Detection of SubsiteID field is the trigger for old-style-subsiteID migration
-			// readonly
+		if(isset($groupFields['SubsiteID'])) {
-			if(!isset($subsiteMap[$this->owner->SubsiteID])) {
+			// Migrate subsite-specific data
-				if($this->owner->SubsiteID) $subsiteTitle = $this->owner->Subsite()->Title;
+		DB::query('INSERT INTO "Group_Subsites" ("GroupID", "SubsiteID")
-				else $subsiteTitle = "All sites";
+				SELECT "ID", "SubsiteID" FROM "Group" WHERE "SubsiteID" > 0');
-				$subsiteMap = array($this->owner->SubsiteID => $subsiteTitle);
+				
+			// Migrate global-access data
+			DB::query('UPDATE "Group" SET "AccessAllSubsites" = 1 WHERE "SubsiteID" = 0');
+			
+			// Move the field out of the way so that this migration doesn't get executed again
+			DB::getConn()->renameField('Group', 'SubsiteID', '_obsolete_SubsiteID');
+			
+		// No subsite access on anything means that we've just installed the subsites module.
+		// Make all previous groups global-access groups
+		} else if(!DB::query('SELECT "ID" FROM "Group" WHERE "AccessAllSubsites" = 1
+			OR "Group_Subsites"."GroupID" IS NOT NULL 
+			LEFT JOIN "Group_Subsites" ON "Group_Subsites"."GroupID" = "Group"."ID"
+			AND "Group_Subsites"."SubsiteID" > 0')->value()) {
+			
+			DB::query('UPDATE "Group" SET "AccessAllSubsites" = 1');
+		}
 	}
 	
-			$dropdown = new DropdownField(
+	function updateCMSFields(&$fields) {
-				'SubsiteID',
+		if($this->owner->canEdit() ){
-				_t('GroupSubsites.SECURITYACCESS', 'Limit CMS access to subsites', PR_MEDIUM, 'Dropdown listing existing subsites which this group has access to'),
+			// i18n tab
-				$subsiteMap
+			$fields->findOrMakeTab('Root.Subsites',_t('GroupSubsites.SECURITYTABTITLE','Subsites'));
-			);
 
+			$subsites = Subsite::accessible_sites(array('ADMIN', 'SECURITY_SUBSITE_GROUP'), true);
+			$subsiteMap = $subsites->toDropdownMap();
+			
+			// Interface is different if you have the rights to modify subsite group values on
+			// all subsites
+			if(isset($subsiteMap[0])) {
+				$fields->addFieldToTab("Root.Subsites", new OptionsetField("AccessAllSubsites", 
+					_t('GroupSubsites.ACCESSRADIOTITLE', 'Give this group access to'),
+					array(
+						1 => _t('GroupSubsites.ACCESSALL', "All subsites"),
+						0 => _t('GroupSubsites.ACCESSONLY', "Only these subsites"),
+					)
+				));
+
+				unset($subsiteMap[0]);
+				$fields->addFieldToTab("Root.Subsites", new CheckboxSetField("Subsites", "",
+					$subsiteMap));
+
+			} else {
 				if (sizeof($subsiteMap) <= 1) $dropdown = $dropdown->transform(new ReadonlyTransformation()) ;
 				$tab->push($dropdown);
+
+				$fields->addFieldToTab("Root.Subsites", new CheckboxSetField("Subsites", 
+					_t('GroupSubsites.ACCESSRADIOTITLE', 'Give this group access to'),
+					$subsiteMap));
+			}
 		}
 	}
 
@@ -54,10 +95,11 @@ class GroupSubsites extends DataObjectDecorator implements PermissionProvider {
 	 * of the security admin interface.
 	 */
 	function alternateTreeTitle() {
-		if($this->owner->SubsiteID == 0) {
+		if($this->owner->AccessAllSubsites) {
 			return $this->owner->Title . ' <i>(global group)</i>';
 		} else {
-			return $this->owner->Title; //. ' <i>(' . $this->owner->Subsite()->Title . ')</i>';
+			$subsites = Convert::raw2xml(implode(", ", $this->owner->Subsites()->column('Title')));
+			return $this->owner->Title . " <i>($subsites)</i>";
 		}
 	}
 
@@ -68,13 +110,11 @@ class GroupSubsites extends DataObjectDecorator implements PermissionProvider {
 		if(Subsite::$disable_subsite_filter) return;
 		if(Cookie::get('noSubsiteFilter') == 'true') return;
 
-		if(defined('DB::USE_ANSI_SQL')) 
+
-			$q="\"";
+		$q = defined('Database::USE_ANSI_SQL') ? "\"" : "`";
-		else $q='`';
 
 		// If you're querying by ID, ignore the sub-site - this is a bit ugly...
-		if(!$query->where || (strpos($query->where[0], ".{$q}ID{$q} = ") === false && strpos($query->where[0], ".{$q}ID{$q} = ") === false && strpos($query->where[0], ".{$q}ID{$q} = ") === false)) {
+		if(!$query->filtersOnID()) {
-
 			if($context = DataObject::context_obj()) $subsiteID = (int) $context->SubsiteID;
 			else $subsiteID = (int) Subsite::currentSubsiteID();
 
@@ -84,22 +124,53 @@ class GroupSubsites extends DataObjectDecorator implements PermissionProvider {
 				$query->where[] = $where;
 				break;
 			}
+			
+			// Don't filter by Group_Subsites if we've already done that
+			$hasGroupSubsites = false;
+			foreach($query->from as $item) if(strpos($item, 'Group_Subsites') !== false) {
+				$hasGroupSubsites = true;
+				break;
+			}
+	
+			if(!$hasGroupSubsites) {
+				if($subsiteID) {
+					$query->leftJoin("Group_Subsites", "{$q}Group_Subsites{$q}.{$q}GroupID{$q} 
+						= {$q}Group{$q}.{$q}ID{$q} AND {$q}Group_Subsites{$q}.{$q}SubsiteID{$q} = $subsiteID");
+					$query->where[] = "({$q}Group_Subsites{$q}.{$q}SubsiteID{$q} IS NOT NULL OR
+						{$q}Group{$q}.{$q}AccessAllSubsites{$q} = 1)";
+				} else {
+					$query->where[] = "{$q}Group{$q}.{$q}AccessAllSubsites{$q} = 1";
+				}
+			}
+			$query->orderby = "{$q}AccessAllSubsites{$q} DESC" . ($query->orderby ? ', ' : '') . $query->orderby;
 		}
 	}
 
-	function augmentBeforeWrite() {
+	function onBeforeWrite() {
-		if((!is_numeric($this->owner->ID) || !$this->owner->ID) && !$this->owner->SubsiteID) $this->owner->SubsiteID = Subsite::currentSubsiteID();
+		// New record test approximated by checking whether the ID has changed.
+		// Note also that the after write test is only used when we're *not* on a subsite
+		if($this->owner->isChanged('ID') && !Subsite::currentSubsiteID()) {
+			$this->owner->AccessAllSubsites = 1;
+		}
+	}
+	
+	function onAfterWrite() {
+		// New record test approximated by checking whether the ID has changed.
+		// Note also that the after write test is only used when we're on a subsite
+		if($this->owner->isChanged('ID') && $currentSubsiteID = Subsite::currentSubsiteID()) {
+			$subsites = $this->owner->Subsites();
+			$subsites->add($currentSubsiteID);
+		}
 	}
 
 	function alternateCanEdit() {
-		// Check the CMS_ACCESS_SecurityAdmin privileges on the subsite that owns this group
+		// Find the sites that this group belongs to and the sites where we have appropriate perm.
-		$oldSubsiteID = Session::get('SubsiteID');
+		$accessibleSites = Subsite::accessible_sites('CMS_ACCESS_SecurityAdmin')->column('ID');
+		$linkedSites = $this->owner->Subsites()->column('ID');
  
-		Subsite::changeSubsite($this->owner->SubsiteID) ;
+		// We are allowed to access this site if at we have CMS_ACCESS_SecurityAdmin permission on
-		$access = Permission::check('CMS_ACCESS_SecurityAdmin');
+		// at least one of the sites
-		Subsite::changeSubsite($oldSubsiteID) ;
+		return (bool)array_intersect($accessibleSites, $linkedSites);
-
-		return $access;
 	}
 
 	/**
@@ -117,10 +188,10 @@ class GroupSubsites extends DataObjectDecorator implements PermissionProvider {
 
 		$group = $this->owner->duplicate(false);
 
-		$subsiteID = ($subsiteID ? $subsiteID : Subsite::currentSubsiteID());
-		$group->SubsiteID = $subsiteID;
 		$group->write();
 
+		$subsite->Groups()->add($group->ID);
+
 		// Duplicate permissions
 		$permissions = $this->owner->Permissions();
 		foreach($permissions as $permission) {

code/LeftAndMainSubsites.php

@@ -13,20 +13,6 @@ class LeftAndMainSubsites extends Extension {
 		Requirements::css('subsites/css/LeftAndMain_Subsites.css');
 		Requirements::javascript('subsites/javascript/LeftAndMain_Subsites.js');
 		Requirements::javascript('subsites/javascript/VirtualPage_Subsites.js');
-		
-		// Switch to the subsite of the current page
-		if ($this->owner->class == 'CMSMain' && $currentPage = $this->owner->currentPage()) {
-			if (Subsite::currentSubsiteID() != $currentPage->SubsiteID) {
-				Subsite::changeSubsite($currentPage->SubsiteID);
-			}
-		}
-		
-		// Switch to a subsite that this user can actually access.
-		$sites = Subsite::accessible_sites("CMS_ACCESS_{$this->owner->class}")->toDropdownMap();
-		if($sites && !isset($sites[Subsite::currentSubsiteID()])) {
-			$siteIDs = array_keys($sites);
-			Subsite::changeSubsite($siteIDs[0]);
-		}
 	}
 	
 	/**
@@ -131,16 +117,24 @@ class LeftAndMainSubsites extends Extension {
 	public function alternateAccessCheck() {
 		$className = $this->owner->class;
 
-		if($result = Permission::check("CMS_ACCESS_$className")) {
+		
-			return $result;
+		// Switch to the subsite of the current page
-		} else {
+		if ($this->owner->class == 'CMSMain' && $currentPage = $this->owner->currentPage()) {
-			$otherSites = Subsite::accessible_sites("CMS_ACCESS_$className");
+			if (Subsite::currentSubsiteID() != $currentPage->SubsiteID) {
-			if($otherSites && $otherSites->TotalItems() > 0) {
+				Subsite::changeSubsite($currentPage->SubsiteID);
-				$otherSites->First()->activate();
-				return Permission::check("CMS_ACCESS_$className");
 			}
 		}
 		
+		// Switch to a subsite that this user can actually access.
+		$sites = Subsite::accessible_sites("CMS_ACCESS_{$this->owner->class}")->toDropdownMap();
+		if($sites && !isset($sites[Subsite::currentSubsiteID()])) {
+			$siteIDs = array_keys($sites);
+			Subsite::changeSubsite($siteIDs[0]);
+			return true;
+		}
+		
+		if(!$sites) return null;
+		
 		return null;
 	}
 	

code/SiteTreeSubsites.php

@@ -232,16 +232,11 @@ class SiteTreeSubsites extends SiteTreeDecorator {
 	function canEdit($member = null) {
 		if(!$member)  $member = Member::currentUser();
 		
-		// Check the CMS_ACCESS_CMSMain privileges on the subsite that owns this group
+		// Find the sites that this user has access to
-		$oldSubsiteID = Session::get('SubsiteID');
+		$goodSites = Subsite::accessible_sites('CMS_ACCESS_CMSMain',true,'all',$member)->column('ID');
 		
-		Subsite::changeSubsite($this->owner->SubsiteID) ;
+		// Return true if they have access to this object's site
-		$access = Permission::checkMember($member, 'CMS_ACCESS_CMSMain');
+		return in_array(0, $goodSites) || in_array($this->owner->SubsiteID, $goodSites);
-		Subsite::changeSubsite($oldSubsiteID);
-		
-		if(!$access) $access = Permission::checkMember($member, 'SUBSITE_ACCESS_ALL');
-		
-		return $access;
 	}
 	
 	/**

code/Subsite.php

@@ -35,6 +35,10 @@ class Subsite extends DataObject implements PermissionProvider {
 		'Domains' => 'SubsiteDomain',
 	);
 	
+	static $belongs_many_many = array(
+		"Groups" => "Group",
+	);
+
 	static $defaults = array(
 		'IsPublic' => 1,
 	);
@@ -353,9 +357,11 @@ JS;
 		$groupCount = DB::query("
 			SELECT COUNT({$q}Permission{$q}.{$q}ID{$q})
 			FROM {$q}Permission{$q}
-			INNER JOIN {$q}Group{$q} ON {$q}Group{$q}.{$q}ID{$q} = {$q}Permission{$q}.{$q}GroupID{$q} AND {$q}Group{$q}.{$q}SubsiteID{$q} = 0
+			INNER JOIN {$q}Group{$q} ON {$q}Group{$q}.{$q}ID{$q} = {$q}Permission{$q}.{$q}GroupID{$q} AND {$q}Group{$q}.{$q}AccessAllSubsites{$q} = 1
 			INNER JOIN {$q}Group_Members{$q} USING({$q}GroupID{$q})
-			WHERE {$q}Permission{$q}.{$q}Code{$q} IN ('$SQL_perms') AND {$q}MemberID{$q} = {$memberID}
+			WHERE
+			{$q}Permission{$q}.{$q}Code{$q} IN ('$SQL_perms')
+			AND {$q}MemberID{$q} = {$memberID}
 		")->value();
 			
 		return ($groupCount > 0);
@@ -420,14 +426,18 @@ JS;
 	 * @param $includeMainSite If true, the main site will be included if appropriate.
 	 * @param $mainSiteTitle The label to give to the main site
 	 */
-	function accessible_sites($permCode, $includeMainSite = false, $mainSiteTitle = "Main site") {
+	function accessible_sites($permCode, $includeMainSite = false, $mainSiteTitle = "Main site", $member = null) {
-		$member = Member::currentUser();
+		// For 2.3 and 2.4 compatibility
+		$q = defined('Database::USE_ANSI_SQL') ? "\"" : "`";
+
+		// Rationalise member arguments
+		if(!$member) $member = Member::currentUser();
+		if(!$member) return new DataObjectSet();
+		if(!is_object($member)) $member = DataObject::get_by_id('Member', $member);
 
 		if(is_array($permCode))	$SQL_codes = "'" . implode("', '", Convert::raw2sql($permCode)) . "'";
 		else $SQL_codes = "'" . Convert::raw2sql($permCode) . "'";
 		
-		if(!$member) return new DataObjectSet();
-		
 		$templateClassList = "'" . implode("', '", ClassInfo::subclassesFor("Subsite_Template")) . "'";
 
 		if(defined('DB::USE_ANSI_SQL')) 
@@ -436,26 +446,38 @@ JS;
 		
 		return DataObject::get(
 			'Subsite',
-			"{$q}Group_Members{$q}.{$q}MemberID{$q} = $member->ID
+			"{$q}Subsite{$q}.{$q}Title{$q} != ''",
-			AND {$q}Permission{$q}.{$q}Code{$q} IN ($SQL_codes, 'ADMIN')
-			AND {$q}Subsite{$q}.{$q}Title{$q} != ''",
 			'',
-			"LEFT JOIN {$q}Group{$q} ON ({$q}SubsiteID{$q}={$q}Subsite{$q}.{$q}ID{$q} OR {$q}SubsiteID{$q} = 0)
+			"LEFT JOIN {$q}Group_Subsites{$q} 
-			LEFT JOIN {$q}Group_Members{$q} ON {$q}Group_Members{$q}.{$q}GroupID{$q}={$q}Group{$q}.{$q}ID{$q}
+				ON {$q}Group_Subsites{$q}.{$q}SubsiteID{$q} = {$q}Subsite{$q}.{$q}ID{$q}
-			LEFT JOIN {$q}Permission{$q} ON {$q}Group{$q}.{$q}ID{$q}={$q}Permission{$q}.{$q}GroupID{$q}"
+			INNER JOIN {$q}Group{$q} ON {$q}Group{$q}.{$q}ID{$q} = {$q}Group_Subsites{$q}.{$q}GroupID{$q}
+				OR {$q}Group{$q}.{$q}AccessAllSubsites{$q} = 1
+			INNER JOIN {$q}Group_Members{$q} 
+				ON {$q}Group_Members{$q}.{$q}GroupID{$q}={$q}Group{$q}.{$q}ID{$q}
+				AND {$q}Group_Members{$q}.{$q}MemberID{$q} = $member->ID
+			INNER JOIN {$q}Permission{$q} 
+				ON {$q}Group{$q}.{$q}ID{$q}={$q}Permission{$q}.{$q}GroupID{$q}
+				AND {$q}Permission{$q}.{$q}Code{$q} IN ($SQL_codes, 'ADMIN')"
 		);
 
 		$rolesSubsites = DataObject::get(
 			'Subsite',
-			"{$q}Group_Members{$q}.{$q}MemberID{$q} = $member->ID
+			"{$q}Subsite{$q}.Title != ''",
-			AND {$q}PermissionRoleCode{$q}.{$q}Code{$q} IN ($SQL_codes, 'ADMIN')
-			AND {$q}Subsite{$q}.Title != ''",
 			'',
-			"LEFT JOIN {$q}Group{$q} ON ({$q}SubsiteID{$q}={$q}Subsite{$q}.{$q}ID{$q} OR {$q}SubsiteID{$q} = 0)
+			"LEFT JOIN {$q}Group_Subsites{$q} 
-			LEFT JOIN {$q}Group_Members{$q} ON {$q}Group_Members{$q}.{$q}GroupID{$q}={$q}Group{$q}.{$q}ID{$q}
+				ON {$q}Group_Subsites{$q}.{$q}SubsiteID{$q} = {$q}Subsite{$q}.{$q}ID{$q}
-			LEFT JOIN {$q}Group_Roles{$q} ON {$q}Group_Roles{$q}.{$q}GroupID{$q}={$q}Group{$q}.{$q}ID{$q}
+			INNER JOIN {$q}Group{$q} ON {$q}Group{$q}.{$q}ID{$q} = {$q}Group_Subsites{$q}.{$q}GroupID{$q}
-			LEFT JOIN {$q}PermissionRole{$q} ON {$q}Group_Roles{$q}.{$q}PermissionRoleID{$q}={$q}PermissionRole{$q}.{$q}ID{$q}
+				OR {$q}Group{$q}.{$q}AccessAllSubsites{$q} = 1
-			LEFT JOIN {$q}PermissionRoleCode{$q} ON {$q}PermissionRole{$q}.{$q}ID{$q}={$q}PermissionRoleCode{$q}.{$q}RoleID{$q}"
+			INNER JOIN {$q}Group_Members{$q} 
+				ON {$q}Group_Members{$q}.{$q}GroupID$q={$q}Group{$q}.{$q}ID{$q}
+				AND {$q}Group_Members{$q}.{$q}MemberID{$q} = $member->ID
+			INNER JOIN {$q}Group_Roles{$q}
+				ON {$q}Group_Roles{$q}.{$q}GroupID{$q}={$q}Group{$q}.{$q}ID{$q}
+			INNER JOIN {$q}PermissionRole{$q}
+				ON {$q}Group_Roles{$q}.{$q}PermissionRoleID{$q}={$q}PermissionRole{$q}.{$q}ID{$q}
+			INNER JOIN {$q}PermissionRoleCode{$q}
+				ON {$q}PermissionRole{$q}.{$q}ID{$q}={$q}PermissionRoleCode{$q}.{$q}RoleID{$q}
+				AND {$q}PermissionRoleCode{$q}.{$q}Code{$q} IN ($SQL_codes, 'ADMIN')"
 		);
 
 		if(!$subsites && $rolesSubsites) return $rolesSubsites;
@@ -603,7 +625,8 @@ class Subsite_Template extends Subsite {
 		 * Copy groups from the template to the given subsites.  Each of the groups will be created and left
 		 * empty.
 		 */
-		$groups = DataObject::get("Group", "{$q}SubsiteID{$q} = '$this->ID'");
+
+		$groups = $this->Groups();
 		if($groups) foreach($groups as $group) {
 			$group->duplicateToSubsite($intranet);
 		}

javascript/LeftAndMain_Subsites.js

@@ -59,6 +59,22 @@ Behaviour.register({
 			
 			return false;
 		}
+	},
+	
+	// Subsite tab of Group editor
+	'#Form_EditForm_AccessAllSubsites' : {
+		initialize: function () {
+			this.showHideSubsiteList();
+			var i=0,items=this.getElementsByTagName('input');
+			for(i=0;i<items.length;i++) {
+				items[i].onchange = this.showHideSubsiteList;
+			}
+		},
+		
+		showHideSubsiteList : function () {
+			$('Form_EditForm_Subsites').parentNode.style.display = 
+				Form.Element.getValue($('Form_EditForm').AccessAllSubsites)==1 ? 'none' : '';
+		}
 	}
 });
 

tests/SubsiteTest.php

@@ -138,6 +138,30 @@ class SubsiteTest extends SapphireTest {
 		
 	}
 	
+	/**
+	 * Test Subsite::accessible_sites()
+	 */
+	function testAccessibleSites() {
+		$member1Sites = Subsite::accessible_sites("CMS_ACCESS_CMSMain", false, null, 
+			$this->objFromFixture('Member', 'subsite1member'));
+		$member1SiteTitles = $member1Sites->column("Title");
+		sort($member1SiteTitles);
+		$this->assertEquals(array('Subsite1 Template'), $member1SiteTitles);
+
+		$adminSites = Subsite::accessible_sites("CMS_ACCESS_CMSMain", false, null, 
+			$this->objFromFixture('Member', 'admin'));
+		$adminSiteTitles = $adminSites->column("Title");
+		sort($adminSiteTitles);
+		$this->assertEquals(array(
+			'Subsite1 Template',
+			'Subsite2 Template',
+			'Template',
+			'Test 1',
+			'Test 2',
+			'Test 3',
+		), $adminSiteTitles);
+	}
+
 	/**
 	 * Publish a change on a master page of a newly created sub-site, and verify that the change has been propagated.
 	 * Verify that if CustomContent is set, then the changes aren't propagated.

tests/SubsiteTest.yml

@@ -72,14 +72,17 @@ Group:
    admin:
       Title: Admin
       Code: admin
+      AccessAllSubsites: 1
    subsite1_group:
       Title: subsite1_group
       Code: subsite1_group
-      SubsiteID: =>Subsite_Template.subsite1
+      AccessAllSubsites: 0
+      Subsites: =>Subsite_Template.subsite1
    subsite2_group:
       Title: subsite2_group
       Code: subsite2_group
-      SubsiteID: =>Subsite_Template.subsite2
+      AccessAllSubsites: 0
+      Subsites: =>Subsite_Template.subsite2
 Permission:
    admin:
       Code: ADMIN