diff --git a/Changes b/Changes index 5ec4bf7a..72100f65 100644 --- a/Changes +++ b/Changes @@ -7,6 +7,11 @@ LIST OF CHANGES FOR NPG-QC PACKAGE 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. + - 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: diff --git a/lib/npg_qc/Schema/Result/Review.pm b/lib/npg_qc/Schema/Result/Review.pm index 1aac0e56..7ce04be9 100644 --- a/lib/npg_qc/Schema/Result/Review.pm +++ b/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 Emg8@sanger.ac.ukE =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. diff --git a/lib/npg_qc/autoqc/checks/review.pm b/lib/npg_qc/autoqc/checks/review.pm index 08a24267..4c1972ab 100644 --- a/lib/npg_qc/autoqc/checks/review.pm +++ b/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) { diff --git a/t/50-schema-result-Review.t b/t/50-schema-result-Review.t index 6be7101c..789bedca 100644 --- a/t/50-schema-result-Review.t +++ b/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'); diff --git a/t/60-autoqc-checks-review.t b/t/60-autoqc-checks-review.t index 36b769a3..b2f74853 100644 --- a/t/60-autoqc-checks-review.t +++ b/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);