Skip to content

Conversation

@alanpoulain
Copy link

@alanpoulain alanpoulain commented Jul 7, 2022

This PR fixes a bug when an attribute association override is used for a many-to-many association with only a join table set.
In this case, joinColumns and inverseJoinColumns will be set to an empty array:

'joinColumns' => $override['joinColumns'] ?? [],
'inverseJoinColumns' => $override['inverseJoinColumns'] ?? [],

Later on, in the method _validateAndCompleteManyToManyMapping, default values will not be set and the empty arrays will be kept because of isset:
if (! isset($mapping['joinTable']['joinColumns'])) {
$mapping['joinTable']['joinColumns'] = [
[
'name' => $this->namingStrategy->joinKeyColumnName($mapping['sourceEntity'], $selfReferencingEntityWithoutJoinColumns ? 'source' : null),
'referencedColumnName' => $this->namingStrategy->referenceColumnName(),
'onDelete' => 'CASCADE',
],
];
}

The first commit adds a failing test. You can see that the configuration works for all the mappings (PHP, XML, annotations...) excepted the attribute mapping.
The second commit fixes it by using null instead of empty arrays.

Related PR (introducing the bug): #9241
@beberlei if you can have a look at this PR since you wrote the code 🙂

@alanpoulain alanpoulain changed the title fix: attribute association override default values not set for joinColumns and inverseJoinColumns if joinTable fix: attribute association override default values not set for joinColumns and inverseJoinColumns if joinTable is set for a many-to-many association Jul 7, 2022
@greg0ire greg0ire requested a review from beberlei July 7, 2022 12:10
@greg0ire
Copy link
Member

greg0ire commented Jul 7, 2022

You can use vendor/bin/phpcbf to fix many of the cs issues.

@greg0ire greg0ire closed this Jul 7, 2022
@greg0ire greg0ire reopened this Jul 7, 2022
@greg0ire
Copy link
Member

greg0ire commented Jul 7, 2022

Sorry didn't mean to close 😅

@alanpoulain alanpoulain force-pushed the fix/association-overrides-default-mapping branch from 1bd84b8 to 0cd672c Compare July 7, 2022 13:13
@alanpoulain
Copy link
Author

Thanks for the quick review @greg0ire! 🙂

If there is an attribute association override with only a join table for
a many-to-many association, default values for join columns and inverse
join columns will not be set.
In an attribute association override with a join table, when there is no value for joinColumns and inverseJoinColumns, set it to null instead of an empty array.
It will allow the method _validateAndCompleteManyToManyMapping to set
the default values instead of keeping the empty array.
@alanpoulain alanpoulain force-pushed the fix/association-overrides-default-mapping branch from 0cd672c to 097c204 Compare July 7, 2022 13:59
@github-actions
Copy link
Contributor

There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.
If you want to continue working on it, please leave a comment.

@github-actions github-actions bot added the Stale label Nov 12, 2024
@github-actions
Copy link
Contributor

This pull request was closed due to inactivity.

@github-actions github-actions bot closed this Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants