Merge pull request #5 from apsislabs/2-joined-model-shorthand

#2 joined model shorthand

Author: wkirby

.gitignore

@@ -14,3 +14,4 @@
 /test/sample/tmp/
 *.sqlite3
 *.log
+.rspec_status

.rspec_status

@@ -7,9 +7,13 @@ module PhiRecord
     included do
       class_attribute :__phi_exclude_methods
       class_attribute :__phi_include_methods
+      class_attribute :__phi_extended_methods
       class_attribute :__phi_methods_wrapped
 
+      after_initialize :wrap_phi
+
       self.__phi_methods_wrapped = []
+      self.__phi_extended_methods = []
     end
 
     class_methods do
@@ -21,6 +25,10 @@ def include_in_phi(*methods)
         self.__phi_include_methods = methods.map(&:to_s)
       end
 
+      def extend_phi_access(*methods)
+        self.__phi_extended_methods = methods.map(&:to_s)
+      end
+
       def allow_phi!(user_id, reason)
         RequestStore.store[:phi_access] ||= {}
 
@@ -42,9 +50,7 @@ def disallow_phi!
       end
     end
 
-    def initialize(*args)
-      super(*args)
-
+    def wrap_phi
       # Disable PHI access by default
       @__phi_access_allowed = false
       @__phi_access_logged = false
@@ -54,11 +60,14 @@ def initialize(*args)
     end
 
     def __phi_wrapped_methods
-      attribute_names - self.class.__phi_exclude_methods.to_a + self.class.__phi_include_methods.to_a - [self.class.primary_key]
+      associations = self.class.reflect_on_all_associations.map(&:name).map(&:to_s)
+      excluded_methods = self.class.__phi_exclude_methods.to_a
+      included_methods = self.class.__phi_include_methods.to_a
+      associations + attribute_names - excluded_methods + included_methods - [self.class.primary_key]
     end
 
     def allow_phi!(user_id, reason)
-      PhiAttrs::Logger.tagged( *phi_log_keys ) do
+      PhiAttrs::Logger.tagged(*phi_log_keys) do
         @__phi_access_allowed = true
         @__phi_user_id = user_id
         @__phi_access_reason = reason
@@ -107,10 +116,21 @@ def phi_wrap_method(method_name)
           raise PhiAttrs::Exceptions::PhiAccessException, "Attempted PHI access for #{self.class.name} #{@__phi_user_id}" unless phi_allowed?
 
           unless @__phi_access_logged
-            PhiAttrs::Logger.info("'#{phi_allowed_by}' accessing #{self.class.name}.\n\t access logging triggered by method: #{method_name}")
+            PhiAttrs::Logger.info("'#{phi_allowed_by}' accessing #{self.class.name}. Triggered by method: #{method_name}")
             @__phi_access_logged = true
           end
 
+          # extend PHI access to relationships if needed
+          if self.class.__phi_extended_methods.include? method_name
+
+            # get the unwrapped relation
+            relation = send(unwrapped_method, *args, &block)
+
+            if relation.class.included_modules.include?(PhiRecord)
+              relation.allow_phi!(phi_allowed_by, phi_access_reason) unless relation.phi_allowed?
+            end
+          end
+
           send(unwrapped_method, *args, &block)
         end
       end

lib/phi_attrs/phi_record.rb

@@ -3,7 +3,7 @@
 
 module PhiAttrs
   class Railtie < Rails::Railtie
-    initializer 'rolify.initialize' do
+    initializer 'phi_attrs.initialize' do
       ActiveSupport.on_load(:active_record) do
         ActiveRecord::Base.send :extend, PhiAttrs
       end

lib/phi_attrs/railtie.rb

@@ -1,3 +1,3 @@
 module PhiAttrs
-  VERSION = '0.1.0'.freeze
+  VERSION = '0.1.2'.freeze
 end

lib/phi_attrs/version.rb

@@ -0,0 +1,4 @@
+class Address < ActiveRecord::Base
+  belongs_to :patient_info
+  phi_model
+end

spec/internal/app/models/address.rb

@@ -1,3 +1,4 @@
 class PatientDetail < ActiveRecord::Base
+  belongs_to :patient_info
   phi_model
 end

spec/internal/app/models/patient_detail.rb

@@ -1,6 +1,11 @@
 class PatientInfo < ActiveRecord::Base
+  has_one :patient_detail, inverse_of: 'patient_info'
+  has_one :address, inverse_of: 'patient_info'
+
   phi_model
 
+  extend_phi_access :patient_detail
+
   exclude_from_phi :last_name
   include_in_phi :birthday
 

spec/internal/app/models/patient_info.rb

