From 9cc895d714e88ea1c2a910f6b578d1f4a38004a0 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Sun, 19 Jan 2025 07:56:21 -0500 Subject: [PATCH 1/3] Lets push these changes out! --- .database_consistency.todo.yml | 29 ++++++++++++----- app/models/user.rb | 31 ++++++++++++++----- app/models/web_request.rb | 8 +++++ ...index_to_web_requests_snapshot_query_id.rb | 14 +++++++++ db/migrate/20250118224749_refine_structure.rb | 10 ++++++ ...525_add_users_id_users_invited_by_id_fk.rb | 5 +++ db/schema.rb | 5 ++- test/fixtures/users.yml | 1 + test/models/user_test.rb | 3 +- 9 files changed, 88 insertions(+), 18 deletions(-) create mode 100644 db/migrate/20250118223041_add_unique_index_to_web_requests_snapshot_query_id.rb create mode 100644 db/migrate/20250118224749_refine_structure.rb create mode 100644 db/migrate/20250119123525_add_users_id_users_invited_by_id_fk.rb diff --git a/.database_consistency.todo.yml b/.database_consistency.todo.yml index 1b15fcdd2..ed016686b 100644 --- a/.database_consistency.todo.yml +++ b/.database_consistency.todo.yml @@ -114,6 +114,9 @@ Case: scorer: ForeignKeyChecker: enabled: false + user_id: + RedundantIndexChecker: + enabled: false CaseMetadatum: id: PrimaryKeyTypeChecker: @@ -237,6 +240,9 @@ Score: user: ColumnPresenceChecker: enabled: false + index_case_scores_annotation_id: + UniqueIndexChecker: + enabled: false Scorer: code: LengthConstraintChecker: @@ -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 @@ -412,16 +418,18 @@ 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 @@ -429,16 +437,21 @@ User: 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 diff --git a/app/models/user.rb b/app/models/user.rb index 853d55b1f..87bd0302c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -48,17 +48,16 @@ # 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, @@ -119,29 +118,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 diff --git a/app/models/web_request.rb b/app/models/web_request.rb index 815dd89ab..18c262b14 100644 --- a/app/models/web_request.rb +++ b/app/models/web_request.rb @@ -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 diff --git a/db/migrate/20250118223041_add_unique_index_to_web_requests_snapshot_query_id.rb b/db/migrate/20250118223041_add_unique_index_to_web_requests_snapshot_query_id.rb new file mode 100644 index 000000000..1b92c63cf --- /dev/null +++ b/db/migrate/20250118223041_add_unique_index_to_web_requests_snapshot_query_id.rb @@ -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 diff --git a/db/migrate/20250118224749_refine_structure.rb b/db/migrate/20250118224749_refine_structure.rb new file mode 100644 index 000000000..33a844666 --- /dev/null +++ b/db/migrate/20250118224749_refine_structure.rb @@ -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 diff --git a/db/migrate/20250119123525_add_users_id_users_invited_by_id_fk.rb b/db/migrate/20250119123525_add_users_id_users_invited_by_id_fk.rb new file mode 100644 index 000000000..66c74f5a3 --- /dev/null +++ b/db/migrate/20250119123525_add_users_id_users_invited_by_id_fk.rb @@ -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 diff --git a/db/schema.rb b/db/schema.rb index 784652e8b..c7b7863ad 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2025_01_18_162642) do +ActiveRecord::Schema[8.0].define(version: 2025_01_19_123525) do create_table "active_storage_attachments", charset: "utf8mb4", collation: "utf8mb4_bin", force: :cascade do |t| t.string "name", null: false t.string "record_type", null: false @@ -620,6 +620,7 @@ t.binary "response", size: :long t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.index ["snapshot_query_id"], name: "index_web_requests_on_snapshot_query_id", unique: true end add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id" @@ -657,4 +658,6 @@ add_foreign_key "teams_scorers", "teams" add_foreign_key "tries", "cases", name: "tries_ibfk_1" add_foreign_key "users", "scorers", column: "default_scorer_id" + add_foreign_key "users", "users", column: "invited_by_id" + add_foreign_key "web_requests", "snapshot_queries" end diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 0e7ad1afd..e6ab8da9d 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -44,6 +44,7 @@ # Foreign Keys # # fk_rails_... (default_scorer_id => scorers.id) +# fk_rails_... (invited_by_id => users.id) # default: &default diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 94d46997c..8b92975dd 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -48,6 +48,7 @@ # Foreign Keys # # fk_rails_... (default_scorer_id => scorers.id) +# fk_rails_... (invited_by_id => users.id) # # rubocop:disable Layout/LineLength @@ -63,7 +64,7 @@ class UserTest < ActiveSupport::TestCase describe 'Defaults' do test 'are set when user is created' do user = User.create(email: 'defaults@email.com', password: 'password') - + assert_not_nil user.completed_case_wizard assert_not_nil user.num_logins From 98d57e6e76b23cd224cd4da4128e9b303216f125 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Sun, 19 Jan 2025 08:05:44 -0500 Subject: [PATCH 2/3] Only one annoucment can be live at a time, so add a index. --- .database_consistency.todo.yml | 2 +- app/models/announcement.rb | 1 + db/migrate/20250119130032_add_announcements_live_index.rb | 5 +++++ db/schema.rb | 3 ++- test/fixtures/announcements.yml | 1 + test/models/announcement_test.rb | 1 + 6 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20250119130032_add_announcements_live_index.rb diff --git a/.database_consistency.todo.yml b/.database_consistency.todo.yml index ed016686b..ecabc0ec3 100644 --- a/.database_consistency.todo.yml +++ b/.database_consistency.todo.yml @@ -26,7 +26,7 @@ Announcement: enabled: false live: MissingUniqueIndexChecker: - enabled: false + enabled: true ThreeStateBooleanChecker: enabled: false text: diff --git a/app/models/announcement.rb b/app/models/announcement.rb index 95ce46748..53129b3ee 100644 --- a/app/models/announcement.rb +++ b/app/models/announcement.rb @@ -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' diff --git a/db/migrate/20250119130032_add_announcements_live_index.rb b/db/migrate/20250119130032_add_announcements_live_index.rb new file mode 100644 index 000000000..ec45fac1a --- /dev/null +++ b/db/migrate/20250119130032_add_announcements_live_index.rb @@ -0,0 +1,5 @@ +class AddAnnouncementsLiveIndex < ActiveRecord::Migration[8.0] + def change + add_index :announcements, :live, name: :index_announcements_live, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index c7b7863ad..9db7f11c1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2025_01_19_123525) do +ActiveRecord::Schema[8.0].define(version: 2025_01_19_130032) do create_table "active_storage_attachments", charset: "utf8mb4", collation: "utf8mb4_bin", force: :cascade do |t| t.string "name", null: false t.string "record_type", null: false @@ -112,6 +112,7 @@ t.datetime "updated_at", null: false t.boolean "live", default: false t.index ["author_id"], name: "index_announcements_author_id" + t.index ["live"], name: "index_announcements_live", unique: true end create_table "api_keys", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| diff --git a/test/fixtures/announcements.yml b/test/fixtures/announcements.yml index 7a5b8fc38..03a847b91 100644 --- a/test/fixtures/announcements.yml +++ b/test/fixtures/announcements.yml @@ -12,6 +12,7 @@ # Indexes # # index_announcements_author_id (author_id) +# index_announcements_live (live) UNIQUE # live_announcement: diff --git a/test/models/announcement_test.rb b/test/models/announcement_test.rb index 7cf997ed8..1a5e0bf90 100644 --- a/test/models/announcement_test.rb +++ b/test/models/announcement_test.rb @@ -14,6 +14,7 @@ # Indexes # # index_announcements_author_id (author_id) +# index_announcements_live (live) UNIQUE # require 'test_helper' From 3a3b1da15784eee9bff84d01b1e7fd179159a8d0 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Sun, 19 Jan 2025 08:05:51 -0500 Subject: [PATCH 3/3] lint --- app/models/user.rb | 5 ++--- test/models/user_test.rb | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 87bd0302c..0c5eec33d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -52,7 +52,6 @@ # class User < ApplicationRecord - # Associations has_many :api_keys, dependent: :destroy @@ -120,8 +119,8 @@ class User < ApplicationRecord # Validations validates :name, - length: { maximum: 255 } - + length: { maximum: 255 } + # https://davidcel.is/posts/stop-validating-email-addresses-with-regex/ validates :email, presence: true, diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 8b92975dd..30adac5f2 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -64,7 +64,7 @@ class UserTest < ActiveSupport::TestCase describe 'Defaults' do test 'are set when user is created' do user = User.create(email: 'defaults@email.com', password: 'password') - + assert_not_nil user.completed_case_wizard assert_not_nil user.num_logins