harmonize world map and device view

Caching will need testing, as well as repercussions of removing lat/lng from user devices

Gemfile

@@ -8,7 +8,7 @@ gem 'doorkeeper', '~> 5'
 # To resize active storage images:
 # Revise if this is needed after Rails 6.0
 gem 'image_processing'
-
+gem 'actionpack-action_caching'
 gem 'ancestry'
 gem 'api-pagination'
 gem 'api_cache'

Gemfile.lock

@@ -53,6 +53,8 @@ GEM
       rack-test (>= 0.6.3)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.0, >= 1.2.0)
+    actionpack-action_caching (1.2.2)
+      actionpack (>= 4.0.0)
     actiontext (6.1.7.3)
       actionpack (= 6.1.7.3)
       activerecord (= 6.1.7.3)
@@ -531,6 +533,7 @@ PLATFORMS
   ruby
 
 DEPENDENCIES
+  actionpack-action_caching
   ancestry
   api-pagination
   api_cache
@@ -616,4 +619,4 @@ RUBY VERSION
    ruby 3.0.6p216
 
 BUNDLED WITH
-   2.4.13
+   2.5.6

app/controllers/v0/devices_controller.rb

@@ -1,10 +1,13 @@
+require 'actionpack/action_caching'
 module V0
   class DevicesController < ApplicationController
-
+    include ActionController::Caching
     before_action :check_if_authorized!, only: [:create]
     after_action :verify_authorized,
       except: [:index, :world_map, :fresh_world_map]
 
+    caches_action :world_map, expires_in: 1.minute
+
     def show
       @device = Device.includes(
         :owner, :sensors,:tags).find(params[:id])
@@ -76,15 +79,12 @@ def destroy
 
     # debug method, must be refactored
     def fresh_world_map
-      render json: Device.for_world_map(current_user&.is_admin?)
+      @devices = Device.for_world_map
+      render :world_map
     end
 
     def world_map
-      unless params[:cachebuster]
-        expires_in 30.seconds, public: true # CRON cURL every 60 seconds to cache
-      end
-
-      render json: Device.for_world_map(current_user&.is_admin?)
+      @devices = Device.for_world_map
     end
 
 private

app/models/device.rb

@@ -69,6 +69,10 @@ class Device < ActiveRecord::Base
     end
   end
 
+  scope :for_world_map, -> {
+    where.not(latitude: nil).where.not(data: nil).where(is_test: false).includes(:owner, :tags)
+  }
+
   def self.ransackable_attributes(auth_object = nil)
     if auth_object == :admin
       # admin can ransack on every attribute
@@ -228,32 +232,30 @@ def self.geocode_all_without_location
     end
   end
 
-  def self.for_world_map(authorized=false)
-    Rails.cache.fetch("world_map", expires_in: 10.seconds) do
-      where
-        .not(latitude: nil)
-        .where.not(data: nil)
-        .where(is_test: false)
-        .includes(:owner,:tags)
-        .map do |device|
-        {
-          id: device.id,
-          name: device.name,
-          description: (device.description.present? ? device.description : nil),
-          owner_id: device.owner_id,
-          owner_username: device.owner_id ? device.owner_username : nil,
-          latitude: device.latitude,
-          longitude: device.longitude,
-          city: device.city,
-          hardware: device.hardware(authorized),
-          country_code: device.country_code,
-          state: device.state,
-          system_tags: device.system_tags,
-          user_tags: device.user_tags,
-          updated_at: device.updated_at,
-          last_reading_at: (device.last_reading_at.present? ? device.last_reading_at : nil)
-        }
-      end
+  def self.old_for_world_map(authorized=false)
+    where
+      .not(latitude: nil)
+      .where.not(data: nil)
+      .where(is_test: false)
+      .includes(:owner,:tags)
+      .map do |device|
+      {
+        id: device.id,
+        name: device.name,
+        description: (device.description.present? ? device.description : nil),
+        owner_id: device.owner_id,
+        owner_username: device.owner_id ? device.owner_username : nil,
+        latitude: device.latitude,
+        longitude: device.longitude,
+        city: device.city,
+        hardware: device.hardware(authorized),
+        country_code: device.country_code,
+        state: device.state,
+        system_tags: device.system_tags,
+        user_tags: device.user_tags,
+        updated_at: device.updated_at,
+        last_reading_at: (device.last_reading_at.present? ? device.last_reading_at : nil)
+      }
     end
   end
 

app/views/v0/devices/_device.jbuilder

