diff --git a/Changes b/Changes index 5ec4bf7a..53eecd57 100644 --- a/Changes +++ b/Changes @@ -6,7 +6,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: 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);