Add tests for advertised ransack parameters and fix regressions

Our documentation advertises that users, devices, and sensors are searchable
with ransack - this commit adds some tests that assert (in a very basic manner) that this will succeed, and fixes a few cases where we weren't actually supporting the behaviour we advertise.

app/controllers/v0/application_controller.rb

@@ -29,6 +29,14 @@ def check_missing_params *params_list
 
     private
 
+    def raise_ransack_errors_as_bad_request(&block)
+      begin
+        block.call
+      rescue ArgumentError => e
+        raise ActionController::BadRequest.new(e.message)
+      end
+    end
+
     def prepend_view_paths
       # is this still necessary?
       prepend_view_path "app/views/v0"

app/controllers/v0/devices_controller.rb

@@ -13,10 +13,11 @@ def show
     end
 
     def index
-      @q = policy_scope(Device)
-        .includes(:owner, :tags, kit: [:components, :sensors])
-        .ransack(params[:q], auth_object: (current_user&.is_admin? ? :admin : nil))
-
+      raise_ransack_errors_as_bad_request do
+        @q = policy_scope(Device)
+          .includes(:owner, :tags, kit: [:components, :sensors])
+          .ransack(params[:q], auth_object: (current_user&.is_admin? ? :admin : nil))
+      end
       # We are here customly adding multiple tags into the Ransack query.
       # Ransack supports this, but how do we add multiple tag names in URL string? Which separator to use?
       # See Issue #186 https://github.com/fablabbcn/smartcitizen-api/issues/186

app/controllers/v0/sensors_controller.rb

@@ -7,7 +7,11 @@ def show
     end
 
     def index
-      @sensors = Sensor.includes(:measurement, :tag_sensors).all
+      raise_ransack_errors_as_bad_request do
+        @q = Sensor.includes(:measurement, :tag_sensors).ransack(params[:q])
+      end
+      @q.sorts = 'id asc' if @q.sorts.empty?
+      @sensors = @q.result(distinct: true)
       @sensors = paginate @sensors
     end
 

app/controllers/v0/users_controller.rb

@@ -9,7 +9,9 @@ def show
     end
 
     def index
-      @q = User.includes(:devices, :profile_picture_attachment).ransack(params[:q])
+      raise_ransack_errors_as_bad_request do
+        @q = User.includes(:devices, :profile_picture_attachment).ransack(params[:q])
+      end
       @q.sorts = 'id asc' if @q.sorts.empty?
       @users = @q.result(distinct: true)
       @users = paginate(@users)

app/models/device.rb

@@ -73,6 +73,15 @@ class Device < ActiveRecord::Base
     end
   end
 
