Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
103 changes: 66 additions & 37 deletions modules/redcap/php/client/redcaphttpclient.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -647,19 +647,19 @@ class RedcapHttpClient
/**
* Get all records for an single instrument.
*
* @param string $instrument_name A REDCap instrument name.
* @param string $unique_event_name A REDCap unique event name.
* @param string $record_id A REDCap record ID.
* @param bool $completed_records_only Only return completed records.
* @param string $instrument_name A REDCap instrument name.
* @param string $unique_event_name A REDCap unique event name.
* @param ?string $record_id A REDCap record ID.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is the ? for ?

And also: why the double spacing ? A new convention ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In PHP ?type means "nullable type", a.k.a a value of type or null. The double spaces are just alignment of the parameter names enforced by the linter.

* @param bool $completed_records_only Only return completed records.
*
* @throws \LorisException
*
* @return RedcapRecord[] an array of records
*/
public function getInstrumentRecord(
string $instrument_name,
string $unique_event_name,
string $record_id,
public function getInstrumentRecords(
string $instrument_name,
string $unique_event_name,
?string $record_id,
bool $completed_records_only = true
): array {
if (empty($instrument_name)) {
Expand All @@ -674,10 +674,6 @@ class RedcapHttpClient
);
}

if (empty($record_id)) {
throw new \LorisException("[redcap] Error: required 'record_id'.");
}

// mapping check
$mapping_instrument_event_exists = $this->hasMappingInstrumentEvent(
$instrument_name,
Expand All @@ -692,65 +688,98 @@ class RedcapHttpClient
}

// request
$records = $this->_getRecords(
$record_dicts = $this->_getRecords(
Comment thread
adamdaudrich marked this conversation as resolved.
[$instrument_name],
[$unique_event_name],
[$record_id]
$record_id !== null ? [$record_id] : [],
);

if (empty($records)) {
if (empty($record_dicts)) {
throw new \LorisException("[redcap] Error: no data found.");
}

// Only keep complete records
if ($completed_records_only) {
$completed = array_filter(
$records,
$record_dicts = array_filter(
$record_dicts,
fn ($record) => $record["{$instrument_name}_complete"] == 2
);

if (count($completed) === 0) {
if (empty($record_dicts)) {
throw new \LorisException(
"[redcap] Error: no complete record found."
);
}
} else {
// if not only completed records
$completed = $records;
}

// Order the records by ${instrument_name}_dtt field value
usort(
$completed,
function ($a, $b) use ($instrument_name) {
$dttField = "{$instrument_name}_dtt";
$a_date = new \DateTimeImmutable($a[$dttField]);
$b_date = new \DateTimeImmutable($b[$dttField]);
return $a_date <=> $b_date;
}
);
$records = [];

// is a repeating instrument?
$final = [];
if ($this->getProjectInfo()->has_repeating_instruments
&& $this->hasRepeatingInstrumentEvent(
$instrument_name,
$unique_event_name
)
) {
foreach ($completed as $index => $record) {
$final[] = new RedcapRepeatingRecord(
// TODO: ${instrument_name}_dtt seems to be HBCD-specific. The code
// could probably be cleaner with a single timestamp abstraction. The
// problem is that the repeating index depends on the order in which the
// records were filled. However, this index might also be obtainable
Comment thread
adamdaudrich marked this conversation as resolved.
// from a field in the record. Investigation needed.
// Order the records by ${instrument_name}_dtt field value
usort(
$record_dicts,
function ($a, $b) use ($instrument_name) {
$dttField = "{$instrument_name}_dtt";
$a_date = new \DateTimeImmutable($a[$dttField]);
$b_date = new \DateTimeImmutable($b[$dttField]);
return $a_date <=> $b_date;
}
);

foreach ($record_dicts as $index => $record) {
$records[] = new RedcapRepeatingRecord(
$instrument_name,
$record,
$index + 1
);
}
} else {
// return the only record
$final[] = new RedcapRecord($instrument_name, $completed[0]);
// Transform the record dictionaries into record objects.
$records = array_map(
fn ($record_dict) => new RedcapRecord(
$instrument_name,
$record_dict,
),
$record_dicts,
);

// Sort the records by datetime.
usort(
$records,
function ($a, $b) {
// If both records have a datetime, compare them.
if ($a->datetime !== null && $b->datetime !== null) {
return $a->datetime <=> $b->datetime;
}

// If only record $a has datetime, it comes first.
if ($a->datetime !== null) {
return -1;
}

// If only record $b has datetime, it comes first.
if ($b->datetime !== null) {
return 1;
}

// If both records have no datetime, maintain the original order.
return 0;
},
);
}

return $final;
return $records;
}

/**
Expand Down
36 changes: 36 additions & 0 deletions modules/redcap/php/redcapmapper.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,42 @@ class RedcapMapper
return $visit_config ? $visit_config : null;
}

/**
* Get the REDCap event associated with a REDCap visit mapping configuration
* node.
*
* @param RedcapConfigVisit $visit_config A REDCap visit mapping configuration
Comment thread
adamdaudrich marked this conversation as resolved.
* node.
*
* @return RedcapEvent The associated REDCap event.
*/
public function getVisitConfigEvent(
RedcapConfigVisit $visit_config,
): RedcapEvent {
// Get the list of all the REDCap arms for this REDCap project.
Copy link
Copy Markdown
Contributor

@adamdaudrich adamdaudrich Dec 9, 2025

Choose a reason for hiding this comment

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

REDCap Arms.
Arms = REDcap,
"Get the list of Arms"

A documentation thought:
Why specify the term "REDCap" at the level of doc-string? At this level of granularity (ie a doc string), everybody knows this is REDCap.

The documentation.md should guide the developer into the overall design of the module: all the more basic REDCap parameters should be clearly outlined there. The deeper level documentation should just be stripped of the redundant association.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same thing as my other reply, this. This is partly due to the way I write comments: I look for consistency and non-ambiguity above all, so if I use a name to refer to something in one place, I will use that same name everywhere, even if it is sometimes verbose. I agree this method could be improved, but as with the other comment, these conventions are already used in the module so I think it is out-of-scope for this PR.

image

$redcap_arms = $this->_redcap_client->getArms();

// Find the REDCap arm that matches the configuration arm name.
$redcap_arm = array_find(
$redcap_arms,
fn($redcap_arm) => $redcap_arm->name === $visit_config->redcap_arm_name,
);

// Get the list of all the REDCap events for this REDCap project.
$redcap_events = $this->_redcap_client->getEvents();

// Find the REDCap event that matches the arm and configuration event name.
$redcap_event = array_find(
$redcap_events,
fn($redcap_event) => (
$redcap_event->arm_number === $redcap_arm->number
&& $redcap_event->name === $visit_config->redcap_event_name
),
);

return $redcap_event;
}

/**
* Check that a session exists or create it using the REDCap module automatic
* session creation configuration. Throw an exception if the session does not
Expand Down
2 changes: 1 addition & 1 deletion modules/redcap/php/redcapnotificationhandler.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ class RedcapNotificationHandler
$this->_acquireNotificationLock();

// get data from redcap
$records = $this->_redcap_client->getInstrumentRecord(
$records = $this->_redcap_client->getInstrumentRecords(
$this->_redcap_notif->instrument_name,
$this->_redcap_notif->unique_event_name,
$this->_redcap_notif->record_id,
Expand Down
Loading
Loading