From 20becbaafd643d8ed3e63ee6c361ee856bebaeb7 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Thu, 26 Jan 2023 19:14:03 -0600 Subject: [PATCH] Remove the course_id from the past_answer table. It is time for this to go. It is an entirely unnecessary column. This is taken care of by the usual course upgrade procedure in the admin course. Don't test this on a course that you want to go back to other branches with! It isn't so easy to add the course_id column back. It can be done though if needed. --- bin/dump_past_answers | 2 +- lib/Caliper/Entity.pm | 15 ++-- lib/WeBWorK/ContentGenerator/CourseAdmin.pm | 73 +++++++++++++------ lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 11 +-- lib/WeBWorK/ContentGenerator/Hardcopy.pm | 4 +- .../Instructor/ProblemGrader.pm | 3 +- lib/WeBWorK/ContentGenerator/Problem.pm | 7 +- lib/WeBWorK/DB.pm | 39 ++-------- lib/WeBWorK/DB/Record/PastAnswer.pm | 1 - lib/WeBWorK/DB/Schema/NewSQL.pm | 6 -- lib/WeBWorK/DB/Schema/NewSQL/Std.pm | 66 ++++++++++++++++- lib/WeBWorK/HTML/SingleProblemGrader.pm | 2 +- lib/WeBWorK/Utils/CourseIntegrityCheck.pm | 33 ++++++--- lib/WeBWorK/Utils/ProblemProcessing.pm | 1 - .../archive_course_confirm.html.ep | 11 ++- .../CourseAdmin/rename_course_confirm.html.ep | 11 ++- 16 files changed, 182 insertions(+), 103 deletions(-) diff --git a/bin/dump_past_answers b/bin/dump_past_answers index d41394bc1d..1228da7ff8 100755 --- a/bin/dump_past_answers +++ b/bin/dump_past_answers @@ -239,7 +239,7 @@ foreach my $courseID (@courses) { $row[24] = defined($tags->{keywords}) ? join(',', @{ $tags->{keywords} }) : ''; } - my @answerIDs = $db->listProblemPastAnswers($courseID, $userID, $setID, $problemID); + my @answerIDs = $db->listProblemPastAnswers($userID, $setID, $problemID); my @answers = $db->getPastAnswers(\@answerIDs); # go through attempts diff --git a/lib/Caliper/Entity.pm b/lib/Caliper/Entity.pm index 11474f4ac3..7a9a95426f 100644 --- a/lib/Caliper/Entity.pm +++ b/lib/Caliper/Entity.pm @@ -285,8 +285,7 @@ sub answer { my $resource_iri = Caliper::ResourseIri->new($ce); my $last_answer_id = - $db->latestProblemPastAnswer($ce->{"courseName"}, $user_id, ($version_id ? "$set_id,v$version_id" : $set_id), - $problem_id); + $db->latestProblemPastAnswer($user_id, ($version_id ? "$set_id,v$version_id" : $set_id), $problem_id); my $last_answer = $db->getPastAnswer($last_answer_id); my @answers = split(/\t/, $last_answer->answer_string()); @@ -322,14 +321,10 @@ sub answer_attempt { ? $db->getMergedProblemVersion($user_id, $set_id, $version_id, $problem_id) : $db->getMergedProblem($user_id, $set_id, $problem_id); my $last_answer_id = - $db->latestProblemPastAnswer($ce->{"courseName"}, $user_id, ($version_id ? "$set_id,v$version_id" : $set_id), - $problem_id); + $db->latestProblemPastAnswer($user_id, ($version_id ? "$set_id,v$version_id" : $set_id), $problem_id); my $last_answer = $db->getPastAnswer($last_answer_id); - my $attempt = - $version_id - ? $version_id - : scalar $db->listProblemPastAnswers($ce->{"courseName"}, $user_id, $set_id, $problem_id); - my $score = $problem_user->status || 0; + my $attempt = $version_id ? $version_id : scalar $db->listProblemPastAnswers($user_id, $set_id, $problem_id); + my $score = $problem_user->status || 0; $score = 0 if ($score > 1 || $score < 0); my $answer_attempt = { @@ -369,7 +364,7 @@ sub problem_set_attempt { } else { my @problem_ids = $db->listGlobalProblems($set_id); for my $problem_id (@problem_ids) { - $attempt += scalar $db->listProblemPastAnswers($ce->{"courseName"}, $user_id, $set_id, $problem_id); + $attempt += scalar $db->listProblemPastAnswers($user_id, $set_id, $problem_id); } } diff --git a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm index eea7f41b73..b589101971 100644 --- a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm +++ b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm @@ -1354,7 +1354,7 @@ sub upgrade_course_confirm ($c) { # Report on database status my ($tables_ok, $dbStatus) = $CIchecker->checkCourseTables($upgrade_courseID); - my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $db_report) = + my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes, $db_report) = $c->formatReportOnDatabaseTables($dbStatus, $upgrade_courseID); my $course_output = $c->c; @@ -1413,6 +1413,21 @@ sub upgrade_course_confirm ($c) { ); } + if ($rebuild_table_indexes) { + push( + @$course_output, + $c->tag( + 'p', + class => 'text-danger fw-bold', + $c->maketext( + 'There are extra database fields which are not defined in the schema and were part of the key ' + . 'for at least one table. These fields must be deleted and the table indexes rebuilt. ' + . 'Warning: This will destroy all data contained in the field and is not undoable!' + ) + ) + ); + } + # Report on directory status my ($directories_ok, $directory_report) = $CIchecker->checkCourseDirectories; push(@$course_output, $c->tag('div', class => 'mb-2', $c->maketext('Directory structure:'))); @@ -1496,7 +1511,7 @@ sub do_upgrade_course ($c) { # Analyze database status and prepare status report ($tables_ok, $dbStatus) = $CIchecker->checkCourseTables($upgrade_courseID); - my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $db_report) = + my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes, $db_report) = $c->formatReportOnDatabaseTables($dbStatus); # Prepend course name @@ -2299,6 +2314,7 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) { my $all_tables_ok = 1; my $extra_database_tables = 0; my $extra_database_fields = 0; + my $rebuild_table_indexes = 0; my $db_report = $c->c; @@ -2334,25 +2350,35 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) { my $field_report = $c->c("$key: $field_status_message{$field_status}"); if ($field_status == WeBWorK::Utils::CourseIntegrityCheck::ONLY_IN_B) { - $extra_database_fields = 1; - push( - @$field_report, - $c->tag( - 'span', - class => 'form-check d-inline-block', - $c->tag( - 'label', - class => 'form-check-label', - $c->c( - $c->check_box( - "$courseID.$table.delete_fieldIDs" => $key, - class => 'form-check-input' - ), - $c->maketext('Delete field when upgrading') - )->join('') - ) - ) - ) if defined $courseID; + if ($fieldInfo{$key}[1]) { + $rebuild_table_indexes = 1; + } else { + $extra_database_fields = 1; + } + if (defined $courseID) { + if ($fieldInfo{$key}[1]) { + push(@$field_report, $c->hidden_field("$courseID.$table.delete_fieldIDs" => $key)); + } else { + push( + @$field_report, + $c->tag( + 'span', + class => 'form-check d-inline-block', + $c->tag( + 'label', + class => 'form-check-label', + $c->c( + $c->check_box( + "$courseID.$table.delete_fieldIDs" => $key, + class => 'form-check-input' + ), + $c->maketext('Delete field when upgrading') + )->join('') + ) + ) + ); + } + } } elsif ($field_status == WeBWorK::Utils::CourseIntegrityCheck::ONLY_IN_A) { $all_tables_ok = 0; } @@ -2367,7 +2393,10 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) { push(@$db_report, $c->tag('p', class => 'text-success', $c->maketext('Database tables are ok'))) if $all_tables_ok; - return ($all_tables_ok, $extra_database_tables, $extra_database_fields, $db_report->join('')); + return ( + $all_tables_ok, $extra_database_tables, $extra_database_fields, + $rebuild_table_indexes, $db_report->join('') + ); } 1; diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 04dd2b5ab0..d2c24cab3a 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -246,12 +246,10 @@ sub attemptResults ($c, $pg) { sub get_instructor_comment ($c, $problem) { return unless ref($problem) =~ /ProblemVersion/; - my $db = $c->db; - my $userPastAnswerID = $db->latestProblemPastAnswer( - $c->ce->{courseName}, - $problem->user_id, $problem->set_id . ',v' . $problem->version_id, - $problem->problem_id - ); + my $db = $c->db; + my $userPastAnswerID = + $db->latestProblemPastAnswer($problem->user_id, $problem->set_id . ',v' . $problem->version_id, + $problem->problem_id); if ($userPastAnswerID) { my $userPastAnswer = $db->getPastAnswer($userPastAnswerID); @@ -1080,7 +1078,6 @@ async sub pre_header_initialize ($c) { # Add to PastAnswer db my $pastAnswer = $db->newPastAnswer(); - $pastAnswer->course_id($c->stash('courseID')); $pastAnswer->user_id($problem->user_id); $pastAnswer->set_id($setVName); $pastAnswer->problem_id($problem->problem_id); diff --git a/lib/WeBWorK/ContentGenerator/Hardcopy.pm b/lib/WeBWorK/ContentGenerator/Hardcopy.pm index ca1603a9cf..10575bfa1a 100644 --- a/lib/WeBWorK/ContentGenerator/Hardcopy.pm +++ b/lib/WeBWorK/ContentGenerator/Hardcopy.pm @@ -1347,8 +1347,8 @@ async sub write_problem_tex ($c, $FH, $TargetUser, $MergedSet, $themeTree, $prob } if ($showComments) { - my $userPastAnswerID = $db->latestProblemPastAnswer($c->stash('courseID'), - $MergedProblem->user_id, $versionName, $MergedProblem->problem_id); + my $userPastAnswerID = + $db->latestProblemPastAnswer($MergedProblem->user_id, $versionName, $MergedProblem->problem_id); my $pastAnswer = $userPastAnswerID ? $db->getPastAnswer($userPastAnswerID) : 0; my $comment = $pastAnswer && $pastAnswer->comment_string ? $pastAnswer->comment_string : ""; diff --git a/lib/WeBWorK/ContentGenerator/Instructor/ProblemGrader.pm b/lib/WeBWorK/ContentGenerator/Instructor/ProblemGrader.pm index 617c09389a..9d9e1263be 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/ProblemGrader.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/ProblemGrader.pm @@ -99,8 +99,7 @@ async sub initialize ($c) { next unless defined $_->{problem}; my $versionID = ref($_->{problem}) =~ /::ProblemVersion/ ? $_->{problem}->version_id : 0; my $userPastAnswerID = - $db->latestProblemPastAnswer($courseName, $user->user_id, $setID . ($versionID ? ",v$versionID" : ''), - $problemID); + $db->latestProblemPastAnswer($user->user_id, $setID . ($versionID ? ",v$versionID" : ''), $problemID); $_->{past_answer} = $db->getPastAnswer($userPastAnswerID) if ($userPastAnswerID); ($_->{problemNumber}, $_->{pageNumber}) = getTestProblemPosition($db, $_->{problem}) if $versionID; diff --git a/lib/WeBWorK/ContentGenerator/Problem.pm b/lib/WeBWorK/ContentGenerator/Problem.pm index ce5451a027..e596fe288d 100644 --- a/lib/WeBWorK/ContentGenerator/Problem.pm +++ b/lib/WeBWorK/ContentGenerator/Problem.pm @@ -1429,11 +1429,8 @@ sub output_misc ($c) { sub output_comments ($c) { my $db = $c->db; - my $userPastAnswerID = $db->latestProblemPastAnswer( - $c->stash('courseID'), - $c->param('effectiveUser'), - $c->stash('setID'), $c->stash('problemID') - ); + my $userPastAnswerID = + $db->latestProblemPastAnswer($c->param('effectiveUser'), $c->stash('setID'), $c->stash('problemID')); # If there is a comment then display it. if ($userPastAnswerID) { diff --git a/lib/WeBWorK/DB.pm b/lib/WeBWorK/DB.pm index 7207c4380c..4d59f2504c 100644 --- a/lib/WeBWorK/DB.pm +++ b/lib/WeBWorK/DB.pm @@ -1103,21 +1103,12 @@ BEGIN { sub countProblemPastAnswers { return scalar shift->listPastAnswers(@_) } sub listProblemPastAnswers { - my ($self, $courseID, $userID, $setID, $problemID); + my ($self, $userID, $setID, $problemID); $self = shift; - $self->checkArgs(\@_, qw/course_id? user_id set_id problem_id/); + $self->checkArgs(\@_, qw/user_id set_id problem_id/); - #if a courseID is not provided then just do the search without a course - #id. This is ok becaus the table is course specific. - my $where; - if ($#_ == 3) { - ($courseID, $userID, $setID, $problemID) = @_; - $where = [ course_id_eq_user_id_eq_set_id_eq_problem_id_eq => $courseID, $userID, $setID, $problemID ]; - - } else { - ($userID, $setID, $problemID) = @_; - $where = [ user_id_eq_set_id_eq_problem_id_eq => $userID, $setID, $problemID ]; - } + ($userID, $setID, $problemID) = @_; + my $where = [ user_id_eq_set_id_eq_problem_id_eq => $userID, $setID, $problemID ]; my $order = ['answer_id']; @@ -1129,21 +1120,12 @@ sub listProblemPastAnswers { } sub latestProblemPastAnswer { - my ($self, $courseID, $userID, $setID, $problemID); + my ($self, $userID, $setID, $problemID); $self = shift; - $self->checkArgs(\@_, qw/course_id? user_id set_id problem_id/); - - #if a courseID is not provided then just do the search without a course - #id. This is ok becaus the table is course specific. - my @answerIDs; - if ($#_ == 3) { - ($courseID, $userID, $setID, $problemID) = @_; - @answerIDs = $self->listProblemPastAnswers($courseID, $userID, $setID, $problemID); + $self->checkArgs(\@_, qw/user_id set_id problem_id/); - } else { - ($userID, $setID, $problemID) = @_; - @answerIDs = $self->listProblemPastAnswers($userID, $setID, $problemID); - } + ($userID, $setID, $problemID) = @_; + my @answerIDs = $self->listProblemPastAnswers($userID, $setID, $problemID); #array should already be returned from lowest id to greatest. Latest answer is greatest return $answerIDs[$#answerIDs]; @@ -1167,11 +1149,6 @@ sub getPastAnswers { sub addPastAnswer { my ($self, $pastAnswer) = shift->checkArgs(\@_, qw/REC:past_answer/); - # we dont have a course table yet but when we do we should check this - - # croak "addPastAnswert: course ", $pastAnswer->course_id, " not found" - # unless $self->{course}->exists($pastAnswer->course_id); - croak "addPastAnswert: user problem ", $pastAnswer->user_id, " ", $pastAnswer->set_id, " ", $pastAnswer->problem_id, " not found" unless $self->{problem_user}->exists($pastAnswer->user_id, $pastAnswer->set_id, $pastAnswer->problem_id); diff --git a/lib/WeBWorK/DB/Record/PastAnswer.pm b/lib/WeBWorK/DB/Record/PastAnswer.pm index 0bc1e6e916..b4a055c549 100644 --- a/lib/WeBWorK/DB/Record/PastAnswer.pm +++ b/lib/WeBWorK/DB/Record/PastAnswer.pm @@ -29,7 +29,6 @@ BEGIN { __PACKAGE__->_fields( answer_id => { type => "INT AUTO_INCREMENT", key => 1 }, - course_id => { type => "VARCHAR(40) NOT NULL", key => 1 }, user_id => { type => "VARCHAR(100) NOT NULL", key => 1 }, set_id => { type => "VARCHAR(100) NOT NULL", key => 1 }, problem_id => { type => "INT NOT NULL", key => 1 }, diff --git a/lib/WeBWorK/DB/Schema/NewSQL.pm b/lib/WeBWorK/DB/Schema/NewSQL.pm index 41086c4e15..8eb5eeef76 100644 --- a/lib/WeBWorK/DB/Schema/NewSQL.pm +++ b/lib/WeBWorK/DB/Schema/NewSQL.pm @@ -57,12 +57,6 @@ sub where_answer_id_eq { return { answer_id => $answer_id }; } -# can be used for past_answers -sub where_course_id_eq_user_id_eq_set_id_eq_problem_id_eq { - my ($self, $flags, $course_id, $user_id, $set_id, $problem_id) = @_; - return { course_id => $course_id, user_id => $user_id, set_id => $set_id, problem_id => $problem_id }; -} - sub where_user_id_eq_set_id_eq_problem_id_eq { my ($self, $flags, $user_id, $set_id, $problem_id) = @_; return { user_id => $user_id, set_id => $set_id, problem_id => $problem_id }; diff --git a/lib/WeBWorK/DB/Schema/NewSQL/Std.pm b/lib/WeBWorK/DB/Schema/NewSQL/Std.pm index 3510f2fd42..f71647df0b 100644 --- a/lib/WeBWorK/DB/Schema/NewSQL/Std.pm +++ b/lib/WeBWorK/DB/Schema/NewSQL/Std.pm @@ -384,6 +384,71 @@ sub _drop_column_field_stmt { my $sql_field_name = $self->sql_field_name($field_name); return "Alter table `$sql_table_name` drop column `$sql_field_name` "; } + +#################################################### +# rebuild indexes for the table +#################################################### + +sub rebuild_indexes { + my ($self) = @_; + + my $sql_table_name = $self->sql_table_name; + my $field_data = $self->field_data; + my %override_fields = reverse %{ $self->{params}{fieldOverride} }; + + # A key field column is going to be removed. The schema will not have the information for this column. So the + # indexes need to be obtained from the database. Note that each element of the returned array is an array reference + # of the form [ Table, Non_unique, Key_name, Seq_in_index, Column_name, ... ] (the information indicated by the + # ellipsis is not needed here). Only the first column in each sequence is needed. + my @indexes = grep { $_->[3] == 1 } @{ $self->dbh->selectall_arrayref("SHOW INDEXES FROM `$sql_table_name`") }; + + # The columns need to be obtained from the database to determine the types of the columns. The information from the + # schema can not be trusted because it doesn't have information about the field being dropped. Note that each + # element of the returned array is an array reference of the form [ Field, Type, Null, Key, Default, Extra ] and + # Extra contains AUTO_INCREMENT for those fields that have that attribute. + my $columns = $self->dbh->selectall_arrayref("SHOW COLUMNS FROM `$sql_table_name`"); + + # First drop all indexes for the table. + my @auto_increment_fields; + for my $index (@indexes) { + # If a field has the AUTO_INCREMENT attribute, then that needs to be removed before the index can be dropped. + my $column = (grep { $index->[4] eq $_->[0] } @$columns)[0]; + if (defined $column && $column->[5] =~ m/AUTO_INCREMENT/i) { + $self->dbh->do("ALTER TABLE `$sql_table_name` MODIFY `$column->[0]` $column->[1]"); + push @auto_increment_fields, $override_fields{ $column->[0] } // $column->[0]; + } + + $self->dbh->do("ALTER TABLE `$sql_table_name` DROP INDEX `$index->[2]`"); + } + + # Add the indices for the table according to the schema. + my @keyfields = $self->keyfields; + for my $start (0 .. $#keyfields) { + my @index_components; + my $sql_field_name = $self->sql_field_name($keyfields[$start]); + + for my $component (@keyfields[ $start .. $#keyfields ]) { + my $sql_field_name = $self->sql_field_name($component); + my $sql_field_type = $field_data->{$component}{type}; + my $length_specifier = $sql_field_type =~ /(text|blob)/i ? '(100)' : ''; + push @index_components, "`$sql_field_name`$length_specifier"; + } + + my $index_string = join(', ', @index_components); + my $index_type = $start == 0 ? 'UNIQUE KEY' : 'KEY'; + + $self->dbh->do("ALTER TABLE `$sql_table_name` ADD $index_type ($index_string)"); + } + + # Finally add the AUTO_INCREMENT attribute back to those columns that is was removed from. + for my $field (@auto_increment_fields) { + my $sql_field_name = $self->sql_field_name($field); + $self->dbh->do("ALTER TABLE `$sql_table_name` MODIFY `$sql_field_name` $field_data->{$field}{type}"); + } + + return 1; +} + #################################################### # checking Tables #################################################### @@ -885,4 +950,3 @@ sub handle_error { sub DESTROY { } 1; - diff --git a/lib/WeBWorK/HTML/SingleProblemGrader.pm b/lib/WeBWorK/HTML/SingleProblemGrader.pm index 35ae7c899b..34ff6cc486 100644 --- a/lib/WeBWorK/HTML/SingleProblemGrader.pm +++ b/lib/WeBWorK/HTML/SingleProblemGrader.pm @@ -44,7 +44,7 @@ sub new ($class, $c, $pg, $userProblem) { # Retrieve the latest past answer and comment (if any). my $userPastAnswerID = - $db->latestProblemPastAnswer($courseID, $studentID, $setID . ($versionID ? ",v$versionID" : ''), $problemID); + $db->latestProblemPastAnswer($studentID, $setID . ($versionID ? ",v$versionID" : ''), $problemID); my $pastAnswer = $userPastAnswerID ? $db->getPastAnswer($userPastAnswerID) : 0; my $comment = $pastAnswer ? $pastAnswer->comment_string : ''; diff --git a/lib/WeBWorK/Utils/CourseIntegrityCheck.pm b/lib/WeBWorK/Utils/CourseIntegrityCheck.pm index 375b5ca137..e6335dadcc 100644 --- a/lib/WeBWorK/Utils/CourseIntegrityCheck.pm +++ b/lib/WeBWorK/Utils/CourseIntegrityCheck.pm @@ -238,17 +238,16 @@ sub checkTableFields { } # Fetch corresponding tables in the database and search for corresponding schema entries. - my $dbh = $self->dbh; # Get a database handle - my $stmt = "SHOW COLUMNS FROM `$table_name`"; # mysql request - # result is array: Field | Type | Null | Key | Default | Extra - my $result = $dbh->selectall_arrayref($stmt); - my %database_field_names = map { ${$_}[0] => [$_] } @$result; # Drill down in the result to the field name level + my $result = $self->dbh->selectall_arrayref("SHOW COLUMNS FROM `$table_name`"); + my %database_fields = map { ${$_}[0] => $_ } @$result; # Construct a hash of field names to field data. - foreach my $field_name (sort keys %database_field_names) { - my $exists = exists($schema_override_field_names{$field_name}); - $fields_ok = 0 unless $exists; - $fieldStatus{$field_name} = [ONLY_IN_B] unless $exists; + for my $field_name (keys %database_fields) { + unless (exists($schema_override_field_names{$field_name})) { + $fields_ok = 0; + $fieldStatus{$field_name} = [ONLY_IN_B]; + push(@{ $fieldStatus{$field_name} }, 1) if $database_fields{$field_name}[3]; + } } return ($fields_ok, \%fieldStatus); @@ -271,20 +270,32 @@ sub updateTableFields { warn "$table_name is a non native table" if $db->{$table}{params}{non_native}; # skip non-native tables my ($fields_ok, $fieldStatus) = $self->checkTableFields($courseName, $table); + my $schema_obj = $db->{$table}; + # Add fields for my $field_name (keys %$fieldStatus) { if ($fieldStatus->{$field_name}[0] == ONLY_IN_A) { - my $schema_obj = $db->{$table}; if ($schema_obj->can('add_column_field') && $schema_obj->add_column_field($field_name)) { push(@messages, [ "Added column '$field_name' to table '$table'", 1 ]); } } } + # Rebuild indexes for the table if a previous key field column is going to be dropped. + if ($schema_obj->can('rebuild_indexes') + && (grep { $fieldStatus->{$_} && $fieldStatus->{$_}[1] } @$delete_field_names)) + { + my $result = eval { $schema_obj->rebuild_indexes }; + if ($@ || !$result) { + push(@messages, [ "There was an error rebuilding indexes for table '$table'", 0 ]); + } else { + push(@messages, [ "Rebuilt indexes for table '$table'", 1 ]); + } + } + # Drop fields if listed in $delete_field_names. for my $field_name (@$delete_field_names) { if ($fieldStatus->{$field_name} && $fieldStatus->{$field_name}[0] == ONLY_IN_B) { - my $schema_obj = $db->{$table}; if ($schema_obj->can('drop_column_field') && $schema_obj->drop_column_field($field_name)) { push(@messages, [ "Dropped column '$field_name' from table '$table'", 1 ]); } diff --git a/lib/WeBWorK/Utils/ProblemProcessing.pm b/lib/WeBWorK/Utils/ProblemProcessing.pm index 7f6fe132ed..3267b7229a 100644 --- a/lib/WeBWorK/Utils/ProblemProcessing.pm +++ b/lib/WeBWorK/Utils/ProblemProcessing.pm @@ -115,7 +115,6 @@ async sub process_and_log_answer ($c) { # add to PastAnswer db my $pastAnswer = $db->newPastAnswer(); - $pastAnswer->course_id($courseID); $pastAnswer->user_id($problem->user_id); $pastAnswer->set_id($problem->set_id); $pastAnswer->problem_id($problem->problem_id); diff --git a/templates/ContentGenerator/CourseAdmin/archive_course_confirm.html.ep b/templates/ContentGenerator/CourseAdmin/archive_course_confirm.html.ep index 29b24becd5..0306175d3c 100644 --- a/templates/ContentGenerator/CourseAdmin/archive_course_confirm.html.ep +++ b/templates/ContentGenerator/CourseAdmin/archive_course_confirm.html.ep @@ -6,7 +6,7 @@

