Skip to content
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

Referrer relations exception #33

Closed
cristianoc72 opened this issue Jan 17, 2017 · 4 comments
Closed

Referrer relations exception #33

cristianoc72 opened this issue Jan 17, 2017 · 4 comments

Comments

@cristianoc72
Copy link
Member

In our test suite, at https://github.com/propelorm/Propel3/blob/master/tests/Fixtures/bookstore/schema.xml#L61 we have 2 references to author table. So, in AuthorEntityMap we have 2 referrer relations named essays, which causes the following exception: Propel\Generator\Exception\EngineException: A relation with the name essays already exists in Propel\Tests\Bookstore\Author.

In Propel2 we don't have this issue, because the name of the relation is built on the foreign key name, which is unique.

Should we make the name unique or should we accept more then one relation with the same name?

@marcj
Copy link
Member

marcj commented Jan 17, 2017

This should actually be handled by this code: https://github.com/propelorm/Propel3/blob/master/src/Propel/Generator/Builder/Om/Component/RelationTrait.php#L182: return ucfirst($className . $this->getRefRelatedBySuffix($relation));

'By' should be automatically added at the referring relation name from the getRefRelatedBySuffix method, so relation should look like:

Essay:firstAuthor -> Author
Essay:secondAuthor -> Author

//referrer
Author:essayByFirstAuthor -> Essay
Author:essayBySecondAuthor -> Essay

Also building of the schema.xml should actually already work. Did we get a regression somewhere? All schemas in the fixtures worked for me in the latest versions

@cristianoc72
Copy link
Member Author

cristianoc72 commented Jan 18, 2017

Oh, it's true!
Anyway, this instruction https://github.com/propelorm/Propel3/blob/master/src/Propel/Generator/Builder/Om/Component/EntityMap/BuildRelationsMethod.php#L45 calls correctly getRefRelationPhpName function, but it seems to always stop here https://github.com/propelorm/Propel3/blob/master/src/Propel/Generator/Builder/Om/Component/RelationTrait.php#L169, returning only the plural form of the referrer field (essays).

I don't know if there's a regression somewhere: the phpunit.agnostic.sh returned these errors from the first time I ran it, after cloning Propel3 repository (my laptop s.o. is Ubuntu 16.04 with php 7.1).

@marcj
Copy link
Member

marcj commented Jan 18, 2017

Oh shit, true. https://github.com/propelorm/Propel3/blob/master/src/Propel/Generator/Model/Relation.php#L187 This is the issue. Outside code thinks it can return null forrefField="" is not defined, so we can work around that, but current implementation of getRefField() returns always a value. When we change the behavior of getRefField to allow returning nothing we should check the usage of getRefField everywhere. Should only be some places.

marcj added a commit that referenced this issue Jan 19, 2017
@cristianoc72
Copy link
Member Author

Closed via #34

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants