Fix type error when index is 0

[DaFa66]

As reported on the Trongate Help Bar, user mjim found that when changing the 'Records Per Page' drop down to '10' on _'trongate_pages/manage'_ it would trigger a type error because index 0 was being passed as an empty string.

This change fixes that problem by removing the 'int' type hinting and ensuring _'$selectedindex' is always an integer when setting the index to the $SESSION variable 'selected_perpage'.

Also removed the if block, that was checking for _isnumeric(), which is not needed and a was setting the wrong index from the default value of 20, which if invoked would throw an index error on the array.

[trongate]

Ahoy!

I appreciate you bringing this one up and we're all super grateful for you.

Onto the thing:

The idea of removing type hinting fixes the issue but it feels a bit like taking a step backwards, doesn't it?

How about the following two ideas:

IDEA 1:

Go to the method called, _get_selected_per_page and simply add a line with: settype($selected_per_page, 'int');

IDEA 2:

Change the method header for set_per_page() to the following:

/**

• Sets the number of items per page.
• @param int|string $selected_index The selected index for the number of items per page.
• @return void */ function set_per_page(int|string $selected_index): void {

JUST TO LET YOU KNOW: The idea with the IF statement was to have the system default to 20 items per page in instances where 'per page' has not be explicitly set. I might be wrong about this but I think the IF statement is good. Of course, I could be wrong.

Any thoughts?

[trongate]

UPDATE:

With a little bit of help from Chat GPT I got this:

/**
 * Gets the selected number of items per page based on user preferences.
 *
 * @return int The selected number of items per page (index in $per_page_options).
 */
function _get_selected_per_page(): int {
    $selected_index = (int) ($_SESSION['selected_per_page'] ?? 1);

    // Ensure the selected index is within the valid range of per_page_options
    $valid_indices = array_keys($this->per_page_options);
    $selected_index = in_array($selected_index, $valid_indices) ? $selected_index : 1;

    return $selected_index;
}

What do you think?

[DaFa66]

Hi DC, I agree with idea 2 on your first post. I initially had that in my commit but thought it was confusing to allow mix types when we are only after integers as an index, however, after thinking some more, you're right, it is a step back.

Idea 1 won't fix the issue, as _set_perpage() is being called from JavaScript in admin.js onchange="setPerPage()" and will still throw a type error if the index is 0 function set_per_page(int $selected_index): void {

The IF statement was setting the index incorrectly to a value of 20, not 1 as you intended: if (!is_numeric($selected_index)) { $selected_index = $this->per_page_options[1]; } you could however reintroduce this with if (!is_numeric($selected_index)) { $selected_index = 1; } but that too is not needed as $_SESSION['selected_per_page'] = (int)$selected_index; will always produce a valid index; unless you really wanted to default to index 1.

The improvement to __get_selected_perpage() is a good idea too 👍

[mjim]

👍 Yes, set_per_page was causing an error with a zero selected_index since it is NULL.

I replied in the help bar and noticed that as well for _get_selected_per_page.

My solution was to get the index based on the value using array_search:

     /**
     * Get the number of records selected per page.
     *
     * @return int The number of records selected per page.
     */
    function _get_selected_per_page(): int {
        $selected_per_page = (isset($_SESSION['selected_per_page'])) ? array_search($_SESSION['selected_per_page'], $this->per_page_options) : 1;
        return $selected_per_page;
    }

However, as DC pointed out in his code, that doesn't prevent an invalid index from being returned if someone sets the value per page to an invalid index, for example 7.

Here is an updated version that works for me:

function _get_selected_per_page(): int {
    if (isset($_SESSION['selected_per_page'])) {
        $key = array_search($_SESSION['selected_per_page'], $this->per_page_options, true);
        if ($key !== false) {
            return $key;
        }
    }
    return 1; // Default key
}

[DaFa66]

Hey DC, I've been digging a bit deeper and found the root cause of this issue. It has nothing to do with the above. The problem is in Core.php I'll send a new Pull request that fixes this very soon.