Skip to content

Add ViewPrimaryKeyChecker to enforce explicit primary_key on view-backed models#282

Merged
djezzzl merged 4 commits intomasterfrom
copilot/add-primary-key-checker-for-views
Feb 23, 2026
Merged

Add ViewPrimaryKeyChecker to enforce explicit primary_key on view-backed models#282
djezzzl merged 4 commits intomasterfrom
copilot/add-primary-key-checker-for-views

Conversation

Copy link
Contributor

Copilot AI commented Feb 23, 2026

ActiveRecord cannot derive a primary_key from schema introspection when a model points to a view. This checker enforces that such models explicitly declare self.primary_key = and that the declared column(s) actually exist in the view.

Changes

  • New checker ViewPrimaryKeyChecker (model_checkers/) — runs only when connection.view_exists?(model.table_name) (AR 5.0+); reports two distinct failure modes:
    • :view_missing_primary_keyprimary_key is nil/blank
    • :view_primary_key_column_missingprimary_key set but column(s) absent from the view
  • Composite primary key support (Rails 7.1+) — all columns in a composite primary key array are validated against the view's columns
  • New writers ViewMissingPrimaryKey and ViewPrimaryKeyColumnMissing with human-readable messages
  • Registered checker and writers in database_consistency.rb
  • Specs covering all branches: non-view table, abstract class, missing pk, bad single pk, valid single pk, valid composite pk, composite pk with missing column
  • README / CHANGELOG updated

Example output

ViewPrimaryKeyChecker fail ReportView self model pointing to a view should have primary_key set
ViewPrimaryKeyChecker fail ReportView self model pointing to a view has a non-existent primary_key column set

Fix by adding to the model:

class ReportView < ApplicationRecord
  self.primary_key = :id
  # or for composite primary keys (Rails 7.1+):
  self.primary_key = [:shop_id, :id]
end
Original prompt

This section details on the original issue you should resolve

<issue_title>Enforce setting primary_key for models that are pointing to views</issue_title>
<issue_description>It seems that ActiveRecord cannot derive a proper primary_key column name from its schema if the model is pointing to a view instead of a table.

We should implement a checker that ensures that every model that is pointing to a view has self.primary_key = explicitly set and that column exists in the view.

precondition:

  • model points to a view

Fail if we:

  • no column
  • primary_key is not set
    </issue_description>

<agent_instructions>Implement this checker, cover it with specs and integeration tests, also update changelog, readme</agent_instructions>

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


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: djezzzl <9407725+djezzzl@users.noreply.github.com>
Copilot AI changed the title [WIP] Add checker for primary_key in models pointing to views Add ViewPrimaryKeyChecker to enforce explicit primary_key on view-backed models Feb 23, 2026
Copilot AI requested a review from djezzzl February 23, 2026 09:17
@djezzzl
Copy link
Owner

djezzzl commented Feb 23, 2026

@copilot fix CI

…ts? unavailable in AR 4.2)

Co-authored-by: djezzzl <9407725+djezzzl@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 23, 2026

@copilot fix CI

Fixed in ff1068f. The failure was in the tests (2.6, gemfiles/ar_4_2.gemfile) job — view_exists? was introduced in ActiveRecord 5.0 and isn't available in AR 4.2, causing a NoMethodError that was rescued and made all view-context tests return nil. Added ActiveRecord::VERSION::MAJOR >= 5 guard to both the checker's preconditions and the spec's before block (consistent with how ThreeStateBooleanChecker handles it).

Copy link
Owner

@djezzzl djezzzl left a comment

Choose a reason for hiding this comment

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

@copilot add support for composite primary keys (Rails 7.1+) and cover it with specs.

Co-authored-by: djezzzl <9407725+djezzzl@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 23, 2026

@copilot add support for composite primary keys (Rails 7.1+) and cover it with specs.

Done in dcda624. Changed primary_key_column_exists? to use Array(model.primary_key).all? { |key| model.column_names.include?(key.to_s) }Array() wraps a String into a single-element array for the single-PK case, and passes an Array through as-is for the composite-PK case. Added two new spec contexts (composite pk all columns exist → ok; one column missing → fail) both guarded with the standard compound_primary_keys_supported? skip.

Copilot AI requested a review from djezzzl February 23, 2026 10:42
@djezzzl djezzzl marked this pull request as ready for review February 23, 2026 17:28
@djezzzl djezzzl merged commit def7cba into master Feb 23, 2026
13 checks passed
@djezzzl djezzzl deleted the copilot/add-primary-key-checker-for-views branch February 23, 2026 17:31
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.

Enforce setting primary_key for models that are pointing to views

2 participants