Dropped uqc QC type for the review check

Changes

@@ -1,12 +1,17 @@
 LIST OF CHANGES FOR NPG-QC PACKAGE
 
- - Previously the code allowed for an empty applicability_criteria hash, which
-   resulted in a particular set of QC criteria being applied to every and any
-   product. Very early on this was an intended behaviour for UKB data. The main
-   filter was the study id. The default section of the product configuration
-   file does not have an external filter, so there is a real danger of the
-   review check being run indiscriminately for any product. While this will
-   never be an intention, small errors in the YML file might have this effect.
+ - npg_qc::autoqc::check::review:
+   1. Previously the code allowed for an empty applicability_criteria hash,
+      which resulted in a particular set of QC criteria being applied to every
+      and any product. Very early on this was an intended behaviour for UKB
+      data. The main filter was the study id. The default section of the product
+      configuration file does not have an external filter, so there is a real
+      danger of the review check being run indiscriminately for any product.
+      While this will never be an intention, small errors in the YML file might
+      have this effect.
+   2. Ability to set UQC outcomes was introduced for the Heron project. This
+      functionality was never used and is unlikely to be used in future, it is
+      now removed.
 
 release 72.2.1 (2024-10-04)
  - Added .github/dependabot.yml file to auto-update GitHub actions

MANIFEST

@@ -523,7 +523,6 @@ t/data/autoqc/review/mqc_type/product_release.yml
 t/data/autoqc/review/lims_applicability_empty/product_release.yml
 t/data/autoqc/review/no_known_applicability_type/product_release.yml
 t/data/autoqc/review/unknown_qc_type/product_release.yml
-t/data/autoqc/review/uqc_type/product_release.yml
 t/data/autoqc/review/default_and_study_section/product_release.yml
 t/data/autoqc/review/default_section/product_release.yml
 t/data/autoqc/review/wrong_default_and_study_section/product_release.yml

lib/npg_qc/autoqc/checks/review.pm

@@ -35,7 +35,7 @@ Readonly::Array  my @APPLICABILITY_CRITERIA_TYPES => (
 Readonly::Scalar my $ACCEPTANCE_CRITERIA_KEY    => q[acceptance_criteria];
 
 Readonly::Scalar my $QC_TYPE_DEFAULT  => q[mqc];
-Readonly::Array  my @VALID_QC_TYPES   => ($QC_TYPE_DEFAULT, q[uqc]);
+Readonly::Array  my @VALID_QC_TYPES   => ($QC_TYPE_DEFAULT);
 
 Readonly::Scalar my $TIMESTAMP_FORMAT_WOFFSET => q[%Y-%m-%dT%T%z];
 
@@ -106,11 +106,6 @@ marked as 'Preliminary' (examples: 'Accepted Final',
 'Rejected Preliminary'). By default the final_qc_outcome flag is
 false and the produced outcomes are preliminary.
 
-A valid User QC outcome is one of the values from the
-uqc_outcome_dict table of the npg_qc database. A concept of
-the finality and, hence, immutability of the outcome is not
-applicable to user QC outcome.
-
 The type of QC outcome can be configured within the Robo QC
 section of product configuration. The default type is library
 Manual QC.
@@ -332,8 +327,9 @@ sub execute {
   }
 
   $self->result->criteria($self->_criteria);
-  my $md5 = $self->result->generate_checksum4data($self->result->criteria);
-  $self->result->criteria_md5($md5);
+  $self->result->criteria_md5(
+    $self->result->generate_checksum4data($self->result->criteria)
+  );
   my $err;
 
   try {
@@ -344,7 +340,7 @@ sub execute {
     $self->result->add_comment($err);
   };
   not $err and $self->result->qc_outcome(
-    $self->generate_qc_outcome($self->_outcome_type(), $md5));
+    $self->generate_qc_outcome($self->_outcome_type()));
 
   return;
 }
