New columns to sp_return_logs

[samathad2023]

🎫 Ticket

Link to the relevant ticket:
LG-13011

🛠 Summary of changes

Three new columns been added to sp_return_logs for billing report

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

• Spec file updates to create sp_return_logs with new columns
• Validate the columns
• Validate the values with profile

[zachmargolis]

personal style, I would line up all the args

Suggested change

	create(
	:profile, :active, :verified, user: current_user,
	initiating_service_provider: current_sp
	)
	create(
	:profile,
	:active,
	:verified,
	user: current_user,
	initiating_service_provider: current_sp,
	)

[zachmargolis]

profile_verified_at is when the user finished IDV.
session_started_at is for the current start of this particular session, which may have no IDV

[zachmargolis]

First, current_user should always be present, so there should be no need for the & directly after it.

Second, we are not always returning profile attributes to partners, maybe we could only check for active_profile if ial > 1 ?

Suggested change

	profile_id: current_user&.active_profile&.id,
	profile_verified_at: current_user&.active_profile&.verified_at,
	profile_requested_issuer: current_user&.active_profile&.initiating_service_provider,
	profile_id: current_user.active_profile&.id,
	profile_verified_at: current_user.active_profile&.verified_at,
	profile_requested_issuer: current_user.active_profile&.initiating_service_provider,

[zachmargolis]

why did we change this part?

[samathad2023]

No changes needed on this.Reverting this change.

[zachmargolis]

please remove this

Suggested change

# binding.pry

[zachmargolis]

1. let's add a spec that shows the new attributes (profile_id, profile_verified_at, profile_requested_issuer) being added? this spec seems to only show that they do not change right?

2. let's save the assignment to make it clearer we're referencing the same row

Suggested change

	SpReturnLog.create(
	request_id: request_id,
	user_id: current_user.id,
	billable: true,
	ial: ial_context.ial,
	issuer: current_sp.issuer,
	requested_at: session_started_at,
	returned_at: Time.zone.now,
	profile_id: active_profile.id,
	profile_verified_at: active_profile.verified_at,
	profile_requested_issuer: active_profile.initiating_service_provider.issuer,
	)
	expect do
	instance.track_billing_events
	end.to_not(change { SpReturnLog.count }.from(1))
	sp_return_log = SpReturnLog.last
	expect(sp_return_log.profile_id).to eq active_profile.id
	expect(sp_return_log.profile_verified_at).to eq active_profile.verified_at
	expect(sp_return_log.profile_requested_issuer).
	to eq active_profile.initiating_service_provider.issuer
	sp_return_log = SpReturnLog.create(
	request_id: request_id,
	user_id: current_user.id,
	billable: true,
	ial: ial_context.ial,
	issuer: current_sp.issuer,
	requested_at: session_started_at,
	returned_at: Time.zone.now,
	profile_id: active_profile.id,
	profile_verified_at: active_profile.verified_at,
	profile_requested_issuer: active_profile.initiating_service_provider.issuer,
	)
	expect do
	instance.track_billing_events
	end.to_not(change { SpReturnLog.count }.from(1))
	sp_return_log.reload
	expect(sp_return_log.profile_id).to eq active_profile.id
	expect(sp_return_log.profile_verified_at).to eq active_profile.verified_at
	expect(sp_return_log.profile_requested_issuer).
	to eq active_profile.initiating_service_provider.issuer

[zachmargolis]

let's link this to a different service provider (this helps us make sure if we ever added a bug that mixed up SPs, that we'd still catch it

Suggested change

	let(:active_profile) do
	create(
	:profile,
	:active,
	:verified,
	user: current_user,
	initiating_service_provider: current_sp,
	let(:profile_sp) { create(:service_provider) }
	let(:active_profile) do
	create(
	:profile,
	:active,
	:verified,
	user: current_user,
	initiating_service_provider: profile_sp,

[zachmargolis]

these assertions are basically checking if the migrations ran, I think we should check the values inside the columns to make sure they're what we expect.

For context, these values are what we're going to generate billing reports later on from, so we want to make sure they are correct

These are the assertions from later in the file

Suggested change

	expect(sp_return_log.respond_to?(:profile_id)).to be true
	expect(sp_return_log.respond_to?(:profile_verified_at)).to be true
	expect(sp_return_log.respond_to?(:profile_requested_issuer)).to be true
	expect(sp_return_log.profile_id).to eq active_profile.profile_id
	expect(sp_return_log.profile_verified_at).to eq active_profile.verified_at
	expect(sp_return_log.profile_requested_issuer).
	to eq active_profile.profile_requested_issuer