Skip to content

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

Merged
mvorisek merged 2 commits intodevelopfrom
join_name_guess_from_table_with_schema
Jun 1, 2022
Merged

Strip schema from table for Join::foreign_field guess#1003
mvorisek merged 2 commits intodevelopfrom
join_name_guess_from_table_with_schema

Conversation

@mvorisek
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

Development

Successfully merging this pull request may close these issues.

2 participants