Fix ForeignKeyCascadeChecker: handle missing primary key and support composite FK

[Copilot]

• Fix ForeignKeyCascadeChecker#preconditions to handle ActiveRecord::UnknownPrimaryKey
• Fix ForeignKeyCascadeChecker#foreign_key to support composite foreign keys
• Fix ForeignKeyCascadeChecker#report_template to support composite key serialization
• Add # rubocop:disable Metrics/AbcSize on report_template to fix CI rubocop failure
• Add tests: no-primary-key, composite FK ok/fail
• Use composite_foreign_key_option_name helper in composite FK tests for Rails 7.1 compatibility (query_constraints vs foreign_key)
• Update CHANGELOG

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Error when running ForeignKeyCascadeChecker</issue_title>
<issue_description>During the first run of database_checker on our codebase, it reported an error and asked for it to be filed here. It might just be an unusual problem in our codebase, so I don't mind if this is closed wontfix!

Metadata:
model: Trace
association: points
checker: DatabaseConsistency::Checkers::ForeignKeyCascadeChecker
Stack trace:
/home/andy/.rbenv/versions/3.0.6/lib/ruby/gems/3.0.0/gems/activerecord-7.1.1/lib/active_record/reflection.rb:345:in `primary_key': Unknown primary key for table gps_points in model Tracepoint. (ActiveRecord::UnknownPrimaryKey)

Full stacktrace attached. The application is the OpenStreetMap-website codebase, if you'd like to reproduce it or see how our models are defined.
</issue_description>

<agent_instructions>Let's add support for multiple foreign and primary keys in ForeignKeyCascadeChecker, and also fix the issue when UnknownPrimaryKey is raised (due to missing primary key definition) and instead return precondition false for that. Don't forget to update changelog with proper thanks to the original issue author. </agent_instructions>

Comments on the Issue (you are @copilot in this section)

@djezzzl Hi @gravitystorm,

This error comes from ActiveRecord, not from the gem. And it seems it is valid for your model.

Based on https://github.com/openstreetmap/openstreetmap-website/blob/master/db/structure.sql and https://github.com/openstreetmap/openstreetmap-website/blob/master/app/models/tracepoint.rb,

TracePoint should have self.primary_key = explicitly defined. Otherwise, when we call TracePoint.primary_key in some of the checkers, it raises an error.

Please check that on your end and let me know if specifying the primary key helped. Feel free to reopen the issue if necessary. </comment_new>
<comment_new>@djezzzl
Hi @gravitystorm,

First, thank you for subscribing to the contributor lists! Appreciated!
Secondly, thank you for your continuing effort to make the gem better!

I have reopened the issue. I agree that, at the very least, this checker shouldn't throw an error when there is no connecting primary key.

On the other hand, I guess ActiveRecord doesn't function well when a table doesn't have a primary key defined. Though it should be investigated first, it can be a different configurable checker (similar to what we will have soon for views). </comment_new>
<comment_new>@djezzzl

The thing is that the TracePoint model (gps_points db table) doesn't have a primary key, by design. The objects in the table are dealt with in groups (e.g. @trace.points.delete_all) and aren't ever handled individually. I realise that it's unusual to have a model in rails without a primary key.

I think this line needs a guard in case the association has no primary key:

BTW, would you say that even when you don't want to have primary_key on the model level, you would still like to have a FK constraint with the cascade deletion option to match your model's on_delete option? </comment_new>

• Fixes Error when running ForeignKeyCascadeChecker #217

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[djezzzl]

@copilot fix CI

[Copilot]

@copilot fix CI

Fixed in bc3d914 — added # rubocop:disable Metrics/AbcSize to report_template, consistent with how ForeignKeyTypeChecker#report_template handles the same offense.

[djezzzl]

@copilot fix CI for Rails 7.1. You need to use query_constraints instead of foreign_key on association. There is a helper for that in spec_helper.rb

[Copilot]

@copilot fix CI for Rails 7.1. You need to use query_constraints instead of foreign_key on association. There is a helper for that in spec_helper.rb

Fixed in 7ccc68f — updated the composite FK test cases to use composite_foreign_key_option_name => instead of foreign_key:, so Rails 7.1 uses query_constraints while other versions use foreign_key.