Skip to content
Merged
Show file tree
Hide file tree
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
25 changes: 10 additions & 15 deletions app/services/idv/session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,28 +90,23 @@ class Session

attr_reader :current_user, :gpo_otp, :service_provider

VALID_SESSION_ATTRIBUTES.each do |attr|
define_method(attr) do
session[attr]
end

define_method(:"#{attr}=") do |val|
session[attr] = val
end
end

def initialize(user_session:, current_user:, service_provider:)
@user_session = user_session
@current_user = current_user
@service_provider = service_provider
set_idv_session
end

def method_missing(method_sym, *arguments, &block)
attr_name_sym = method_sym.to_s.delete_suffix('=').to_sym
if VALID_SESSION_ATTRIBUTES.include?(attr_name_sym)
return session[attr_name_sym] if arguments.empty?
session[attr_name_sym] = arguments.first
else
super
end
end

def respond_to_missing?(method_sym, include_private)
attr_name_sym = method_sym.to_s.delete_suffix('=').to_sym
VALID_SESSION_ATTRIBUTES.include?(attr_name_sym) || super
end

# @return [Profile]
def create_profile_from_applicant_with_password(
user_password, is_enhanced_ipp:, proofing_components:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
def add_to_idv_session_applicant(pii:)
pii_hash = Pii::StateId.members.index_with(nil).merge(pii)

idv_session.applicant(pii_hash)
idv_session.applicant = pii_hash
end

def add_to_gpo_pending_profile(pii:)
Expand Down
15 changes: 6 additions & 9 deletions spec/services/idv/session_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,27 +47,24 @@
end
end

describe '#method_missing' do
it 'disallows un-supported attributes' do
describe 'attribute methods' do
it 'disallows un-supported setters' do
expect { subject.foo = 'bar' }.to raise_error NoMethodError
end

it 'allows supported attributes' do
it 'allows using supported setters and getters' do
Idv::Session::VALID_SESSION_ATTRIBUTES.each do |attr|
subject.send attr, 'foo'
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So before calling idv_session.address_edited('foo') would act as a setter.

That is very uncommon! I made a call to remove that, now it will throw an argument error because now reader and writer are two separate methods.

Open to changing this back if we are worried, but I expect our test suite would catch any lingering use of that style

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed this too, glad to see a fix

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed two sneaky-setter usages in f2426a2

expect(subject.send(attr)).to eq 'foo'
expect(subject.send(attr)).to eq nil
subject.send :"#{attr}=", 'foo'
expect(subject.send(attr)).to eq 'foo'
end
end
end

describe '#respond_to_missing?' do
it 'disallows un-supported attributes' do
it 'allows checking for un-supported attributes' do
expect(subject.respond_to?(:foo=, false)).to eq false
end

it 'allows supported attributes' do
it 'allows checking for supported attributes' do
Idv::Session::VALID_SESSION_ATTRIBUTES.each do |attr|
expect(subject.respond_to?(attr, false)).to eq true
expect(subject.respond_to?(:"#{attr}=", false)).to eq true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
current_user: nil,
service_provider: service_provider,
)
idv_session.applicant(applicant_pii)
idv_session.applicant = applicant_pii
idv_session
end

Expand Down