fix: correctly record the datetime for last_active

[datamweb]

Hello friends,
correctly record the datetime for last_active in case of change defaultLocale.

Before:

+----+--------------------+--------+----------------+--------+--------------------+--------------------+--------------------+------------+
| id | username           | status | status_message | active | last_active        | created_at         | updated_at         | deleted_at |
+----+--------------------+--------+----------------+--------+--------------------+--------------------+--------------------+------------+
| 1  | pooyaparsadadas... |        |                | 1      | 0000-00-00 00:0... | 2022-08-25 15:3... | 2022-08-25 15:3... |            |
+----+--------------------+--------+----------------+--------+--------------------+--------------------+--------------------+------------+

After:

+----+--------------------+--------+----------------+--------+--------------------+--------------------+--------------------+------------+
| id | username           | status | status_message | active | last_active        | created_at         | updated_at         | deleted_at |
+----+--------------------+--------+----------------+--------+--------------------+--------------------+--------------------+------------+
| 1  | pooyaparsadadas... |        |                | 1      | 2022-08-25 15:4... | 2022-08-25 15:3... | 2022-08-25 15:3... |            |
+----+--------------------+--------+----------------+--------+--------------------+--------------------+--------------------+------------+

Ref #238

[kenjis]

I think we've fixed this bug in the past.
Why again?

[datamweb]

@kenjis, This bug was created from #392.
But my question is, is the test written in Commit valid?

Oh, maybe you mean problem 0000-00-00 00:00:00.
Because SessionAuth is not enabled by default. In fact, when SessionAuth is not set, last_active will not be updated. So we didn't see it because it wasn't set.
I was checking 392 last night when I noticed this.

[datamweb]

@kenjis see here!!

public function recordActiveDate(): void
{
if (! $this->user instanceof User) {
throw new InvalidArgumentException(
__METHOD__ . '() requires logged in user before calling.'
);
}
-$this->user->last_active = Time::now()->format('Y-m-d H:i:s');
+$time = Time::now()->format('Y-m-d H:i:s');

+$this->user->last_active = $time;

+echo ($time);

+echo ($this->user->last_active);
+exit;

        $this->provider->updateActiveDate($this->user);
    }

output :

2022-08-26 23:10:12 and ۲۰۲۲-۰۸-۲۶ ۲۳:۱۰:۱۲ way?

Is it related to Handling Business Logic?!؟

Temporary solution:
$this->user->last_active = new Time('now','','en-US');

[datamweb]

oh! I noticed something.
If we delete:

shield/src/Entities/User.php

Line 37 in b7b6720

'last_active',

And then

 $this->user->last_active = Time::now()->format('Y-m-d H:i:s');

The date is recorded without any problems!

See Here!

[kenjis]

@datamweb Ah, you're correct! Date mutator converts date string to Time object.

This is a problem in the database layer.
So we should provide the database safe date string at
https://github.com/codeigniter4/shield/pull/392/files#diff-459f49e48ae4f20464cf08fd3256652536435610f4f7db89305fe7269e539ac9R329

[kenjis]

But my question is, is the test written in Commit valid?

It is valid. last_active is updated after the creation of the object.

┌──────────────────────────────────────────────────────────────────────────────┐
│ auth(...)->user()->updated_at                                                │
└──────────────────────────────────────────────────────────────────────────────┘
CodeIgniter\I18n\Time (6) (
    protected 'timezone' -> DateTimeZone (2) (
        public 'timezone_type' -> integer 3
        public 'timezone' -> string (15) "America/Chicago"
    )
    protected 'locale' -> string (2) "en"
    protected 'toStringFormat' -> string (19) "yyyy-MM-dd HH:mm:ss"
    public 'date' -> string (26) "2022-08-26 16:12:13.000000"
    public 'timezone_type' -> integer 3
    public 'timezone' -> string (15) "America/Chicago"
)
┌──────────────────────────────────────────────────────────────────────────────┐
│ auth(...)->user()->last_active                                               │
└──────────────────────────────────────────────────────────────────────────────┘
CodeIgniter\I18n\Time (6) (
    protected 'timezone' -> DateTimeZone (2) (
        public 'timezone_type' -> integer 3
        public 'timezone' -> string (15) "America/Chicago"
    )
    protected 'locale' -> string (2) "en"
    protected 'toStringFormat' -> string (19) "yyyy-MM-dd HH:mm:ss"
    public 'date' -> string (26) "2022-08-26 16:12:13.119705"
    public 'timezone_type' -> integer 3
    public 'timezone' -> string (15) "America/Chicago"
)
════════════════════════════════════════════════════════════════════════════════

[MGatner]

That was my faulty approval. I didn't realize that the $dates property actually stores a Time instance 🤦‍♂️ IMO this is a mistake, since the values in $attributes should be ready for persistence.

[kenjis]

I didn't realize that the $dates property actually stores a Time instance 🤦‍♂️ IMO this is a mistake, since the values in $attributes should be ready for persistence.

I thought that. But if we convert it to string, it will lose the timezone.

[datamweb]

I don't like to have a PR about 0000-00-00 00:00:00 again.
And what about this? is there a need to change?

shield/src/Entities/UserIdentity.php

Lines 37 to 40 in b7b6720

	protected $dates = [
	'expires',
	'last_used_at',
	];

[kenjis]

The coding rule that we must follow now is:

• When we use a DateTime column, we must pass database safe datetime string to the QueryBuilder

So the following code is incorrect, because it passes a Time object.

$time = Time::now();
$this->builder->set('last_active', $time)

But it works in locale en, etc. Because it will be converted to string like 2022-08-29 13:00:00 when it is cast to string.
But it will be '۲۰۲۲-۰۸-۲۹ ۱۳:۰۰:۰۰' in locale fa, and does not work.

I would like to fix it as a bug to work in all locales, because Time is a CI4 standard class.

[MGatner]

We can continue this discussion on the framework issue, but just to reinforce what @kenjis said:

• Query Builder statements should always use a string input for datetimes
• Model/Entity interactions can use either strings or Time; by default Entities with $dates fields will cast to Time and Model will handle the string conversion

[datamweb]

Kenjis and MGatner Thanks for the review and useful information.
I merged.