Skip to content

Comments

Remove word ID dependencies from QiraatJunctureSegment#778

Open
al-imam wants to merge 2 commits intodofrom
qiraat-remove-word-ids
Open

Remove word ID dependencies from QiraatJunctureSegment#778
al-imam wants to merge 2 commits intodofrom
qiraat-remove-word-ids

Conversation

@al-imam
Copy link
Collaborator

@al-imam al-imam commented Feb 16, 2026

Summary

This PR refactors the QiraatJunctureSegment model to remove hard dependencies on start_word_id and end_word_id, making these fields nullable and optional. This simplifies the model and removes unnecessary foreign key constraints.

Changes

  • Migration: Added migration to make start_word_id and end_word_id nullable in qiraat_juncture_segments table
  • Foreign Keys: Removed foreign key constraints and indexes for word ID columns
  • Model Updates:
    • Made start_word and end_word associations optional
    • Removed validations that enforced word ownership and ordering constraints
    • Updated methods to handle nullable word IDs gracefully
  • API Updates: Removed word ID fields from API response in matrix controller
  • Performance: Removed unused includes from queries to improve efficiency

Testing

  • Updated specs to validate segments with null word IDs
  • Added test coverage for edge cases with missing word associations
  • All existing tests pass with the new nullable structure

Breaking Changes

API responses no longer include start_word_id, end_word_id, start_word_position, and end_word_position fields in the segments endpoint.

Migration

Run the new migration to update the database schema:

rails db:migrate

- Made `start_word_id` and `end_word_id` nullable in `qiraat_juncture_segments` to allow for optional associations.
- Updated validations and methods in `QiraatJunctureSegment` to handle cases where word IDs may be null.
- Adjusted scopes in `QiraatJuncture` to remove unused associations with `start_word` and `end_word`.
- Modified JSON response in the matrix view to exclude `start_word` and `end_word` data.
- Added migration to update the database schema accordingly.

These changes enhance flexibility in handling segments without requiring word associations, improving the overall robustness of the model.
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4b9a2a8330

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the QiraatJunctureSegment model to make word associations optional by allowing start_word_id and end_word_id to be nullable. This architectural change removes hard dependencies on word references while maintaining backward compatibility for existing functionality.

Changes:

  • Makes start_word_id and end_word_id nullable and removes associated foreign key constraints and indexes
  • Updates the model to handle nullable word IDs gracefully throughout all methods
  • Removes validation constraints that enforced word ownership and ordering
  • Removes word ID and position fields from the matrix segments API response
  • Optimizes query includes by removing unused associations

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
db/migrate/20260216100000_make_qiraat_juncture_segments_word_ids_nullable.rb Migration to make word ID columns nullable and remove foreign key constraints
app/models/qiraat_juncture_segment.rb Updates model with optional word associations and removes related validations
app/models/qiraat_juncture.rb Updates scope to exclude unused word associations and adopts filter_map pattern
app/controllers/api/qdc/qiraat/matrix_controller.rb Removes unused word associations from includes
app/views/api/qdc/qiraat/matrix/by_verse.json.streamer Removes word ID and position fields from segments API response
spec/models/qiraat_juncture_segment_spec.rb Comprehensive test coverage for nullable word IDs and edge cases

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

1 participant