formtools / module-form_builder

The Form Builder module.
https://docs.formtools.org/modules/form_builder/
15 stars 21 forks source link

Forms get incorrectly flagged as offline #22

Closed benkeen closed 6 years ago

benkeen commented 6 years ago

See discussion here, with @stefanomarty's explanation: https://github.com/formtools/core/issues/213#issuecomment-389608565

stefanomarty commented 6 years ago

First of all, a quick fix for all those having this issue: just put a future date in the "Automatically take form offline" field (see here below).

schermata 2018-05-17 alle 11 57 12

@benkeen I don't know why this issue is not reproduced in your system, maybe it's related to OS (mine is Debian 8) or other components. If it could be of any help, I can send you admin access to my system, just let me know.

Regarding the offline_date field, again I do not understand the choice to set the 0000-00-00 00:00:00 value instead of NULL, but I see it's handled in many other parts, so maybe it's safer to let it there and just change the if clause.

The problem (on my system) is that this clause: if ($config["is_online"] == "yes" && $config["offline_date"] != "0000-00-00 00:00:00") and specifically, the second part: $config["offline_date"] != "0000-00-00 00:00:00") evaluates to TRUE even when the offline_date field is saved as "0000-00-00 00:00:00" in the database, as the expression $config["offline_date"] evaluates to the empty string.

You can check it by printing $config["offline_date"] before the if clause: it correctly prints a date if it's set to something different than "0000-... etc", but it prints an empty string when the offline_date field is cleared.

        // first, check the form shouldn't be taken offline right now
        print_r($config["offline_date"]);   <-- insert the print instruction here

        if ($config["is_online"] == "yes" && $config["offline_date"] != "0000-00-00 00:00:00") {
        //if ($config["is_online"] == "yes" && $config["offline_date"] != "") {
            $default_timezone_offset = Settings::get("default_timezone_offset");

When you open the Publish tag, you can see the $config["offline_date" value printed on top of the table, like this:

schermata 2018-05-17 alle 12 54 42

Now, if you clear the value in "Automatically take form offline" field:

schermata 2018-05-17 alle 12 53 22

then save and exit, you can see that the $config["offline_date" is printed as an empty string and the form is put offline:

schermata 2018-05-17 alle 12 53 43

Checking the database with phpMyAdmin shows the offline_date field is actually saved as "0000-... etc.":

schermata 2018-05-17 alle 13 21 23
benkeen commented 6 years ago

Bizarre! I still haven't been able to reproduce this, but I'm going to do a refactor that I think will solve it anyway. The old 0000-00-00 00:00:00 checks are far too fussy. I'll try to release a new version tonight with the updated code. Keep you posted.

Edit: huh... now I see the problem.

benkeen commented 6 years ago

Hi @stefanomarty, I just release 2.0.8 - mind giving it a go when you get a moment?

This new version allows null for the offline_date field, and converts any existing 0000-00... values to null. All comparison checks also use null, which is much clearer I think.

Do let me know if you spot anything wonky with it!

stefanomarty commented 6 years ago

Hi @benkeen, I made the upgrade and a quick test, it looks good, I tried some different configurations and they all work fine. I'll keep trying and see how it works under normal conditions.

In the meantime, I may have found another minor bug: while deleting my previously published test forms, I got into this message:

schermata 2018-05-21 alle 00 37 21

The message was due to the fact the file wasn't there anymore (I had deleted it manually).

So, this message is OK, but the link under "click here" is not, as it points to a non existent edit.php:

schermata 2018-05-21 alle 00 37 03

I checked the /formtools/admin/forms/edit/ folder and actually there's no edit.php there. Looks like a broken link to me. This is not a very common error message to be seen, but I thought I let you know.

benkeen commented 6 years ago

Ah, thanks @stefanomarty! Yeah, bug! Just to keep things organized, I'll create a separate ticket for this.