@@ -8,8 +8,14 @@ def change
     end
 
     create_table :patient_details do |t|
+      t.belongs_to :patient_info
       t.string :detail
       t.timestamps
     end
+
+    create_table :addresses do |t|
+      t.belongs_to :patient_info
+      t.string :address
+    end
   end
 end

spec/internal/db/migrate/20170214100255_create_patient_infos.rb

@@ -84,31 +84,64 @@
   end
 
   context 'instance authorized' do
-    it 'allows access to an authorized instance' do
-      expect { patient_jane.first_name }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
+    context 'single record' do
+      it 'allows access to an authorized instance' do
+        expect { patient_jane.first_name }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
 
-      patient_jane.allow_phi! 'test', 'unit tests'
+        patient_jane.allow_phi! 'test', 'unit tests'
 
-      expect { patient_jane.first_name }.not_to raise_error
-    end
+        expect { patient_jane.first_name }.not_to raise_error
+      end
 
-    it 'only allows access to the authorized instance' do
-      patient_jane.allow_phi! 'test', 'unit tests'
+      it 'only allows access to the authorized instance' do
+        patient_jane.allow_phi! 'test', 'unit tests'
 
-      expect { patient_jane.first_name }.not_to raise_error
-      expect { patient_john.first_name }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
-    end
+        expect { patient_jane.first_name }.not_to raise_error
+        expect { patient_john.first_name }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
+      end
 
-    it 'revokes access after calling disallow_phi!' do
-      expect { patient_jane.first_name }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
+      it 'revokes access after calling disallow_phi!' do
+        expect { patient_jane.first_name }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
 
-      patient_jane.allow_phi! 'test', 'unit tests'
+        patient_jane.allow_phi! 'test', 'unit tests'
 
-      expect { patient_jane.first_name }.not_to raise_error
+        expect { patient_jane.first_name }.not_to raise_error
 
-      patient_jane.disallow_phi!
+        patient_jane.disallow_phi!
 
-      expect { patient_jane.first_name }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
+        expect { patient_jane.first_name }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
+      end
+
+      it 'allows access on an instance that already exists' do
+        john = PatientInfo.create(first_name: 'John', last_name: 'Doe')
+        expect { john.first_name }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
+
+        john_id = john.id
+        john = nil
+
+        john = PatientInfo.find(john_id)
+        expect { john.first_name }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
+
+        john.allow_phi! 'test', 'unit tests'
+        expect { john.first_name }.not_to raise_error
+        expect(john.first_name).to eq 'John'
+      end
+    end
+
+    context 'collection' do
+      it 'allows access when fetched as a collection' do
+        jay = PatientInfo.create(first_name: "Jay")
+        bob = PatientInfo.create(first_name: "Bob")
+        moe = PatientInfo.create(first_name: "Moe")
+
+        patients = PatientInfo.all
+
+        expect(patients).to contain_exactly(jay, bob, moe)
+        expect { patients.map(&:first_name) }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
+
+        patients.map { |p| p.allow_phi! 'test', 'unit tests' }
+        expect { patients.map(&:first_name) }.not_to raise_error
+      end
     end
   end
 
@@ -138,4 +171,37 @@
       expect { patient_jane.first_name }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
     end
   end
+
+  context 'extended authorization' do
+    let(:mary_detail)  { PatientDetail.create(detail: 'Lorem Ipsum') }
+    let(:mary_address) { Address.create(address: '123 Street Ave') }
+    let(:patient_mary) { PatientInfo.create(first_name: 'Mary', last_name: 'Jay', address: mary_address, patient_detail: mary_detail) }
+
+    it 'extends access to extended association' do
+      expect { patient_mary.first_name }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
+      expect { patient_mary.patient_detail }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
+      expect { patient_mary.patient_detail.detail }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
+
+      patient_mary.allow_phi! 'test', 'unit tests'
+
+      expect { patient_mary.first_name }.not_to raise_error
+      expect { patient_mary.patient_detail.detail }.not_to raise_error
+      expect(patient_mary.patient_detail.detail).to eq 'Lorem Ipsum'
+    end
+
+    it 'does not extend to unextended association' do
+      expect { patient_mary.first_name }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
+      expect { patient_mary.address }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
+      expect { patient_mary.address.address }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
+
+      patient_mary.allow_phi! 'test', 'unit tests'
+      expect { patient_mary.first_name }.not_to raise_error
+      expect { patient_mary.address }.not_to raise_error
+      expect { patient_mary.address.address }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
+
+      patient_mary.address.allow_phi! 'test', 'unit test'
+      expect { patient_mary.address.address }.not_to raise_error
+      expect(patient_mary.address.address).to eq '123 Street Ave'
+    end
+  end
 end