Y26-680 Automated buffer addition for cherrypicking

[andrewsparkes]

Closes #y26-680

Changes proposed in this pull request

Optional automated buffer addition for cherrypicking

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.24%. Comparing base (430e83a) to head (fe10426).
⚠️ Report is 6 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #5565   +/-   ##
========================================
  Coverage    87.24%   87.24%           
========================================
  Files         1461     1464    +3     
  Lines        33003    33083   +80     
  Branches      3470     3489   +19     
========================================
+ Hits         28792    28862   +70     
- Misses        4190     4200   +10     
  Partials        21       21           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[yoldas]

Looks good. My comments are notes. After UAT, we might consider describing the options and limitations at https://ssg-confluence.internal.sanger.ac.uk/spaces/PSDPUB/pages/232620342/Cherrypicking+options+in+Sequencescape .

[KatyTaylor]

There's a lot of code :) any chance of a brief summary / description of changes, and a screenshot or two of the UI changes? Thanks

Edit by Andrew as can't seem to reply: see Abdullah's original story with the screenshots here

[KatyTaylor]

Didn't manage to finish reviewing, will look again Monday

[KatyTaylor]

All just suggestions, although don't understand the 1, on bit. Also I haven't reviewed the tests I'm afraid.

[KatyTaylor]

Think this algorithm is good and works. It took me a while to understand so I asked ChatGPT to refactor it. Here's its suggestion, feel free to accept, adapt or reject! My opinion is that methods 1, 2 and 4 are an improvement, and 3 I'm not sure - includes slightly obscure stuff like filter_map.

Haven't tested it so may not work. Think returns something slightly different as in doesn't cut out the source object (but shouldn't matter).

def data_object_for_buffers(data_object)
  buffer_volume = @batch&.buffer_volume_for_empty_wells
  return data_object unless buffer_volume

  destinations = data_object['destination'].transform_values do |plate_details|
    build_destination_plate(plate_details, buffer_volume)
  end

  { 'destination' => destinations }
end

private

def build_destination_plate(plate_details, buffer_volume)
  plate_size = plate_details['plate_size']
  
  {
    'name' => plate_details['name'],
    'plate_size' => plate_size,
    'mapping' => build_mapping(plate_details['mapping'], plate_size, buffer_volume)
  }
end

def build_mapping(existing_mapping, plate_size, buffer_volume)
  plate = Plate.find_by_barcode(plate_details['name'])

  index_to_mapping = existing_mapping.index_by do |entry|
    description_to_column_index(entry['dst_well'], plate_size)
  end

  (1..plate_size).filter_map do |index|
    existing = index_to_mapping[index]
    next existing if existing

    dst_well = column_index_to_description(index, plate_size)
    next unless well_empty?(plate, dst_well)

    {
      'dst_well' => dst_well,
      'buffer_volume' => buffer_volume
    }
  end
end

def well_empty?(plate, dst_well)
  well = plate.find_well_by_name(dst_well)
  well.blank? || well.empty?
end

[andrewsparkes]

refactored

[KatyTaylor]

Thank you for refactoring (and making the other changes).

I think it's generally more readable. Think this particular method mapping_or_buffer_entry is a little abstract and maybe I would just put this logic into the build_buffer_mapping method.

Happy to approve anyway though.