Update GlossaryEntryProposal commands to manage status and add Search tests

iHiD · claude · iHiD · commit 0ea47fd8b7b7 · 2025-09-10T12:09:33.000+02:00
- Update GlossaryEntryProposal::Approve to set glossary_entry status to :checked - Update GlossaryEntryProposal::Reject to set status to :unchecked when no pending proposals remain - Add locale parameter support to GlossaryEntry::Search - Create comprehensive test suite for GlossaryEntry::Search following Original::SearchTest pattern - Update assembler to pass filter_locale parameter to Search command 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
diff --git a/app/assemblers/assemble_localization_glossary_entries.rb b/app/assemblers/assemble_localization_glossary_entries.rb
@@ -24,7 +24,8 @@ def glossary_entries
       user,
       criteria: params[:criteria],
       status: params[:status],
-      page: params[:page]
+      page: params[:page],
+      locale: params[:filter_locale]
     )
   end
 end
diff --git a/app/commands/localization/glossary_entry/search.rb b/app/commands/localization/glossary_entry/search.rb
@@ -8,11 +8,12 @@ def self.default_per
     DEFAULT_PER
   end
 
-  def initialize(user, page: nil, per: nil, criteria: nil, status: nil)
+  def initialize(user, page: nil, per: nil, criteria: nil, status: nil, locale: nil)
     @user = user
 
     @criteria = criteria
     @status = status
+    @locale = locale
     @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
@@ -22,6 +23,7 @@ def call
 
     filter_criteria!
     filter_status!
+    filter_locale!
 
     paginated_glossary_entries = @glossary_entries.
       order(:locale, :term).
@@ -37,7 +39,7 @@ def call
   def locales = user.translator_locales - [:en]
 
   private
-  attr_reader :user, :per, :page, :criteria, :status
+  attr_reader :user, :per, :page, :criteria, :status, :locale
 
   def filter_criteria!
     return if criteria.blank?
@@ -53,4 +55,10 @@ def filter_status!
 
     @glossary_entries = @glossary_entries.where("localization_glossary_entries.status": status)
   end
+
+  def filter_locale!
+    return if locale.blank?
+
+    @glossary_entries = @glossary_entries.where("localization_glossary_entries.locale": locale)
+  end
 end
diff --git a/app/commands/localization/glossary_entry_proposal/approve.rb b/app/commands/localization/glossary_entry_proposal/approve.rb
@@ -25,7 +25,8 @@ def handle_addition!
       locale: proposal.locale,
       term: proposal.term,
       translation: proposal.translation,
-      llm_instructions: proposal.llm_instructions
+      llm_instructions: proposal.llm_instructions,
+      status: :checked
     )
   end
 
@@ -34,7 +35,8 @@ def handle_modification!
 
     proposal.glossary_entry.update!(
       translation: proposal.translation,
-      llm_instructions: proposal.llm_instructions
+      llm_instructions: proposal.llm_instructions,
+      status: :checked
     )
   end
 
diff --git a/app/commands/localization/glossary_entry_proposal/reject.rb b/app/commands/localization/glossary_entry_proposal/reject.rb
@@ -9,6 +9,12 @@ def call
         status: :rejected,
         reviewer: user
       )
+
+      # Set glossary_entry status to unchecked if no pending proposals remain (for modifications only)
+      if proposal.type == :modification && proposal.glossary_entry
+        glossary_entry = proposal.glossary_entry
+        glossary_entry.update!(status: :unchecked) unless glossary_entry.proposals.pending.exists?
+      end
     end
   end
 end
