Skip to content
This repository was archived by the owner on Jul 5, 2023. It is now read-only.

Zip plucked values with tstruct keys in the same order for pluck_to_tstruct - #563

Open
gnaratil2017 wants to merge 1 commit into
chanzuckerberg:masterfrom
gnaratil2017:assign_in_order_for_pluck_to_tstruct
Open

Zip plucked values with tstruct keys in the same order for pluck_to_tstruct#563
gnaratil2017 wants to merge 1 commit into
chanzuckerberg:masterfrom
gnaratil2017:assign_in_order_for_pluck_to_tstruct

Conversation

@gnaratil2017

Copy link
Copy Markdown

Fixes #562

pluck(*pluck_keys).map do |row|
row = [row] if keys_one
value = Hash[map_nil_values_to_default(tstruct_props, tstruct_keys.zip(row))]
value = Hash[map_nil_values_to_default(tstruct_props, tstruct_keys_in_pluck_order.zip(row))]

@gnaratil2017 gnaratil2017 Jun 8, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

When we zip the tstruct keys with the plucked values, we want to make sure the corresponding key and value match up correctly.

This requires that our tstruct keys are in the same order as the pluck_keys, which is why we want to subtract the associations_keys and then append them to the end of tstruct_keys on line 34. This matches the operations and order changes we performed for the pluck_keys on line 33.

const :name, String
const :house, String
const :wand_wood_type, String
const :house, String

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

With this change in the order of the tstruct props, the spec on line 138 fails without the new changes to pluck_to_tstruct.rb.

context 'pluck with associations' do
associations = { wand_wood_type: "wands.wood_type" }
expected = [
WizardWithWandT.new(name: "Harry Potter", house: "Gryffindor", wand_wood_type: "Holly"),
WizardWithWandT.new(name: "Hermione Granger", house: "Gryffindor", wand_wood_type: "Vine"),
]
it_should_behave_like 'pluck_to_tstruct with associations', WizardWithWandT, associations, expected
end

Rather than the expected array, the actual array would look like:

[
     WizardWithWandT.new(name: "Harry Potter", house: "Holly", wand_wood_type: "Gryffindor"),
     WizardWithWandT.new(name: "Hermione Granger", house: "Vine", wand_wood_type: "Gryffindor"),
]

@codecov

codecov Bot commented Jun 13, 2023

Copy link
Copy Markdown

Codecov Report

Merging #563 (5482a42) into master (447ecbe) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

❗ Current head 5482a42 differs from pull request most recent head 6fe9c91. Consider uploading reports for the commit 6fe9c91 to get more accurate results

@@            Coverage Diff             @@
##           master     #563      +/-   ##
==========================================
- Coverage   97.48%   97.44%   -0.04%     
==========================================
  Files         115      115              
  Lines        2978     2979       +1     
==========================================
  Hits         2903     2903              
- Misses         75       76       +1     
Impacted Files Coverage Δ
lib/sorbet-rails/rails_mixins/pluck_to_tstruct.rb 100.00% <100.00%> (ø)
spec/pluck_to_tstruct_spec.rb 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pluck_to_tstruct assigns plucked values to the wrong tstruct keys depending on the order in which the tstruct props for associations are listed

1 participant