Merge pull request #1023 from unikent/php72

PHP 7.2 support
unikent · May 1, 2024 · 7120ae6 · 7120ae6
2 parents 223b887 + f058fff
commit 7120ae6
Show file tree

Hide file tree

Showing 12 changed files with 72 additions and 92 deletions.
diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml
@@ -16,12 +16,12 @@ jobs:
       with:
         submodules: true
 
-    - name: Setup PHP 5.4 with composer
+    - name: Setup PHP 7.2 with composer
       uses: shivammathur/setup-php@v2
       with:
-        php-version: '5.4'
+        php-version: '7.2'
         coverage: none
-        tools: composer, phpunit:v3.7
+        tools: composer, phpunit:v5.7
 
     - name: Validate composer.json and composer.lock
       run: composer validate

diff --git a/.travis.yml b/.travis.yml
diff --git a/application/models/fees.php b/application/models/fees.php
@@ -84,30 +84,34 @@ public static function get_fee_mapping($year){
 	 * generate_fee_map - Create fee mapping data & shove it in a cache
 	 * 
 	 * @param $year
-	 * @return Fee Data array
+	 * @return array | bool
+	 *					- array of fee data,
+	 *					- empty array if no fee data found
+	 *                  - true if fees unchanged
+	 * 					- false if fees config path is not configured
 	 */
 	public static function generate_fee_map($year, $cache_exists = true){
 
 		$path = Config::get('fees.path');
 		if($path == '') return false;
 
 
-        if ($year ==='preview'){
-            $path = explode('/',$path);
+		if ($year ==='preview'){
+			$path = explode('/',$path);
 
-            array_pop($path);
+			array_pop($path);
 
-            $path = implode('/',$path) . '/preview-fees';
+			$path = implode('/',$path) . '/preview-fees';
 
-            // If no cache, open up feedbands and mapping csv files for preview
-            $fees = Fees::load_csv_from_webservice("{$path}/preview-feebands.csv");
-            $courses = Fees::load_csv_from_webservice("{$path}/preview-mapping.csv");
-        }else {
-            // If no cache, open up feedbands and mapping csv files for given year
-            $fees = Fees::load_csv_from_webservice("{$path}/{$year}-feebands.csv");
-            $courses = Fees::load_csv_from_webservice("{$path}/{$year}-mapping.csv");
+			// If no cache, open up feedbands and mapping csv files for preview
+			$fees = Fees::load_csv_from_webservice("{$path}/preview-feebands.csv");
+			$courses = Fees::load_csv_from_webservice("{$path}/preview-mapping.csv");
+		}else {
+			// If no cache, open up feedbands and mapping csv files for given year
+			$fees = Fees::load_csv_from_webservice("{$path}/{$year}-feebands.csv");
+			$courses = Fees::load_csv_from_webservice("{$path}/{$year}-mapping.csv");
 
-        }
+		}
 
 		// Ensure data was found
 		if(!$fees || !$courses || empty($fees) || empty($courses)) return array();
@@ -151,14 +155,14 @@ public static function generate_fee_map($year, $cache_exists = true){
 
 		// Flush output caches, so new data is reflected
 		try
-        {
+		{
 			API::purge_fees_cache($year);
-            API::purge_output_cache();
-        }
-        catch(Exception $e)
-        {
+			API::purge_output_cache();
+		}
+		catch(Exception $e)
+		{
 
-        }
+		}
 
 		// return data
 		return $mapping;

diff --git a/application/models/programme.php b/application/models/programme.php
@@ -288,7 +288,7 @@ public static function generate_api_programme($iid, $year, $revision = false)
 		}
 
 		// Return false if there is no live revision
-		if(sizeof($revision) === 0 || $revision === null){
+		if($revision === null){
 			return false;
 		}
 

diff --git a/application/models/revisionable.php b/application/models/revisionable.php
@@ -506,7 +506,7 @@ public static function generate_api_data($year = false, $revision = false){
 		} 
 
 		// Return false if there is no live revision
-		if(sizeof($revision) === 0 || $revision === null){
+		if($revision === null){
 			return false;
 		} 
 

diff --git a/application/tasks/sitsimport.php b/application/tasks/sitsimport.php
@@ -266,7 +266,7 @@ public function getProgramme($course, $level, $processYears = null)
 		}
 
 		$model = $level === "ug" ? "UG_Programme" : "PG_Programme";
-		$courseID = substr($course->progID, 0, count($course->progID) - 3);
+		$courseID = substr($course->progID, 0, strlen((string) $course->progID) - 3);
 
 		return $model::where(
 			"instance_id",

diff --git a/application/tasks/update-fees.php b/application/tasks/update-fees.php
@@ -1,10 +1,10 @@
 <?php
 
-class Update_fees_Task {
-
+class Update_fees_Task
+{
     /**
      * Generate the fee data
-     * 
+     *
      * @param array  $arguments The arguments sent to the moduledata command.
      */
     public function run($arguments = array())
@@ -15,18 +15,18 @@ public function run($arguments = array())
         $years = array($year);
 
         // If the one is "current"
-        if($year == 'current'){
+        if ($year == 'current') {
 
             // If current we may have differing UG/PG years, so we may need to
             // load two years in at this point
 
             $ug_year = Setting::get_setting("ug_current_year");
             $pg_year = Setting::get_setting("pg_current_year");
 
-            if($ug_year==$pg_year){
+            if ($ug_year == $pg_year) {
                 // The same, so just load once
                 $years = array($ug_year);
-            }else{
+            } else {
                 // Load both
                 $years = array($ug_year, $pg_year);
             }
@@ -35,26 +35,30 @@ public function run($arguments = array())
         echo "\nGenerating fee mappings for: \n\n";
 
         // Regen fee data for year
-        foreach($years as $year){
+        foreach ($years as $year) {
             $state = Fees::generate_fee_map($year);
 
             echo "- {$year}";
             // report state
-            if($state === true){
-                echo " (no change)";
-            }elseif($state === true){
-               echo " (update failed)";
-            }else{
-                echo " (updated)";
+            $status = "";
+            if (is_array($state)) {
+                if (empty($state)) {
+                    $status = "no data found";
+                } else {
+                    $status = "fees updated";
+                }
+            } else {
+                if (is_bool($state)) {
+                    if ($state) {
+                        $status = "no change";
+                    } else {
+                        $status = "fee config not found";
+                    }
+                }
             }
 
-            echo " \n";
+            echo " ($status)\n";
         }
-
         echo "\nDone :)";
-
     }
-
-}
-
-
+}
diff --git a/application/tests/controllers/api.test.php b/application/tests/controllers/api.test.php
@@ -222,13 +222,12 @@ public function testget_indexReturnsJSONWithData()
         $course = $this->make_programme_live($input['id']);
 
         $response = $this->get('api@index', array($input['year'], 'undergraduate'));
-        $returned_data = json_decode($response->render());
+        $returned_data = json_decode($response->render(), true);
 
-        $returned_data = $returned_data->$input['id'];
+        $returned_programme = $returned_data[$input['id']];
 
-
-        $this->assertEquals($input['id'], $returned_data->id);
-        $this->assertEquals($input['programme_title_1'], $returned_data->name);
+        $this->assertEquals($input['id'], $returned_programme['id']);
+        $this->assertEquals($input['programme_title_1'], $returned_programme['name']);
     }
 
     public function testget_indexReturnsJSONWithSuspendedWithdrawnData()
@@ -248,12 +247,12 @@ public function testget_indexReturnsJSONWithSuspendedWithdrawnData()
         $course = $this->make_programme_live($input['id']);
 
         $response = $this->get('api@index', array($input['year'], 'undergraduate'));
-        $returned_data = json_decode($response->render());
+        $returned_data = json_decode($response->render(), true);
 
-        $returned_data = $returned_data->$input['id'];
+        $returned_programme = $returned_data[$input['id']];
 
-        $this->assertEquals($input['id'], $returned_data->id);
-        $this->assertEquals($input['programme_title_1'], $returned_data->name);
+        $this->assertEquals($input['id'], $returned_programme['id']);
+        $this->assertEquals($input['programme_title_1'], $returned_programme['name']);
     }
 
     public function testget_programmeReturns204WithNoCache()

diff --git a/application/tests/models/api.test.php b/application/tests/models/api.test.php
@@ -588,20 +588,16 @@ public function testpurge_output_cache(){
 	public function testarray_to_xml(){
 		$record = array(
 			'column_1' => 'column_1',
-			'column_2~*' => 'column_2',
-			'2' => 'item'
+			'column_2~*' => 'column_2', // test key removes special characters
+			'2' => 'item'				// test numeric keys get renamed as 'item'
 		);
 
 		$xml = API::array_to_xml($record);
-
 		$xml_object = new SimpleXMLElement($xml);
 
 		foreach ($record as $key => $value) {
 			$result = $xml_object->xpath("/response/{$value}");
-			while(list( , $node) = each($result)) {
-				$this->assertEquals($node, $value);
-				break;
-			}
+			$this->assertEquals((string) $result[0], $value);
 		}
 	}
 

diff --git a/application/tests/models/revisionable.test.php b/application/tests/models/revisionable.test.php
@@ -232,9 +232,9 @@ public function testall_as_listCheckNumberWeGetOutWithNumberWePutInWithYear()
 		$this->populate('RevisionableThing', array('name' => 'Widget 2015', 'year' => 2015, 'id' => 2));
 
 		$this->assertCount(1, RevisionableThing::all_as_list(2014));
-		$this->assertEquals(count(RevisionableThing::where('year', '=', '2014')), count(RevisionableThing::all_as_list(2014)));
+		$this->assertEquals(count(RevisionableThing::where('year', '=', '2014')->get()), count(RevisionableThing::all_as_list(2014)));
 
-		$this->assertEquals(count(RevisionableThing::where('year', '=', '2015')), count(RevisionableThing::all_as_list(2015)));
+		$this->assertEquals(count(RevisionableThing::where('year', '=', '2015')->get()), count(RevisionableThing::all_as_list(2015)));
 		$this->assertCount(1, RevisionableThing::all_as_list(2015));
 	}
 

diff --git a/application/tests/models/simpledata.test.php b/application/tests/models/simpledata.test.php
@@ -313,10 +313,10 @@ public function testall_as_listCheckNumberWeGetOutWithNumberWePutInWithYear()
 	{
 		$this->populate_two_years();
 
-		$this->assertEquals(count(Thing::where('year', '=', '2014')), count(Thing::all_as_list(2014)));
+		$this->assertEquals(count(Thing::where('year', '=', '2014')->get()), count(Thing::all_as_list(2014)));
 		$this->assertCount(1, Thing::all_as_list(2014));
 
-		$this->assertEquals(count(Thing::where('year', '=', '2015')), count(Thing::all_as_list(2015)));
+		$this->assertEquals(count(Thing::where('year', '=', '2015')->get()), count(Thing::all_as_list(2015)));
 		$this->assertCount(1, Thing::all_as_list(2015));
 	}
 

diff --git a/composer.json b/composer.json
@@ -2,7 +2,6 @@
 	"name": "unikent/programmes-plant",
 	"description": "The Programmes Plant is part of the XCRI-CAP project at the University of Kent",
     "require": {
-        "phpunit/phpunit": "3.7.*@dev",
         "unikent/programmes-plant-modules": "dev-master",
 		"mockery/mockery": "0.9.4",
 		"unikent/curl": "dev-master"
@@ -18,5 +17,8 @@
 
     "config": {
         "bin-dir": "bin/"
-    }
+    },
+	"require-dev": {
+		"phpunit/phpunit": "^5"
+	}
 }