Removed unused code and improved tests

IUREDCap · Jul 25, 2019 · 09d255b · 09d255b
1 parent ac51f0e
commit 09d255b
Show file tree

Hide file tree

Showing 25 changed files with 97 additions and 166 deletions.
diff --git a/.gitignore b/.gitignore
@@ -11,6 +11,9 @@ composer.phar
 # ignore swap files
 *.swp
 
+# ignore cache files
+*.cache
+
 # Eclipse project file and settings directory
 .project
 .settings/

diff --git a/RedCapEtlModule.php b/RedCapEtlModule.php
@@ -61,6 +61,9 @@ public function redcap_module_link_check_display($project_id, $link)
  return $link;
  }
 
+ # If you are in a project (vs. being on admin page) and you have permissions
+ # to use REDCap-ETL on the page, then return the REDCap-ETL link (so that it
+ # can be displayed)
  if (!empty($project_id) && Authorization::hasRedCapUserRightsForEtl($this, USERID)) {
  return $link;
  }
@@ -1124,19 +1127,7 @@ public function renderProjectPageContentHeader($selfUrl, $errorMessage, $warning
  $this->renderWarningMessageDiv($warningMessage);
  $this->renderSuccessMessageDiv($successMessage);
  }
-
- public function renderProjectPageHeader()
- {
- ob_start();
- require_once APP_PATH_DOCROOT . 'ProjectGeneral/header.php';
- $buffer = ob_get_clean();
- #$cssFile = $this->getUrl('resources/redcap-etl.css');
- #$link = '<link href="'.$cssFile.'" rel="stylesheet" type="text/css" media="all">';
- #$buffer = str_replace('</head>', " ".$link."\n</head>", $buffer);
- return $buffer;
- }
-
-
+
 
  /**
  * Checks if the user has permission to a user (non-admin) page, and

diff --git a/classes/AdminConfig.php b/classes/AdminConfig.php
@@ -29,15 +29,6 @@ class AdminConfig implements \JsonSerializable
  /** @var string certificate authority certificate file used for SSL verification of REDCap. */
  private $caCertFile;
 
- # private $allowEmbeddedServer; // Allow embedded REDCap-ETL server to be used
-
- /** @var string log file (if any) on REDCap server to use for the embedded ETL server. */
- # private $embeddedServerLogFile;
-
- /** @var string E-mail from address to use for embedded server
- * (must be set for e-mail logging to work for embedded server). */
- #private $embeddedServerEmailFromAddress;
-
  private $allowOnDemand; // Allow the ETL process to be run on demand
 
  private $allowCron;
@@ -58,9 +49,9 @@ public function __construct()
  $this->allowedCronTimes[$day] = array();
  foreach (range(0, 23) as $hour) {
  if ($day === 0 || $day === 6 || $hour < 8 || $hour > 17) {
- $this->allowedCronTimes[$day][$hour] = '1';
+ $this->allowedCronTimes[$day][$hour] = true;
  } else {
- $this->allowedCronTimes[$day][$hour] = '0';
+ $this->allowedCronTimes[$day][$hour] = false;
  }
  }
  }
