Update to allow phi access on new records

Author: wkirby

lib/phi_attrs/phi_record.rb

@@ -109,7 +109,7 @@ def allow_phi!(user_id = nil, reason = nil)
       #
       def allow_phi(user_id = nil, reason = nil, allow_only: nil, &block)
         get_phi(user_id, reason, allow_only: allow_only, &block)
-        return
+        nil
       end
 
       # Enable PHI access for any instance of this class in the block given only
@@ -149,7 +149,7 @@ def get_phi(user_id = nil, reason = nil, allow_only: nil)
             allow_only.each { |t| t.allow_phi!(user_id, reason) }
           end
 
-          return yield
+          yield
         ensure
           __instances_with_extended_phi.each do |obj|
             if frozen_instances.include?(obj)
@@ -362,7 +362,7 @@ def allow_phi!(user_id = nil, reason = nil)
     #
     def allow_phi(user_id = nil, reason = nil, &block)
       get_phi(user_id, reason, &block)
-      return
+      nil
     end
 
     # Enable PHI access for a single instance of this class inside the block.
@@ -388,7 +388,7 @@ def get_phi(user_id = nil, reason = nil)
       begin
         allow_phi!(user_id, reason)
 
-        return yield
+        yield
       ensure
         new_extensions = @__phi_relations_extended - extended_instances
         disallow_last_phi!(preserve_extensions: true)
@@ -487,7 +487,7 @@ def phi_access_reason
     # @return [Boolean] whether PHI access is allowed for this instance
     #
     def phi_allowed?
-      !phi_context.nil? && phi_context[:phi_access_allowed]
+      new_record? || (!phi_context.nil? && phi_context[:phi_access_allowed])
     end
 
     # Require phi access. Raises an error pre-emptively if it has not been granted.
@@ -672,7 +672,7 @@ def phi_wrap_extension(method_name)
       self.class.send(:define_method, wrapped_method) do |*args, **kwargs, &block|
         relation = send(unwrapped_method, *args, **kwargs, &block)
 
-        if phi_allowed? && (relation.present? && relation_klass(relation).included_modules.include?(PhiRecord))
+        if phi_allowed? && relation.present? && relation_klass(relation).included_modules.include?(PhiRecord)
           relations = relation.is_a?(Enumerable) ? relation : [relation]
           relations.each do |r|
             r.allow_phi!(phi_allowed_by, phi_access_reason) unless @__phi_relations_extended.include?(r)
@@ -720,7 +720,7 @@ def relation_klass(rel)
       return rel.klass if rel.is_a?(ActiveRecord::Relation)
       return rel.first.class if rel.is_a?(Enumerable)
 
-      return rel.class
+      rel.class
     end
 
     def wrapped_extended_name(method_name)

spec/dummy/app/models/patient_info.rb

@@ -12,6 +12,8 @@ class PatientInfo < ApplicationRecord
   exclude_from_phi :last_name
   include_in_phi :birthday
 
+  validates :first_name, presence: true
+
   def birthday
     Time.current
   end

spec/dummy/db/schema.rb

@@ -23,16 +23,6 @@
     t.index ["patient_info_id"], name: "index_health_records_on_patient_info_id"
   end
 
-  create_table "missing_attribute_models", force: :cascade do |t|
-    t.datetime "created_at", precision: nil, null: false
-    t.datetime "updated_at", precision: nil, null: false
-  end
-
-  create_table "missing_extend_models", force: :cascade do |t|
-    t.datetime "created_at", precision: nil, null: false
-    t.datetime "updated_at", precision: nil, null: false
-  end
-
   create_table "patient_details", force: :cascade do |t|
     t.integer "patient_info_id"
     t.string "detail"

spec/phi_attrs/phi_record/phi_wrapping_spec.rb

@@ -15,4 +15,15 @@
       expect { missing_extend_model }.to raise_error(NameError)
     end
   end
+
+  context 'FactoryBot create with validation on PHI attribute' do
+    it 'allows access during validation for new records' do
+      expect { create(:patient_info) }.not_to raise_error
+    end
+
+    it 'requires allow_phi for saved records' do
+      patient = create(:patient_info)
+      expect { patient.first_name }.to raise_error(PhiAttrs::Exceptions::PhiAccessException)
+    end
+  end
 end