Skip to content

Commit fc3afa4

Browse files
drgrice1drgrice1
committed
Expermintation with moving the version_id into its own field.
1 parent e15c575 commit fc3afa4

File tree

12 files changed

+147
-342
lines changed

12 files changed

+147
-342
lines changed

‎lib/WeBWorK/ContentGenerator/CourseAdmin.pm

Lines changed: 32 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1340,9 +1340,7 @@ sub upgrade_course_confirm ($c) {
13401340

13411341
# Report on database status
13421342
my ($tables_ok, $dbStatus) = $CIchecker->checkCourseTables($upgrade_courseID);
1343-
my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes,
1344-
$incorrect_type_database_fields, $db_report)
1345-
= $c->formatReportOnDatabaseTables($dbStatus, $upgrade_courseID);
1343+
my $dbReport = $c->formatReportOnDatabaseTables($dbStatus, $upgrade_courseID);
13461344

13471345
my $course_output = $c->c;
13481346

@@ -1368,9 +1366,9 @@ sub upgrade_course_confirm ($c) {
13681366
);
13691367
push(@$course_output, $c->tag('h2', $c->maketext('Report for course [_1]:', $upgrade_courseID)));
13701368
push(@$course_output, $c->tag('div', class => 'mb-2', $c->maketext('Database:')));
1371-
push(@$course_output, $db_report);
1369+
push(@$course_output, $dbReport->{summary});
13721370