@@ -96,19 +87,21 @@ public function set($properties)
  if (array_key_exists(self::ALLOWED_CRON_TIMES, $properties)) {
  $times = $properties[self::ALLOWED_CRON_TIMES];
  if (is_array($times)) {
- foreach ($times as $rownum => $row) {
- if (is_array($row)) {
- foreach ($row as $colnum => $value) {
- $times[$rownum][$colnum] = Filter::sanitizeLabel($value);
+ for ($row = 0; $row < count($this->allowedCronTimes); $row++) {
+ if (array_key_exists($row, $times)) {
+ $dayTimes = $times[$row];
+ if (is_array($dayTimes)) {
+ for ($col = 0; $col < count($this->allowedCronTimes[$row]); $col++) {
+ if (array_key_exists($col, $dayTimes)) {
+ $this->allowedCronTimes[$row][$col] = true;
+ } else {
+ $this->allowedCronTimes[$row][$col] = false;
+ }
+ }
  }
  }
  }
- } else {
- $times = array();
  }
- $this->allowedCronTimes = $times;
- } else {
- $this->allowedCronTimes = array();
  }
 
  # Set flag that indicates if users can run jobs on demand
@@ -237,11 +230,6 @@ public function getAllowOnDemand()
  return $this->allowOnDemand;
  }
 
- public function setAllowOnDemand($allowOnDemand)
- {
- $this->allowOnDemand = $allowOnDemand;
- }
-
  public function getAllowCron()
  {
  return $this->allowCron;
@@ -258,11 +246,6 @@ public function getSslVerify()
  return $this->sslVerify;
  }
 
- public function setSslVerify($sslVerify)
- {
- $this->sslVerify = $sslVerify;
- }
-
  public function getCaCertFile()
  {
  return $this->caCertFile;

diff --git a/classes/Authorization.php b/classes/Authorization.php
@@ -26,10 +26,10 @@ public static function hasEtlProjectPagePermission($module)
  $hasPermission = false;
 
  $projectId = $module->getProjectId();
- 
+
  if ($module->isSuperUser()) {
  $hasPermission = true;
- } elseif (!empty($projectId)) {
+ } elseif (!empty($projectId)) { // @codeCoverageIgnore
  if (self::hasRedCapUserRightsForEtl($module)) {
  $userEtlProjects = $module->getUserEtlProjects();
  if (in_array($projectId, $userEtlProjects)) {
@@ -104,7 +104,7 @@ public static function hasEtlRequestPermission($module)
 
  if ($module->isSuperUser()) {
  $hasPermission = true;
- } elseif (!empty($projectId)) {
+ } elseif (!empty($projectId)) { // @codeCoverageIgnore
  $rights = $module->getUserRights();
  if (self::hasRedCapUserRightsForEtl($module) && $rights['data_export_tool'] > 0) {
  $hasPermission = true;

diff --git a/classes/Filter.php b/classes/Filter.php
@@ -43,12 +43,6 @@ public static function escapeForUrlParameter($value)
  return urlencode($value);
  }
 
- public static function escapeForJavaScriptInSingleQuotes($value)
- {
- # REDCap's JavaScript escape function for single quotes
- return js_escape($value);
- }
-
  public static function escapeForJavaScriptInDoubleQuotes($value)
  {
  # REDCap's JavaScript escape function for double quotes

diff --git a/tests/unit/AdminConfigTest.php b/tests/unit/AdminConfigTest.php
@@ -10,13 +10,33 @@
 
 class AdminConfigTest extends TestCase
 {
- public function setup()
- {
- }
-
  public function testCreate()
  {
  $adminConfig = new AdminConfig();
  $this->assertNotNull($adminConfig, 'Object creation test');
+
+ $caCertFile = $adminConfig->getCaCertFile();
+ $this->assertNull($caCertFile, 'CA cert file null check');
+
+
+ $expectedCaCertFile = '/tmp/cacert.pem';
+
+ $properties = array();
+ $properties[AdminConfig::ALLOWED_CRON_TIMES] = array([0 => 'on'], [], [], [], [], [], []);
+ $properties[AdminConfig::CA_CERT_FILE] = $expectedCaCertFile;
+ $adminConfig->set($properties);
+
+ $allowed = $adminConfig->isAllowedCronTime(1, 12);
+ $this->assertFalse($allowed, 'Allowed cron time false test');
+
+ $allowed = $adminConfig->isAllowedCronTime(0, 0);
+ $this->assertTrue($allowed, 'Allowed cron time true test');
+
+ $caCertFile = $adminConfig->getCaCertFile();
+ $this->assertEquals($expectedCaCertFile, $caCertFile, 'CA cert file check');
+
+ # SSL verify - should be false, because no value was provided for properties in set call
+ $sslVerify = $adminConfig->getSslVerify();
+ $this->assertFalse($sslVerify, 'SSL verify check');
  }
 }
diff --git a/tests/unit/AuthorizationTest.php b/tests/unit/AuthorizationTest.php
@@ -10,10 +10,6 @@
 
 class AuthorizationTest extends TestCase
 {
- public function setup()
- {
- }
-
  public function testCreate()
  {
  $authorization = new Authorization();

diff --git a/tests/unit/ConfigurationTest.php b/tests/unit/ConfigurationTest.php
@@ -14,11 +14,6 @@
 
 class ConfigurationTest extends TestCase
 {
-
- public function setup()
- {
- }
-
  public function testCreate()
  {
  $config = new Configuration('test');

diff --git a/tests/unit/CsrfTest.php b/tests/unit/CsrfTest.php
@@ -10,10 +10,6 @@
 
 class CsrfTest extends TestCase
 {
- public function setup()
- {
- }
-
  public function testCreate()
  {
  $csrf = new Csrf();

diff --git a/tests/unit/FilterTest.php b/tests/unit/FilterTest.php
@@ -10,10 +10,6 @@
 
 class FilterTest extends TestCase
 {
- public function setup()
- {
- }
-
  public function testCreate()
  {
  $filter = new Filter();

diff --git a/tests/unit/HelpTest.php b/tests/unit/HelpTest.php
@@ -10,10 +10,6 @@
 
 class HelpTest extends TestCase
 {
- public function setup()
- {
- }
-
  public function testCreate()
  {
  $help = new Help();

diff --git a/tests/unit/ProjectInfoTest.php b/tests/unit/ProjectInfoTest.php
@@ -10,10 +10,6 @@
 
 class ProjectInfoTest extends TestCase
 {
- public function setup()
- {
- }
-
  public function testCreate()
  {
  $projectInfo = new ProjectInfo();

diff --git a/tests/unit/RedCapDbTest.php b/tests/unit/RedCapDbTest.php
@@ -10,10 +10,6 @@
 
 class RedCapDbTest extends TestCase
 {
- public function setup()
- {
- }
-
  public function testCreate()
  {
  $redCapDb = new RedCapDb();

diff --git a/tests/unit/ServerConfigTest.php b/tests/unit/ServerConfigTest.php
@@ -10,10 +10,6 @@
 
 class ServerConfigTest extends TestCase
 {
- public function setup()
- {
- }
-
  public function testCreate()
  {
  $serverConfig = new ServerConfig('test');

diff --git a/tests/unit/ServersTest.php b/tests/unit/ServersTest.php
@@ -10,10 +10,6 @@
 
 class ServersTest extends TestCase
 {
- public function setup()
- {
- }
-
  public function testCreate()
  {
  $servers = new Servers();

diff --git a/tests/unit/SettingsTest.php b/tests/unit/SettingsTest.php
@@ -10,10 +10,6 @@
 
 class SettingsTest extends TestCase
 {
- public function setup()
- {
- }
-
  public function testCreate()
  {
  $module = null;

diff --git a/tests/unit/UserListTest.php b/tests/unit/UserListTest.php
@@ -13,10 +13,6 @@
  */
 class UserListTest extends TestCase
 {
- public function setup()
- {
- }
-
  public function testCreate()
  {
  $userList = new UserList();

diff --git a/tests/web/features/admin-help.feature b/tests/web/features/admin-help.feature
@@ -11,6 +11,21 @@ Feature: Admin Help Customization
  Background:
  Given I am on "/"
 
+ Scenario: Use help topic selection to access a help topic
+ When I access the admin interface
+ And I follow "Help Edit"
+ And I follow "Edit"
+ And I select "E-mail To List" from "Help Topic"
+ Then I should see "A comma-separated list of e-mail addresses"
+
+ Scenario: Try to save help with no help topic selected
+ When I access the admin interface
+ And I follow "Help Edit"
+ And I follow "Edit"
+ And I press "Save"
+ Then I should see "Error:"
+ And I should see "No help topic specified"
+
  Scenario: Preview custom help prepended to default help
  When I access the admin interface
  And I follow "Help Edit"

diff --git a/tests/web/features/admin-project-access.feature b/tests/web/features/admin-project-access.feature
@@ -0,0 +1,24 @@
+#-------------------------------------------------------
+# Copyright (C) 2019 The Trustees of Indiana University
+# SPDX-License-Identifier: BSD-3-Clause
+#-------------------------------------------------------
+
+Feature: Admin-Interface
+ In order to help users
+ As an admin
+ I need to be able to access their projects' ETL information
+
+ Background:
+ Given I am on "/"
+ When I access the admin interface
+
+ Scenario: Access the REDCap-ETL page of a user's project
+ When I follow "Users"
+ And I follow "Search"
+ And I search for user
+ And I select the test project
+ And I go to new window in 4 seconds
+ And I follow "REDCap-ETL"
+ Then I should see "ETL Configurations"
+ And I go to old window
+
diff --git a/tests/web/features/schedule.feature b/tests/web/features/schedule.feature
@@ -44,15 +44,8 @@ I need to be able to create, copy, rename and delete configurations
  And I select "(embedded server)" from "server"
  And I schedule for next hour
  And I press "Save"
- And I wait for 10 seconds
+ And I wait for 4 seconds
  Then I should see "Configuration:"
  And I should see "Server:"
  And I should see "(embedded server)"
- #And I select "behat" from "configName"
- #And I press "Run"
- #Then I should see "Configuration:"
- #And I should see "Created table"
- #And I should see "Number of record_ids found: 100"
- #And I should see "Processing complete."
- #But I should not see "Error:"