Cphalcon: [BUG]: When updating including a relation, the relation destination is not updated

[fagai]

Describe the bug

The following updates will not be updated.

$user = User::find(1);
$user->userCard->token = $token;
$user->save(); // not update(4.0), update(3.4)

You can reinsert the object changed in this way,

$user = User::find(1);
$userCard = $user->userCard;
$userCard->token = $token;
$user->userCard = $userCard;
$user->save(); // update!

It works if you run update directly at the relation destination.

$user = User::find(1);
$user->userCard->token = $token;
$user->userCard->save(); // update!

I think this is because it has not been possible to detect that the data of the relation destination has been dirty.
This worked until v3.4.

Details

• Phalcon version: 4.0.6
• PHP Version: 7.4.9
• Operating System: debian(docker)
• Installation type: Compiling from source
• Zephir version (if any):
• Server: Apache
• Other related info (Database, table schema):

Additional context
Add any other context about the problem here.

[zsilbi]

I couldn't find anything in the docs (https://docs.phalcon.io/4.0/en/db-models-relationships) about this use case.
The fixes I made in #14035 affected this issue because I was not aware of this possible functionality.

I've always used it like your second example for updates:

$user = User::find(1);
$userCard = $user->userCard;
$userCard->token = $token;
$user->userCard = $userCard;
$user->save(); // update!

@phalcon/core-team, what's your opinion?

[gponcon]

I have the same problem:

# (Address BelongsTo User)
$address = Address::find(1);
$user = new User();
$user->address = $address;

# 3.4: _preSaveRelatedRecords() triggered, _preSave() triggered
# 4.0: _preSaveRelatedRecords() triggered, _preSave() triggered
$user->save();

$user->address->city = 'Paris';

# 3.4: _preSaveRelatedRecords() triggered, _preSave() triggered (saved)
# 4.0: _preSaveRelatedRecords() NOT triggered, _preSave() triggered (NOT saved)
$user->save();

However, reinserting the related record does not work for me either:

# ...
$address = $user->address;
$address->city = 'Paris';
$user->address = $address;

# 3.4: _preSaveRelatedRecords() triggered, _preSave() triggered (saved)
# 4.0: _preSaveRelatedRecords() NOT triggered, _preSave() triggered (NOT saved)
$user->save();

Setting keepSnapshots to true or call explicitly _preSaveRelatedRecords() in _preSave() has no effect.

I did not find an adequate option in the documentation, I'm just Imagine that it should work using ->save() (https://docs.phalcon.io/4.0/en/db-models-relationships#save).

• Phalcon 4.0.6
• PHP 7.4.11
• Alpine Linux on Docker

Any idea?

[zsilbi]

We are going to fix and document this before the next release.

[zsilbi]

Fixed in https://github.com/phalcon/cphalcon/pull/15195 and just released with 4.1

[gponcon]

Thank you very much for your great work @zsilbi.

Your fix works for me, but maybe only partially.

With hasOne / hasMany relationships (as in your test), it's perfect. However, if you replace hasOne() by belongsTo() in the Invoices.php model, values are not updated, I don't know why:

1) SaveCest: Test Mvc\Model - save() with related records property
 Test tests/database/Mvc/Model/SaveCest.php:mvcModelSaveWithRelatedRecordsProperty
 Step Assert equals "new_firstName", "test_firstName_1
 Fail Failed asserting that two strings are equal.
- Expected | + Actual
@@ @@
-'new_firstName
+'test_firstName_1'.

Scenario Steps:

 5. $I->assertEquals("new_firstName", "test_firstName_1") at tests/database/Mvc/Model/SaveCest.php:240

But maybe it's a problem of understanding on my part...

[Jeckerson]

@gponcon Seems that in your case belongsTo() must be not inside Invoices model, but inside another, inside hasOne() declaration.

class Invoice
   hasOne('id', InvoiceDocument::class, 'invoice_id')

class InvoiceDocument
  belongsTo('invoice_id', Invoice::class, 'id')

[gponcon]

Thank you @Jeckerson for this clarification.

I asked this question because we have a lot of hasMany() <-> belongsTo() relations that work with Phalcon 3.4 and not with Phalcon 4, for example:

class Customer
   hasMany('id', Invoice::class, 'invoice_id')

class Invoice
   belongsTo('customer_id', Customer::class, 'id')

Invoice is saved before Customer when doing this:

$invoice->customer->name = 'baz';
$invoice->save();

With Phalcon 4, Customer is no longer saved.

In the unit tests, we have a hasMany() <-> hasOne() relationship as in the documentation.

[zsilbi]

Let's reopen this until I add some more tests to verify this.

[gponcon]

You can try with CustomersKeepSnapshots for example:

public function mvcModelSaveWithRelatedManyAndBelongsRecordsProperty(DatabaseTester $I)
{
    $I->wantToTest('Mvc\Model - save() with related records property (relation many - belongs)');

    /** @var \PDO $connection */
    $connection = $I->getConnection();

    $invoicesMigration = new InvoicesMigration($connection);
    $invoicesMigration->clear();
    $invoicesMigration->insert(77, 1, 0, uniqid('inv-', true));

    $customersMigration = new CustomersMigration($connection);
    $customersMigration->clear();
    $customersMigration->insert(1, 1, 'test_firstName_1', 'test_lastName_1');

    /**
     * @var Invoices $invoice
     */
    $invoice = InvoicesKeepSnapshots::findFirst(77);

    $I->assertEquals(
        1,
        $invoice->customer->id
    );

    $invoice->customer->cst_name_first  = 'new_firstName';
    $invoice->customer->cst_status_flag = 0;

    $I->assertTrue(
        $invoice->save()
    );

    /**
     * @var Customers $customer
     */
    $customer = Customers::findFirst(1);

    $I->assertEquals(
        'new_firstName',
        $customer->cst_name_first
    );

    $I->assertEquals(
        0,
        $customer->cst_status_flag
    );
}

[fagai]

I have not yet verified if this issue has been resolved.
To confirm this issue correctly

• Enable keepSnapshots
• Enable reusable options

It is necessary to set the above two and verify whether it works.

[gponcon]

For both InvoicesKeepSnapshots and CustomersKeepSnapshots models, keepSnapshots and reusable seem to be activated in their initialize():

// ...
public function initialize()
{
    $this->keepSnapshots(true);
    $this->setSource('co_customers');

    $this->hasMany(
        cst_id',
        InvoicesKeepSnapshots::class,
        inv_cst_id',
        [
            alias' => 'invoices',
            reusable' => true,
        ]
    );
}
// ...

(but InvoicesKeepSnapshots is linked to Customers...)

I'm trying to investigate.

[gponcon]

I don't know if I've done it right, but if I make these changes it seems to work:

https://github.com/phalcon/cphalcon/pull/15203/files

Note that I am not yet familiar with Zephir and the algorithm behind the MVC models.

At your disposal.