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

Strip schema from table for Join::foreign_field guess #1003

Merged
merged 2 commits into from
Jun 1, 2022

Conversation

mvorisek
Copy link
Member

related with #1002, at least Join probably needs a simillar fix

/cc @samsouthardjr

@mvorisek mvorisek force-pushed the join_name_guess_from_table_with_schema branch from dfbd2aa to 0183de2 Compare May 31, 2022 14:13
@mvorisek mvorisek changed the title Strip schema from table for HasMany::their_name guess Strip schema from table for Join::foreign_field guess May 31, 2022
@samsouthardjr
Copy link
Contributor

related with #1002, at least Join probably needs a simillar fix

/cc @samsouthardjr

I'm not an expert at Join or github, but you're right. The exact same fix should be made at line 357 of Join.php. Are you going to put that into this branch, or were you looking for me to do so? I've never contributed to a pull request that someone else has started, but if you're looking to me to do it I can always try and learn.

@mvorisek
Copy link
Member Author

Technically you can open a PR into join_name_guess_from_table_with_schema branch, but I fixed it - the only thing to fix is the test and you can offer (such small) fix using a GH suggestion:

image

@samsouthardjr
Copy link
Contributor

Technically you can open a PR into join_name_guess_from_table_with_schema branch, but I fixed it - the only thing to fix is the test and you can offer (such small) fix using a GH suggestion:

image

I couldn't find that button -- is it in githubdesktop, maybe?

In any case, I'll just comment on it here, since it's your pull. The current pull doesn't address that the same change is needed in Join's hasMany() method, at line 357.

@mvorisek mvorisek force-pushed the join_name_guess_from_table_with_schema branch from 602fd57 to c576364 Compare June 1, 2022 22:30
@mvorisek mvorisek force-pushed the join_name_guess_from_table_with_schema branch from c576364 to aad22d0 Compare June 1, 2022 22:34
@mvorisek mvorisek marked this pull request as ready for review June 1, 2022 22:34
@mvorisek
Copy link
Member Author

mvorisek commented Jun 1, 2022

Impl. fixed, but Join should be dropped sooner or later with Model impl. to support full/nested modelling.

@mvorisek mvorisek merged commit 5b939ab into develop Jun 1, 2022
@mvorisek mvorisek deleted the join_name_guess_from_table_with_schema branch June 1, 2022 22:45
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

Successfully merging this pull request may close these issues.

2 participants