Variables initialized as strings and then used as arrays cause fatal errors if PHP >= 7.1

I looked at the code in this file...

The code sequence:

$var[] = date;
$var[] = time;
$var = implode(' ', $var);
...

is used something like 13 times... Some the $var is declared as an empty string, some it is not declared and a few it is declared as an empty array.

It seems to me that the best approach would actually be to move the above code sequence into a single function within this, or the parent class and simply call it 13 times with the appropriate date and time field values.

But I also guess that there may already be a function somewhere else in vtiger which does this anyway ;-)

@uma.s comments? I'd be happy to create a MR but it would be good to know if there is already a static or common function elsewhere which does this...

There is one use of this sequence in the SaveFollowupAjax class as well. Both inherit from the parent class Calendar_SaveAjax_Action.

So I would suggest adding a small helper function in there something like:

getFormattedDateTime($date, $time) {
    $dateTime = array();
    $dateTime[] = $date;
    $dateTime[] = $time;
    return implode(' ', $dateTime);
}

Make sense?

@lord_alan Maybe I'm missing something, but wouldn't something like this be more simple?

getFormattedDateTime($date, $time) {
    return $date . ' ' . $time;
}

@ruben.estrada I guess most of the time that will be OK. But if the $time is empty. you will be adding a ' ' to the date which is probably not wanted. I guess that is why they adopted this format.

It's a bit like the CONTACT_WS function in MySQL; if a value is Null the separator is not added...

Of course you could do something like this:

getFormattedDateTime($date, $time) {
    if($date && $time) {
        return $date . ' ' . $time;
    } elseif(!$date) {
        return $time;
    }
    return $date;
}

Or:

getFormattedDateTime($date, $time) {
    return trim($date . ' ' . $time);
}

Very good - I can't see any problem with that solution 👍

@lord_alan I will review this and update my observation soon.

@lord_alan Yes! your right we have this code duplication at around 13 places in DragDropAjax.php and also at SaveFollowupAjax.php.

I checked through the source, where i couldn't find any helper api's which supports this formatting.

It would be great if you guys can share the Merge request for this fix, if you already have one with an helper api. If not i will write one please do confirm.

@ruben.estrada

@ruben.estrada Do you want to write the MR? Or I'm happy to do it if you want.

@lord_alan please go ahead! Thanks for taking care of it.

@uma.s I've been asking this question a lot and maybe you can answer it. What is the official or recommended php version to use with vtiger 7.1/7.2/7.3 ? We should have a compatibility table somewhere publicly available not just for php, but also for mysql/mariadb.

@ruben.estrada Vtiger is officially compatible with php 7.2. We would recommend the php version <= 7.2

We have addressed many signature compatible warnings, and we are on the way to do more to make it fully compatible with higher versions.

Your right, We will update this in our website soon with the upcoming release.

Thanks a lot Uma, that's very valuable information. Talking about the table, I actually started my own but I'm not sure about it and there are many holes. But just to give you the idea, it's this:

Title changed from Drag n Drop functionality Calendar broken for PHP >= 7.1 to Drag n Drop functionality in Calendar broken for PHP >= 7.1

mentioned in merge request !642 (merged)

I've created an MR - not had time to test it but take a look if you get chance - else I'll test it next week.

@lord_alan Thanks! for the merge request, we will validate and review.

after further testing I can confirm the following: The bug reported in this issue is not a bug anymore. It was already reported by Alan here: #1058 (closed) and fixed.

The bug was corrected simply by initializing the variables as arrays like here: https://code.vtiger.com/vtiger/vtigercrm/blob/master/modules/Calendar/actions/DragDropAjax.php#L254

However many initializations were left unchanged in the same file and maybe cause other bugs. Thus, I think Alan's MR is still valid.

Title changed from Drag n Drop functionality in Calendar broken for PHP >= 7.1 to Variables initialized as strings and then used as arrays cause fatal errors if PHP >= 7.1

Variables initialized as strings and then used as arrays cause fatal errors if PHP >= 7.1

Designs

Child items ...

Activity

Admin message

Variables initialized as strings and then used as arrays cause fatal errors if PHP >= 7.1

Activity