Merge pull request #1887 from codidact/0valt/1865/filters

"Open" system filter & a bit of cleanup

Author: Oaphi

app/controllers/users_controller.rb

@@ -94,7 +94,9 @@ def preferences
     end
   end
 
-  # Helper method to convert it to the form expected by the client
+  # Converts a given filter to JSON
+  # @param filter {Filter} filter to convert
+  # @return {Hash}
   def filter_json(filter)
     {
       min_score: filter.min_score,
@@ -105,20 +107,23 @@ def filter_json(filter)
       exclude_tags: Tag.where(id: filter.exclude_tags).map { |tag| [tag.name, tag.id] },
       source: filter.source,
       status: filter.status,
-      system: filter.user_id == -1
+      system: filter.system?
     }
   end
 
-  def filters_json
-    system_filters = Rails.cache.fetch 'default_system_filters', expires_in: 1.day do
+  # Gets system filters as JSON
+  # @return [Hash{String => Hash}]
+  def system_filters_json
+    Rails.cache.fetch 'default_system_filters', expires_in: 1.day do
       system_user.filters.to_h { |filter| [filter.name, filter_json(filter)] }
     end
+  end
 
+  def filters_json
     if user_signed_in?
-      current_user.filters.to_h { |filter| [filter.name, filter_json(filter)] }
-                  .merge(system_filters)
+      current_user.filters.to_h { |filter| [filter.name, filter_json(filter)] }.merge(system_filters_json)
     else
-      system_filters
+      system_filters_json
     end
   end
 

app/models/filter.rb

@@ -4,4 +4,8 @@ class Filter < ApplicationRecord
   validates :name, uniqueness: { scope: :user }
   serialize :include_tags, coder: YAML, type: Array
   serialize :exclude_tags, coder: YAML, type: Array
+
+  def system?
+    user_id == -1
+  end
 end

db/seeds/filters.yml

@@ -11,4 +11,8 @@
   user_id: -1
   min_answers: 0
   max_answers: 0
-  status: 'open'
+  status: 'open'
+
+- name: Open
+  user_id: -1
+  status: 'open'

test/controllers/users_controller_test.rb

@@ -565,6 +565,19 @@ class UsersControllerTest < ActionController::TestCase
     assert_json_success
   end
 
+  test 'filters should only return system filters for anonymous users' do
+    try_filters
+
+    assert_response(:success)
+    assert_valid_json_response
+
+    parsed = JSON.parse(response.body)
+    assert parsed.any?
+    parsed.each do |name, filter|
+      assert filter['system'], "'#{name}' is not a system filter"
+    end
+  end
+
   private
 
   def create_other_user
@@ -575,6 +588,12 @@ def create_other_user
     other_user
   end
 
+  def try_filters
+    get :filters, params: {
+      format: :json
+    }
+  end
+
   def try_default_filter(category)
     get :default_filter, params: {
       category: category&.id,

test/fixtures/filters.yml

@@ -17,3 +17,8 @@ two:
   min_answers: 1
   max_answers: 1
   status: MyString
+
+system:
+  name: System filter
+  user: system
+  status: open

test/models/filter_test.rb

@@ -1,7 +1,11 @@
 require 'test_helper'
 
 class FilterTest < ActiveSupport::TestCase
-  # test "the truth" do
-  #   assert true
-  # end
+  test 'system? should correctly determine if a filter is a system filter' do
+    sys_usr = users(:system)
+
+    filters.each do |filter|
+      assert_equal filter.system?, filter.user_id == sys_usr
+    end
+  end
 end