[BUG]: Saving related entity in Phalcon 5

[slechtic]

Discussed in #16203

^{Originally posted by slechtic November 11, 2022}
Hi,

I'm trying to migrate from Phalcon v3 to v5 but I have a problem with updating related entity.

I have an entity Document and this entity has related entity DocumentHistory.

On Document entity there is hasMany relationship defined:

$this->hasMany('id', DocumentHistory::class, 'idDocument', ['alias' => 'documentHistories']);

DocumentHistory entity has belongsTo relationship:

$this->belongsTo('idDocument', Document::class, 'id',['alias' => 'document']);

when I create new DocumentHistory entity and assign existing Document entity, change some values on this entity (Document) and save DocumentHistory entity, only DocumentHistory is saved (created). Document entity is not updated but foreign key is set on DocumentHistory entity correctly. In case when both entities exist it works correctly.

$documentHistory = new DocumentHistory();

$document = Document::findFirst($id);
$document->setNumber($number);

$documentHistory->setDocument($document);
$documentHistory->save();

On DocumentHistory there is a setter:

public function setDocument(Document $document) {
    $this->document = $document;
}

This works for Phalcon v3. This will not be supported in v5 or it is a bug? Thank you.

My environent:

PHP 8.1.
Phalcon 5.1.0.
Postgresql 11

[slechtic]

Hi guys, did you have any chance to check this problem? We need to know whether it is a bug or expected behavior so that we can proceed with Phalcon 5 upgrade.

[Jeckerson]

Did you tried to migrate to v4 first?

[slechtic]

Did you tried to migrate to v4 first?

Yes I did. But there was a problem with segmentation fault and suggestion was migrate to v5. #16028

[slechtic]

Here you can test it https://github.com/slechtic/phalcon5_relationships

[markofo]

Hi @Jeckerson
could you please let us know whether this is expected behavior, bug, or we are using it wrong?

In the previous post from @slechtic, there is a docker image with the example of the issue where you can easily test it.

Is there someone from Phalcon team who can help us even as a paid consultation on this matter?

Thank you in advance!

Marek

[slechtic]

Hi @Jeckerson, any progress? Did you try to run my example project which I attached?

[GeoffreyCZ]

Hi @Jeckerson,

I have encountered the same issue as the OP. Could you give us an information whether it is a bug or not? I need to decide if I need to migrate my project to another FW.

[MartinZubek]

Hi, I've run into the same issue while migrating a project to newer Phalcon.

Šlechtič's docker image (from the link above) clearly illustrates the issue I have.

Can you please check it and tell us what went wrong?
Thanks in advance

[FPEPOSHI]

Hi @Jeckerson, same here. We are stuck with the upgrade of our current version to v5.

[Jeckerson]

I'll investigate.

[FPEPOSHI]

Hi @Jeckerson, do we have any update about the issue reported?

[maxgalbu]

I'm not sure this is related to this bug, but this doesn't work either:

public function initialize() {
  $this->hasOne('code', 'ApiCore\App\Models\VendorPricingConfig', 'vendor_code', ['alias' => 'pricingConfig']);
}

public static function suspendVendor($vendorCode, DateTime $toDate) {
  $model = self::findFirst(array(
     'conditions' => 'code = ?1',
     'bind' => array(1 => $vendorCode)
  ));
  
  $model->pricingConfig->auto_disabled = 1;
  $model->pricingConfig->auto_disabled_date = $toDate->format('Y-m-d H:i:s');
  $model->save();
}

This only sets auto_disabled on $model->pricingConfig to 1 and doesn't set auto_disabled_date. If I invert those two lines

$model->pricingConfig->auto_disabled_date = $toDate->format('Y-m-d H:i:s');
$model->pricingConfig->auto_disabled = 1;

..it only sets auto_disabled_date.

#16402 doesn't fix it

[rudiservo]

@maxgalbu there are two problems,
1 - the getRelated fetchs always a new record
so everytime you do $model->pricingConfig, you effectively call a new model.

2 - lack of a First Level Cache
Hopefully I can add it to phalcon5.

Anyway a way to solve this:

$pricingConfig = $model->pricingConfig;
$pricingConfig->auto_disabled = 1;
$pricingConfig->auto_disabled_date = $toDate->format('Y-m-d H:i:s');

$model->pricingConfig = $pricingConfig;
$model->save();

The $model->pricingConfig = $pricingConfig; could be $model->pricingConfig = $model->pricingConfig;

This would put the pricingConfig in the dirtyRelated array, that is the first one to be fetched.
And it's the best way to mark a dirtyRelation, so it's probably going to be a default behaviour.

Hope this helps.

[maxgalbu]

@rudiservo yes, I solved it like you said. Still, it's weird? that there's no cache on the related models

[rudiservo]

@maxgalbu yeah, trying to fix that, will make a few PR to see how it goes, but still to make stuff dirty I do recommend the $model->pricingConfig = $model->pricingConfig to set it has dirtyRelated, will make it easier in the save specially if I can add teh option for collectRelatedToSave to only return the dirtyRelated instead of all relations.

Hopefully in the near future.

[markofo]

@rudiservo Hi, do you already have any plan to fix and release the issue with saving the related entity? Thank you!

[rudiservo]

@markofo Plans, yes, actual code yes, but still ironing out some kinks, release I am not sure, it adds some behaviour, although some is optional, it's still pretty new.
The focus is v6 for the time, we do not want to add to many things to v5 ATM.
I do have a repo with every idea I have but it's not stable enough, I need to test it for 2 more weeks.
PR #16402 will need a small function and it's stable.
Anyway, has soon has I get my branches stable and v6 is out, I will try and put out a release for v5, although with v6 might be a better option to work with IMO.
Sorry.

[markofo]

@rudiservo it is not a new thing, this worked in Phalcon 3, that's why we were wondering about getting this fixed in 5. We have to workaround this issue for the time being.

Regarding Phalcon 6 that you mentioned, when do you expect the 6 to be released, anytime soon?

@markofo Plans, yes, actual code yes, but still ironing out some kinks, release I am not sure, it adds some behaviour, although some is optional, it's still pretty new. The focus is v6 for the time, we do not want to add to many things to v5 ATM. I do have a repo with every idea I have but it's not stable enough, I need to test it for 2 more weeks. PR #16402 will need a small function and it's stable. Anyway, has soon has I get my branches stable and v6 is out, I will try and put out a release for v5, although with v6 might be a better option to work with IMO. Sorry.

[rudiservo]

@markofo it worked on phalcon3 but it had it's own set of issues that needed a workaround.

No ETA on phalcon6, I am swamped at work ATM.