diff --git a/test/commands/localization/glossary_entry/search_test.rb b/test/commands/localization/glossary_entry/search_test.rb
@@ -0,0 +1,226 @@
+require 'test_helper'
+
+class Localization::GlossaryEntry::SearchTest < ActiveSupport::TestCase
+  test "searches for user's locales" do
+    user = create :user
+    user.stubs(translator_locales: %i[hu nl])
+
+    glossary_entry_1 = create :localization_glossary_entry, locale: "hu", term: "term1", status: :checked
+    glossary_entry_2 = create :localization_glossary_entry, locale: "nl", term: "term2", status: :checked
+
+    # Different locale (not in user's translator_locales)
+    create :localization_glossary_entry, locale: "fr", term: "term3", status: :checked
+
+    # English locale (should be excluded)
+    create :localization_glossary_entry, locale: "en", term: "term4", status: :checked
+
+    actual = Localization::GlossaryEntry::Search.(user)
+
+    expected = [glossary_entry_1, glossary_entry_2]
+    assert_equal expected, actual.to_a
+  end
+
+  test "searches for unchecked glossary entries" do
+    user = create :user
+    user.stubs(translator_locales: %i[hu nl])
+
+    glossary_entry_1 = create :localization_glossary_entry, locale: "hu", term: "term1", status: :unchecked
+    glossary_entry_2 = create :localization_glossary_entry, locale: "nl", term: "term2", status: :unchecked
+
+    # Proposed
+    create :localization_glossary_entry, locale: "hu", term: "term3", status: :proposed
+
+    # Checked
+    create :localization_glossary_entry, locale: "hu", term: "term4", status: :checked
+
+    actual = Localization::GlossaryEntry::Search.(user, status: :unchecked)
+
+    expected = [glossary_entry_1, glossary_entry_2]
+    assert_equal expected, actual.to_a
+  end
+
+  test "searches for checked glossary entries" do
+    user = create :user
+    user.stubs(translator_locales: %i[hu nl])
+
+    glossary_entry_1 = create :localization_glossary_entry, locale: "hu", term: "term1", status: :checked
+    glossary_entry_2 = create :localization_glossary_entry, locale: "nl", term: "term2", status: :checked
+
+    # Unchecked
+    create :localization_glossary_entry, locale: "hu", term: "term3", status: :unchecked
+
+    # Proposed
+    create :localization_glossary_entry, locale: "hu", term: "term4", status: :proposed
+
+    actual = Localization::GlossaryEntry::Search.(user, status: :checked)
+
+    expected = [glossary_entry_1, glossary_entry_2]
+    assert_equal expected, actual.to_a
+  end
+
+  test "honours criteria for term" do
+    user = create :user
+    user.stubs(translator_locales: %i[hu nl])
+
+    glossary_entry_1 = create :localization_glossary_entry, locale: "hu", term: "foobar", translation: "something", status: :checked
+    create :localization_glossary_entry, locale: "nl", term: "bazqux", translation: "other", status: :checked
+
+    # Different locale (should not be included)
+    create :localization_glossary_entry, locale: "fr", term: "foobar", translation: "something", status: :checked
+
+    actual = Localization::GlossaryEntry::Search.(user, criteria: "foo")
+
+    expected = [glossary_entry_1]
+    assert_equal expected, actual.to_a
+  end
+
+  test "honours criteria for translation" do
+    user = create :user
+    user.stubs(translator_locales: %i[hu nl])
+
+    glossary_entry_1 = create :localization_glossary_entry, locale: "hu", term: "term1", translation: "contains foo here",
+      status: :checked
+    create :localization_glossary_entry, locale: "nl", term: "term2", translation: "contains bar here",
+      status: :checked
+
+    # Different locale (should not be included)
+    create :localization_glossary_entry, locale: "fr", term: "term3", translation: "contains foo here",
+      status: :checked
+
+    actual = Localization::GlossaryEntry::Search.(user, criteria: "foo")
+
+    expected = [glossary_entry_1]
+    assert_equal expected, actual.to_a
+  end
+
+  test "filters by locale" do
+    user = create :user
+    user.stubs(translator_locales: %i[hu nl fr])
+
+    glossary_entry_1 = create :localization_glossary_entry, locale: "hu", term: "term1", status: :checked
+    glossary_entry_2 = create :localization_glossary_entry, locale: "hu", term: "term2", status: :checked
+    create :localization_glossary_entry, locale: "nl", term: "term3", status: :checked
+    create :localization_glossary_entry, locale: "fr", term: "term4", status: :checked
+
+    actual = Localization::GlossaryEntry::Search.(user, locale: "hu")
+
+    expected = [glossary_entry_1, glossary_entry_2]
+    assert_equal expected, actual.to_a
+  end
+
+  test "combines filters" do
+    user = create :user
+    user.stubs(translator_locales: %i[hu nl])
+
+    glossary_entry_1 = create :localization_glossary_entry, locale: "hu", term: "foobar", status: :unchecked
+    create :localization_glossary_entry, locale: "hu", term: "bazqux", status: :unchecked
+    create :localization_glossary_entry, locale: "hu", term: "foobar", status: :checked
+    create :localization_glossary_entry, locale: "nl", term: "foobar", status: :unchecked
+
+    actual = Localization::GlossaryEntry::Search.(user, criteria: "foo", status: :unchecked, locale: "hu")
+
+    expected = [glossary_entry_1]
+    assert_equal expected, actual.to_a
+  end
+
+  test "orders by locale then term" do
+    user = create :user
+    user.stubs(translator_locales: %i[hu nl])
+
+    glossary_entry_1 = create :localization_glossary_entry, locale: "nl", term: "zebra", status: :checked
+    glossary_entry_2 = create :localization_glossary_entry, locale: "hu", term: "apple", status: :checked
+    glossary_entry_3 = create :localization_glossary_entry, locale: "hu", term: "banana", status: :checked
+    glossary_entry_4 = create :localization_glossary_entry, locale: "nl", term: "apple", status: :checked
+
+    actual = Localization::GlossaryEntry::Search.(user)
+
+    expected = [glossary_entry_2, glossary_entry_3, glossary_entry_4, glossary_entry_1]
+    assert_equal expected, actual.to_a
+  end
+
+  test "paginates" do
+    Localization::GlossaryEntry::Search.stubs(:default_per).returns(1)
+
+    user = create :user
+    user.stubs(translator_locales: %i[hu nl])
+
+    glossary_entry_1 = create :localization_glossary_entry, locale: "hu", term: "term1", status: :checked
+    glossary_entry_2 = create :localization_glossary_entry, locale: "nl", term: "term2", status: :checked
+
+    actual = Localization::GlossaryEntry::Search.(user, page: 1)
+    assert_equal 2, actual.total_count
+    assert_equal 1, actual.size
+    assert_includes [glossary_entry_1.id, glossary_entry_2.id], actual.first.id
+    assert_equal 1, actual.current_page
+    assert_equal 2, actual.total_pages
+
+    actual = Localization::GlossaryEntry::Search.(user, page: 2)
+    assert_equal 2, actual.total_count
+    assert_equal 1, actual.size
+    assert_includes [glossary_entry_1.id, glossary_entry_2.id], actual.first.id
+    assert_equal 2, actual.current_page
+    assert_equal 2, actual.total_pages
+
+    # Ensure we got different items on each page
+    page1_ids = Localization::GlossaryEntry::Search.(user, page: 1).map(&:id)
+    page2_ids = Localization::GlossaryEntry::Search.(user, page: 2).map(&:id)
+    assert_equal [glossary_entry_1.id, glossary_entry_2.id].sort, (page1_ids + page2_ids).sort
+  end
+
+  test "handles custom per page" do
+    user = create :user
+    user.stubs(translator_locales: [:hu])
+
+    create :localization_glossary_entry, locale: "hu", term: "term1", status: :checked
+    create :localization_glossary_entry, locale: "hu", term: "term2", status: :checked
+    create :localization_glossary_entry, locale: "hu", term: "term3", status: :checked
+
+    actual = Localization::GlossaryEntry::Search.(user, per: 2, page: 1)
+    assert_equal 3, actual.total_count
+    assert_equal 2, actual.size
+    assert_equal 1, actual.current_page
+    assert_equal 2, actual.total_pages
+
+    actual = Localization::GlossaryEntry::Search.(user, per: 2, page: 2)
+    assert_equal 3, actual.total_count
+    assert_equal 1, actual.size
+    assert_equal 2, actual.current_page
+    assert_equal 2, actual.total_pages
+  end
+
+  test "excludes english locale" do
+    user = create :user
+    user.stubs(translator_locales: %i[en hu nl])
+
+    glossary_entry_1 = create :localization_glossary_entry, locale: "hu", term: "term1", status: :checked
+    glossary_entry_2 = create :localization_glossary_entry, locale: "nl", term: "term2", status: :checked
+    create :localization_glossary_entry, locale: "en", term: "term3", status: :checked
+
+    actual = Localization::GlossaryEntry::Search.(user)
+
+    expected = [glossary_entry_1, glossary_entry_2]
+    assert_equal expected, actual.to_a
+  end
+
+  test "returns empty array when user has no translator locales" do
+    user = create :user
+    user.stubs(translator_locales: [])
+
+    create :localization_glossary_entry, locale: "hu", term: "term1", status: :checked
+
+    actual = Localization::GlossaryEntry::Search.(user)
+
+    assert_empty actual.to_a
+  end
+
+  test "returns empty array when user only has english locale" do
+    user = create :user
+    user.stubs(translator_locales: [:en])
+
+    create :localization_glossary_entry, locale: "en", term: "term1", status: :checked
+
+    actual = Localization::GlossaryEntry::Search.(user)
+
+    assert_empty actual.to_a
+  end
+end
diff --git a/test/commands/localization/glossary_entry_proposal/approve_test.rb b/test/commands/localization/glossary_entry_proposal/approve_test.rb
@@ -24,6 +24,7 @@ class Localization::GlossaryEntryProposal::ApproveTest < ActiveSupport::TestCase
     assert_equal term, glossary_entry.term
     assert_equal translation, glossary_entry.translation
     assert_equal llm_instructions, glossary_entry.llm_instructions