@@ -1,5 +1,8 @@
-with_owner = true unless local_assigns.has_key?(:with_owner)
-with_data = true unless local_assigns.has_key?(:with_data)
+local_assigns[:with_owner] = true unless local_assigns.has_key?(:with_owner)
+local_assigns[:with_data] = true unless local_assigns.has_key?(:with_data)
+local_assigns[:with_postprocessing] = true unless local_assigns.has_key?(:with_postprocessing)
+local_assigns[:with_location] = true unless local_assigns.has_key?(:with_location)
+local_assigns[:slim_owner] = false unless local_assigns.has_key?(:slim_owner)
 
 json.(
   device,
@@ -8,7 +11,6 @@ json.(
   :name,
   :description,
   :state,
-  :postprocessing,
   :system_tags,
   :user_tags,
   :is_private,
@@ -26,25 +28,26 @@ if authorized
 else
   json.merge! device_token: '[FILTERED]'
 end
-
-json.merge!(location: device.formatted_location)
+json.merge!(postprocessing: device.postprocessing) if local_assigns[:with_postprocessing]
+json.merge!(location: device.formatted_location) if local_assigns[:with_location]
 json.merge!(hardware: device.hardware(authorized))
 
-if with_owner && device.owner
+if local_assigns[:with_owner] && device.owner
   json.owner do
     json.id device.owner.id
     json.uuid device.owner.uuid
     json.username device.owner.username
-    json.avatar device.owner.avatar
-
-    json.profile_picture profile_picture_url(device.owner)
-
     json.url device.owner.url
-    json.location device.owner.location
-    json.device_ids device.owner.cached_device_ids
+
+    unless local_assigns[:slim_owner]
+      json.avatar device.owner.avatar
+      json.profile_picture profile_picture_url(device.owner)
+      json.location device.owner.location
+      json.device_ids device.owner.cached_device_ids
+    end
   end
 end
 
-json.data device.formatted_data if with_data
+json.data device.formatted_data if local_assigns[:with_data]
 
 

app/views/v0/devices/world_map.jbuilder

@@ -0,0 +1 @@
+json.array! @devices, partial: 'device', as: :device, local_assigns: { with_data: false, with_postprocessing: false, slim_owner: true }

app/views/v0/users/_user.jbuilder

@@ -12,7 +12,9 @@ json.(user,
 
 json.profile_picture profile_picture_url(user)
 
-if current_user and (current_user.is_admin? or current_user == user)
+authorized = current_user && current_user == user || current_user&.is_admin?
+
+if authorized
   json.merge! email: user.email
   json.merge! legacy_api_key: user.legacy_api_key
 else
@@ -21,14 +23,7 @@ else
 end
 
 json.devices user.devices.filter { |d|
-  !d.is_private? || current_user == user || current_user&.is_admin?
+  !d.is_private? || authorized
 }.map do |device|
-  json.partial! "devices/device", device: device, with_data: false, with_owner: false
-  if current_user == user || current_user&.is_admin?
-    json.merge!(
-      location: device.location,
-      latitude: device.latitude,
-      longitude: device.longitude,
-    )
-  end
+  json.partial! "devices/device", device: device, with_data: false, with_owner: false, with_location: authorized
 end

spec/requests/v0/devices_spec.rb

@@ -24,7 +24,7 @@
       expect(json.length).to eq(2)
       # expect(json[0]['name']).to eq(first.name)
       # expect(json[1]['name']).to eq(second.name)
-      expect(json[0].keys).to eq(%w(id uuid name description state postprocessing system_tags user_tags is_private notify_low_battery notify_stopped_publishing last_reading_at created_at updated_at device_token location hardware owner data))
+      expect(json[0].keys).to eq(%w(id uuid name description state system_tags user_tags is_private notify_low_battery notify_stopped_publishing last_reading_at created_at updated_at device_token postprocessing location hardware owner data))
     end
 
     describe "when not logged in" do

spec/requests/v0/users_spec.rb

@@ -58,8 +58,6 @@
         it "does not include the device locations" do
           j = api_get "users/testguy"
           expect(j["devices"].map { |d| d["location"]}.compact).to be_empty
-          expect(j["devices"].map { |d| d["latitude"]}.compact).to be_empty
-          expect(j["devices"].map { |d| d["longitude"]}.compact).to be_empty
         end
       end
 
@@ -73,8 +71,6 @@
         it "includes the device locations" do
           j = api_get "users/testguy?access_token=#{token.token}"
           expect(j["devices"].map { |d| d["location"]}.compact).not_to be_empty
-          expect(j["devices"].map { |d| d["latitude"]}.compact).not_to be_empty
-          expect(j["devices"].map { |d| d["longitude"]}.compact).not_to be_empty
         end
       end
 
@@ -95,8 +91,6 @@
         it "does not include the device locations" do
           j = api_get "users/testguy?access_token=#{requesting_token.token}"
           expect(j["devices"].map { |d| d["location"]}.compact).to be_empty
-          expect(j["devices"].map { |d| d["latitude"]}.compact).to be_empty
-          expect(j["devices"].map { |d| d["longitude"]}.compact).to be_empty
         end
       end
 
@@ -117,8 +111,6 @@
         it "does not include the device locations" do
           j = api_get "users/testguy?access_token=#{requesting_token.token}"
           expect(j["devices"].map { |d| d["location"]}.compact).to be_empty
-          expect(j["devices"].map { |d| d["latitude"]}.compact).to be_empty
-          expect(j["devices"].map { |d| d["longitude"]}.compact).to be_empty
         end
       end
 
@@ -139,8 +131,6 @@
         it "includes the device locations" do
           j = api_get "users/testguy?access_token=#{requesting_token.token}"
           expect(j["devices"].map { |d| d["location"]}.compact).not_to be_empty
-          expect(j["devices"].map { |d| d["latitude"]}.compact).not_to be_empty
-          expect(j["devices"].map { |d| d["longitude"]}.compact).not_to be_empty
         end
       end
     end