Skip to content

Commit

Permalink
Merge pull request #29 from drgrice1/copy-course-suggestions
Browse files Browse the repository at this point in the history 
Suggestions from my review of #2290.
  • Loading branch information
@Alex-Jordan
Alex-Jordan authored Jan 10, 2024
2 parents 1abdd9e + 8c721d8 commit cc0e554
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 93 deletions.
9 changes: 3 additions & 6 deletions lib/WeBWorK/ContentGenerator/CourseAdmin.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,6 @@ sub do_add_course ($c) {
my $add_courseTitle = trim_spaces($c->param('add_courseTitle')) // '';
my $add_courseInstitution = trim_spaces($c->param('add_courseInstitution')) // '';

my $add_admin_users = $c->param('add_admin_users') || '';

my $add_initial_userID = trim_spaces($c->param('add_initial_userID')) // '';
my $add_initial_password = trim_spaces($c->param('add_initial_password')) // '';
my $add_initial_confirmPassword = trim_spaces($c->param('add_initial_confirmPassword')) // '';
Expand All @@ -317,7 +315,7 @@ sub do_add_course ($c) {
my @users;

# copy users from current (admin) course if desired
if ($add_admin_users ne '') {
if ($c->param('add_admin_users')) {
for my $userID ($db->listUsers) {
if ($userID eq $add_initial_userID) {
$c->addbadmessage($c->maketext(
Expand Down Expand Up @@ -363,9 +361,8 @@ sub do_add_course ($c) {
# Include any optional arguments, including a template course and the course title and course institution.
my %optional_arguments;
if ($copy_from_course ne '') {
%optional_arguments = map { $_ => 1 } ($c->param('copy_component'));
$optional_arguments{copyFrom} = $copy_from_course;
$optional_arguments{copyConfig} = $c->param('copy_config_file') || '';
%optional_arguments = map { $_ => 1 } $c->param('copy_component');
$optional_arguments{copyFrom} = $copy_from_course;
}
if ($add_courseTitle ne '') {
$optional_arguments{courseTitle} = $add_courseTitle;
Expand Down
48 changes: 29 additions & 19 deletions lib/WeBWorK/Utils/CourseManagement.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,8 +313,8 @@ sub addCourse {

##### step 3: populate course database #####

# db object for course to copy things from
my $db0;
# database and course environment objects for the course to copy things from.
my ($db0, $ce0);
if (
$sourceCourse ne ''
&& !(grep { $sourceCourse eq $_ } @{ $ce->{modelCoursesForCopy} })
Expand All @@ -325,7 +325,7 @@ sub addCourse {
|| $options{copyInstitution})
)
{
my $ce0 = WeBWorK::CourseEnvironment->new({ courseName => $sourceCourse });
$ce0 = WeBWorK::CourseEnvironment->new({ courseName => $sourceCourse });
$db0 = WeBWorK::DB->new($ce0->{dbLayouts}{$dbLayoutName});
}

Expand All @@ -334,11 +334,15 @@ sub addCourse {
debug("not adding users to the course database: 'user' table is non-native.\n");
} else {
if ($db0 && $options{copyNonStudents}) {
# If the course.conf file is being copied, then the student role from the source course needs to be used,
# as the role might be customized in that file.
my @non_student_ids =
map {@$_} ($db0->listPermissionLevelsWhere(
{ permission => { not_like => '0' }, user_id => { not_like => 'set_id:%' } }, 'user_id'
));
my %user_args = map { $_->[0]{user_id} => 1 } (@users);
map {@$_} ($db0->listPermissionLevelsWhere({
permission =>
{ '!=' => $options{copyConfig} ? $ce0->{userRoles}{student} : $ce->{userRoles}{student} },
user_id => { not_like => 'set_id:%' }
}));
my %user_args = map { $_->[0]{user_id} => 1 } @users;

for my $user_id (@non_student_ids) {
next if $user_args{$user_id};
Expand Down Expand Up @@ -419,20 +423,24 @@ sub addCourse {
}
}

##### step 4: write course.conf file #####
##### step 4: write course.conf file (unless that is going to be copied from a source course) #####

my $courseEnvFile = $ce->{courseFiles}->{environment};
open my $fh, ">:utf8", $courseEnvFile
or die "failed to open $courseEnvFile for writing.\n";
writeCourseConf($fh, $ce, %courseOptions);
close $fh;
unless ($sourceCourse ne '' && $options{copyConfig}) {
my $courseEnvFile = $ce->{courseFiles}{environment};
open my $fh, ">:utf8", $courseEnvFile
or die "failed to open $courseEnvFile for writing.\n";
writeCourseConf($fh, $ce, %courseOptions);
close $fh;
}

##### step 5: copy templates, html, simple.conf, course.conf if desired #####

if ($sourceCourse ne '') {
my $sourceCE = WeBWorK::CourseEnvironment->new({ get_SeedCE($ce), courseName => $sourceCourse });
my $sourceDir = $sourceCE->{courseDirs}{templates};
my $sourceCE = WeBWorK::CourseEnvironment->new({ get_SeedCE($ce), courseName => $sourceCourse });

if ($options{copyTemplatesHtml}) {
my $sourceDir = $sourceCE->{courseDirs}{templates};

## copy templates ##
if (-d $sourceDir) {
my $destDir = $ce->{courseDirs}{templates};
Expand All @@ -452,8 +460,8 @@ sub addCourse {
warn "Failed to copy templates from course '$sourceCourse': "
. "templates directory '$sourceDir' does not exist.\n";
}

## copy html ##
## this copies the html/tmp directory as well which is not optimal
$sourceDir = $sourceCE->{courseDirs}{html};
if (-d $sourceDir) {
my $destDir = $ce->{courseDirs}{html};
Expand All @@ -474,8 +482,10 @@ sub addCourse {
. "html directory '$sourceDir' does not exist.\n";
}
}

## copy config files ##
# this copies the simple.conf file if desired

# this copies the simple.conf file if desired
if ($options{copySimpleConfig}) {
my $sourceFile = $sourceCE->{courseFiles}{simpleConfig};
if (-e $sourceFile) {
Expand All @@ -492,7 +502,8 @@ sub addCourse {
}
}
}
# this copies the course.conf file if desired

# this copies the course.conf file if desired
if ($options{copyConfig}) {
my $sourceFile = $sourceCE->{courseFiles}{environment};
if (-e $sourceFile) {
Expand All @@ -509,7 +520,6 @@ sub addCourse {
}
}
}

}
}

Expand Down
135 changes: 67 additions & 68 deletions templates/ContentGenerator/CourseAdmin/add_course_form.html.ep
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
<%= $c->hidden_authen_fields =%>
<%= $c->hidden_fields('subDisplay') =%>
%
<p><%= maketext(
<div class="mb-2"><%= maketext(
'Specify an ID, title, and institution for the new course. The course ID may contain only letters, '
. 'numbers, hyphens, and underscores, and may have at most [_1] characters.',
$ce->{maxCourseIdLength}) %>
</p>
<div class="row mb-2">
</div>
<div class="row mb-3">
<div class="col-lg-8 col-md-10">
<div class="form-floating mb-1">
<%= text_field add_courseID => '',
Expand All @@ -27,7 +27,7 @@
class => 'form-control' =%>
<%= label_for add_courseTitle => maketext('Course Title') =%>
</div>
<div class="form-floating mb-1">
<div class="form-floating">
<%= text_field add_courseInstitution => '',
id => 'add_courseInstitution',
placeholder => '',
Expand All @@ -41,7 +41,7 @@
<%= maketext(
'To add the WeBWorK administrators to the new course (as administrators) check the box below.') =%>
</div>
<div class="form-check mb-2">
<div class="form-check">
<label class="form-check-label">
<%= check_box 'add_admin_users' => 1, class => 'form-check-input' =%>
<%= maketext('Add WeBWorK administrators to new course') =%>
Expand All @@ -54,7 +54,7 @@
. 'The user ID may contain only numbers, letters, hyphens, periods (dots), commas,and underscores.'
) =%>
</div>
<div class="row mb-2">
<div class="row mb-3">
<div class="col-lg-4 col-md-5 col-sm-6">
<div class="form-floating mb-1">
<%= text_field add_initial_userID => '',
Expand Down Expand Up @@ -103,9 +103,10 @@
</div>
</div>
<div class="mb-1">
<%= maketext('To copy components from an existing course, select the course and check which components to copy.') =%>
<%= maketext('To copy components from an existing course, '
. 'select the course and check which components to copy.') =%>
</div>
<div class="row mb-3">
<div class="row mb-1">
% my @existingCourses = sort { lc($a) cmp lc($b) } grep { $_ ne stash('courseID') } listCourses($ce);
% unshift(@existingCourses, sort { lc($a) cmp lc($b) } @{ $ce->{modelCoursesForCopy} });
%
Expand All @@ -119,67 +120,65 @@
id => 'copy_from_course',
class => 'form-select' =%>
</div>
<div class="mb-3">
<div class="col-form-label fw-bold">
Copy These Components:
</div>
<div class="form-check">
<label class="form-check-label">
<%= check_box select_all => 1, class => 'select-all form-check-input',
'data-select-group' => 'copy_component'=%>
<%= maketext('select all', 'templates', 'html') =%>
</label>
</div>
<div class="form-check">
<label class="form-check-label">
<%= check_box copy_component => 'copyTemplatesHtml', class => 'form-check-input' =%>
<%= maketext('[_1] and [_2] folders', 'templates', 'html') =%>
</label>
</div>
<div class="form-check">
<label class="form-check-label">
<%= check_box copy_component => 'copySimpleConfig', class => 'form-check-input' =%>
<%= maketext('simple configuration file') =%>
</label>
</div>
<div class="form-check">
<label class="form-check-label">
<%= check_box copy_config_file => 1, class => 'form-check-input' =%>
<%= maketext('course configuration file') =%>
</label>
</div>
<div class="form-check">
<label class="form-check-label">
<%= check_box copy_component => 'copyNonStudents', class => 'form-check-input' =%>
<%= maketext('non-student users') =%>
</label>
</div>
<div class="form-check">
<label class="form-check-label">
<%= check_box copy_component => 'copySets', class => 'form-check-input' =%>
<%= maketext('assignments/sets') =%>
</label>
</div>
<div class="form-check">
<label class="form-check-label">
<%= check_box copy_component => 'copyAchievements', class => 'form-check-input' =%>
<%= maketext('achievements') =%>
</label>
</div>
<div class="form-check">
<label class="form-check-label">
<%= check_box copy_component => 'copyTitle', class => 'form-check-input' =%>
<%= maketext('course title (will override "Course Title" input above)') =%>
</label>
</div>
<div class="form-check">
<label class="form-check-label">
<%= check_box copy_component => 'copyInstitution', class => 'form-check-input' =%>
<%= maketext('course institution (will override "Institution" input above)') =%>
</label>
</div>
</div>
</div>
<fieldset class="mb-3">
<legend class="fw-bold fs-6">Copy These Components:</legend>
<div class="form-check">
<label class="form-check-label">
<%= check_box select_all => 1, class => 'select-all form-check-input',
data => { select_group => 'copy_component' } =%>
<%= maketext('select all', 'templates', 'html') =%>
</label>
</div>
<div class="form-check">
<label class="form-check-label">
<%= check_box copy_component => 'copyTemplatesHtml', class => 'form-check-input' =%>
<%= maketext('[_1] and [_2] folders', 'templates', 'html') =%>
</label>
</div>
<div class="form-check">
<label class="form-check-label">
<%= check_box copy_component => 'copySimpleConfig', class => 'form-check-input' =%>
<%= maketext('simple configuration file') =%>
</label>
</div>
<div class="form-check">
<label class="form-check-label">
<%= check_box copy_component => 'copyConfig', class => 'form-check-input' =%>
<%= maketext('course configuration file') =%>
</label>
</div>
<div class="form-check">
<label class="form-check-label">
<%= check_box copy_component => 'copyNonStudents', class => 'form-check-input' =%>
<%= maketext('non-student users') =%>
</label>
</div>
<div class="form-check">
<label class="form-check-label">
<%= check_box copy_component => 'copySets', class => 'form-check-input' =%>
<%= maketext('assignments/sets') =%>
</label>
</div>
<div class="form-check">
<label class="form-check-label">
<%= check_box copy_component => 'copyAchievements', class => 'form-check-input' =%>
<%= maketext('achievements') =%>
</label>
</div>
<div class="form-check">
<label class="form-check-label">
<%= check_box copy_component => 'copyTitle', class => 'form-check-input' =%>
<%= maketext('course title (will override "Course Title" input above)') =%>
</label>
</div>
<div class="form-check">
<label class="form-check-label">
<%= check_box copy_component => 'copyInstitution', class => 'form-check-input' =%>
<%= maketext('course institution (will override "Institution" input above)') =%>
</label>
</div>
</fieldset>
<%= hidden_field add_dbLayout => 'sql_single' =%>
<%= submit_button maketext('Add Course'), name => 'add_course', class => 'btn btn-primary' =%>
<% end =%>

0 comments on commit cc0e554

Please sign in to comment.