+    assert_equal :checked, glossary_entry.status
   end
 
   test "modification" do
@@ -38,8 +39,9 @@ class Localization::GlossaryEntryProposal::ApproveTest < ActiveSupport::TestCase
 
     assert_equal :approved, proposal.reload.status
     assert_equal user, proposal.reviewer
-    assert_equal translation, glossary_entry.translation
+    assert_equal translation, glossary_entry.reload.translation
     assert_equal llm_instructions, glossary_entry.llm_instructions
+    assert_equal :checked, glossary_entry.status
   end
 
   test "deletion" do
diff --git a/test/commands/localization/glossary_entry_proposal/reject_test.rb b/test/commands/localization/glossary_entry_proposal/reject_test.rb
@@ -3,7 +3,7 @@
 class Localization::GlossaryEntryProposal::RejectTest < ActiveSupport::TestCase
   test "reject proposal with no other ones" do
     user = create :user
-    glossary_entry = create :localization_glossary_entry, term: "wild_term", locale: "en"
+    glossary_entry = create :localization_glossary_entry, term: "wild_term", locale: "en", status: :checked
     proposal = create :localization_glossary_entry_proposal, :modification, glossary_entry: glossary_entry, proposer: user,
       translation: "Something wiiiiiild"
 
@@ -13,11 +13,12 @@ class Localization::GlossaryEntryProposal::RejectTest < ActiveSupport::TestCase
     assert_equal user, proposal.reviewer
 
     assert_equal 0, glossary_entry.proposals.pending.count
