Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[candidate] Create Sex table and remove Sex enum #9025

Merged
merged 24 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ changes in the following format: PR #1234***
#### Features
- Add OpenID Connect authorization support to LORIS (PR #8255)

#### Updates and Improvements
- Create new `sex` table to hold candidate sex options, and change Sex and ProbandSex columns of `candidate` table to a varchar(255) datatype that is restricted by the `sex` table (PR #9025)

#### Bug Fixes
- Fix examiner site display (PR #8967)
- bvl_feedback updates in real-time (PR #8966)
Expand Down
19 changes: 16 additions & 3 deletions SQL/0000-00-00-schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,15 @@ CREATE TABLE `language` (
INSERT INTO language (language_code, language_label) VALUES
('en-CA', 'English');

CREATE TABLE `sex` (
`SexID` int(10) unsigned NOT NULL AUTO_INCREMENT,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What purpose does this column have? It is not being used as a foreign key by any other tables and the other column in this table already has a not null/unique constraint and is being used as a foreign key into this table.

Removing it would solve the "what should it be named" debate in the thread below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, this column is not useful. I don't mind removing it

`SexValue` varchar(255) NOT NULL,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicky but as I said during the meeting, I find "SexValue" to be a little generic, and think something like "SexName" would be more descriptive (this also applies to the rest of this PR obviously).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated "SexValue" to "Sex", and also modified charset to utf8mb4

PRIMARY KEY (`SexID`),
UNIQUE KEY `SexValue` (`SexValue`)
) ENGINE=InnoDB AUTO_INCREMENT=1 DEFAULT CHARSET=utf8 COMMENT='Stores sex options available for candidates in LORIS';

INSERT INTO sex (SexValue) VALUES ('Male'), ('Female'), ('Other');

CREATE TABLE `users` (
`ID` int(10) unsigned NOT NULL auto_increment,
`UserID` varchar(255) NOT NULL default '',
Expand Down Expand Up @@ -151,7 +160,7 @@ CREATE TABLE `candidate` (
`DoB` date DEFAULT NULL,
`DoD` date DEFAULT NULL,
`EDC` date DEFAULT NULL,
`Sex` enum('Male','Female','Other') DEFAULT NULL,
`Sex` varchar(255) DEFAULT NULL,
`RegistrationCenterID` integer unsigned NOT NULL DEFAULT '0',
`RegistrationProjectID` int(10) unsigned NOT NULL,
`Ethnicity` varchar(255) DEFAULT NULL,
Expand All @@ -166,7 +175,7 @@ CREATE TABLE `candidate` (
`flagged_other_status` enum('not_answered') DEFAULT NULL,
`Testdate` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
`Entity_type` enum('Human','Scanner') NOT NULL DEFAULT 'Human',
`ProbandSex` enum('Male','Female','Other') DEFAULT NULL,
`ProbandSex` varchar(255) DEFAULT NULL,
`ProbandDoB` date DEFAULT NULL,
PRIMARY KEY (`CandID`),
UNIQUE KEY `ID` (`ID`),
Expand All @@ -175,9 +184,13 @@ CREATE TABLE `candidate` (
KEY `CandidateActive` (`Active`),
KEY `FK_candidate_2_idx` (`flagged_reason`),
KEY `PSCID` (`PSCID`),
KEY `FK_candidate_sex_1` (`Sex`),
KEY `FK_candidate_sex_2` (`ProbandSex`),
CONSTRAINT `FK_candidate_1` FOREIGN KEY (`RegistrationCenterID`) REFERENCES `psc` (`CenterID`),
CONSTRAINT `FK_candidate_2` FOREIGN KEY (`flagged_reason`) REFERENCES `caveat_options` (`ID`) ON DELETE RESTRICT ON UPDATE CASCADE,
CONSTRAINT `FK_candidate_RegistrationProjectID` FOREIGN KEY (`RegistrationProjectID`) REFERENCES `Project` (`ProjectID`) ON UPDATE CASCADE
CONSTRAINT `FK_candidate_RegistrationProjectID` FOREIGN KEY (`RegistrationProjectID`) REFERENCES `Project` (`ProjectID`) ON UPDATE CASCADE,
CONSTRAINT `FK_candidate_sex_1` FOREIGN KEY (`Sex`) REFERENCES `sex` (`SexValue`) ON DELETE RESTRICT ON UPDATE CASCADE,
CONSTRAINT `FK_candidate_sex_2` FOREIGN KEY (`ProbandSex`) REFERENCES `sex` (`SexValue`) ON DELETE RESTRICT ON UPDATE CASCADE
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

CREATE TABLE `session` (
Expand Down
6 changes: 1 addition & 5 deletions modules/candidate_list/jsx/candidateListIndex.js
Original file line number Diff line number Diff line change
Expand Up @@ -282,11 +282,7 @@ class CandidateListIndex extends Component {
name: 'sex',
type: 'select',
hide: this.state.hideFilter,
options: {
'Male': 'Male',
'Female': 'Female',
'Other': 'Other',
},
options: options.Sex,
},
},
{
Expand Down
1 change: 1 addition & 0 deletions modules/candidate_list/php/candidate_list.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ class Candidate_List extends \DataFrameworkMenu
'cohort' => $cohort_options,
'participantstatus' => $participant_status_options,
'useedc' => $config->getSetting("useEDC"),
'Sex' => \Utility::getSexList(),
];
}

Expand Down
1 change: 1 addition & 0 deletions modules/candidate_parameters/ajax/getData.php
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ function getProbandInfoFields()
'ageDifference' => $ageDifference,
'extra_parameters' => $extra_parameters,
'parameter_values' => $parameter_values,
'sexOptions' => \Utility::getSexList(),
];

return $result;
Expand Down
15 changes: 10 additions & 5 deletions modules/candidate_parameters/jsx/ProbandInfo.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
import React, {Component} from 'react';
import PropTypes from 'prop-types';
import Loader from 'Loader';
import {
FormElement,
StaticElement,
SelectElement,
ButtonElement,
DateElement,
TextareaElement,
} from 'jsx/Form';

/**
* Proband Info Component.
Expand All @@ -16,11 +24,7 @@ class ProbandInfo extends Component {
super(props);

this.state = {
sexOptions: {
Male: 'Male',
Female: 'Female',
Other: 'Other',
},
sexOptions: {},
Data: [],
formData: {},
updateResult: null,
Expand Down Expand Up @@ -66,6 +70,7 @@ class ProbandInfo extends Component {
formData: formData,
Data: data,
isLoaded: true,
sexOptions: data.sexOptions,
});
},
error: (error) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class CandidateQueryEngine extends \LORIS\Data\Query\SQLQueryEngine
"Sex",
"Candidate's biological sex",
$candscope,
new \LORIS\Data\Types\Enumeration('Male', 'Female', 'Other'),
new \LORIS\Data\Types\StringType(255),
kongtiaowang marked this conversation as resolved.
Show resolved Hide resolved
new Cardinality(Cardinality::SINGLE),
),
new DictionaryItem(
Expand Down
5 changes: 1 addition & 4 deletions modules/genomic_browser/jsx/tabs_content/cnv.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,7 @@ class CNV extends Component {
filter: {
name: 'Sex',
type: 'select',
options: {
Male: 'Male',
Female: 'Female',
},
options: options.Sex,
},
},
{
Expand Down
5 changes: 1 addition & 4 deletions modules/genomic_browser/jsx/tabs_content/methylation.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,7 @@ class Methylation extends Component {
filter: {
name: 'Sex',
type: 'select',
options: {
Male: 'Male',
Female: 'Female',
},
options: options.Sex,
},
},
{
Expand Down
5 changes: 1 addition & 4 deletions modules/genomic_browser/jsx/tabs_content/profiles.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,7 @@ class Profiles extends Component {
filter: {
name: 'Sex',
type: 'select',
options: {
Male: 'Male',
Female: 'Female',
},
options: options.Sex,
},
},
{
Expand Down
5 changes: 1 addition & 4 deletions modules/genomic_browser/jsx/tabs_content/snp.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,7 @@ class SNP extends Component {
filter: {
name: 'Sex',
type: 'select',
options: {
Male: 'Male',
Female: 'Female',
},
options: options.Sex,
},
},
{
Expand Down
1 change: 1 addition & 0 deletions modules/genomic_browser/php/views/cnv.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class CNV
'Sites' => $sites,
'Cohorts' => $cohorts,
'Platform' => $platform_options,
'Sex' => \Utility::getSexList(),
];
}

Expand Down
1 change: 1 addition & 0 deletions modules/genomic_browser/php/views/methylation.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class Methylation
'genotyping_platform',
'Name'
),
'Sex' => \Utility::getSexList(),
];
}

Expand Down
1 change: 1 addition & 0 deletions modules/genomic_browser/php/views/profiles.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class Profiles
$this->_formElement = [
'Sites' => $sites,
'Cohorts' => $cohorts,
'Sex' => \Utility::getSexList(),
];
}

Expand Down
1 change: 1 addition & 0 deletions modules/genomic_browser/php/views/snp.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class SNP
'Sites' => $sites,
'Cohorts' => $cohorts,
'Platform' => $platform_options,
'Sex' => \Utility::getSexList(),
];
}

Expand Down
13 changes: 5 additions & 8 deletions modules/new_profile/php/new_profile.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,11 @@ class New_Profile extends \NDB_Form
$ageMin = $config->getSetting('ageMin');
$dobFormat = $config->getSetting('dobFormat');
$edc = $config->getSetting('useEDC');
$sex = [
'Male' => 'Male',
'Female' => 'Female',
'Other' => 'Other',
];
$pscidSet = "false";
$minYear = (isset($startYear, $ageMax)) ? $startYear - $ageMax : null;
$maxYear = (isset($endYear, $ageMin)) ? $endYear - $ageMin : null;
$sex = \Utility::getSexList();

$pscidSet = "false";
$minYear = (isset($startYear, $ageMax)) ? $startYear - $ageMax : null;
$maxYear = (isset($endYear, $ageMin)) ? $endYear - $ageMin : null;

// Get sites for the select dropdown
$user_list_of_sites = $user->getCenterIDs();
Expand Down
17 changes: 17 additions & 0 deletions php/libraries/Utility.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,23 @@ class Utility
return $result;
}

/**
* Returns list of sex options in the database
*
* @return array An associative array of sex values.
*/
static function getSexList(): array
{
$factory = NDB_Factory::singleton();
$DB = $factory->database();

$query = "SELECT SexValue FROM sex";

$result = $DB->pselectCol($query, []);

return array_combine($result, $result);
}

/**
* Returns a list of sites in the database
*
Expand Down
19 changes: 9 additions & 10 deletions src/StudyEntities/Candidate/Sex.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,7 @@ class Sex implements \JsonSerializable
/* @var string */
public $value;

private const VALID_VALUES = array(
'Male',
'Female',
'Other',
);
private $validValues = [];

/**
* Calls validate() immediately.
Expand All @@ -43,10 +39,12 @@ class Sex implements \JsonSerializable
*/
public function __construct(string $value)
{
if (!self::validate($value)) {
$this->validValues = \Utility::getSexList();

if (!self::validate($value, $this->validValues)) {
throw new \DomainException(
'The value is not valid. Must be one of: '
. implode(', ', self::VALID_VALUES)
. implode(', ', $this->validValues)
);
}
$this->value = $value;
Expand All @@ -55,13 +53,14 @@ public function __construct(string $value)
/**
* Ensures that the value is well-formed.
*
* @param string $value The value to be validated
* @param string $value The value to be validated
* @param array $laidValues The restricted optional values of $value
*
* @return bool True if the value format is valid
*/
public static function validate(string $value): bool
public static function validate(string $value, array $validValues): bool
{
return in_array($value, self::VALID_VALUES, true);
return in_array($value, $validValues, true);
Copy link
Contributor

@kongtiaowang kongtiaowang Feb 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getSexList will return array(
'Male' => 'Male',
'Female' => 'Female',
'Other' => 'Other'
) so you need to change to [return in_array($value, array_values($validValues), true);]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kongtiaowang Made that change but still having trouble unfortunately!

}

/**
Expand Down
4 changes: 2 additions & 2 deletions tools/CouchDB_Import_Demographics.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class CouchDBDemographicsImporter
],
'Sex' => [
'Description' => 'Candidate\'s biological sex',
'Type' => "enum('Male', 'Female', 'Other')"
'Type' => "varchar(255)"
],
'Site' => [
'Description' => 'Site that this visit took place at',
Expand Down Expand Up @@ -324,7 +324,7 @@ function _updateDataDict()
if ($config->getSetting("useProband") === "true") {
$this->Dictionary["Sex_proband"] = [
'Description' => 'Proband\'s biological sex',
'Type' => "enum('Male','Female', 'Other')"
'Type' => "varchar(255)"
];
$this->Dictionary["Age_difference"] = [
'Description' => 'Age difference between the candidate and ' .
Expand Down
Loading