+  def self.ransackable_attributes(auth_object = nil)
+    if auth_object == :admin
+      # admin can ransack on every attribute
+      self.authorizable_ransackable_attributes
+    else
+      ["id", "name", "description", "created_at", "updated_at", "last_recorded_at", "state","geohash", "uuid", "kit_id"]
+    end
+  end
+
   def self.ransackable_associations(auth_object = nil)
     [
       "components", "devices_tags", "kit", "owner",
@@ -268,16 +277,6 @@ def remove_mac_address_for_newly_registered_device!
     update(old_mac_address: mac_address, mac_address: nil)
   end
 
-  def self.ransackable_attributes(auth_object = nil)
-    if auth_object == :admin
-      # admin can ransack on every attribute
-      super
-    else
-      # normal users can NOT ransack on device_token
-      column_names - ['device_token'] + _ransackers.keys
-    end
-  end
-
   private
 
     def set_state

app/models/postprocessing.rb

@@ -4,4 +4,8 @@ class Postprocessing < ApplicationRecord
   def self.ransackable_attributes(auth_object = nil)
     ["blueprint_url", "created_at", "device_id", "forwarding_params", "hardware_url", "id", "latest_postprocessing", "meta", "updated_at"]
   end
+
+  def self_ransackable_associations(auth_object = nil)
+    []
+  end
 end

app/models/sensor.rb

@@ -18,6 +18,14 @@ class Sensor < ActiveRecord::Base
   has_ancestry
   validates_presence_of :name, :description#, :unit
 
+  def self.ransackable_attributes(auth_object = nil)
+    ["ancestry", "created_at", "description", "id", "measurement_id", "name", "unit", "updated_at", "uuid"]
+  end
+
+  def self.ransackable_associations(auth_object = nil)
+    []
+  end
+
   def tags
     tag_sensors.map(&:name)
   end

app/models/tag.rb

@@ -10,4 +10,12 @@ class Tag < ActiveRecord::Base
 
   extend FriendlyId
   friendly_id :name
+
+  def self.ransackable_attributes(auth_object = nil)
+    ["created_at", "description", "id", "name", "updated_at", "uuid"]
+  end
+
+  def ransackable_associations(auth_object = nil)
+    []
+  end
 end

app/models/user.rb

@@ -42,7 +42,7 @@ class User < ActiveRecord::Base
   alias_attribute :joined_at, :created_at
 
   def self.ransackable_attributes(auth_object = nil)
-    [ "city", "country_code", "email", "id", "url", "username", "uuid", "created_at", "joined_at", "updated_at"]
+    [ "city", "country_code", "id", "username", "uuid", "created_at", "updated_at"]
   end
 
   def self.ransackable_associations(auth_object = nil)

config/initializers/ransack.rb

@@ -0,0 +1,5 @@
+Ransack.configure do |config|
+    # Raise errors if a query contains an unknown predicate or attribute.
+    # Default is true (do not raise error on unknown conditions).
+    config.ignore_unknown_conditions = false
+  end

spec/controllers/v0/devices_controller_spec.rb

@@ -2,4 +2,6 @@
 
 RSpec.describe V0::DevicesController do
   skip { is_expected.to permit(:name,:description,:mac_address,:latitude,:longitude,:elevation,:exposure,:meta,:kit_id,:user_tags).for(:create) }
+
+
 end

spec/models/device_spec.rb

@@ -298,8 +298,10 @@
       expect(Device.count).to eq(3)
       # Admins can ransack on device_token_contains
       expect(Device.ransack({device_token_cont: '123'}, auth_object: :admin).result.count).to eq(1)
-      # Normal users get all deviecs returned. They cannot ransack on device_token
-      expect(Device.ransack({device_token_cont: '123'}).result.count).to eq(3)
+      # Normal users cannot ransack on device_token
+      expect {
+        Device.ransack({device_token_cont: '123'})
+      }.to raise_error(ArgumentError)
     end
 
     it 'does not validate uniqueness when nil' do

spec/requests/v0/devices_spec.rb

@@ -114,11 +114,92 @@
         expect(response.status).to eq(400)
       end
 
+      it "allows searching by id" do
+        json = api_get "devices?q[id_eq]=1"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by name" do
+        json = api_get "devices?q[name_eq]=Name"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by description" do
+        json = api_get "devices?q[description_eq]=Desc"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by created_at" do
+        json = api_get "devices?q[created_at_lt]=2023-09-26"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by updated_at" do
+        json = api_get "devices?q[updated_at_lt]=2023-09-26"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by last_recorded_at" do
+        json = api_get "devices?q[last_recorded_at_lt]=2023-09-26"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by state" do
+        json = api_get "devices?q[state_eq]=state"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by geohash" do
+        json = api_get "devices?q[geohash_eq]=geohash"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by uuid" do
+        json = api_get "devices?q[uuid_eq]=uuid"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by owner id" do
+        json = api_get "devices?q[owner_id_eq]=1"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by owner username" do
+        json = api_get "devices?q[owner_username_eq]=test"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by tag name" do
+        json = api_get "devices?q[tags_name_eq]=test"
+        expect(response.status).to eq(200)
+      end
+
       it "allows searching by presence of postprocessing" do
         json = api_get "devices?q[postprocessing_id_not_null]=1"
         expect(response.status).to eq(200)
       end
 
+      it "allows searching by postprocessing id" do
+        json = api_get "devices?q[postprocessing_id_eq]=1"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by mac address by admins" do
+        json = api_get "devices?q[mac_address_eq]=00:00:00:00:00:00&access_token=#{admin_token.token}"
+        expect(response.status).to eq(200)
+      end
+
+      it "does not allow searching by mac address by non-admins" do
+        expect {
+          api_get "devices?q[mac_address_eq]=00:00:00:00:00:00"
+        }.to raise_error(ActionController::BadRequest)
+      end
+
+      it "does not allow searching on disallowed parameters" do
+        expect {
+          api_get "devices?q[disallowed_eq]=1"
+        }.to raise_error(ActionController::BadRequest)
+      end
     end
   end
 

spec/requests/v0/sensors_spec.rb

@@ -26,6 +26,61 @@
       expect(j[0]['name']).to eq('testing sensor')
       expect(response.status).to eq(200)
     end
+
+    describe "smoke tests for ransack" do
+
+      it "allows searching by ancestry" do
+        json = api_get "sensors?q[ancestry_eq]=1"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by created_at" do
+        json = api_get "sensors?q[created_at_eq]=1"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by description" do
+        json = api_get "sensors?q[description_eq]=1"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by id" do
+        json = api_get "sensors?q[id_lt]=100"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by measurement_id" do
+        json = api_get "sensors?q[measurement_id_eq]=1"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by name" do
+        json = api_get "sensors?q[name_eq]=name"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by unit" do
+        json = api_get "sensors?q[unit_eq]=ppm"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by updated_at" do
+        json = api_get "sensors?q[updated_at_eq]=1"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by uuid" do
+        json = api_get "sensors?q[uuid_eq]=1"
+        expect(response.status).to eq(200)
+      end
+
+      it "does not allow searching on disallowed parameters" do
+        expect {
+          api_get "sensors?q[disallowed_eq]=1"
+        }.to raise_error(ActionController::BadRequest)
+      end
+
+    end
   end
 
   describe "POST /sensors" do

spec/requests/v0/users_spec.rb

@@ -42,6 +42,54 @@
       expect(j['email']).to eq(user.email)
     end
 
+    describe "smoke tests for ransack" do
+      it "does not allow searching by first name" do
+        expect {
+          api_get "users?q[first_name_eq]=Tim"
+        }.to raise_error(ActionController::BadRequest)
+      end
+
+      it "allows searching by city" do
+        json = api_get "users?q[city_eq]=Barcelona"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by country code" do
+        json = api_get "users?q[country_code_eq]=es"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by id" do
+        json = api_get "users?q[id_eq]=1"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by username" do
+        json = api_get "users?q[username_eq]=mistertim"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by uuid" do
+        json = api_get "users?q[uuid_eq]=1"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by created_at" do
+        json = api_get "users?q[created_at_eq]=1"
+        expect(response.status).to eq(200)
+      end
+
+      it "allows searching by updated_at" do
+        json = api_get "users?q[updated_at_eq]=1"
+        expect(response.status).to eq(200)
+      end
+
+      it "does not allow searching on disallowed parameters" do
+        expect {
+          api_get "users?q[disallowed_eq]=1"
+        }.to raise_error(ActionController::BadRequest)
+      end
+    end
   end
 
   # index