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

Fix foreign keys #268

Merged
merged 11 commits into from
Aug 21, 2023
Merged

Fix foreign keys #268

merged 11 commits into from
Aug 21, 2023

Conversation

Tigrov
Copy link
Member

@Tigrov Tigrov commented Aug 2, 2023

  • Fix multiple foreign keys referencing to one table
  • Fix possible null columns for reference
Q A
Is bugfix? ✔️
New feature?
Breaks BC?
Fixed issues -

@what-the-diff
Copy link

what-the-diff bot commented Aug 2, 2023

PR Summary

  • Improved Foreign Key Handling
    Improved how our system handles foreign keys. Now, it can efficiently process multiple foreign keys from the same table. In the case where a specific parameter is undefined, it will smartly use primary key column names as the foreign key column names.

  • Updated Test Cases
    A new test case has been added to ensure the correct functioning of the improved foreign key handling mechanism. This will help maintain high standards of system reliability and correctness.

  • Added New Database Tables for Testing
    Enhanced the database used for testing by incorporating two new tables. These tables, 'foreign_keys_parent' and 'foreign_keys_child', will be significant in testing and ensuring the proper functioning of foreign keys for multiple tables.

@codecov
Copy link

codecov bot commented Aug 2, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.19% ⚠️

Comparison is base (aa85945) 98.21% compared to head (6f2ac76) 98.02%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #268      +/-   ##
============================================
- Coverage     98.21%   98.02%   -0.19%     
- Complexity      325      328       +3     
============================================
  Files            17       17              
  Lines          1006     1013       +7     
============================================
+ Hits            988      993       +5     
- Misses           18       20       +2     
Files Changed Coverage Δ
src/TableSchema.php 0.00% <ø> (-100.00%) ⬇️
src/Schema.php 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Tigrov Tigrov marked this pull request as ready for review August 2, 2023 09:04
Copy link
Member

@vjik vjik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we mark TableSchema::compositeForeignKey() as deprecated and remove it in the next major version?

Seems, TableSchemaInterface::compositeForeignKey() used only here, and after merge of this PR can be marked as deprecated also.

Copy link
Member

@vjik vjik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment and need line in changelog also.

And several optional suggestions.

src/Schema.php Outdated Show resolved Hide resolved
src/Schema.php Outdated Show resolved Hide resolved
src/Schema.php Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@vjik vjik requested a review from a team August 6, 2023 19:35
@vjik vjik added the status:code review The pull request needs review. label Aug 6, 2023
@terabytesoftw terabytesoftw merged commit 807ffb6 into yiisoft:master Aug 21, 2023
15 of 17 checks passed
@Tigrov Tigrov deleted the fix_foreign_keys branch August 21, 2023 08:47
Tigrov added a commit to Tigrov/db that referenced this pull request Sep 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:code review The pull request needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants