Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new data policy fields #331

Merged
merged 7 commits into from
Jun 27, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app/controllers/v0/devices_controller.rb
Original file line number Diff line number Diff line change
@@ -97,6 +97,7 @@ def device_params
:device_token,
:notify_low_battery,
:notify_stopped_publishing,
:precise_location,
:exposure,
:meta,
:user_tags,
@@ -105,7 +106,7 @@ def device_params

# Researchers + Admins can update is_private
if current_user.role_mask >= 2
params_to_permit.push(:is_private, :is_test)
params_to_permit.push(:is_private, :is_test, :enable_forwarding)
end

params.permit(
27 changes: 26 additions & 1 deletion app/models/device.rb
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@ class Device < ActiveRecord::Base

default_scope { with_active_state }

include ActiveModel::Dirty
include Workflow
include WorkflowActiverecord
include ArchiveWorkflow
@@ -37,6 +38,7 @@ class Device < ActiveRecord::Base
with: /\A([0-9a-fA-F]{2}[:-]){5}[0-9a-fA-F]{2}\z/, allow_nil: true

before_save :nullify_other_mac_addresses, if: :mac_address
before_save :truncate_and_fuzz_location!, if: :location_changed?
before_save :calculate_geohash
after_validation :do_geocoding

@@ -245,6 +247,14 @@ def update_component_timestamps(timestamp, sensor_ids)
end
end

def data_policy(authorized=false)
{
is_private: authorized ? is_private : "[FILTERED]",
enable_forwarding: authorized ? enable_forwarding : "[FILTERED]",
precise_location: authorized ? precise_location : "[FILTERED]"
}
end

def hardware(authorized=false)
{
name: hardware_name,
@@ -272,19 +282,34 @@ def hardware_slug
end

def forward_readings?
owner.forward_device_readings?
enable_forwarding && owner.forward_device_readings?
end

def forwarding_token
owner.forwarding_token
end

def truncate_and_fuzz_location!
if latitude && longitude
fuzz_decimal_places = 3
total_decimal_places = 5
lat_fuzz = self.precise_location ? 0.0 : (Random.rand * 1/10.0**fuzz_decimal_places)
long_fuzz = self.precise_location ? 0.0 : (Random.rand * 1/10.0**fuzz_decimal_places)
self.latitude = (self.latitude + lat_fuzz).truncate(total_decimal_places)
self.longitude = (self.longitude + long_fuzz).truncate(total_decimal_places)
end
end

private

def set_state
self.state = self.soft_state
end

def location_changed?
latitude_changed? || longitude_changed? || precise_location_changed?
end

def calculate_geohash
# include ActiveModel::Dirty
# if latitude.changed? or longitude.changed?
2 changes: 1 addition & 1 deletion app/views/v0/devices/_device.jbuilder
Original file line number Diff line number Diff line change
@@ -14,7 +14,6 @@ json.(
:state,
:system_tags,
:user_tags,
:is_private,
:last_reading_at,
:created_at,
:updated_at
@@ -35,6 +34,7 @@ else
end
json.merge!(postprocessing: device.postprocessing) if local_assigns[:with_postprocessing]
json.merge!(location: device.formatted_location) if local_assigns[:with_location]
json.merge!(data_policy: device.data_policy(authorized))
json.merge!(hardware: device.hardware(authorized))

if local_assigns[:with_owner] && device.owner
8 changes: 8 additions & 0 deletions db/migrate/20240624161155_add_data_policy_fields_to_device.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class AddDataPolicyFieldsToDevice < ActiveRecord::Migration[6.1]
def change
add_column :devices, :precise_location, :boolean, null: false, default: false
add_column :devices, :enable_forwarding, :boolean, null: false, default: false
# Existing devices have precise locations, despite the default for all new ones.
execute "UPDATE devices SET precise_location = true"
end
end
8 changes: 8 additions & 0 deletions lib/tasks/devices.rake
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
namespace :devices do
task :truncate_and_fuzz_locations => :environment do
Device.all.each do |device|
device.truncate_and_fuzz_location!
device.save!(validate: false)
end
end
end
103 changes: 99 additions & 4 deletions spec/models/device_spec.rb
Original file line number Diff line number Diff line change
@@ -192,7 +192,7 @@

it "calculates geohash on save" do
barcelona = create(:device)
expect(barcelona.geohash).to match('sp3e9bh31y')
expect(barcelona.geohash).to match(/^sp3e/)
end

skip "calculates elevation on save", :vcr do
@@ -203,6 +203,60 @@

end

describe "location truncation and fuzzing" do
context "when the location is changed" do
context "when the device has precise location set" do
let(:device) {
create(:device, precise_location: true)

}

it "truncates the location to 5 decimal places" do
device.update(latitude: 0.1234567, longitude: 10.1234567)
device.save
expect(device.latitude).to eq(0.12345)
expect(device.longitude).to eq(10.12345)
end
end

context "when the device has no precise location set" do
let(:device) {
create(:device, precise_location: false)

}


it "truncates the location to 3 decimal places and adds 2 extra dp of randomness" do
device.update(latitude: 0.12345, longitude: 10.12345)
device.save
expect(device.latitude).not_to eq(0.12345)
expect(device.longitude).not_to eq(10.12345)
expect((device.latitude - 0.12345).abs.truncate(5)).to be <= 0.001
expect((device.longitude - 10.12345).abs.truncate(5)).to be <= 0.001
end
end
end

context "when the location is not changed" do
let(:device) {
create(:device, precise_location: false, latitude: 0.12345, longitude: 0.12345)

}

it "does not re-fuzz the device location" do
lat_before = device.latitude
long_before = device.longitude

device.update(name: "not updating location")
device.save!

expect(device.reload.latitude).to eq(lat_before)
expect(device.reload.longitude).to eq(long_before)
end
end
end


it "has to_s" do
device = create(:device, name: 'cool device')
expect(device.to_s).to eq('cool device')
@@ -524,10 +578,51 @@

describe "forwarding" do
describe "#forward_readings?" do
context "when the device has forwarding enabled" do
let(:device) {
create(:device, enable_forwarding: true)
}

context "when forward_device_readings is true on the owning user" do
it "forwardds readings" do
expect(device.owner).to receive(:forward_device_readings?).and_return(true)
expect(device.forward_readings?).to be(true)
end
end

context "when forward_device_readings is not true on the owning user" do
it "does not forward readings" do
expect(device.owner).to receive(:forward_device_readings?).and_return(false)
expect(device.forward_readings?).to be(false)
end
end
end

context "when the device does not have forwarding enabled" do
let(:device) {
create(:device, enable_forwarding: false)
}

context "when forward_device_readings is true on the owning user" do
it "does not forward readings" do
allow(device.owner).to receive(:forward_device_readings?).and_return(true)
expect(device.forward_readings?).to be(false)
expect(device.owner).not_to have_received(:forward_device_readings?)
end
end

context "when forward_device_readings is not true on the owning user" do
it "does not forward readings" do
allow(device.owner).to receive(:forward_device_readings?).and_return(false)
expect(device.forward_readings?).to be(false)
expect(device.owner).not_to have_received(:forward_device_readings?)
end
end


end

it "delegates to the forward_device_readings? method on the device owner" do
forward_readings = double(:forward_readings)
expect(device.owner).to receive(:forward_device_readings?).and_return(forward_readings)
expect(device.forward_readings?).to eq(forward_readings)
end
end

24 changes: 23 additions & 1 deletion spec/requests/v0/devices_spec.rb
Original file line number Diff line number Diff line change
@@ -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 system_tags user_tags is_private last_reading_at created_at updated_at notify device_token postprocessing location hardware owner data))
expect(json[0].keys).to eq(%w(id uuid name description state system_tags user_tags last_reading_at created_at updated_at notify device_token postprocessing location data_policy hardware owner data))
end

describe "when not logged in" do
@@ -46,6 +46,13 @@
json = api_get 'devices'
expect(json[0]['hardware']['last_status_message']).to eq("[FILTERED]")
end

it "does not show data policies" do
first = create(:device)
second = create(:device)
json = api_get 'devices'
expect(json[0]['data_policy']['enable_forwarding']).to eq("[FILTERED]")
end
end

describe "when logged in as a normal user" do
@@ -67,6 +74,14 @@
json = api_get 'devices', { access_token: token.token }
expect(json[0]['hardware']['last_status_message']).to eq("[FILTERED]")
end

it "only shows device policies for the owning user" do
first = create(:device)
second = create(:device, owner: user)
json = api_get 'devices', { access_token: token.token}
expect(json[0]['data_policy']['enable_forwarding']).to eq('[FILTERED]')
expect(json[1]['data_policy']['enable_forwarding']).not_to eq('[FILTERED]')
end
end

describe "when logged in as an admin" do
@@ -88,6 +103,13 @@
json = api_get 'devices', { access_token: admin_token.token}
expect(json[0]['hardware']['last_status_message']).not_to eq('[FILTERED]')
end

it "shows device policies" do
first = create(:device)
second = create(:device)
json = api_get 'devices', { access_token: admin_token.token}
expect(json[0]['data_policy']['enable_forwarding']).not_to eq('[FILTERED]')
end
end

describe "world map" do