Merge pull request wtsi-npg#875 from mgcam/optional_lib_type4review

Made library_type optional in the review check.

Changes

@@ -9,7 +9,12 @@ LIST OF CHANGES FOR NPG-QC PACKAGE
    https://github.com/gugod/App-perlbrew/blob/master/perlbrew-install, so
    the change originates from GitHub and can be trusted. Our CI flow compares
    the checksum of the downloaded script to the expected value. We now store
-   an updated expeced checksum value, which corresponds to the latest release.
+   an updated expected checksum value, which corresponds to the latest release.
+ - npg_qc::autoqc::check::review:
+  The forthcoming lane-level evaluations are impossible if the code continues
+  to error when library_type attribute is not defined for the entity under
+  consideration. The library_type attribute is now set when possible, no error
+  if it is undefined.
 
 release 72.2.0 (2024-08-30)
  - npg_qc::autoqc::check::review:

lib/npg_qc/Schema/Result/Review.pm

@@ -357,14 +357,12 @@ around [qw/update insert/] => sub {
 
   ##### 
   # Do not accept half-baked results, ie if we have evaluation
-  # results, we should also have library type and criteria.
+  # results, we should also have criteria.
   if ($data->{'evaluation_results'} and keys %{$data->{'evaluation_results'}}) {
-    foreach my $name (qw/library_type criteria/) {
-      my $value = $data->{$name};
-      my $m = "Evaluation results present, but $name absent";
-      $value or croak $m;
-      ((not ref $value) or keys %{$value}) or croak $m;
-    }
+    my $value = $data->{'criteria'};
+    my $m = 'Evaluation results present, but criteria absent';
+    $value or croak $m;
+    ((not ref $value) or keys %{$value}) or croak $m;
   }
 
   #####
@@ -475,7 +473,7 @@ Marina Gourtovaia E<lt>[email protected]<gt>
 
 =head1 LICENSE AND COPYRIGHT
 
-Copyright (C) 2019,2020 Genome Research Ltd.
+Copyright (C) 2019,2020, 2924 Genome Research Ltd.
 
 This file is part of NPG.
 

lib/npg_qc/autoqc/checks/review.pm

@@ -568,11 +568,11 @@ has '_criteria' => (
 sub _build__criteria {
   my $self = shift;
 
-  # Save redundant library_type.
-  # TODO: Save details about applicability instead.
+  # Library type might be undefined. Example - lane level object.
   my $lib_type = $self->lims->library_type;
-  $lib_type or croak 'Library type is not defined for ' .  $self->_entity_desc;
-  $self->result->library_type($lib_type);
+  if ($lib_type) {
+    $self->result->library_type($lib_type);
+  }
 
   my $num_criteria = scalar @{$self->_applicable_criteria};
   if ($num_criteria == 0) {

t/50-schema-result-Review.t

@@ -19,19 +19,9 @@ my $schema = Moose::Meta::Class->create_anon_class(
      ->new_object({})->create_test_db(q[npg_qc::Schema], 't/data/fixtures');
 
 subtest 'reject incomplete results on insert' => sub {
-  plan tests => 4;
+  plan tests => 2;
 
   my $values = {
-    evaluation_results => {"e1"=>1,"e2"=>0},
-    criteria           => {"and"=>["e1","e2"]},
-    pass               => 0,
-    path               => 't/data'
-  };
-  throws_ok {$schema->resultset($table)->create($values)}
-    qr/Evaluation results present, but library_type absent/,
-    'library type is not defined - error on insert';
-
-  $values = {
     library_type       => 'common_type',
     criteria           => {},
     evaluation_results => {"e1"=>1,"e2"=>0},
@@ -41,18 +31,6 @@ subtest 'reject incomplete results on insert' => sub {
     qr/Evaluation results present, but criteria absent/,
     'criteria are not defined - error on insert';
 
-  $values = {
-    evaluation_results => {"e1"=>1,"e2"=>0},
-    criteria           => {},
-    qc_outcome =>
-    {"mqc_outcome"=>"Rejected final","timestamp"=>"2018-06-03T12:53:46+0000","username"=>"robo_qc"},
-    pass => 0,
-    path => 't/data'
-  };
-  throws_ok {$schema->resultset($table)->create($values)}
-    qr/Evaluation results present, but library_type absent/,
-    'criteria and library type are not defined - error on insert';
-
   $values = {
     library_type       => 'common_type',
     evaluation_results => {"e1"=>1,"e2"=>0},
@@ -68,13 +46,27 @@ subtest 'reject incomplete results on insert' => sub {
 };
 
 subtest 'insert a basic record, do not allow incomplete data in update' => sub {
-  plan tests => 9;
+  plan tests => 10;
 
   my $id_seq_composition = t::autoqc_util::find_or_save_composition(
                 $schema, {'id_run'    => 1111,
                           'position'  => 1,
-                          'tag_index' => 8});
+                          'tag_index' => 7});
   my $values = {
+    evaluation_results => {"e1"=>1,"e2"=>0},
+    criteria           => {"and"=>["e1","e2"]},
+    pass               => 0,
+    path               => 't/data',
+    id_seq_composition => $id_seq_composition
+  };
+  lives_ok {$schema->resultset($table)->create($values)}
+    'library type is not defined - no error on insert';
+
+  $id_seq_composition = t::autoqc_util::find_or_save_composition(
+                $schema, {'id_run'    => 1111,
+                          'position'  => 1,
+                          'tag_index' => 8});
+  $values = {
     evaluation_results => {},
     criteria           => {},
     qc_outcome         => {},
@@ -108,9 +100,8 @@ subtest 'insert a basic record, do not allow incomplete data in update' => sub {
     pass               => 0,
     path               => 't/data'
   };
-  throws_ok {$new->update($values)}
-    qr/Evaluation results present, but library_type absent/,
-    'library type is not defined - error on update';
+  lives_ok {$new->update($values)}
+    'library type is not defined - no error on update';
 
   $values = {
     id_seq_composition => $id_seq_composition,
@@ -277,7 +268,7 @@ subtest 'a full insert/update record with mqc outcome' => sub {
   };
   my $created=$schema->resultset($table)->create($values);
   $rs = $schema->resultset($table)->search({});
-  is ($rs->count, 2, q[two rows in the table]);
+  is ($rs->count, 3, q[three rows in the table]);
 
   $mqc_rs = $schema->resultset($mqc_table)->search({id_seq_composition => $id_seq_composition});
   is ($mqc_rs->count, 1, 'one mqc record for the entity');

t/60-autoqc-checks-review.t

@@ -401,7 +401,7 @@ subtest 'single expression evaluation' => sub {
 };
 
 subtest 'evaluation within the execute method' => sub {
-  plan tests => 42;
+  plan tests => 48;
 
   local $ENV{NPG_CACHED_SAMPLESHEET_FILE} =
     't/data/autoqc/review/samplesheet_29524.csv';
@@ -439,6 +439,8 @@ subtest 'evaluation within the execute method' => sub {
   foreach my $check (@check_objects) {
     lives_ok { $check->execute } 'execute method runs OK';
     is ($check->result->pass, 1, 'result pass attribute is set to 1');
+    is ($check->result->library_type, 'HiSeqX PCR free',
+      'result library_type attribute is set correctly');
     my %expected = map { $_ => 1 } @{$criteria_list};
     is_deeply ($check->result->evaluation_results(), \%expected,
       'evaluation results are saved');
@@ -454,7 +456,19 @@ subtest 'evaluation within the execute method' => sub {
     $count++;
   }
 
+  # Undefined library type should not be a problem.
+  my $lane_lims = (st::api::lims->new(id_run=> 29524)->children())[0];
+  is ($lane_lims->library_type, undef, 'library type is undefined on lane level');
   my $check = npg_qc::autoqc::checks::review->new(
+    conf_path => $test_data_dir,
+    qc_in     => $test_data_dir,
+    rpt_list  => $rpt_list,
+    lims      => $lane_lims
+  );
+  lives_ok { $check->execute } 'execute method runs OK';
+  is ($check->result->library_type, undef, 'library_type attribute is unset');
+
+  $check = npg_qc::autoqc::checks::review->new(
     conf_path => "$test_data_dir/unknown_qc_type",
     qc_in     => $dir,
     rpt_list  => $rpt_list);