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

Look at database consistency issues #1189

Merged
merged 3 commits into from
Jan 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 22 additions & 9 deletions .database_consistency.todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Announcement:
enabled: false
live:
MissingUniqueIndexChecker:
enabled: false
enabled: true
ThreeStateBooleanChecker:
enabled: false
text:
Expand Down Expand Up @@ -114,6 +114,9 @@ Case:
scorer:
ForeignKeyChecker:
enabled: false
user_id:
RedundantIndexChecker:
enabled: false
CaseMetadatum:
id:
PrimaryKeyTypeChecker:
Expand Down Expand Up @@ -237,6 +240,9 @@ Score:
user:
ColumnPresenceChecker:
enabled: false
index_case_scores_annotation_id:
UniqueIndexChecker:
enabled: false
Scorer:
code:
LengthConstraintChecker:
Expand Down Expand Up @@ -395,12 +401,12 @@ User:
enabled: false
company:
LengthConstraintChecker:
enabled: false
enabled: true
email:
ColumnPresenceChecker:
enabled: false
LengthConstraintChecker:
enabled: false
enabled: true
id:
PrimaryKeyTypeChecker:
enabled: false
Expand All @@ -412,33 +418,40 @@ User:
enabled: false
invitation_token:
LengthConstraintChecker:
enabled: false
enabled: true
invited_by:
ForeignKeyChecker:
enabled: false
enabled: true
locked:
ThreeStateBooleanChecker:
enabled: false
name:
LengthConstraintChecker:
enabled: false
enabled: true
ColumnPresenceChecker:
enabled: false
owned_scorers:
MissingIndexChecker:
enabled: true
password:
ColumnPresenceChecker:
enabled: false
LengthConstraintChecker:
enabled: false
enabled: true
permissions:
MissingIndexChecker:
enabled: true
profile_pic:
LengthConstraintChecker:
enabled: false
enabled: true
reset_password_token:
LengthConstraintChecker:
enabled: false
enabled: true
stored_raw_invitation_token:
LengthConstraintChecker:
enabled: true

WebRequest:
index_web_requests_on_snapshot_query_id:
UniqueIndexChecker:
enabled: false
1 change: 1 addition & 0 deletions app/models/announcement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
# Indexes
#
# index_announcements_author_id (author_id)
# index_announcements_live (live) UNIQUE
#
class Announcement < ApplicationRecord
belongs_to :author, class_name: 'User'
Expand Down
28 changes: 21 additions & 7 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,15 @@
# Foreign Keys
#
# fk_rails_... (default_scorer_id => scorers.id)
# fk_rails_... (invited_by_id => users.id)
#

class User < ApplicationRecord
# Associations

has_many :api_keys, dependent: :destroy

belongs_to :default_scorer, class_name: 'Scorer', optional: true # for communal scorers there isn't a owner

# has_many :cases,
# dependent: :nullify # sometimes a case belongs to a team, so don't just delete it.
has_many :cases,
class_name: 'Case',
foreign_key: :owner_id,
Expand Down Expand Up @@ -119,29 +117,45 @@ class User < ApplicationRecord

has_many :announcements, foreign_key: 'author_id', dependent: :destroy, inverse_of: :author

# has_many :ai_judges, dependent: :destroy
# has_many :books, through: :ai_judges

# Validations
validates :name,
length: { maximum: 255 }

# https://davidcel.is/posts/stop-validating-email-addresses-with-regex/
validates :email,
presence: true,
uniqueness: true,
format: { with: URI::MailTo::EMAIL_REGEXP },
length: { maximum: 80 },
unless: :ai_judge?

validates :password,
presence: true
presence: true,
length: { maximum: 80 }

validates :password, confirmation: { message: 'should match confirmation' }

validates :reset_password_token,
length: { maximum: 255 }
validates :invitation_token,
length: { maximum: 255 }
validates :stored_raw_invitation_token,
length: { maximum: 255 }

validates :company,
length: { maximum: 255 }
validates :profile_pic,
length: { maximum: 4000 }

validates_with ::DefaultScorerExistsValidator

validates :agreed,
acceptance: { message: 'checkbox must be clicked to signify you agree to the terms and conditions.' },
if: :terms_and_conditions?

validates :openai_key, length: { maximum: 255 }, allow_nil: true
validates :system_prompt, length: { maximum: 4000 }, allow_nil: true

def terms_and_conditions?
Rails.application.config.terms_and_conditions_url.present?
end
Expand Down
8 changes: 8 additions & 0 deletions app/models/web_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@
# updated_at :datetime not null
# snapshot_query_id :integer
#
# Indexes
#
# index_web_requests_on_snapshot_query_id (snapshot_query_id) UNIQUE
#
# Foreign Keys
#
# fk_rails_... (snapshot_query_id => snapshot_queries.id)
#
class WebRequest < ApplicationRecord
belongs_to :snapshot_query, optional: true
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Loaded configurations: .database_consistency.yml, .database_consistency.todo.yml
# UniqueIndexChecker fail Score index_case_scores_annotation_id index is unique in the database but do not have uniqueness validator
# LengthConstraintChecker fail User system_prompt column has limit in the database but do not have length validator
# LengthConstraintChecker fail User openai_key column has limit in the database but do not have length validator
# ForeignKeyChecker fail WebRequest snapshot_query should have foreign key in the database
# MissingIndexChecker fail SnapshotQuery web_request associated model should have proper unique index in the database

class AddUniqueIndexToWebRequestsSnapshotQueryId < ActiveRecord::Migration[8.0]
def change
remove_index :web_requests, :snapshot_query_id if index_exists?(:web_requests, :snapshot_query_id)
add_index :web_requests, :snapshot_query_id, unique: true
add_foreign_key :web_requests, :snapshot_queries
end
end
10 changes: 10 additions & 0 deletions db/migrate/20250118224749_refine_structure.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# RedundantIndexChecker fail Case user_id index is redundant as idx_owner_archived covers it

class RefineStructure < ActiveRecord::Migration[8.0]
def change
if index_exists?(:cases, [:user_id, :archived], name: 'idx_owner_archived')
remove_index :cases, :user_id if index_exists?(:cases, :user_id)
end

end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddUsersIdUsersInvitedByIdFk < ActiveRecord::Migration[8.0]
def change
add_foreign_key :users, :users, column: :invited_by_id, primary_key: :id
end
end
5 changes: 5 additions & 0 deletions db/migrate/20250119130032_add_announcements_live_index.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddAnnouncementsLiveIndex < ActiveRecord::Migration[8.0]
def change
add_index :announcements, :live, name: :index_announcements_live, unique: true
end
end
6 changes: 5 additions & 1 deletion db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions test/fixtures/announcements.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# Indexes
#
# index_announcements_author_id (author_id)
# index_announcements_live (live) UNIQUE
#

live_announcement:
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/users.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
# Foreign Keys
#
# fk_rails_... (default_scorer_id => scorers.id)
# fk_rails_... (invited_by_id => users.id)
#

default: &default
Expand Down
1 change: 1 addition & 0 deletions test/models/announcement_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
# Indexes
#
# index_announcements_author_id (author_id)
# index_announcements_live (live) UNIQUE
#
require 'test_helper'

Expand Down
1 change: 1 addition & 0 deletions test/models/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
# Foreign Keys
#
# fk_rails_... (default_scorer_id => scorers.id)
# fk_rails_... (invited_by_id => users.id)
#

# rubocop:disable Layout/LineLength
Expand Down
Loading