From debe0ef3ddb66b98507c905cd20e9a504f5ca896 Mon Sep 17 00:00:00 2001
From: mgcam <mg8@sanger.ac.uk>
Date: Fri, 9 Aug 2024 15:01:15 +0100
Subject: [PATCH]  Added the runfolder_path attribute to the review check.
 (#869)

---
 Changes                            |  3 +++
 lib/npg_qc/autoqc/checks/review.pm | 39 +++++++++++++++++++++++++++++-
 t/60-autoqc-autoqc.t               | 10 +++++++-
 t/60-autoqc-checks-review.t        | 19 +++++++++++++--
 4 files changed, 67 insertions(+), 4 deletions(-)

diff --git a/Changes b/Changes
index ed8167b95..3989322f5 100644
--- a/Changes
+++ b/Changes
@@ -2,6 +2,9 @@ LIST OF CHANGES FOR NPG-QC PACKAGE
 
  - docs
   - automated creation/update of confluence pages for docs
+ - To enable access to information about a sequencing run (from RunInfo.xml,
+   {r,R}unParameters.xml), added an optional 'runfolder_path' attribute to
+   the review check. The attribute will have to be set by the caller.
 
 release 72.1.2 (2024-08-05)
  - handle cases where the same tag sequence is seen in both index reads
diff --git a/lib/npg_qc/autoqc/checks/review.pm b/lib/npg_qc/autoqc/checks/review.pm
index 0c8e7d2ab..7cfbb9152 100644
--- a/lib/npg_qc/autoqc/checks/review.pm
+++ b/lib/npg_qc/autoqc/checks/review.pm
@@ -219,6 +219,43 @@ npg_tracking::util::pipeline_config
 A method. Returns the path of the product configuration file.
 Inherited from npg_tracking::util::pipeline_config
 
+=head2 runfolder_path
+
+The runfolder path, an optional attribute. In case of complex products
+(multi-component compositions) is only relevant if all components belong
+to the same sequencing run. This attribute is used to retrieve information
+from RunInfo.xml and {r,R}unParameters.xml files. Some 'robo' configuration
+might not require information of this nature, thus the attribute is optional.
+If the information from the above-mentioned files is required, but the access
+to the staging run folder is not available, the check cannot be run.
+
+=cut
+
+has 'runfolder_path' => (
+  isa => 'Str',
+  is  => 'ro',
+  required => 0,
+  predicate => 'has_runfolder_path',
+);
+
+
+=head2 BUILD
+
+A method that is run before returning the new object instance to the caller.
+Errors if any attributes of the object are are in conflict.
+
+=cut
+
+sub BUILD {
+  my $self = shift;
+  if ($self->has_runfolder_path && !$self->get_id_run) {
+    my $m = sprintf
+      'Product defined by rpt list %s does not belong to a single run.',
+      $self->rpt_list;
+    croak "$m 'runfolder_path' attribute should not be set.";
+  }
+}
+
 =head2 can_run
 
 Returns true if the check can be run, meaning a robo configuration
@@ -764,7 +801,7 @@ Marina Gourtovaia
 
 =head1 LICENSE AND COPYRIGHT
 
-Copyright (C) 2019,2020 Genome Research Ltd.
+Copyright (C) 2019,2020,2024 Genome Research Ltd.
 
 This file is part of NPG.
 
diff --git a/t/60-autoqc-autoqc.t b/t/60-autoqc-autoqc.t
index e3876860f..7af743ca1 100644
--- a/t/60-autoqc-autoqc.t
+++ b/t/60-autoqc-autoqc.t
@@ -1,6 +1,6 @@
 use strict;
 use warnings;
-use Test::More tests => 24;
+use Test::More tests => 26;
 use Test::Exception;
 use File::Temp qw/tempdir/;
 
@@ -88,4 +88,12 @@ my $dir = tempdir( CLEANUP => 1 );
     'check is an instance of the foo1 autoqc class');
 }
 
+{
+  local @ARGV = qw(--check review --runfolder_path t --rpt_list 4:1 --qc_out);
+  push @ARGV, $dir;
+  my $check = npg_qc::autoqc::autoqc->new_with_options()->create_check_object();
+  isa_ok($check, 'npg_qc::autoqc::checks::review');  
+  is ($check->runfolder_path, 't', '--runfolder_path option is passed through');
+}
+
 1;
diff --git a/t/60-autoqc-checks-review.t b/t/60-autoqc-checks-review.t
index b6fd53f2d..e8595507a 100644
--- a/t/60-autoqc-checks-review.t
+++ b/t/60-autoqc-checks-review.t
@@ -31,8 +31,8 @@ my $criteria_list = [
   'verify_bam_id.freemix < 0.01'
 ];
 
-subtest 'construction object, deciding whether to run' => sub {
-  plan tests => 27;
+subtest 'constructing object, deciding whether to run' => sub {
+  plan tests => 29;
 
   my $check = npg_qc::autoqc::checks::review->new(
     conf_path => $test_data_dir,
@@ -40,6 +40,21 @@ subtest 'construction object, deciding whether to run' => sub {
     rpt_list  => '27483:1:2');
   isa_ok ($check, 'npg_qc::autoqc::checks::review');
   isa_ok ($check->result, 'npg_qc::autoqc::results::review');
+
+  lives_ok { npg_qc::autoqc::checks::review->new(
+    conf_path      => $test_data_dir,
+    qc_in          => $test_data_dir,
+    runfolder_path => $test_data_dir,
+    rpt_list       => '27483:1:2;27483:2:2')
+  } 'object created OK for components from the same run';
+  throws_ok { npg_qc::autoqc::checks::review->new(
+    conf_path      => $test_data_dir,
+    qc_in          => $test_data_dir,
+    runfolder_path => $test_data_dir,
+    rpt_list       => '27483:1:2;27484:2:2')
+  } qr/'runfolder_path' attribute should not be set/,
+    'error creating an object for components from different runs';
+
   my $can_run;
   warnings_like { $can_run = $check->can_run }
     [qr/Study config not found for/],