1373-
if ($extra_database_tables) {
1371+
if ($dbReport->{extra_database_tables}) {
13741372
$extra_database_tables_exist = 1;
13751373
push(
13761374
@$course_output,
@@ -1384,7 +1382,7 @@ sub upgrade_course_confirm ($c) {
13841382
);
13851383
}
13861384

1387-
if ($extra_database_fields) {
1385+
if ($dbReport->{extra_database_fields}) {
13881386
$extra_database_fields_exist = 1;
13891387
push(
13901388
@$course_output,
@@ -1400,7 +1398,7 @@ sub upgrade_course_confirm ($c) {
14001398
);
14011399
}
14021400

1403-
if ($rebuild_table_indexes) {
1401+
if ($dbReport->{rebuild_table_indexes}) {
14041402
push(
14051403
@$course_output,
14061404
$c->tag(
@@ -1415,7 +1413,7 @@ sub upgrade_course_confirm ($c) {
14151413
);
14161414
}
14171415

1418-
if ($incorrect_type_database_fields) {
1416+
if ($dbReport->{incorrect_type_database_fields}) {
14191417
$incorrect_type_database_fields_exist = 1;
14201418
push(
14211419
@$course_output,
@@ -1547,15 +1545,13 @@ sub do_upgrade_course ($c) {
15471545
# Analyze database status and prepare status report
15481546
($tables_ok, $dbStatus) = $CIchecker->checkCourseTables($upgrade_courseID);
15491547

1550-
my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes,
1551-
$incorrect_type_database_fields, $db_report)
1552-
= $c->formatReportOnDatabaseTables($dbStatus);
1548+
my $dbReport = $c->formatReportOnDatabaseTables($dbStatus);
15531549

15541550
# Prepend course name
1555-
$db_report = $c->c($c->tag('div', class => 'mb-2', $c->maketext('Database:')), $db_report);
1551+
my $db_report = $c->c($c->tag('div', class => 'mb-2', $c->maketext('Database:')), $dbReport->{summary});
15561552

15571553
# Report on databases and report summary
1558-
if ($extra_database_tables) {
1554+
if ($dbReport->{extra_database_tables}) {
15591555
push(
15601556
@$db_report,
15611557
$c->tag(
@@ -1565,7 +1561,7 @@ sub do_upgrade_course ($c) {
15651561
)
15661562
);
15671563
}
1568-
if ($extra_database_fields) {
1564+
if ($dbReport->{extra_database_fields}) {
15691565
push(
15701566
@$db_report,
15711567
$c->tag(
@@ -1577,7 +1573,7 @@ sub do_upgrade_course ($c) {
15771573
);
15781574
}
15791575

1580-
if ($incorrect_type_database_fields) {
1576+
if ($dbReport->{incorrect_type_database_fields}) {
15811577
push(
15821578
@$db_report,
15831579
$c->tag(
@@ -2785,13 +2781,14 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) {
27852781
)
27862782
);
27872783

2788-
my $all_tables_ok = 1;
2789-
my $extra_database_tables = 0;
2790-
my $extra_database_fields = 0;
2791-
my $rebuild_table_indexes = 0;
2792-
my $incorrect_type_database_fields = 0;
2793-
2794-
my $db_report = $c->c;
2784+
my $dbReport = {
2785+
all_tables_ok => 1,
2786+
extra_database_tables => 0,
2787+
extra_database_fields => 0,
2788+
rebuild_table_indexes => 0,
2789+
incorrect_type_database_fields => 0,
2790+
summary => $c->c
2791+
};
27952792

27962793
for my $table (sort keys %$dbStatus) {
27972794
my $table_report = $c->c;
@@ -2800,9 +2797,9 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) {
28002797
push(@$table_report, $table . ': ', $table_status_message{$table_status});
28012798

28022799
if ($table_status == WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_A) {
2803-
$all_tables_ok = 0;
2800+
$dbReport->{all_tables_ok} = 0;
28042801
} elsif ($table_status == WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_B) {
2805-
$extra_database_tables = 1;
2802+
$dbReport->{extra_database_tables} = 1;
28062803
push(
28072804
@$table_report,
28082805
$c->tag(
@@ -2826,9 +2823,9 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) {
28262823

28272824
if ($field_status == WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_B) {
28282825
if ($fieldInfo{$key}[1]) {
2829-
$rebuild_table_indexes = 1;
2826+
$dbReport->{rebuild_table_indexes} = 1;
28302827
} else {
2831-
$extra_database_fields = 1;
2828+
$dbReport->{extra_database_fields} = 1;
28322829
}
28332830
if (defined $courseID) {
28342831
if ($fieldInfo{$key}[1]) {
@@ -2855,9 +2852,9 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) {
28552852
}
28562853
}
28572854
} elsif ($field_status == WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_A) {
2858-
$all_tables_ok = 0;
2855+
$dbReport->{all_tables_ok} = 0;
28592856
} elsif ($field_status == WeBWorK::Utils::CourseDBIntegrityCheck::DIFFER_IN_A_AND_B) {
2860-
$incorrect_type_database_fields = 1;
2857+
$dbReport->{incorrect_type_database_fields} = 1;
28612858
if (defined $courseID) {
28622859
push(
28632860
@$field_report,
@@ -2887,18 +2884,17 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) {
28872884
}
28882885
push(@$table_report, $c->tag('ul', $fields_report->join('')));
28892886
}
2890-
push(@$db_report, $c->tag('li', $table_report->join('')));
2887+
push(@{ $dbReport->{summary} }, $c->tag('li', $table_report->join('')));
28912888
}
28922889

2893-
$db_report = $c->c($c->tag('ul', $db_report->join('')));
2890+
$dbReport->{summary} = $c->c($c->tag('ul', $dbReport->{summary}->join('')));
28942891

2895-
push(@$db_report, $c->tag('p', class => 'text-success', $c->maketext('Database tables are ok'))) if $all_tables_ok;
2892+
push(@{ $dbReport->{summary} }, $c->tag('p', class => 'text-success', $c->maketext('Database tables are ok')))
2893+
if $dbReport->{all_tables_ok};
28962894

2897-
return (
2898-
$all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes,
2899-
$incorrect_type_database_fields,
2900-
$db_report->join('')
2901-
);
2895+
$dbReport->{summary} = $dbReport->{summary}->join('');
2896+
2897+
return $dbReport;
29022898
}
29032899

29042900
1;

‎lib/WeBWorK/ContentGenerator/ProblemSet.pm

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ use WeBWorK::Utils::DateTime qw(after);
1414
use WeBWorK::Utils::Files qw(path_is_subdir);
1515
use WeBWorK::Utils::Rendering qw(renderPG);
1616
use WeBWorK::Utils::Sets qw(is_restricted grade_set format_set_name_display);
17-
use WeBWorK::DB::Utils qw(grok_versionID_from_vsetID_sql);
1817
use WeBWorK::Localize;
1918
use WeBWorK::AchievementItems;
2019

@@ -49,8 +48,7 @@ async sub initialize ($c) {
4948
if ($c->{set}->assignment_type =~ /gateway/) {
5049
$c->{setVersions} = [
5150
$db->getMergedSetVersionsWhere(
52-
{ user_id => $eUserID, set_id => { like => $c->{set}->set_id . ',v%' } },
53-
\grok_versionID_from_vsetID_sql($db->{set_version_merged}->sql->_quote('set_id'))
51+
{ user_id => $eUserID, set_id => $c->{set}->set_id, version_id => { '>', 0 } }, 'version_id'
5452
)
5553
];
5654
$c->{achievementItems} = WeBWorK::AchievementItems::UserItems($c, $eUserID, $c->{set}, $c->{setVersions});
@@ -140,8 +138,7 @@ sub siblings ($c) {
140138

141139
# Note that listUserSets does not list versioned sets, but listUserSetsWhere does. On the other hand, listUserSets
142140
# cannot sort in the database, while listUserSetsWhere can.
143-
my @setIDs =
144-
map { $_->[1] } $db->listUserSetsWhere({ user_id => $eUserID, set_id => { not_like => '%,v%' } }, 'set_id');
141+
my @setIDs = map { $_->[1] } $db->listUserSetsWhere({ user_id => $eUserID, version_id => 0 }, 'set_id');
145142

146143
# Do not show hidden siblings unless user is allowed to view hidden sets.
147144
unless ($authz->hasPermissions($user, 'view_hidden_sets')) {

‎lib/WeBWorK/DB.pm

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1392,8 +1392,6 @@ sub getUserSets {
13921392
return $self->{set_user}->gets(@userSetIDs);
13931393
}
13941394

1395-
# the code from addUserSet() is duplicated in large part following in
1396-
# addVersionedUserSet; changes here should accordingly be propagated down there
13971395
sub addUserSet {
13981396
my ($self, $UserSet) = shift->checkArgs(\@_, qw/REC:set_user/);
13991397
WeBWorK::DB::Ex::DependencyNotFound->throw(error => 'addUserSet: user ' . $UserSet->user_id . ' not found')
@@ -1403,8 +1401,6 @@ sub addUserSet {
14031401
return $self->{set_user}->add($UserSet);
14041402
}
14051403

1406-
# the code from putUserSet() is duplicated in large part in the following
1407-
# putVersionedUserSet; c.f. that routine
14081404
sub putUserSet {
14091405
my ($self, $UserSet) = shift->checkArgs(\@_, qw/REC:set_user/);
14101406
my $rows = $self->{set_user}->put($UserSet); # DBI returns 0E0 for 0.
@@ -1452,7 +1448,7 @@ sub getMergedSets {
14521448
}
14531449

14541450
################################################################################
1455-
# set_version functions (NEW)
1451+
# set_version functions
14561452
################################################################################
14571453

14581454
BEGIN {

‎lib/WeBWorK/DB/Record/PastAnswer.pm

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,11 @@ use warnings;
1212

1313
BEGIN {
1414
__PACKAGE__->_fields(
15-
answer_id => { type => "INT AUTO_INCREMENT", key => 1 },
16-
user_id => { type => "VARCHAR(100) NOT NULL", key => 1 },
17-
set_id => { type => "VARCHAR(100) NOT NULL", key => 1 },
18-
problem_id => { type => "INT NOT NULL", key => 1 },
15+
answer_id => { type => "INT AUTO_INCREMENT", key => 1 },
16+
user_id => { type => "VARCHAR(100) NOT NULL", key => 1 },
17+
set_id => { type => "VARCHAR(100) NOT NULL", key => 1 },
18+
version_id => { type => "INT NOT NULL DEFAULT 0", key => 1 },
19+
problem_id => { type => "INT NOT NULL", key => 1 },
1920
source_file => { type => "TEXT" },
2021
timestamp => { type => "BIGINT" },
2122
scores => { type => "TINYTEXT" },

‎lib/WeBWorK/DB/Record/UserProblem.pm

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@ use warnings;
1313

1414
BEGIN {
1515
__PACKAGE__->_fields(
16-
user_id => { type => "VARCHAR(100) NOT NULL", key => 1 },
17-
set_id => { type => "VARCHAR(100) NOT NULL", key => 1 },
18-
problem_id => { type => "INT NOT NULL", key => 1 },
16+
user_id => { type => "VARCHAR(100) NOT NULL", key => 1 },
17+
set_id => { type => "VARCHAR(100) NOT NULL", key => 1 },
18+
version_id => { type => "INT NOT NULL DEFAULT 0", key => 1 },
19+
problem_id => { type => "INT NOT NULL", key => 1 },
1920
source_file => { type => "TEXT" },
2021
# FIXME i think value should be able to hold decimal values...
2122
value => { type => "INT" },

‎lib/WeBWorK/DB/Record/UserSet.pm

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@ use warnings;
1212

1313
BEGIN {
1414
__PACKAGE__->_fields(
15-
user_id => { type => "VARCHAR(100) NOT NULL", key => 1 },
16-
set_id => { type => "VARCHAR(100) NOT NULL", key => 1 },
15+
user_id => { type => "VARCHAR(100) NOT NULL", key => 1 },
16+
set_id => { type => "VARCHAR(100) NOT NULL", key => 1 },
17+
version_id => { type => "INT NOT NULL DEFAULT 0", key => 1 },
1718
psvn => { type => "INT UNIQUE NOT NULL AUTO_INCREMENT" },
1819
set_header => { type => "TEXT" },
1920
hardcopy_header => { type => "TEXT" },

‎lib/WeBWorK/DB/Schema/NewSQL/NonVersioned.pm

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,41 +11,28 @@ use WeBWorK::DB::Utils qw(make_vsetID);
1111

1212
use constant TABLES => qw(set_user problem_user);
1313

14-
################################################################################
15-
# where clause
16-
################################################################################
17-
18-
# Override where clause generators that can be used with non-versioned sets so
19-
# that they only match non-versioned sets.
14+
# Override where clause generators that can be used with non-versioned sets or problems so that they
15+
# only match non-versioned sets or problems (those are sets or problems with version_id equal to 0).
2016

2117
sub where_DEFAULT {
2218
my ($self, $flags) = @_;
23-
return { set_id => { NOT_LIKE => make_vsetID("%", "%") } };
19+
return { version_id => 0 };
2420
}
2521

2622
sub where_user_id_eq {
2723
my ($self, $flags, $user_id) = @_;
28-
return { user_id => $user_id, set_id => { NOT_LIKE => make_vsetID("%", "%") } };
24+
return { user_id => $user_id, version_id => 0 };
2925
}
3026

3127
sub where_user_id_like {
3228
my ($self, $flags, $user_id) = @_;
33-
return { user_id => { LIKE => $user_id }, set_id => { NOT_LIKE => make_vsetID("%", "%") } };
29+
return { user_id => { LIKE => $user_id }, version_id => 0 };
3430
}
3531

36-
################################################################################
37-
# override keyparts_to_where to limit scope of where clauses
38-
################################################################################
39-
4032
sub keyparts_to_where {
4133
my ($self, @keyparts) = @_;
42-
4334
my $where = $self->SUPER::keyparts_to_where(@keyparts);
44-
45-
unless (exists $where->{set_id}) {
46-
$where->{set_id} = { NOT_LIKE => make_vsetID("%", "%") };
47-
}
48-
35+
$where->{version_id} = 0 unless exists $where->{version_id};
4936
return $where;
5037
}
5138

‎lib/WeBWorK/DB/Schema/NewSQL/Std.pm

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -359,16 +359,16 @@ sub rebuild_indexes {
359359
my $sql_table_name = $self->sql_table_name;
360360
my $field_data = $self->field_data;
361361

362-
# A key field column is going to be removed. The schema will not have the information for this column. So the
363-
# indexes need to be obtained from the database. Note that each element of the returned array is an array reference
364-
# of the form [ Table, Non_unique, Key_name, Seq_in_index, Column_name, ... ] (the information indicated by the
365-
# ellipsis is not needed here). Only the first column in each sequence is needed.
362+
# If a key field column is going to be removed, then the schema will not have the information for this column. So
363+
# the indexes need to be obtained from the database. Note that each element of the returned array is an array
364+
# reference of the form [ Table, Non_unique, Key_name, Seq_in_index, Column_name, ... ] (the information indicated
365+
# by the ellipsis is not needed here). Only the first column in each sequence is needed.
366366
my @indexes = grep { $_->[3] == 1 } @{ $self->dbh->selectall_arrayref("SHOW INDEXES FROM `$sql_table_name`") };
367367

368368
# The columns need to be obtained from the database to determine the types of the columns. The information from the
369-
# schema cannot be trusted because it doesn't have information about the field being dropped. Note that each
370-
# element of the returned array is an array reference of the form [ Field, Type, Null, Key, Default, Extra ] and
371-
# Extra contains AUTO_INCREMENT for those fields that have that attribute.
369+
# schema cannot be trusted because it doesn't have information about fields being dropped. Note that each element
370+
# of the returned array is an array reference of the form [ Field, Type, Null, Key, Default, Extra ] and Extra
371+
# contains AUTO_INCREMENT for those fields that have that attribute.
372372
my $columns = $self->dbh->selectall_arrayref("SHOW COLUMNS FROM `$sql_table_name`");
373373

374374
# First drop all indexes for the table.

0 commit comments

Comments
 (0)