Skip to content

Commit

Permalink
code review
Browse files Browse the repository at this point in the history
  • Loading branch information
@sajp
sajp committed Jan 2, 2013
1 parent 701bba9 commit 34c624e
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 46 deletions.
31 changes: 20 additions & 11 deletions lib/LIMS2/Model/Plugin/Process.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ sub create_process {

sub pspec_add_recombinase_data {
return {
plate_name => { validate => 'plate_name' },
plate_name => { validate => 'existing_plate_name' },
well_name => { validate => 'well_name' },
recombinase => { validate => 'existing_recombinase' },
plate_name => { validate => 'plate_name' },
plate_name => { validate => 'existing_plate_name' },
well_name => { validate => 'well_name' },
recombinase => { validate => 'existing_recombinase' },
};
}

Expand All @@ -55,12 +55,9 @@ sub add_recombinase_data {

my $well = $self->retrieve_well( $validated_params );

LIMS2::Exception::Validation->throw(
"Well does not exist"
) unless $well;

my @process = $well->parent_processes;

#TODO use $self->throw
LIMS2::Exception::Validation->throw(
"could not retreive process"
) unless @process ;
Expand All @@ -86,10 +83,10 @@ sub upload_recombinase_file_data {
my $error_log;
my $line = 1;


foreach my $recombinase (@{$recombinase_data}){
$line++;

#TODO $self->throw
LIMS2::Exception::Validation->throw(
"invalid column names or data"
) unless $recombinase->{plate_name} && $recombinase->{well_name} && $recombinase->{recombinase};
Expand All @@ -98,12 +95,24 @@ sub upload_recombinase_file_data {
$params->{well_name} = $recombinase->{well_name};
$params->{recombinase} = $recombinase->{recombinase};
try{
#TODO see if you can not use $params here, use $recombinase
add_recombinase_data( $self, $params );
}
catch{
$error_log .= 'line ' . $line . ': plate ' . $params->{plate_name} . ', well ' . $params->{well_name} . ' , recombinase ' . $params->{recombinase} . ' ERROR: ' . $_ ;
}
$error_log
.= 'line '
. $line
. ': plate '
. $params->{plate_name}
. ', well '
. $params->{well_name}
. ' , recombinase '
. $params->{recombinase}
. ' ERROR: '
. $_;
};
}
#TODO $self->throw
LIMS2::Exception::Validation->throw(
"$error_log"
)if $error_log;
Expand Down
47 changes: 30 additions & 17 deletions lib/LIMS2/Model/Plugin/Well.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -506,18 +506,19 @@ sub pspec_update_well_colony_picks {

sub update_well_colony_picks{
my ( $self, $params ) = @_;
use Smart::Comments;
my $validated_params = $self->check_params( $params, $self->pspec_update_well_colony_picks, ignore_unknown => 1);

my $plate_name = $validated_params->{plate_name};
my $well_name = $validated_params->{well_name};
my $validated_params = $self->check_params( $params, $self->pspec_update_well_colony_picks,
ignore_unknown => 1 );

my @colony_types = map { $_->id } $self->schema->resultset('ColonyCountType')->all;
my $well = $self->retrieve_well({ plate_name => $plate_name, well_name => $well_name });
foreach my $colony_type (@colony_types){
my $well = $self->retrieve_well(
{ plate_name => $validated_params->{plate_name},
well_name => $validated_params->{well_name}
}
);

foreach my $colony_type (@colony_types){
if (exists $params->{$colony_type} and $params->{$colony_type} =~ /^\d+$/){

$well->update_or_create_related( 'well_colony_counts' => {
colony_count_type_id => $colony_type,
colony_count => $params->{$colony_type},
Expand All @@ -542,21 +543,34 @@ sub upload_well_colony_picks_file_data {
$line++;
foreach my $column (keys %{$well_colony_picks}){

#TODO self->throw
#TODO check this is doing what you think its doing
LIMS2::Exception::Validation->throw(
"invalid column names or data"
) unless ( grep( /^$column$/, @columns ) );
#TODO use List::MoreUtils qw( none )

$params->{$column} = $well_colony_picks->{$column};
#TODO in wrong place
$params->{created_by} = $created_by;
};
#TODO just use $well_colony_picks and add created_by to this
try{
update_well_colony_picks( $self, $params )
}
catch{
$error_log .= 'line ' . $line . ': plate ' . $params->{plate_name} . ', well ' . $params->{well_name} . ' ERROR: $_';
$error_log
.= 'line '
. $line
. ': plate '
. $params->{plate_name}
. ', well '
. $params->{well_name}
. ' ERROR: $_';
};
$params = undef;
}
#TODO self->throw
LIMS2::Exception::Validation->throw(
"$error_log"
)if $error_log;
Expand All @@ -568,17 +582,16 @@ sub get_well_colony_pick_fields_values {
my ( $self, $params ) = @_;

my @colony_data;
my %fields = map { $_->id => {label => $_->id, name => $_->id } } $self->schema->resultset('ColonyCountType')->all;
my %fields = map { $_->id => { label => $_->id, name => $_->id } }
$self->schema->resultset('ColonyCountType')->all;

if (exists $params->{plate_name} && exists $params->{well_name}){
my $well = $self->retrieve_well( $params );
if ($well){
@colony_data = $well->well_colony_counts;

if (@colony_data) {
foreach (@colony_data){
$fields{$_->colony_count_type_id}{att_values} = $_->colony_count;
}
my $well = $self->retrieve_well( $params );
@colony_data = $well->well_colony_counts;

if (@colony_data) {
foreach (@colony_data){
$fields{$_->colony_count_type_id}{att_values} = $_->colony_count;
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions lib/LIMS2/Model/Util/CreateProcess.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,7 @@ sub pspec_create_process_aux_data_recombinase {
return { recombinase => { validate => 'existing_recombinase' }, };
}

#NOTE order recombinaes added is just the order that they are specified in the array
## no critic(Subroutines::ProhibitUnusedPrivateSubroutine)
sub create_process_aux_data_recombinase {
my ( $model, $params, $process ) = @_;
Expand All @@ -541,6 +542,7 @@ sub create_process_aux_data_recombinase {
"recombinase process should have 1 or more recombinases"
) unless @{ $validated_params->{recombinase} };

#TODO fix this
if ($process->process_recombinases->find( { 'recombinase_id' => $validated_params->{recombinase} } )){
LIMS2::Exception::Validation->throw(
"recombinase process already exists"
Expand Down
30 changes: 17 additions & 13 deletions lib/LIMS2/WebApp/Controller/User/RecombinaseUpload.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,32 +19,38 @@ Catalyst Controller.
sub recombinase_upload :Path( '/user/recombinase_upload' ) :Args(0) {
my ( $self, $c ) = @_;

my $process_type = 'recombinase';

$c->stash(
process_fields => $c->model('Golgi')->get_process_fields( { process_type => $process_type } )
process_fields => $c->model('Golgi')->get_process_fields( { process_type => 'recombinase' } )
);
return;
}


sub add_recombinase :Path( '/user/add_recombinase' ) :Args(0) {
my ( $self, $c ) = @_;

unless ($c->request->params->{plate_name} and $c->request->params->{well_name} and $c->request->params->{recombinase}){
$c->flash->{error_msg} = 'Data must be specified for all three fields; Plate Name, Well Name and Recombinase';
#TODO make plate form auto complete
unless ($c->request->params->{plate_name}
and $c->request->params->{well_name}
and $c->request->params->{recombinase} )
{
$c->flash->{error_msg}
= 'Data must be specified for all three fields; Plate Name, Well Name and Recombinase';
$c->res->redirect( $c->uri_for('/user/recombinase_upload') );
return;
}

$c->assert_user_roles('edit');
try{
$c->model('Golgi')->txn_do(
sub {
shift->add_recombinase_data( $c->request->params );
$c->flash->{success_msg} = 'Add ' . $c->request->params->{recombinase} . ' recombinase for well ' . $c->request->params->{well_name} . ' on plate ' . $c->request->params->{plate_name} ;
}
sub { shift->add_recombinase_data( $c->request->params ) }
);
$c->flash->{success_msg}
= 'Add '
. $c->request->params->{recombinase}
. ' recombinase for well '
. $c->request->params->{well_name}
. ' on plate '
. $c->request->params->{plate_name};
}
catch {
s/\{(.*?)\}//;
Expand All @@ -54,7 +60,6 @@ sub add_recombinase :Path( '/user/add_recombinase' ) :Args(0) {
return;
}


sub upload_recombinase_file :Path( '/user/upload_recombinase_file' ) :Args(0) {
my ( $self, $c ) = @_;

Expand All @@ -71,9 +76,9 @@ sub upload_recombinase_file :Path( '/user/upload_recombinase_file' ) :Args(0) {
$c->model('Golgi')->txn_do(
sub {
shift->upload_recombinase_file_data( $recombinase_data->fh, $c->request->params );
$c->flash->{success_msg} = 'Successfully added recombinase to wells';
}
);
$c->flash->{success_msg} = 'Successfully added recombinase to wells';
}
catch{
$c->flash->{error_msg} = "$_";
Expand All @@ -83,7 +88,6 @@ sub upload_recombinase_file :Path( '/user/upload_recombinase_file' ) :Args(0) {
return;
}


=head1 AUTHOR
Peter Matthews
Expand Down
8 changes: 3 additions & 5 deletions lib/LIMS2/WebApp/Controller/User/WellData.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,11 @@ sub genotyping_qc_data : Path( '/user/genotyping_qc_data') : Args(0){
return;
}

#TODO re-write
sub update_colony_picks : Path( '/user/update_colony_picks' ) :Args(0) {
my ( $self, $c ) = @_;
my $params = $c->request->params;
my $model =$c->model('Golgi');
my $model = $c->model('Golgi');

unless (exists $params->{plate_name} && exists $params->{well_name}){
$c->stash(
Expand Down Expand Up @@ -151,11 +152,9 @@ sub update_colony_picks : Path( '/user/update_colony_picks' ) :Args(0) {

}


sub upload_well_colony_picks_file_data : Path( '/user/upload_well_colony_counts_file_data' ) :Args(0){
my ( $self, $c ) = @_;


my $well_colony_picks_data = $c->request->upload('datafile');
$c->request->params->{created_by} = $c->user->name;

Expand All @@ -170,17 +169,16 @@ sub upload_well_colony_picks_file_data : Path( '/user/upload_well_colony_counts
$c->model('Golgi')->txn_do(
sub {
shift->upload_well_colony_picks_file_data( $well_colony_picks_data->fh, $c->request->params );
$c->flash->{success_msg} = 'Successfully added well colony counts to wells';
}
);
$c->flash->{success_msg} = 'Successfully added well colony counts to wells';
}
catch{
$c->flash->{error_msg} = "$_";
};

$c->res->redirect( $c->uri_for('/user/update_colony_picks') );
return;

}

=head1 AUTHOR
Expand Down

0 comments on commit 34c624e

Please sign in to comment.