<%= $_->[0] %>

% } % } -% my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $db_report) = +% my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes, $db_report) = % $c->formatReportOnDatabaseTables($dbStatus); <%= $db_report %> % if ($extra_database_tables) { @@ -25,6 +25,15 @@ ) =%>

% } +% if ($rebuild_table_indexes) { +

+ <%= maketext( + 'There are extra database fields which are not defined in the schema and were part of the key ' + . 'for at least one table. These fields need to be deleted and the table indexes need to be rebuilt. ' + . 'This will be done when upgrading the course.' + ) =%> +

+% } % if (!$all_tables_ok) {

<%= maketext('The course database must be upgraded before archiving this course.') =%> diff --git a/templates/ContentGenerator/CourseAdmin/rename_course_confirm.html.ep b/templates/ContentGenerator/CourseAdmin/rename_course_confirm.html.ep index c36f4a3116..e32033dc85 100644 --- a/templates/ContentGenerator/CourseAdmin/rename_course_confirm.html.ep +++ b/templates/ContentGenerator/CourseAdmin/rename_course_confirm.html.ep @@ -6,7 +6,7 @@

<%= $_->[0] %>

% } % } -% my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $db_report) = +% my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes, $db_report) = % $c->formatReportOnDatabaseTables($dbStatus); <%= $db_report =%> % if ($extra_database_tables) { @@ -25,6 +25,15 @@ ) =%>

% } +% if ($rebuild_table_indexes) { +

+ <%= maketext( + 'There are extra database fields which are not defined in the schema and were part of the key ' + . 'for at least one table. These fields need to be deleted and the table indexes need to be rebuilt. ' + . 'This will be done when upgrading the course.' + ) =%> +

+% } % if (!$all_tables_ok) {

<%= maketext('The course database must be upgraded before renaming this course.') =%>