Merge pull request #257 from fablabbcn/bugfix/256-test-advertised-ran…

…sack-parameters-function

Add tests for advertised ransack parameters and fix regressions

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
+        render json: { message: e.message, status: 400 }, status: 400
+      end
+    end
+
     def prepend_view_paths
       # is this still necessary?
       prepend_view_path "app/views/v0"

app/controllers/v0/devices_controller.rb

@@ -13,34 +13,34 @@ 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))
-
-      # 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
-      # If we figure it out, we can remove the next 3 lines, but remember to document in:
-      # https://developer.smartcitizen.me/#basic-searching
-      if params[:with_tags]
-        @q.tags_name_in = params[:with_tags].split('|')
-      end
+      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))
+        # 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
+        # If we figure it out, we can remove the next 3 lines, but remember to document in:
+        # https://developer.smartcitizen.me/#basic-searching
+        if params[:with_tags]
+          @q.tags_name_in = params[:with_tags].split('|')
+        end
 
-      @devices = @q.result(distinct: true)
-
-      if params[:near]
-        if params[:near] =~ /\A(\-?\d+(\.\d+)?),\s*(\-?\d+(\.\d+)?)\z/
-          @devices = @devices.near(
-            params[:near].split(','), (params[:within] || 1000))
-        else
-          return render json: { id: "bad_request",
-            message: "Malformed near parameter",
-            url: 'https://developer.smartcitizen.me/#get-all-devices',
-            errors: nil }, status: :bad_request
+        @devices = @q.result(distinct: true)
+
+        if params[:near]
+          if params[:near] =~ /\A(\-?\d+(\.\d+)?),\s*(\-?\d+(\.\d+)?)\z/
+            @devices = @devices.near(
+              params[:near].split(','), (params[:within] || 1000))
+          else
+            return render json: { id: "bad_request",
+              message: "Malformed near parameter",
+              url: 'https://developer.smartcitizen.me/#get-all-devices',
+              errors: nil }, status: :bad_request
+          end
         end
+        @devices = paginate(@devices)
       end
-
-      @devices = paginate(@devices)
     end
 
     def update

app/controllers/v0/sensors_controller.rb

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

app/controllers/v0/users_controller.rb

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

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
+        json = api_get "devices?q[mac_address_eq]=00:00:00:00:00:00"
+        expect(response.status).to eq(400)
+        expect(json["status"]).to eq(400)
+      end
+
+      it "does not allow searching on disallowed parameters" do
+        json = api_get "devices?q[disallowed_eq]=1"
+        expect(response.status).to eq(400)
+        expect(json["status"]).to eq(400)
+      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
+        json = api_get "sensors?q[disallowed_eq]=1"
+        expect(response.status).to eq(400)
+        expect(json["status"]).to eq(400)
+      end
+
+    end
   end
 
   describe "POST /sensors" do