diff --git a/AGENTS.md b/AGENTS.md index 730fdf7012..840dec02c1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -116,6 +116,11 @@ rm -rf .built-assets/ # Clear asset cache if needed 4. Add appropriate tests (see testing docs) 5. Run pre-commit validation commands +**Code Style:** + +- **Always use symbol key syntax (`key:`) in hashes** - Use `{ name: "value" }` not `{ "name" => "value" }` unless there's a specific reason to use string keys (like when the key contains special characters or spaces) +- This applies to serializers, API responses, and all Ruby hash structures + ## Reference Documentation - `docs/context/overview.md` - Complete codebase documentation and setup guide diff --git a/app/assemblers/assemble_localization_glossary_entries.rb b/app/assemblers/assemble_localization_glossary_entries.rb new file mode 100644 index 0000000000..dabd408593 --- /dev/null +++ b/app/assemblers/assemble_localization_glossary_entries.rb @@ -0,0 +1,30 @@ +class AssembleLocalizationGlossaryEntries + include Mandate + + initialize_with :user, :params + + def self.keys + %i[criteria status page] + end + + def call + SerializePaginatedCollection.( + glossary_entries, + serializer: SerializeLocalizationGlossaryEntries, + serializer_args: user, + meta: { + unscoped_total: Localization::GlossaryEntry.count + } + ) + end + + memoize + def glossary_entries + Localization::GlossaryEntry::Search.( + user, + criteria: params[:criteria], + status: params[:status], + page: params[:page] + ) + end +end diff --git a/app/commands/localization/glossary_entry/search.rb b/app/commands/localization/glossary_entry/search.rb new file mode 100644 index 0000000000..ce6da2c84f --- /dev/null +++ b/app/commands/localization/glossary_entry/search.rb @@ -0,0 +1,54 @@ +class Localization::GlossaryEntry::Search + include Mandate + + DEFAULT_PAGE = 1 + DEFAULT_PER = 24 + + def self.default_per + DEFAULT_PER + end + + def initialize(user, page: nil, per: nil, criteria: nil, status: nil) + @user = user + + @criteria = criteria + @status = status + @page = page.present? && page.to_i.positive? ? page.to_i : DEFAULT_PAGE + @per = per.present? && per.to_i.positive? ? per.to_i : self.class.default_per + end + + def call + @glossary_entries = Localization::GlossaryEntry.where(locale: locales) + + filter_criteria! + filter_status! + + paginated_glossary_entries = @glossary_entries.page(page).per(per) + + Kaminari.paginate_array( + paginated_glossary_entries, + total_count: @glossary_entries.count + ).page(page).per(per) + end + + memoize + def locales = user.translator_locales - [:en] + + private + attr_reader :user, :per, :page, :criteria, :status + + def filter_criteria! + return if criteria.blank? + + @glossary_entries = @glossary_entries.where( + "localization_glossary_entries.term LIKE ? OR localization_glossary_entries.translation LIKE ?", + "%#{criteria}%", "%#{criteria}%" + ) + end + + def filter_status! + return if status.blank? + + @glossary_entries = @glossary_entries.where("localization_glossary_entries.status": status) + end +end diff --git a/app/commands/localization/glossary_entry_proposal/approve.rb b/app/commands/localization/glossary_entry_proposal/approve.rb index 9caf60887f..49268df2e3 100644 --- a/app/commands/localization/glossary_entry_proposal/approve.rb +++ b/app/commands/localization/glossary_entry_proposal/approve.rb @@ -30,16 +30,16 @@ def handle_addition! end def handle_modification! - raise "No glossary entry to modify" if glossary_entry.nil? + raise "No glossary entry to modify" if proposal.glossary_entry.nil? - glossary_entry.update!( + proposal.glossary_entry.update!( translation: proposal.translation, llm_instructions: proposal.llm_instructions ) end def handle_deletion! - raise "No glossary entry to delete" if glossary_entry.nil? + raise "No glossary entry to delete" if proposal.glossary_entry.nil? # Retrieve this before destroying else we have a race glossary_entry = proposal.glossary_entry @@ -48,6 +48,4 @@ def handle_deletion! proposal.update!(glossary_entry: nil) glossary_entry.destroy! end - - delegate :glossary_entry, to: :proposal end diff --git a/app/commands/localization/glossary_entry_proposal/create.rb b/app/commands/localization/glossary_entry_proposal/create.rb new file mode 100644 index 0000000000..89301f50eb --- /dev/null +++ b/app/commands/localization/glossary_entry_proposal/create.rb @@ -0,0 +1,12 @@ +class Localization::GlossaryEntryProposal + class Create + include Mandate + + initialize_with :glossary_entry, :user, :value + + def call + # For existing glossary entries, create a modification proposal + CreateModification.(glossary_entry, user, value, "") + end + end +end diff --git a/app/commands/localization/glossary_entry_proposal/reject.rb b/app/commands/localization/glossary_entry_proposal/reject.rb index 30e6c36f67..43bc84978d 100644 --- a/app/commands/localization/glossary_entry_proposal/reject.rb +++ b/app/commands/localization/glossary_entry_proposal/reject.rb @@ -11,6 +11,4 @@ def call ) end end - - delegate :glossary_entry, to: :proposal end diff --git a/app/controllers/api/localization/glossary_entry_proposals_controller.rb b/app/controllers/api/localization/glossary_entry_proposals_controller.rb index 463aed617a..ecb901b390 100644 --- a/app/controllers/api/localization/glossary_entry_proposals_controller.rb +++ b/app/controllers/api/localization/glossary_entry_proposals_controller.rb @@ -6,24 +6,23 @@ def create Localization::GlossaryEntryProposal::Create.(@glossary_entry, current_user, params[:value]) render json: { - glossary_entry: SerializeLocalizationGlossaryEntry.(@glossary_entry) - + glossary_entry: SerializeLocalizationGlossaryEntry.(@glossary_entry, current_user) }, status: :created end def approve - Localization::GlossaryEntryProposal::Approve.(@glossary_entry, current_user) + Localization::GlossaryEntryProposal::Approve.(@proposal, current_user) render json: { - glossary_entry: SerializeLocalizationGlossaryEntry.(@glossary_entry) + glossary_entry: SerializeLocalizationGlossaryEntry.(@glossary_entry, current_user) } end def reject - Localization::GlossaryEntryProposal::Reject.(@glossary_entry, current_user) + Localization::GlossaryEntryProposal::Reject.(@proposal, current_user) render json: { - glossary_entry: SerializeLocalizationGlossaryEntry.(@glossary_entry) + glossary_entry: SerializeLocalizationGlossaryEntry.(@glossary_entry, current_user) } end @@ -36,7 +35,7 @@ def update end render json: { - glossary_entry: SerializeLocalizationGlossaryEntry.(@glossary_entry) + glossary_entry: SerializeLocalizationGlossaryEntry.(@glossary_entry, current_user) } end diff --git a/app/controllers/localization/glossary_entries_controller.rb b/app/controllers/localization/glossary_entries_controller.rb index f8bb73c6a7..389be94ab0 100644 --- a/app/controllers/localization/glossary_entries_controller.rb +++ b/app/controllers/localization/glossary_entries_controller.rb @@ -1,4 +1,4 @@ -class Localization::Controller < ApplicationController +class Localization::GlossaryEntriesController < ApplicationController def index @glossary_entries = AssembleLocalizationGlossaryEntries.(current_user, params)[:results] @glossary_entries_params = params.permit(:criteria, :status, :page) diff --git a/app/models/localization/glossary_entry.rb b/app/models/localization/glossary_entry.rb index ef003694b2..6e2730eb40 100644 --- a/app/models/localization/glossary_entry.rb +++ b/app/models/localization/glossary_entry.rb @@ -1,3 +1,18 @@ class Localization::GlossaryEntry < ApplicationRecord + enum :status, { + generating: 0, + unchecked: 1, + proposed: 2, + checked: 3 + } + has_many :proposals, class_name: "Localization::GlossaryEntryProposal", dependent: :destroy + + before_create do + self.uuid = SecureRandom.uuid if uuid.blank? + self.status = :unchecked if status.blank? + end + + def to_param = uuid + def status = super.to_sym end diff --git a/app/serializers/serialize_localization_glossary_entries.rb b/app/serializers/serialize_localization_glossary_entries.rb new file mode 100644 index 0000000000..96ea310ff1 --- /dev/null +++ b/app/serializers/serialize_localization_glossary_entries.rb @@ -0,0 +1,31 @@ +class SerializeLocalizationGlossaryEntries + include Mandate + + initialize_with :glossary_entries, :user + + def call + glossary_entries_with_includes.map do |glossary_entry| + { + uuid: glossary_entry.uuid, + locale: glossary_entry.locale, + term: glossary_entry.term, + translation: glossary_entry.translation, + status: glossary_entry.status.to_s, + llm_instructions: glossary_entry.llm_instructions, + proposals_count: proposals[glossary_entry.id]&.length || 0 + } + end + end + + def glossary_entries_with_includes + glossary_entries.to_active_relation + end + + memoize + def proposals + Localization::GlossaryEntryProposal. + where(glossary_entry: glossary_entries). + where.not(status: :rejected). + group_by(&:glossary_entry_id) + end +end diff --git a/app/serializers/serialize_localization_glossary_entry.rb b/app/serializers/serialize_localization_glossary_entry.rb new file mode 100644 index 0000000000..99a86ed65f --- /dev/null +++ b/app/serializers/serialize_localization_glossary_entry.rb @@ -0,0 +1,35 @@ +class SerializeLocalizationGlossaryEntry + include Mandate + + initialize_with :glossary_entry, :user, proposals: nil + + def call + { + uuid: glossary_entry.uuid, + locale: glossary_entry.locale, + term: glossary_entry.term, + translation: glossary_entry.translation, + status: glossary_entry.status.to_s, + llm_instructions: glossary_entry.llm_instructions, + proposals: proposals.map do |proposal| + { + uuid: proposal.uuid, + type: proposal.type.to_s, + status: proposal.status.to_s, + term: proposal.term, + translation: proposal.translation, + llm_instructions: proposal.llm_instructions, + proposer_id: proposal.proposer&.id, + reviewer_id: proposal.reviewer&.id + } + end + } + end + + memoize + def proposals + Array.new( + @proposals || @glossary_entry.proposals.where.not(status: :rejected) + ) + end +end diff --git a/config/routes/api.rb b/config/routes/api.rb index b82de4be0e..319321e5cd 100644 --- a/config/routes/api.rb +++ b/config/routes/api.rb @@ -128,6 +128,13 @@ patch :reject, on: :member end end + + resources :glossary_entries, only: %i[index show] do + resources :proposals, only: %i[create update], controller: "glossary_entry_proposals" do + patch :approve, on: :member + patch :reject, on: :member + end + end end get "/scratchpad/:category/:title" => "scratchpad_pages#show", as: :scratchpad_page diff --git a/config/routes/website.rb b/config/routes/website.rb index a032f997c7..17560b74b4 100644 --- a/config/routes/website.rb +++ b/config/routes/website.rb @@ -196,6 +196,8 @@ root to: "dashboard#show" resources :originals, only: %i[index show] do end + resources :glossary_entries, only: %i[index show] do + end end resource :user_onboarding, only: %i[show create], controller: "user_onboarding" diff --git a/db/migrate/20250905142739_add_status_to_localization_glossary_entries.rb b/db/migrate/20250905142739_add_status_to_localization_glossary_entries.rb new file mode 100644 index 0000000000..67e6ed6822 --- /dev/null +++ b/db/migrate/20250905142739_add_status_to_localization_glossary_entries.rb @@ -0,0 +1,7 @@ +class AddStatusToLocalizationGlossaryEntries < ActiveRecord::Migration[7.1] + def change + add_column :localization_glossary_entries, :status, :integer, default: 1, null: false + add_column :localization_glossary_entries, :uuid, :string, null: false + add_index :localization_glossary_entries, :uuid, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index d4b9859062..bd5d6d6331 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[7.1].define(version: 2025_08_28_143607) do +ActiveRecord::Schema[7.1].define(version: 2025_09_05_142739) do create_table "active_storage_attachments", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| t.string "name", null: false t.string "record_type", null: false @@ -739,6 +739,9 @@ t.text "llm_instructions", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.integer "status", default: 1, null: false + t.string "uuid", null: false + t.index ["uuid"], name: "index_localization_glossary_entries_on_uuid", unique: true end create_table "localization_glossary_entry_proposals", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| diff --git a/test/controllers/api/localization/glossary_entries_controller_test.rb b/test/controllers/api/localization/glossary_entries_controller_test.rb index 9f99f0c778..b959b8c0a0 100644 --- a/test/controllers/api/localization/glossary_entries_controller_test.rb +++ b/test/controllers/api/localization/glossary_entries_controller_test.rb @@ -24,8 +24,7 @@ class API::Localization::GlossaryEntriesControllerTest < API::BaseTestCase test "show returns a localization glossary entry with proposals" do setup_user glossary_entry = create :localization_glossary_entry - translation = create :localization_translation, key: glossary_entry.key, locale: "en" - create(:localization_translation_proposal, translation:) + create :localization_glossary_entry_proposal, :modification, glossary_entry: glossary_entry, proposer: @current_user get api_localization_glossary_entry_path(glossary_entry.uuid), headers: @headers, as: :json diff --git a/test/controllers/api/localization/glossary_entries_proposals_controller_test.rb b/test/controllers/api/localization/glossary_entries_proposals_controller_test.rb index ee00c5ceca..664b73a6f5 100644 --- a/test/controllers/api/localization/glossary_entries_proposals_controller_test.rb +++ b/test/controllers/api/localization/glossary_entries_proposals_controller_test.rb @@ -13,42 +13,42 @@ class API::Localization::GlossaryEntriesProposalsControllerTest < API::BaseTestC post api_localization_glossary_entry_proposals_path(glossary_entry), params: { value: value }, headers: @headers, as: :json assert_response :created - expected = { glossary_entry: SerializeLocalizationGlossaryEntry.(glossary_entry) } + expected = { glossary_entry: SerializeLocalizationGlossaryEntry.(glossary_entry, @current_user) } assert_json_response(expected) end test "approve proxies to approval" do setup_user glossary_entry = create :localization_glossary_entry - proposal = create :localization_glossary_entry_proposal, glossary_entry: glossary_entry, proposer: @current_user + proposal = create :localization_glossary_entry_proposal, :modification, glossary_entry: glossary_entry, proposer: @current_user - Localization::GlossaryEntryProposal::Approve.expects(:call).with(glossary_entry, @current_user) + Localization::GlossaryEntryProposal::Approve.expects(:call).with(proposal, @current_user) patch approve_api_localization_glossary_entry_proposal_path(glossary_entry, proposal), headers: @headers, as: :json assert_response :ok - expected = { glossary_entry: SerializeLocalizationGlossaryEntry.(glossary_entry) } + expected = { glossary_entry: SerializeLocalizationGlossaryEntry.(glossary_entry, @current_user) } assert_json_response(expected) end test "reject proxies to rejection" do setup_user glossary_entry = create :localization_glossary_entry - proposal = create :localization_glossary_entry_proposal, glossary_entry: glossary_entry, proposer: @current_user + proposal = create :localization_glossary_entry_proposal, :modification, glossary_entry: glossary_entry, proposer: @current_user - Localization::GlossaryEntryProposal::Reject.expects(:call).with(glossary_entry, @current_user) + Localization::GlossaryEntryProposal::Reject.expects(:call).with(proposal, @current_user) patch reject_api_localization_glossary_entry_proposal_path(glossary_entry, proposal), headers: @headers, as: :json assert_response :ok - expected = { glossary_entry: SerializeLocalizationGlossaryEntry.(glossary_entry) } + expected = { glossary_entry: SerializeLocalizationGlossaryEntry.(glossary_entry, @current_user) } assert_json_response(expected) end test "edit proxies to update value if it's the same user" do setup_user glossary_entry = create :localization_glossary_entry - proposal = create :localization_glossary_entry_proposal, glossary_entry: glossary_entry, proposer: @current_user + proposal = create :localization_glossary_entry_proposal, :modification, glossary_entry: glossary_entry, proposer: @current_user new_value = "Updated proposal value" Localization::GlossaryEntryProposal::UpdateValue.expects(:call).with(proposal, @current_user, new_value) @@ -57,14 +57,14 @@ class API::Localization::GlossaryEntriesProposalsControllerTest < API::BaseTestC as: :json assert_response :ok - expected = { glossary_entry: SerializeLocalizationGlossaryEntry.(glossary_entry) } + expected = { glossary_entry: SerializeLocalizationGlossaryEntry.(glossary_entry, @current_user) } assert_json_response(expected) end test "rejects and creates a new proposal if the user is different" do setup_user glossary_entry = create :localization_glossary_entry - proposal = create :localization_glossary_entry_proposal, glossary_entry: glossary_entry + proposal = create :localization_glossary_entry_proposal, :modification, glossary_entry: glossary_entry new_value = "Updated proposal value" Localization::GlossaryEntryProposal::Reject.expects(:call).with(proposal, @current_user) @@ -74,7 +74,7 @@ class API::Localization::GlossaryEntriesProposalsControllerTest < API::BaseTestC as: :json assert_response :ok - expected = { glossary_entry: SerializeLocalizationGlossaryEntry.(glossary_entry) } + expected = { glossary_entry: SerializeLocalizationGlossaryEntry.(glossary_entry, @current_user) } assert_json_response(expected) end end diff --git a/test/models/localization/glossary_entry_test.rb b/test/models/localization/glossary_entry_test.rb index e733ea301e..689afb4e8f 100644 --- a/test/models/localization/glossary_entry_test.rb +++ b/test/models/localization/glossary_entry_test.rb @@ -1,7 +1,30 @@ require "test_helper" class Localization::GlossaryEntryTest < ActiveSupport::TestCase - # test "the truth" do - # assert true - # end + test "has statuses" do + assert_equal 4, Localization::GlossaryEntry.statuses.size + assert_equal %i[generating unchecked proposed checked], Localization::GlossaryEntry.statuses.keys.map(&:to_sym) + end + + test "status defaults to unchecked on create" do + glossary_entry = create :localization_glossary_entry + assert_equal :unchecked, glossary_entry.status + end + + test "generates uuid on create" do + glossary_entry = create :localization_glossary_entry + refute_nil glossary_entry.uuid + assert_equal 36, glossary_entry.uuid.length + end + + test "to_param returns uuid" do + glossary_entry = create :localization_glossary_entry + assert_equal glossary_entry.uuid, glossary_entry.to_param + end + + test "status returns symbol" do + glossary_entry = build :localization_glossary_entry + glossary_entry.status = :checked + assert_equal :checked, glossary_entry.status + end end