@@ -371,34 +367,26 @@ sub evaluate {
 
 Returns a hash reference representing the QC outcome.
 
-  my $u_outcome = $r->generate_qc_outcome('uqc', $md5);
   my $m_outcome = $r->generate_qc_outcome('mqc');
-
+  
 =cut
 
 sub generate_qc_outcome {
-  my ($self, $outcome_type, $md5) = @_;
+  my ($self, $outcome_type) = @_;
 
   $outcome_type or croak 'outcome type should be defined';
 
   my $package_name = 'npg_qc::Schema::Mqc::OutcomeDict';
   my $pass = $self->result->pass;
   #####
   # Any of Accepted, Rejected, Undecided outcomes can be returned here
-  my $outcome = ($outcome_type eq $QC_TYPE_DEFAULT)
-    ? $package_name->generate_short_description(
-      $self->final_qc_outcome ? 1 : 0, $pass)
-    : $package_name->generate_short_description_prefix($pass);
+  my $outcome = $package_name->generate_short_description(
+    $self->final_qc_outcome ? 1 : 0, $pass);
 
   $outcome_type .= '_outcome';
   my $outcome_info = { $outcome_type => $outcome,
                        timestamp   => create_current_timestamp(),
                        username    => $ROBO_KEY};
-  if ($outcome_type =~ /\Auqc/xms) {
-    my @r = ($ROBO_KEY, $VERSION);
-    $md5 and push @r, $md5;
-    $outcome_info->{'rationale'} = join q[ ], @r;
-  }
 
   return $outcome_info;
 }

t/50-schema-result-Review.t

@@ -1,6 +1,6 @@
 use strict;
 use warnings;
-use Test::More tests => 6;
+use Test::More tests => 5;
 use Test::Exception;
 use Moose::Meta::Class;
 use Digest::MD5 qw/md5_hex/;
@@ -306,135 +306,17 @@ subtest 'a full insert/update record with mqc outcome' => sub {
   is ($rs->next->description, 'Accepted final', 'outcome has not changed');
 };
 
-subtest 'a full insert/update record with uqc outcome' => sub {
-  plan tests => 36;
-
-  my $initial_num_records = $schema->resultset($table)->search({})->count();
-
-  my $uqc_table = 'UqcOutcomeEnt';
-  my $id_seq_composition = t::autoqc_util::find_or_save_composition(
-                $schema, {'id_run'    => 1111,
-                          'position'  => 1,
-                          'tag_index' => 33});
-
-  my $uqc_rs = $schema->resultset($uqc_table)->search({id_seq_composition => $id_seq_composition});
-  is ($uqc_rs->count, 0, 'no uqc records for this entity');
-  my $rs = $schema->resultset($table)->search({id_seq_composition => $id_seq_composition});
-  is ($rs->count, 0, 'no review records for this entity');
-
-  my $qc_outcome = {"uqc_outcome" => "Rejected",
-                    "timestamp"   => "2018-06-03T12:53:46+0000",
-                    "username"    => "robo_qc",
-                    "rationale"   => "testrobo"};
-  my $values = {
-    id_seq_composition => $id_seq_composition,
-    library_type       => 'common_type',
-    evaluation_results => {"e1"=>1,"e2"=>0},
-    criteria           => {"and"=>["e1","e2"]},
-    qc_outcome         => $qc_outcome,
-    pass => 0,
-    path => 't/data'
-  };
-  my $cmd5 = md5_hex(JSON::XS->new()->canonical(1)->encode($values->{criteria}));
-
-  isa_ok($schema->resultset($table)->create($values), 'npg_qc::Schema::Result::' . $table);
-  is ($schema->resultset($table)->search({})->count(), $initial_num_records + 1,
-    'total number of records in teh review table went up by one');
-  $rs = $schema->resultset($table)->search({id_seq_composition => $id_seq_composition});
-  is ($rs->count, 1, 'one row created in the review table');
-  my $row = $rs->next;
-  is_deeply ($row->qc_outcome, $qc_outcome, 'qc outcome saved');
-  is_deeply ($row->evaluation_results, {"e1"=>1,"e2"=>0}, 'evaluation results saved');
-  is_deeply ($row->criteria, {"and"=>["e1","e2"]}, 'criteria saved');
-  is ($row->criteria_md5, $cmd5, 'checksum saved');
-  $uqc_rs = $schema->resultset($uqc_table)->search({id_seq_composition => $id_seq_composition});
-  is ($uqc_rs->count, 1, 'one row created in the uqc table');
-  my $outcome = $uqc_rs->next;
-  is ($outcome->rationale, 'testrobo', 'rationale recorded');
-  is ($outcome->description, 'Rejected', 'correct uqc outcome');
-  is ($outcome->modified_by, $ENV{USER}, 'correct user');
-  is ($outcome->username, 'robo_qc', 'correct user');
-  my $dt = $outcome->last_modified();
-  is ($dt->year, 2018, 'correct year');
-  is ($dt->month, '6', 'correct month');
-  is ($dt->minute, '53', 'correct minute');
-  is ($dt->second, '46', 'correct second');
-
-  $qc_outcome = {"uqc_outcome" => "Accepted",
-                 "timestamp"   => "2018-08-05T12:53:46+0000",
-                 "username"    => "robo_qc",
-                 "rationale"   => "testrobo1"};
-  $values = {
-    id_seq_composition => $id_seq_composition,
-    library_type       => 'common_type',
-    evaluation_results => {"e1"=>1,"e2"=>1},
-    criteria           => {"and"=>["e1","e2"]},
-    qc_outcome         => $qc_outcome,
-  };
-  $row->update($values);
-  is_deeply ($row->evaluation_results, {"e1"=>1,"e2"=>1}, 'evaluation results updated');
-  is_deeply ($row->qc_outcome, $qc_outcome, 'qc outcome updated');
-  $outcome = $schema->resultset($uqc_table)->search({id_seq_composition => $id_seq_composition})
-                    ->next;
-  is ($outcome->rationale, 'testrobo1', 'rationale updated');
-  is ($outcome->description, 'Accepted', 'uqc outcome updated');
-  is ($outcome->last_modified()->month, 8, 'month updated');
-  is ($outcome->last_modified()->year, 2018, 'correct year');
-
-  $id_seq_composition = t::autoqc_util::find_or_save_composition(
-                $schema, {'id_run'    => 1111,
-                          'position'  => 1,
-                          'tag_index' => 34});
-  $uqc_rs = $schema->resultset($uqc_table)->search({id_seq_composition => $id_seq_composition});
-  is ($uqc_rs->count, 0, 'no uqc records for this entity');
-  my $new_uqc = $schema->resultset($uqc_table)
-                ->new_result({id_seq_composition => $id_seq_composition});
-  $new_uqc->update_outcome({uqc_outcome => 'Accepted', rationale => 'test1'}, 'user1', 'test');
-  $uqc_rs = $schema->resultset($uqc_table)->search({id_seq_composition => $id_seq_composition});
-  is ($uqc_rs->count, 1, 'record created');
-
-  $values = {
-    id_seq_composition => $id_seq_composition,
-    library_type       => 'common_type',
-    evaluation_results => '{"e1":1,"e2":1}',
-    criteria           => '{"and":["e1","e2"]}',
-    qc_outcome => {"uqc_outcome" => "Undecided",
-                   "timestamp"   => "2018-09-03T12:58:43+0000",
-                   "username"    => "robo_qc",
-                   "rationale"   => "testrobo2"},
-    pass => undef
-  };
-  my $created=$schema->resultset($table)->create($values);
-
-  is ($schema->resultset($table)->search({})->count(), $initial_num_records + 2,
-    'total number of records in teh review table went up by one');
-  $uqc_rs = $schema->resultset($uqc_table)->search({id_seq_composition => $id_seq_composition});
-  is ($uqc_rs->count, 1, 'one uqc record for the entity');
-  $outcome = $uqc_rs->next;
-  is ($outcome->rationale, 'testrobo2', 'rationale updated');
-  is ($outcome->description, 'Undecided', 'outcome updated');
-  is ($outcome->modified_by, $ENV{USER}, 'correct user');
-  is ($outcome->username, 'robo_qc', 'correct user');
-  $dt = $outcome->last_modified();
-  is ($dt->year, 2018, 'correct year');
-  is ($dt->month, '9', 'correct month');
-  is ($dt->minute, '58', 'correct minute');
-  is ($dt->second, '43', 'correct second');
-};
-
-
 subtest 'unknown outcome type should give an error' => sub {
   plan tests => 5;
 
-  my $uqc_table = 'UqcOutcomeEnt';
   my $id_seq_composition = t::autoqc_util::find_or_save_composition(
                 $schema, {'id_run'    => 1111,
                           'position'  => 1,
                           'tag_index' => 43});
 
   my $no_records = sub {
-    is ($schema->resultset($uqc_table)->search({id_seq_composition => $id_seq_composition})->count,
-      0, 'no uqc records for this entity');
+    is ($schema->resultset($mqc_table)->search({id_seq_composition => $id_seq_composition})->count,
+      0, 'no mqc records for this entity');
     is ($schema->resultset($table)->search({id_seq_composition => $id_seq_composition})->count,
       0, 'no review records for this entity');
   };

t/60-autoqc-checks-review.t

@@ -417,7 +417,7 @@ subtest 'single expression evaluation' => sub {
 };
 
 subtest 'evaluation within the execute method' => sub {
-  plan tests => 48;
+  plan tests => 40;
 
   local $ENV{NPG_CACHED_SAMPLESHEET_FILE} =
     't/data/autoqc/review/samplesheet_29524.csv';
@@ -436,7 +436,6 @@ subtest 'evaluation within the execute method' => sub {
   is ($o->result->pass, undef, 'result pass attribute is unset');
 
   my @check_objects = ();
-
   push @check_objects, npg_qc::autoqc::checks::review->new(
     runfolder_path => $rf_path,
     conf_path => $test_data_dir,
@@ -446,12 +445,7 @@ subtest 'evaluation within the execute method' => sub {
     conf_path => "$test_data_dir/mqc_type",
     qc_in     => $dir,
     rpt_list  => $rpt_list);
-  push @check_objects, npg_qc::autoqc::checks::review->new(
-    conf_path => "$test_data_dir/uqc_type",
-    qc_in     => $dir,
-    rpt_list  => $rpt_list);
 
-  my $count = 0;
   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');
@@ -461,15 +455,9 @@ subtest 'evaluation within the execute method' => sub {
     is_deeply ($check->result->evaluation_results(), \%expected,
       'evaluation results are saved');
     my $outcome = $check->result->qc_outcome;
-    if ($count < 2) {
-      is ($outcome->{'mqc_outcome'} , 'Accepted preliminary', 'correct outcome string');
-    } elsif ($count == 2) {
-      is ($outcome->{'uqc_outcome'} , 'Accepted', 'correct outcome string');
-      ok ($outcome->{'rationale'} , 'rationale is set');
-    }
+    is ($outcome->{'mqc_outcome'} , 'Accepted preliminary', 'correct outcome string');
     is ($outcome->{'username'}, 'robo_qc', 'correct process id');
     ok ($outcome->{'timestamp'}, 'timestamp saved');
-    $count++;
   }
 
   # Undefined library type should not be a problem.