From 105e9356cd1664b352a00b9a695240a90fe4e0ab Mon Sep 17 00:00:00 2001 From: Ian Neilson Date: Tue, 27 Nov 2018 10:36:55 +0000 Subject: [PATCH 1/2] Basic scope creation and tag edits functional with tag display added to edit/success screens --- config/gocdb_schema.xml | 5 ++ config/local_info.xml | 27 ------ .../controllers/admin/edit_scope.php | 10 ++- htdocs/web_portal/controllers/scope.php | 1 + htdocs/web_portal/controllers/utils.php | 8 +- htdocs/web_portal/css/web_portal.php | 11 ++- htdocs/web_portal/views/admin/add_scope.php | 2 + htdocs/web_portal/views/admin/edit_scope.php | 3 +- .../web_portal/views/admin/edited_scope.php | 10 ++- htdocs/web_portal/views/scope.php | 4 + htdocs/web_portal/views/scopes.php | 16 +++- lib/Doctrine/deploy/AddScopes.php | 13 ++- lib/Doctrine/deploy/sampleData/Scopes.xml | 6 ++ lib/Doctrine/entities/Scope.php | 21 ++++- lib/Gocdb_Services/Scope.php | 86 ++++++++++--------- .../lib/Gocdb_Services/ScopeServiceTest.php | 11 ++- 16 files changed, 139 insertions(+), 95 deletions(-) diff --git a/config/gocdb_schema.xml b/config/gocdb_schema.xml index 18d4ec715..7b86b4c62 100644 --- a/config/gocdb_schema.xml +++ b/config/gocdb_schema.xml @@ -442,6 +442,11 @@ /^[^`'\";<>]+$/ + + RESERVED + 1 + /^[01]$/ + diff --git a/config/local_info.xml b/config/local_info.xml index d804c7652..cf9deefe6 100755 --- a/config/local_info.xml +++ b/config/local_info.xml @@ -83,33 +83,6 @@ 1 - - - wlcg - tier1 - tier2 - alice - atlas - cms - lhcb - elixir - FedCloud - - 20 diff --git a/htdocs/web_portal/controllers/admin/edit_scope.php b/htdocs/web_portal/controllers/admin/edit_scope.php index 09b46195b..df92c20ba 100644 --- a/htdocs/web_portal/controllers/admin/edit_scope.php +++ b/htdocs/web_portal/controllers/admin/edit_scope.php @@ -57,7 +57,8 @@ function draw() { $params = array('Name' => $scope->getName(), 'Id' => $scope->getId(), - 'Description' => $scope->getDescription()); + 'Description' => $scope->getDescription(), + 'Reserved' => $scope->getReserved()); //show the add service type view show_view("admin/edit_scope.php", $params, "Edit Scope"); @@ -83,12 +84,13 @@ function submit() { $user = \Factory::getUserService()->getUserByPrinciple($dn); try { - //function will through error if user does not have the correct permissions + //function will throw an error if user does not have the correct permissions $scope = $serv->editScope($scope, $values, $user); $params = array('Name' => $scope->getName(), 'ID'=> $scope->getId(), - 'Description' => $scope->getDescription()); - show_view("admin/edited_scope.php", $params, $params['Name']. " succesffully updated"); + 'Description' => $scope->getDescription(), + 'Reserved' => $scope->getReserved()); + show_view("admin/edited_scope.php", $params, $params['Name']. " successfully updated"); } catch (Exception $e) { show_view('error.php', $e->getMessage()); die(); diff --git a/htdocs/web_portal/controllers/scope.php b/htdocs/web_portal/controllers/scope.php index 6db2e1af2..15fa736b0 100644 --- a/htdocs/web_portal/controllers/scope.php +++ b/htdocs/web_portal/controllers/scope.php @@ -40,6 +40,7 @@ function view_scope() { $params['Name'] = $scope->getName(); $params['Description'] = $scope->getDescription(); $params['ID'] = $scope->getId(); + $params['Reserved'] = $scope->getReserved(); $params['NGIs'] = $serv->getNgisFromScope($scope); $params['Sites'] = $serv->getSitesFromScope($scope); $params['ServiceGroups'] = $serv->getServiceGroupsFromScope($scope); diff --git a/htdocs/web_portal/controllers/utils.php b/htdocs/web_portal/controllers/utils.php index 08e5efcf0..3a6d87ed1 100644 --- a/htdocs/web_portal/controllers/utils.php +++ b/htdocs/web_portal/controllers/utils.php @@ -113,7 +113,7 @@ function getEntityScopesAsJSON2($targetScopedEntity = null, $parentScopedEntity $parentScopes = $parentScopedEntity->getScopes()->toArray(); } - $reservedScopeNames = \Factory::getConfigService()->getReservedScopeList(); + // $reservedScopeNames = \Factory::getConfigService()->getReservedScopeList(); $allScopes = \Factory::getScopeService()->getScopes(); $optionalScopeIds = array(); $reservedOptionalScopeIds = array(); @@ -139,7 +139,7 @@ function getEntityScopesAsJSON2($targetScopedEntity = null, $parentScopedEntity } // Is scope tag in the reserved list ? - if(in_array($scope->getName(), $reservedScopeNames)){ + if($scope->getReserved()){ // A reserved scope tag: if($parentChecked || $targetChecked){ if($parentChecked){ @@ -656,6 +656,10 @@ function getDateFormat() { function getScopeDataFromWeb() { $scopeData ['Name'] = trim($_REQUEST ['Name']); $scopeData ['Description'] = trim($_REQUEST ['Description']); + // 'Reserved' value is a checkbox ==>> absent if not checked + if (array_key_exists('Reserved', $_REQUEST)){ + $scopeData ['Reserved'] = ($_REQUEST ['Reserved'] == '1'); + } if (array_key_exists('Id', $_REQUEST)){ $scopeData ['Id'] = $_REQUEST ['Id']; } diff --git a/htdocs/web_portal/css/web_portal.php b/htdocs/web_portal/css/web_portal.php index 4a68c0c3a..72ef71a99 100644 --- a/htdocs/web_portal/css/web_portal.php +++ b/htdocs/web_portal/css/web_portal.php @@ -311,14 +311,13 @@ margin-bottom: 0.3em; } -.input_input_text, -.input_input_date, -.input_input_check { - margin-left: 2em; +.input_input_checkbox, .input_input_date, .input_input_text { + margin-bottom: 1em; + margin-left: 2em !important; } -.input_input_text, -.input_input_date { + +.input_input_text, .input_input_date { width: 90%; margin-bottom: 1em; } diff --git a/htdocs/web_portal/views/admin/add_scope.php b/htdocs/web_portal/views/admin/add_scope.php index c9c1e7c2a..88fade79c 100644 --- a/htdocs/web_portal/views/admin/add_scope.php +++ b/htdocs/web_portal/views/admin/add_scope.php @@ -6,6 +6,8 @@ Description + Reserved - check to create a reserved scope + name="Reserved" class="input_input_checkbox">
diff --git a/htdocs/web_portal/views/admin/edit_scope.php b/htdocs/web_portal/views/admin/edit_scope.php index 5e397dad2..821d2684b 100644 --- a/htdocs/web_portal/views/admin/edit_scope.php +++ b/htdocs/web_portal/views/admin/edit_scope.php @@ -6,7 +6,8 @@ Description - + Reserved - check to set scope as Reserved + name="Reserved" class="input_input_checkbox">

diff --git a/htdocs/web_portal/views/admin/edited_scope.php b/htdocs/web_portal/views/admin/edited_scope.php index a14d9ec05..294bddc1e 100644 --- a/htdocs/web_portal/views/admin/edited_scope.php +++ b/htdocs/web_portal/views/admin/edited_scope.php @@ -1,14 +1,17 @@

Success


- - - has been successfully edited as follows: + + + has been successfully edited as follows:

Name:
Description: +
+ Reserved scope: +

@@ -17,4 +20,3 @@

- diff --git a/htdocs/web_portal/views/scope.php b/htdocs/web_portal/views/scope.php index a70664866..34e2ebcf5 100644 --- a/htdocs/web_portal/views/scope.php +++ b/htdocs/web_portal/views/scope.php @@ -4,6 +4,7 @@ $description = $params['Description']; $ngis = $params['NGIs']; $ngiCount = count($ngis); +$reserved = $params['Reserved']; $sites = $params['Sites']; $siteCount = count($sites); $serviceGroups = $params['ServiceGroups']; @@ -19,6 +20,9 @@

Scope:

+ + This scope is . + 0):?> In total, there are diff --git a/htdocs/web_portal/views/scopes.php b/htdocs/web_portal/views/scopes.php index 29cc01652..189979c0d 100644 --- a/htdocs/web_portal/views/scopes.php +++ b/htdocs/web_portal/views/scopes.php @@ -149,8 +149,18 @@
- getDescription()); ?> - + +
+ + getDescription()); ?> + +
+ + + getReserved() == 1):?> + + + @@ -189,4 +199,4 @@ }); }) - \ No newline at end of file + diff --git a/lib/Doctrine/deploy/AddScopes.php b/lib/Doctrine/deploy/AddScopes.php index a83b96e52..1a23abeac 100644 --- a/lib/Doctrine/deploy/AddScopes.php +++ b/lib/Doctrine/deploy/AddScopes.php @@ -13,12 +13,19 @@ foreach($scopes as $scope) { $doctrineScope = new Scope(); $name = ""; + $reserved = false; foreach($scope as $key => $value) { - if($key == "name") { - $name = (string) $value; - } + switch ($key) { + case "name": + $name = (string) $value; + break; + case "reserved": + $reserved = ( $value == 1 ); + break; + } } $doctrineScope->setName($name); + $doctrineScope->setReserved($reserved); $entityManager->persist($doctrineScope); } diff --git a/lib/Doctrine/deploy/sampleData/Scopes.xml b/lib/Doctrine/deploy/sampleData/Scopes.xml index 80b16439f..e12b35d06 100644 --- a/lib/Doctrine/deploy/sampleData/Scopes.xml +++ b/lib/Doctrine/deploy/sampleData/Scopes.xml @@ -3,8 +3,14 @@ Local + 0 EGI + 0 + + + wlcg + 1 diff --git a/lib/Doctrine/entities/Scope.php b/lib/Doctrine/entities/Scope.php index 7df30816e..a36ac7af8 100644 --- a/lib/Doctrine/entities/Scope.php +++ b/lib/Doctrine/entities/Scope.php @@ -36,6 +36,9 @@ class Scope { /** @Column(type="string", nullable=true) */ protected $description; + + /** @Column(type="boolean", options={"default": false}) */ + protected $reserved = false; /** * @return int The PK of this entity or null if not persisted @@ -59,6 +62,14 @@ public function getName() { public function getDescription() { return $this->description; } + + /** + * Get the reserved status of this scope. + * @return boolean + */ + public function getReserved() { + return $this->reserved; + } /** * Set the unique name of this Scope instance. @@ -75,7 +86,15 @@ public function setName($name) { public function setDescription($description) { $this->description = $description; } - + + /** + * Set the reserved status of this scope. + * @param boolean $reserved + */ + public function setReserved($reserved) { + $this->reserved = $reserved; + } + /** * Returns the unique name of this Scope instance. * @return string diff --git a/lib/Gocdb_Services/Scope.php b/lib/Gocdb_Services/Scope.php index 5d770d284..e3eb000c9 100644 --- a/lib/Gocdb_Services/Scope.php +++ b/lib/Gocdb_Services/Scope.php @@ -71,14 +71,13 @@ public function getScopes($scopeIdArray = NULL) { $dql = "SELECT s from Scope s ORDER BY s.name"; $query = $this->em->createQuery($dql); return $query->getResult(); - - } else if(count($scopeIdArray) > 0){ + } + if(count($scopeIdArray) > 0){ $dql = "SELECT s from Scope s WHERE s.id IN(:scopeIdArray) ORDER BY s.name"; $query = $this->em->createQuery($dql)->setParameter('scopeIdArray', $scopeIdArray); return $query->getResult(); - } else { - return array(); } + return array(); } @@ -120,12 +119,12 @@ public function getScopesFilterByParams(array $filterParams, $scopeArray) { // Check the parameter keys are supoported $supportedParams = array('excludeNonDefault', 'excludeDefault', 'excludeReserved', 'excludeNonReserved'); - $testParamKeys = array_keys($filterParams); - foreach ($testParamKeys as $key) { + $testParamKeys = array_keys($filterParams); + foreach ($testParamKeys as $key) { // if givenkey is not defined in supportedkeys it is unsupported - if (!in_array($key, $supportedParams)) { - throw new \InvalidArgumentException('Unsupported parameter key'); - } + if (!in_array($key, $supportedParams)) { + throw new \InvalidArgumentException('Unsupported parameter key'); + } } $defaultScopeName = $this->configService->getDefaultScopeName(); @@ -145,29 +144,18 @@ public function getScopesFilterByParams(array $filterParams, $scopeArray) { } } if(isset($filterParams['excludeReserved']) && $filterParams['excludeReserved'] == TRUE){ - $reservedScopes = $this->configService->getReservedScopeList(); - foreach ($allScopes as $scope) { - foreach($reservedScopes as $rs){ - if($scope->getName() == $rs){ - unset($allScopes[array_search($scope, $allScopes)]); - } - } + foreach ($allScopes as $scope) { + if($scope->getReserved()){ + unset($allScopes[array_search($scope, $allScopes)]); } + } } if(isset($filterParams['excludeNonReserved']) && $filterParams['excludeNonReserved'] == TRUE){ - $reservedScopes = $this->configService->getReservedScopeList(); - foreach ($allScopes as $scope) { - $isReserved = false; - foreach($reservedScopes as $rs){ - if($scope->getName() == $rs){ - $isReserved = true; - break; - } - } - if(!$isReserved){ - unset($allScopes[array_search($scope, $allScopes)]); - } + foreach ($allScopes as $scope) { + if(!$scope->getReserved()){ + unset($allScopes[array_search($scope, $allScopes)]); } + } } return $allScopes; } @@ -277,13 +265,13 @@ public function deleteScope(\scope $scope, \User $user= null, $inUseOveride=fals throw new Exception("This scope tag is still applied to one or more NGIs. ". $scope->getName() ."can not be deleted until these links are removed"); } if(sizeof($sites)>0){ - throw new Exception("This scope tag is still applied to one or more NGIs. ". $scope->getName() ."can not be deleted until these links are removed"); + throw new Exception("This scope tag is still applied to one or more Sites. ". $scope->getName() ."can not be deleted until these links are removed"); } if(sizeof($serviceGroups)>0){ - throw new Exception("This scope tag is still applied to one or more NGIs. ". $scope->getName() ."can not be deleted until these links are removed"); + throw new Exception("This scope tag is still applied to one or more Service Groups. ". $scope->getName() ."can not be deleted until these links are removed"); } if(sizeof($services)>0){ - throw new Exception("This scope tag is still applied to one or more NGIs. ". $scope->getName() ."can not be deleted until these links are removed"); + throw new Exception("This scope tag is still applied to one or more Services. ". $scope->getName() ."can not be deleted until these links are removed"); } } @@ -343,9 +331,8 @@ public function addScope($values, \user $user = null){ try { //new scope object $scope = new \Scope(); - //set name - $scope->setName($values['Name']); - $scope->setDescription($values['Description']); + + $this->populateScope($scope, $values); $this->em->persist($scope); $this->em->flush(); @@ -383,6 +370,8 @@ public function editScope(\Scope $scope, $newValues, \User $user = null){ //set name $scope->setName($newValues['Name']); $scope->setDescription($newValues['Description']); + + $this->populateScope($scope, $newValues); $this->em->merge($scope); $this->em->flush(); @@ -479,13 +468,23 @@ private function scopeNameIsUnique($name){ $query = $this->em->createQuery($dql); $result = $query->setParameter('name', $name)->getResult(); - if(count($result)==0){ - return true; - } - else { - return false; - } + return count($result)==0; + } + /** + * Populate the scope instance + * Note that to reserve a scope the Reserved key MUST be set in the input array + * This is to be consistent with html checkbox behaviour + */ + private function populateScope(\Scope $scope, $values) { + + $scope->setName($values['Name']); + $scope->setDescription($values['Description']); + $scope->setReserved(false); + + if (array_key_exists('Reserved', $values)) { + $scope->setReserved($values['Reserved'] == true); + } } /** @@ -523,6 +522,13 @@ private function validate($scopeData, $scopeIsNew, $oldScopeName = '') { throw new \Exception("Scope names must be unique, '" . $scopeData['Name'] . "' is already in use"); } } + + // if reserved status specified it must be TRUE or false + if (array_key_exists('Reserved', $scopeData)) { + if ($scopeData['Reserved'] != 0 and $scopeData['Reserved'] != 1){ + throw new \Exception("Scope reserved status must be true(1) or false(0), '" . $scopeData['Reserved'] . "' is invalid."); + } + } //remove the ID fromt the values file if present (which it may be for an edit) if (array_key_exists("Id", $scopeData)) { diff --git a/tests/unit/lib/Gocdb_Services/ScopeServiceTest.php b/tests/unit/lib/Gocdb_Services/ScopeServiceTest.php index 37881df8c..d810a9b55 100644 --- a/tests/unit/lib/Gocdb_Services/ScopeServiceTest.php +++ b/tests/unit/lib/Gocdb_Services/ScopeServiceTest.php @@ -138,9 +138,12 @@ private function insertTestScopes() { $scopeCount = 10; $scopes = array(); - // create scopes and persist - for ($i = 0; $i < $scopeCount; $i++) { - $scopes[] = TestUtil::createSampleScope("scope " . $i, "Scope" . $i); + // create scopes and persist + for($i=0; $i<$scopeCount; $i++){ + $scopes[] = TestUtil::createSampleScope("scope ".$i, "Scope".$i); + if($i <= 3){ + $scopes[$i]->setReserved(TRUE); + } $this->em->persist($scopes[$i]); } } @@ -178,7 +181,7 @@ public function testGetScopesFilterByParams1() // exclude all the 'Reserved' scopes $filterParams = array('excludeReserved' => true); - $excludedScopeNames = $configService->getReservedScopeList();//'Scope0', 'Scope1', 'Scope2', 'Scope3' + $excludedScopeNames = array('Scope0', 'Scope1', 'Scope2', 'Scope3'); $filteredScopes = $scopeService->getScopesFilterByParams($filterParams, null); foreach ($filteredScopes as $scope) { if (in_array($scope->getName(), $excludedScopeNames)) { From bbddb16a24656035ed96aa1cdee67dc6ab8a654b Mon Sep 17 00:00:00 2001 From: Ian Neilson Date: Thu, 13 Jun 2019 16:08:30 +0100 Subject: [PATCH 2/2] Remove unused code and adjust comments Removed unused method for loading reserved scopes from local_config.xml and adjust some comments. --- lib/Gocdb_Services/Config.php | 22 ---------------------- lib/Gocdb_Services/Scope.php | 4 ++-- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/lib/Gocdb_Services/Config.php b/lib/Gocdb_Services/Config.php index 19e350b15..b1c37b375 100644 --- a/lib/Gocdb_Services/Config.php +++ b/lib/Gocdb_Services/Config.php @@ -411,28 +411,6 @@ public function getMinimumScopesRequired($entityType){ return intval($numScopesRequired); } - /** - * Get an array of 'reserved' scope strings or an empty array if non are configured. - * - *

- * Reserved scopes can only be assiged by gocdb admin (and in future by selected roles); - * they can't be freely assigned to resources by their users/owners. - * - * @return array Reserved scopes as Strings - */ - public function getReservedScopeList() { - $reservedScopes = array (); - /* @var $reserved_scopes \SimpleXMLElement */ - $reserved_scopes = $this->GetLocalInfoXML()->reserved_scopes; - if ($reserved_scopes != null) { - /* @var $scope \SimpleXMLElement */ - foreach ( $reserved_scopes->children () as $scope ) { - $reservedScopes [] = ( string ) $scope; - } - } - return $reservedScopes; - } - public function getDefaultFilterByScope () { if (strtolower($this->GetLocalInfoXML()->default_filter_by_scope) == 'true'){ diff --git a/lib/Gocdb_Services/Scope.php b/lib/Gocdb_Services/Scope.php index e3eb000c9..84eafd766 100644 --- a/lib/Gocdb_Services/Scope.php +++ b/lib/Gocdb_Services/Scope.php @@ -91,9 +91,9 @@ public function getScopes($scopeIdArray = NULL) { * default value listed in the 'local_info.xml' config file *

  • 'excludeNonDefault' => boolean (if true exclude scopes that are not default)
    • *
  • 'excludeReserved' => boolean (if true, exclude 'reserved' scopes, - * i.e. those that are listed as reserved in the the 'local_info.xml' config file
    • + * i.e. those that are flagged as reserved in the database scopes table *
  • 'excludeNonReserved' => boolean (if true, exclude 'normal' scopes, - * i.e. those that are not listed as reserved in the the 'local_info.xml' config file
    • + * i.e. those that are not flagged as reserved in the database scopes table * * * @param array $filterParams Associative array