improve the boolean column's logic to more closely match PHP's boolea…

[joelmellon]

WHY

The "boolean" column's logic is unintuitive.

BEFORE - What was wrong? What was happening before this PR?

The "boolean" column logic is unintuitive, and isn't very flexible, requiring the use of custom column types (eg. "closure") for otherwise straight-forward use cases.

AFTER - What is happening after this PR?

The "boolean" column logic is more consistent with PHP's typing, so various database column datatypes can be cast to booleans more easily.

HOW

How did you achieve that, in technical terms?

Check for falsy values first.

Is it a breaking change?

Yes.

How can we test the before & after?

Before: Truthy values like 123, 'asd', etc. are cast to a falsy "No" in the UI, eg. on a list operation page. After: Truthy values like 123, 'asd', etc. are cast to a truthy "Yes", eg. on a list operation page.

Example:

// assuming we have a model like this:
$entry->deleted_at = '2022-04-18 00:06:02';

// in PHP casting the deleted_at attribute to boolean would return true:
(bool) $entry->deleted_at; // true

// but using Backpack's "boolean" column doesn't function the same way, logically speaking
$this->crud->addColumn([
    'name' => 'deleted_at',
    'label'  => 'Deleted',
    'type'  => 'boolean',
]);

// in the blade template, the deleted_at value is not one of [true, 1, '1'] so it's assumed falsy

[joelmellon]

You could also make it completely consistent with PHP's code by using (bool) or boolval(...) directly. Is there a reason why you choose not to do so?

That's an option. The whole idea is to take what's in the database and cast it to a boolean. If your "boolean" column was strings of either true/false, what would you expect to 'false' to evaluate to, true or false?

This is sort of an aside, but if you look at the old code '1' is explicitly treated as true. I guess it's in case Eloquent doesn't cast a database's string '1' to an integer 1, but most "bool" database columns are tinyint's or a similar integer type, so I don't get why this would check for a string '1' but not other common boolean strings, like 'true' or 'TRUE'. Casting '1' to a bool is no different than any other string in my mind. If someone is storing their 0/1 flags in string columns, that's just as non-standard as string true/false or on/off, yes/no in the DB. TL;DR: there's no reason to account for string '1'. /tangent

I really don't care about the details though. I'm mostly trying to account for a very common use-case: soft-delete columns when cast to boolean should return true for non-null values. This is logically how soft delete scopes work in Laravel.

If the consensus is that the new logic should simply use boolval() that's fine with me, but I think it's less intuitive, even if it's not perfectly aligned with PHP's logic.

I think we can all agree that the old code is the least intuitive of our options though, so I think we should press forward with improvements.

[tabacitu]

Ugh... this one seems to have slipped through the cracks, thanks @joelmellon .

I agree there's an inconsistency here that we need to fix. It's not ok to evaluate '1' to true and 'true' to false - didn't even know this was happening. Your use case with deleted_at being casted to boolean also makes perfect sense to me.

I've scheduled to take a closer look at this next month (5.2), and figure out a solution if we cannot merge this as-is. The difficult part will be making this a non-breaking change (if possible at all).

Thanks again!