lsgs / redcap-event-navigation-tweaks

REDCap external module with enhancements to event and arm navigation.
GNU General Public License v3.0
1 stars 2 forks source link

Updates for better compatibility with PHP 8 #2

Closed biggeeves closed 1 year ago

biggeeves commented 1 year ago

PHP 8 added check for optional parameters before required parameters.

REDCap EM documentation has updated their function calls by declaring the type within the parameters. The following function need to be updated to declare the type. Insert the type before each of the variables and the php error message goes away.

void redcap_data_entry_form_top ( int $project_id, string $record = NULL, string $instrument, int $event_id, int $group_id = NULL, int $repeat_instance = 1 )

void redcap_survey_page_top ( int $project_id, string $record = NULL, string $instrument, int $event_id, int $group_id = NULL, string $survey_hash, int $response_id = NULL, int $repeat_instance = 1 )

void redcap_save_record ( int $project_id, string $record = NULL, string $instrument, int $event_id, int $group_id = NULL, string $survey_hash = NULL, int $response_id = NULL, int $repeat_instance = 1 )

lsgs commented 1 year ago

Thanks for the report @biggeeves. I'm not sure I understand what the problem is the release v1.0.0 of this EM though. The method definitions may not be up-to-date with type specifications, but that should not be a problem. There are no default values specified, so there are no optional parameters occurring before required parameters (which - yes - is indeed an issue with PHP 8).

Can you explain further please? Are you receiving some error message because the methods of this EM do not contain the type specifications?

biggeeves commented 1 year ago

When using default arguments, any defaults should be on the right side of any non-default arguments; otherwise, things will not work as expected.

$instrument and $event_id are after the first default variable $record. If $record was AFTER $instrument and $event_id, this would not be an issue. But it does come before, thus the warning in PHP 8.

redcap_data_entry_form_top ( $project_id, $record = NULL, $instrument, $event_id, $group_id = NULL, $repeat_instance = 1 )

lsgs commented 1 year ago

I understand the principle, @biggeeves, and have made this fix in some of my other EMs. I can't see how it is a problem in this one though...

public function redcap_data_entry_form_top($project_id, $record, $instrument, $event_id, $group_id, $repeat_instance) {
biggeeves commented 1 year ago

It is not a problem. But it does issue a notice in our PHP logs. I like keeping the logs as clean as possible, making it easier to troubleshoot other issues when they arise.

On Thu, Jul 6, 2023 at 9:12 PM Luke @.***> wrote:

I understand the principle, @biggeeves https://github.com/biggeeves, and have made this fix in some of my other EMs. I can't see how it is a problem in this one though...

public function redcap_data_entry_form_top($project_id, $record, $instrument, $event_id, $group_id, $repeat_instance) {

— Reply to this email directly, view it on GitHub https://github.com/lsgs/redcap-event-navigation-tweaks/issues/2#issuecomment-1624674817, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5ICNJXTO5HY7TQSV4RZ6LXO6EBJANCNFSM6AAAAAAZQ3DS64 . You are receiving this because you were mentioned.Message ID: @.***>

-- Greg Neils 917-656-6722

lsgs commented 1 year ago

A new report of an exception thrown in an instance running on PHP 8.1:

The 'event_navigation_tweaks' module threw the following exception when calling the hook method 'redcap_data_entry_form_top':

TypeError: array_key_exists(): Argument #2 ($array) must be of type array, null given in /var/www/redcap/modules/event_navigation_tweaks_v1.0.0/EventNavigationTweaks.php:111
Stack trace:
#0 /var/www/redcap/modules/event_navigation_tweaks_v1.0.0/EventNavigationTweaks.php(111): array_key_exists()
#1 /var/www/redcap/modules/event_navigation_tweaks_v1.0.0/EventNavigationTweaks.php(18): MCRI\EventNavigationTweaks\EventNavigationTweaks->makeEventNavPopoverContent()
#2 /var/www/redcap/redcap_v13.7.13/ExternalModules/classes/ExternalModules.php(3165): MCRI\EventNavigationTweaks\EventNavigationTweaks->redcap_data_entry_form_top()
#3 /var/www/redcap/redcap_v13.7.13/ExternalModules/classes/ExternalModules.php(3330): ExternalModules\ExternalModules::startHook()
#4 /var/www/redcap/redcap_v13.7.13/ExternalModules/classes/ExternalModules.php(3363): ExternalModules\ExternalModules::ExternalModules\{closure}()
#5 /var/www/redcap/redcap_v13.7.13/Classes/Hooks.php(42): ExternalModules\ExternalModules::callHook()
#6 /var/www/redcap/redcap_v13.7.13/DataEntry/index.php(533): Hooks::call()
#7 {main}
lsgs commented 1 year ago

Function argument default values checked. Release v1.01 contains bug fix for PHP8 relating to array access along with a couple of things for Bootstrap 5 compatibility.