+    assert_equal :unchecked, glossary_entry.reload.status
   end
 
   test "reject proposals with other pending ones" do
     user = create :user
-    glossary_entry = create :localization_glossary_entry, term: "wild_term", locale: "hu"
+    glossary_entry = create :localization_glossary_entry, term: "wild_term", locale: "hu", status: :checked
     proposal_1 = create :localization_glossary_entry_proposal, :modification, glossary_entry: glossary_entry, proposer: user,
       translation: "Something wiiiiiild"
     create :localization_glossary_entry_proposal, :modification, glossary_entry: glossary_entry, proposer: user,
@@ -29,5 +30,35 @@ class Localization::GlossaryEntryProposal::RejectTest < ActiveSupport::TestCase
     assert_equal user, proposal_1.reviewer
 
     assert_equal 1, glossary_entry.proposals.pending.count
+    assert_equal :checked, glossary_entry.reload.status # Should remain checked since there's still a pending proposal
+  end
+
+  test "reject addition proposal" do
+    user = create :user
+    proposal = create :localization_glossary_entry_proposal, :addition, proposer: user,
+      term: "new_term", locale: "en", translation: "A new term"
+
+    Localization::GlossaryEntryProposal::Reject.(proposal, user)
+
+    assert_equal :rejected, proposal.reload.status
+    assert_equal user, proposal.reviewer
+
+    # No glossary entry should be created for rejected additions
+    assert_equal 0, Localization::GlossaryEntry.where(term: "new_term", locale: "en").count
+  end
+
+  test "reject deletion proposal" do
+    user = create :user
+    glossary_entry = create :localization_glossary_entry, term: "term_to_delete", locale: "en", status: :checked
+    proposal = create :localization_glossary_entry_proposal, :deletion, glossary_entry: glossary_entry, proposer: user
+
+    Localization::GlossaryEntryProposal::Reject.(proposal, user)
+
+    assert_equal :rejected, proposal.reload.status
+    assert_equal user, proposal.reviewer
+
+    # Glossary entry should still exist and status should remain unchanged
+    assert glossary_entry.reload
+    assert_equal :checked, glossary_entry.status
   end
 end
