Adds variant details to split_details endpoint#45
Conversation
|
Needs somebody from @Betterment/test_track_core to claim domain review. Use the shovel operator to claim, e.g.:
|
| text-transform: capitalize; | ||
| } | ||
| .DescriptionTable { | ||
| @include standard-table(100%, 15%, 35%); |
There was a problem hiding this comment.
Added a new table style to support the split show page layout
| @@ -1 +1,2 @@ | |||
| json.(@split_detail, :name, :hypothesis, :assignment_criteria, :description, :owner, :location, :platform) | |||
| json.variant_details @split_detail.split.variant_details, partial: 'variant_detail', as: :variant_detail | |||
There was a problem hiding this comment.
Should I delegate variant_details to split here?
There was a problem hiding this comment.
you can do that. you'll also want to add includes(:variant_details) wherever the split is being queried
|
nanda? |
|
Needs somebody from @Betterment/test_track_core and @dschaub to claim domain review. Use the shovel operator to claim, e.g.:
|
|
<< domain tafn |
|
@agirlnamedsophia needs to incorporate feedback from @dschaub. Bump when done. |
|
bump @dschaub |
|
Needs @dschaub to provide domain review. When you finish a round of review, be sure to say you've finished or sign off on the PR, e.g.:
If you're too busy to review, unclaim the PR, e.g.:
|
|
|
||
| def update | ||
| @split_detail = Split.find(params[:split_id]).detail | ||
| @split_detail = Split.includes(:variant_details).find(params[:split_id]).detail |
There was a problem hiding this comment.
my bad, I read the view wrong and thought we were iterating over the splits and causing an n+1, the way this worked was totally fine since it's a single query per table.
|
domain lgtm |
|
Ready to merge! :squirrel: 🎈 🤘 |
3 participants
/domain @Betterment/test_track_core @dschaub