Match progress report - support for duplicates program names #894
Conversation
| @@ -19,7 +19,7 @@ def index | |||
| respond_to do |format| | |||
| format.html {} | |||
| format.xlsx do | |||
| @included_sub_programs = sub_program_list.invert.slice(*report_params[:sub_programs]) | |||
| @included_sub_programs = sub_program_list.select { |sp| sp.last.in?(report_params[:sub_programs]) } | |||
There was a problem hiding this comment.
Just checking: we're confident that the sub_program_list ids are of the same type as report_params[:sub_programs] (strings / ints)?
This is totally fine as is; might be a little clearer to use filter instead of select and name the filtered vars.
| @included_sub_programs = sub_program_list.select { |sp| sp.last.in?(report_params[:sub_programs]) } | |
| @included_sub_programs = sub_program_list.filter { |_name, sp_id| sp_id.in?(report_params[:sub_programs]) } |
There was a problem hiding this comment.
Thanks, I definitely like that syntax better.
And yes, double checked, ints all the way down.
| [ | ||
| [project_name, sub_project_name].join('|'), | ||
| id, | ||
| [sp[:program], sp[:sub_program], sp[:route]].compact_blank.join('|'), |
There was a problem hiding this comment.
It looks like using compact here introduces ambiguity around which names are for which type of entity. Maybe that doesn't matter unless something is parsing them out (for example this would be incorrect program, sub_program, route = name.split('|'))
There was a problem hiding this comment.
This is just used for display, I'd rather turn this into a sentence or more descriptive text, but didn't want to mess with the wording too much until we confirm this works.
Merging this PR
Description
Type of change
Checklist before requesting review