New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Method to check if relationship was loaded #12772

Closed
wants to merge 48 commits into
base: 3.2.x
from

Conversation

Projects
None yet
10 participants
@Surt
Copy link
Contributor

Surt commented Apr 6, 2017

Hello!

  • Type: new feature
  • Link to issue:

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the Contributing Guidelines?
  • I have checked that another pull request for this purpose does not exist.
  • I wrote some tests for this PR.

Method to check if a relationship was loaded. So we can check it without invoking the lazy loading.

Thanks

sergeyklay and others added some commits Mar 23, 2017

add sanitizing URL
Add unit test for filter URL

Add changelog entry and correct typo in filter url unit test message
Merge pull request #12759 from Jurigag/3.2.x-7.1
Remove php 7.1 from allowed failures
Merge pull request #12271 from andrew-demb/fix-mvc-query-builder-inte…
…rface-join

Amended Phalcon\Mvc\Model\Query\BuilderInterface::join() to specify type join
Fixed call Imagick::getVersion() (#12729)
* Update imagick.zep

Imagick none-static method getVersion

* update CHANGELOG.md

* Update CHANGELOG.md [ci skip]

sergeyklay and others added some commits Apr 6, 2017

Merge pull request #12771 from sergeyklay/merge-master
Merge stable code base from master
Method to check if relationship is loaded
Check if a relationship has been loaded (to avoid lazy loading when invoking the alias property directly)

@Surt Surt changed the title Method to check if relationship is loaded Method to check if relationship was loaded Apr 6, 2017

Surt added some commits Apr 6, 2017

@grigoriy-ivanov

This comment has been minimized.

Copy link

grigoriy-ivanov commented Apr 6, 2017

_related used in the EagerLoading (and somewhere else). If you use phalcon relations they will be store in modelsManager.

@sergeyklay

This comment has been minimized.

Copy link
Member

sergeyklay commented Apr 6, 2017

@Surt Could you please cover your additions by small test?

@grigoriygithub Can you please explain your comment a bit more?

@Jurigag

This comment has been minimized.

Copy link
Member

Jurigag commented Apr 6, 2017

Also update changelog.

@grigoriy-ivanov

This comment has been minimized.

Copy link

grigoriy-ivanov commented Apr 6, 2017

@sergeyklay
Models:

class Pages extends Model
{
    protected $id;
    protected $section_id;

    public function initialize()
    {
        $this->belongsTo('section_id', Sections::class, 'id', [
            'alias' => 'Section',
            'reusable' => true
        ]);
    }
// getters-setters
}
class Sections extends Model
{
    protected $id;
// getters-setters
}

Case: 1 Phalcon Relations

 $Page = Pages::findFirst();
 $PageSection = $Page->getSection();
/*
in Page Model:
protected '_related' => null
*/
 var_dump($Page); // page
/*
in Models Manager: 
  protected '_reusable' => 
    array (size=2)
      'Sections[[[id] = :APR0:,[0],]]' => object(Section)
*/
 var_dump($this->modelsManager);

Case: 2 Eager Loading from Incubator

 $Page = Pages::findFirstWith('Section');
/*
in Page Model:
protected '_related'  => 
    array (size=1)
      'section' => object(Section)
public 'section' => object(Section)
*/
 var_dump($Page); // page
/*
in Models Manager: 
  protected '_reusable' => null
*/
 var_dump($this->modelsManager);
@Surt

This comment has been minimized.

Copy link
Contributor Author

Surt commented Apr 19, 2017

Anyway this would not work. The _related property is filled just for belongs-to as pointed in https://github.com/phalcon/cphalcon/blob/master/phalcon/mvc/model.zep#L4355

@Surt

This comment has been minimized.

Copy link
Contributor Author

Surt commented Apr 19, 2017

@gdluck

This comment has been minimized.

Copy link
Contributor

gdluck commented on 346259c Apr 26, 2017

Hello, in model.zep you forgot to add getter for old snapshot which is very very useful:

	public function getOldSnapshotData() -> array
	{
		return this-> _oldSnapshot;
	}

This comment has been minimized.

Copy link
Member

Jurigag replied Apr 26, 2017

Well perhaps it should be added, wasn't sure. You can add it if you want.

@sergeyklay sergeyklay force-pushed the phalcon:3.2.x branch 2 times, most recently from e63249b to 821011d Jun 18, 2017

@sergeyklay

This comment has been minimized.

Copy link
Member

sergeyklay commented Jun 19, 2017

@Surt Could you please only submit commits relevant to the PR.

@sergeyklay sergeyklay added this to the unplanned milestone Jun 19, 2017

@Surt

This comment has been minimized.

Copy link
Contributor Author

Surt commented Jun 20, 2017

Would be great if someone have the time to finish it.
Last time I checked there was a problem where _related does not contain all the relationship types, but now when I checked again it seems that already has been changed.

The old code only setted _related on belongs-to relationships

Anyway this would not work. The _related property is filled just for belongs-to as pointed in https://github.com/phalcon/cphalcon/blob/master/phalcon/mvc/model.zep#L4355

@sergeyklay sergeyklay force-pushed the phalcon:3.2.x branch 2 times, most recently from 14919a9 to 13ce09f Oct 21, 2017

@niden

This comment has been minimized.

Copy link
Member

niden commented Nov 1, 2018

@Surt I am happy to help with this but I am having trouble identifying the changes. Also it took me a while to get to this so this one points to 3.2 vs the current 3.4.x branch.

I will try and address this as soon as I can.

@niden niden self-assigned this Nov 1, 2018

@CameronHall CameronHall referenced this pull request Nov 24, 2018

Merged

Method to check if relationship was loaded for 4.0.x #13616

3 of 3 tasks complete
@CameronHall

This comment has been minimized.

Copy link
Member

CameronHall commented Nov 24, 2018

I've redone this for the 4.0.x branch. @Surt I tried to cherry-pick the commits but my git kept wigging out, so I've made sure you're noted as a co-author.

@sergeyklay

This comment has been minimized.

Copy link
Member

sergeyklay commented Nov 24, 2018

Closed in favor of #13616

@sergeyklay sergeyklay closed this Nov 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment