From 14aec6c90ab53e7e029f7861655ca0cf8ae02a72 Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Sun, 8 Nov 2026 10:51:51 -0700 Subject: [PATCH] Remove use of Undefined_Set. Problems can currently be rendered and tested in the problem editor, set details page, and library browser. Support for viewing them in their own window is historical from when it was not always possible to test a problem directly within those pages. To support viewing a a problem using an `Undefined_Set` required extra handling, fake sets, problems, and extra logic. Furthermore extra care is needed when viewing these problems to not allow submission, showing another version, or doing actions which may alter the database or not be valid for only testing a problem. This removes links and extra handling around using an `Undefined_Set`, and problems are now only viewed/tested within the page they are rendered in. This simplifies the logic in many pages, specifically the Problem.pm and GatewayQuiz.pm modules, which need lots of additional logic to handle this case. This also adds `show_me_another` to checkSet. There is now currently no use of `fake_test_version`, but `fake_set`, `fake_test`, and `fake_problem` are still used by renderRPC. Initial code removal was by me, then tested by Claude and cleaned up some of the things found I missed. Co-Authored-By: Claude Opus 5.5 --- htdocs/js/PGProblemEditor/pgproblemeditor.js | 49 +--- lib/WeBWorK/Authz.pm | 15 +- lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 265 ++++++------------ .../Instructor/PGProblemEditor.pm | 224 +++------------ lib/WeBWorK/ContentGenerator/Problem.pm | 233 ++++----------- lib/WeBWorK/ContentGenerator/ProblemSet.pm | 14 - lib/WeBWorK/ContentGenerator/ProblemSets.pm | 21 +- .../ContentGenerator/GatewayQuiz.html.ep | 4 +- .../PGProblemEditor/save_as_form.html.ep | 4 +- .../PGProblemEditor/save_form.html.ep | 10 - .../PGProblemEditor/view_form.html.ep | 8 - .../Instructor/ProblemSetDetail.html.ep | 50 ++-- .../Instructor/SetMaker/problem_row.html.ep | 35 +-- templates/ContentGenerator/Problem.html.ep | 1 - .../InstructorPGProblemEditor.html.ep | 15 +- 15 files changed, 208 insertions(+), 740 deletions(-) diff --git a/htdocs/js/PGProblemEditor/pgproblemeditor.js b/htdocs/js/PGProblemEditor/pgproblemeditor.js index 328a23547f..a009cfc50d 100644 --- a/htdocs/js/PGProblemEditor/pgproblemeditor.js +++ b/htdocs/js/PGProblemEditor/pgproblemeditor.js @@ -344,51 +344,11 @@ } const actionView = document.getElementById('view'); - const editorForm = document.getElementById('editor'); - - // Make sure this is reset on each click so that a new window isn't always opened once that has been done once. - if (editorForm) editorForm.target = '_self'; if (actionView && actionView.classList.contains('active')) { - if (document.getElementById('newWindowView')?.checked) { - document.getElementById('revert-tab')?.classList.remove('disabled'); - document.getElementById('revert-to-tmp-container')?.classList.remove('d-none'); - const revertRadio = document.getElementById('action_revert_type_revert_id'); - if (revertRadio && revertRadio.disabled) { - revertRadio.disabled = false; - revertRadio.checked = true; - } - - // Add the temporary file coloring and change the current file to the saved file. - document.querySelectorAll('.set-file-info').forEach((nfo) => nfo.classList.add('temporaryFile')); - for (const currentFile of document.querySelectorAll('.current-file')) { - currentFile.textContent = currentFile.dataset.tmpFile; - } - - if (editorForm) editorForm.target = 'WW_View'; - } else { - e.preventDefault(); - await render(); - - saveTempFile(); - } - } - - const actionSave = document.getElementById('save'); - if ( - actionSave && - actionSave.classList.contains('active') && - document.getElementById('newWindowSave')?.checked && - editorForm - ) { - if (document.getElementById('backupFile')?.checked) { - document.getElementById('show-backups-comment')?.classList.remove('d-none'); - const deleteBackupCheck = document.getElementById('deleteBackup'); - if (deleteBackupCheck) deleteBackupCheck.disabled = false; - } - document.getElementById('revert-tab')?.classList.remove('disabled'); - - editorForm.target = 'WW_View'; + e.preventDefault(); + await render(); + saveTempFile(); } const actionHardcopy = document.getElementById('hardcopy'); @@ -671,7 +631,8 @@ if (fileType === 'hardcopy_header') { renderArea.innerHTML = '
' + - 'Hardcopy header contents can only be viewed in a new window.
'; + 'Hardcopy header contents cannot be viewed in the problem editor. ' + + 'Use "Generate Hardcopy" to view the contents.'; resolve(); return; } diff --git a/lib/WeBWorK/Authz.pm b/lib/WeBWorK/Authz.pm index 6005d7e1c7..bafcfedcb6 100644 --- a/lib/WeBWorK/Authz.pm +++ b/lib/WeBWorK/Authz.pm @@ -189,10 +189,11 @@ sub checkSet ($self) { # First check to see if we have to worried about set-level access restrictions. return 0 - unless grep {/^$node_name$/} + unless grep { $node_name eq $_ } (qw( problem_list problem_detail + show_me_another gateway_quiz gateway_quiz_version proctored_gateway_quiz @@ -215,9 +216,6 @@ sub checkSet ($self) { if ($set && $set->set_id eq $setName && $set->user_id eq $effectiveUserName && $set->version_id == $setVersion) { # If we have all of this, then we can just use this set and skip the rest. - } elsif ($setName eq 'Undefined_Set' && $self->hasPermissions($userName, 'access_instructor_tools')) { - # This is the case of previewing a problem from a 'try it' link. - return 0; } else { if ($db->existsSetVersion($effectiveUserName, $setName, $setVersion)) { $set = $db->getMergedSetVersion($effectiveUserName, $setName, $setVersion); @@ -241,10 +239,6 @@ sub checkSet ($self) { } else { if ($db->existsUserSet($effectiveUserName, $setName)) { $set = $db->getMergedSet($effectiveUserName, $setName); - } elsif ($setName eq 'Undefined_Set' && $self->hasPermissions($userName, 'access_instructor_tools')) { - # This is the case of the library browser, when we don't actually have a set to look at. This only - # happens for instructor tool users. - return 0; } else { return $c->maketext('Requested set "[_1]" is not assigned to user [_2].', $setName, $effectiveUserName); } @@ -301,7 +295,10 @@ sub checkSet ($self) { } # Check to be sure that gateways are being sent to the correct content generator. - if (defined $set->assignment_type && $set->assignment_type =~ /gateway/ && $node_name eq 'problem_detail') { + if (defined $set->assignment_type + && $set->assignment_type =~ /gateway/ + && ($node_name eq 'problem_detail' || $node_name eq 'show_me_another')) + { return $c->maketext( 'Requested set "[_1]" is a test but the regular homework assignment content ' . 'generator [_2] was called. Try re-entering the set from the problem sets listing page.', diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 6099a44e32..63d27a3cf4 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -13,13 +13,12 @@ use Mojo::JSON qw(encode_json decode_json); use WeBWorK::Utils qw(encodeAnswers decodeAnswers max wwRound); use WeBWorK::Utils::DateTime qw(before between after); -use WeBWorK::Utils::Files qw(path_is_subdir); use WeBWorK::Utils::Instructor qw(assignSetVersionToUser); use WeBWorK::Utils::Logs qw(writeLog writeCourseLog); use WeBWorK::Utils::ProblemProcessing qw(create_ans_str_from_responses compute_reduced_score); use WeBWorK::Utils::Rendering qw(getTranslatorDebuggingOptions renderPG); use WeBWorK::Utils::Sets qw(is_restricted); -use WeBWorK::DB::Utils qw(global2user fake_set fake_set_version fake_problem); +use WeBWorK::DB::Utils qw(global2user); use WeBWorK::Debug qw(debug); use PGrandom; use WeBWorK::Authen::LTI::GradePassback qw(passbackGradeOnSubmit); @@ -101,7 +100,6 @@ sub can_showProblemGrader ($c, $user, $effectiveUser, $set, $problem, $tmplSet) return ($authz->hasPermissions($user->user_id, 'access_instructor_tools') && $authz->hasPermissions($user->user_id, 'problem_grader') - && $set->set_id ne 'Undefined_Set' && !$c->{invalidSet}); } @@ -124,9 +122,6 @@ sub can_showSolutions ($c, $user, $effectiveUser, $set, $problem, $tmplSet) { sub can_recordAnswers ($c, $user, $effectiveUser, $set, $problem, $tmplSet = 0, $submitAnswers = 0) { my $authz = $c->authz; - # Never record answers for undefined sets - return 0 if $set->set_id eq 'Undefined_Set'; - if ($user->user_id ne $effectiveUser->user_id) { # If the user is not allowed to record answers as another user, return that permission. If the user is allowed # to record an unsubmitted test, allow that. If the user is allowed to record only set version answers, then @@ -331,7 +326,6 @@ async sub initialize ($c) { my $setID = $c->stash('setID'); my $userID = $c->authen->{user_id}; my $effectiveUserID = $c->param('effectiveUser'); - my $isFakeSet = 0; my $user = $c->authen->{user}; my $effectiveUser = $c->authen->{effective_user}; @@ -344,151 +338,81 @@ async sub initialize ($c) { # We need the template (user) set, the merged set version, and a problem from the set to be able to test whether # we're creating a new set version. - my ($tmplSet, $set, $problem) = (0, 0, 0); - - # If the set comes in as "Undefined_Set", then we're trying/editing a single problem in a set, and so create a fake - # set with which to work if the user has the authorization to do that. - if ($setID eq 'Undefined_Set') { - $isFakeSet = 1; - # Make sure these are defined - $requestedVersion = 1; - $c->{assignment_type} = 'gateway'; - - if (!$authz->hasPermissions($userID, 'modify_problem_sets')) { - $c->{invalidSet} = - $c->maketext('You do not have the authorization level required to view/edit undefined sets.'); - - # Define these so that we can drop through to report the error in body. - $tmplSet = fake_set($db); - $set = fake_set_version($db); - $problem = fake_problem($db); - } else { - # In this case we're creating a fake set from the input, so the input must include a source file. - if (!$c->param('sourceFilePath')) { - $c->{invalidSet} = $c->maketext( - 'An Undefined_Set was requested, but no source file for the contained problem was provided.'); - - # Define these so that we can drop through to report the error in body. - $tmplSet = fake_set($db); - $set = fake_set_version($db); - $problem = fake_problem($db); - - } else { - my $sourceFPath = $c->param('sourceFilePath'); - die('sourceFilePath is unsafe!') - unless path_is_subdir($sourceFPath, $ce->{courseDirs}{templates}, 1); - - $tmplSet = fake_set($db); - $set = fake_set_version($db); - $problem = fake_problem($db); - - my $creation_time = time; - - $tmplSet->assignment_type('gateway'); - $tmplSet->attempts_per_version(0); - $tmplSet->time_interval(0); - $tmplSet->versions_per_interval(1); - $tmplSet->version_time_limit(0); - $tmplSet->version_creation_time($creation_time); - $tmplSet->problem_randorder(0); - $tmplSet->problems_per_page(1); - $tmplSet->hide_score('N'); - $tmplSet->hide_score_by_problem('N'); - $tmplSet->hide_work('N'); - $tmplSet->time_limit_cap('0'); - $tmplSet->restrict_ip('No'); - - $set->assignment_type('gateway'); - $set->time_interval(0); - $set->versions_per_interval(1); - $set->version_time_limit(0); - $set->version_creation_time($creation_time); - $set->time_limit_cap('0'); - - $problem->problem_id(1); - $problem->source_file($sourceFPath); - $problem->user_id($effectiveUserID); - $problem->value(1); - $problem->problem_seed($c->param('problemSeed')) if ($c->param('problemSeed')); - } - } - } else { - # If there is a cap on problems per page, make sure that is respected in the global set in - # case something higher snuck in. - if ($ce->{test}{maxProblemsPerPage}) { - my $globalSet = $db->getGlobalSet($setID); - if ( - $ce->{test}{maxProblemsPerPage} - && ($globalSet->problems_per_page == 0 - || $globalSet->problems_per_page > $ce->{test}{maxProblemsPerPage}) - ) - { - $globalSet->problems_per_page($ce->{test}{maxProblemsPerPage}); - $db->putGlobalSet($globalSet); - } + my ($tmplSet, $set, $problem); + + # If there is a cap on problems per page, make sure that is respected in the global set in + # case something higher snuck in. + if ($ce->{test}{maxProblemsPerPage}) { + my $globalSet = $db->getGlobalSet($setID); + if ( + $ce->{test}{maxProblemsPerPage} + && ($globalSet->problems_per_page == 0 + || $globalSet->problems_per_page > $ce->{test}{maxProblemsPerPage}) + ) + { + $globalSet->problems_per_page($ce->{test}{maxProblemsPerPage}); + $db->putGlobalSet($globalSet); } + } - # Get the template set, i.e., the non-versioned set that's assigned to the user. - # If this failed in authz->checkSet, then $c->{invalidSet} is set. - $tmplSet = $db->getMergedSet($effectiveUserID, $setID); - - # Now that is has been validated that this is a gateway test, save the assignment test for the processing of - # graded proctored tests. If a set was not obtained from the database, store a fake value here - # to be able to continue. - $c->{assignment_type} = $tmplSet->assignment_type || 'gateway'; + # Get the template set, i.e., the non-versioned set that's assigned to the user. + $tmplSet = $db->getMergedSet($effectiveUserID, $setID); - # If there is a cap on problems per page, make sure that is respected in case something higher snuck in. - if ($ce->{test}{maxProblemsPerPage} - && ($tmplSet->problems_per_page == 0 || $tmplSet->problems_per_page > $ce->{test}{maxProblemsPerPage})) - { - $tmplSet->problems_per_page($ce->{test}{maxProblemsPerPage}); - } + # Now that is has been validated that this is a gateway test, save the assignment type for the processing of + # graded proctored tests. Fallback to 'gateway' in case assignment_type was empty. + $c->{assignment_type} = $tmplSet->assignment_type || 'gateway'; - # next, get the latest (current) version of the set if we don't have a - # requested version number - my @allVersionIds = $db->listSetVersions($effectiveUserID, $setID); - my $latestVersion = (@allVersionIds ? $allVersionIds[-1] : 0); + # If there is a cap on problems per page, make sure that is respected in case something higher snuck in. + if ($ce->{test}{maxProblemsPerPage} + && ($tmplSet->problems_per_page == 0 || $tmplSet->problems_per_page > $ce->{test}{maxProblemsPerPage})) + { + $tmplSet->problems_per_page($ce->{test}{maxProblemsPerPage}); + } - # Double check that the requested version makes sense. - $requestedVersion = $latestVersion - if ($requestedVersion !~ /^\d+$/ - || $requestedVersion > $latestVersion - || $requestedVersion < 0); + # next, get the latest (current) version of the set if we don't have a + # requested version number + my @allVersionIds = $db->listSetVersions($effectiveUserID, $setID); + my $latestVersion = (@allVersionIds ? $allVersionIds[-1] : 0); + + # Double check that the requested version makes sense. + $requestedVersion = $latestVersion + if ($requestedVersion !~ /^\d+$/ + || $requestedVersion > $latestVersion + || $requestedVersion < 0); + + die('No requested version when returning to problem?!') + if ( + ( + $c->param('previewAnswers') + || $c->param('checkAnswers') + || $c->param('submitAnswers') + || $c->param('newPage') + ) + && !$requestedVersion + ); - die('No requested version when returning to problem?!') - if ( - ( - $c->param('previewAnswers') - || $c->param('checkAnswers') - || $c->param('submitAnswers') - || $c->param('newPage') - ) - && !$requestedVersion - ); - - # To check for a proctored test, the set version is needed, not the template. So get that. - if ($requestedVersion) { - $set = $db->getMergedSetVersion($effectiveUserID, $setID, $requestedVersion); - } elsif ($latestVersion) { - $set = $db->getMergedSetVersion($effectiveUserID, $setID, $latestVersion); - } else { - # If there is not a requested version or a latest version, then create dummy set to proceed. - # FIXME RETURN TO: should this be global2version? - $set = global2user($db->{set_version}{record}, $db->getGlobalSet($setID)); - $set->user_id($effectiveUserID); - $set->psvn('000'); - $set->set_id($setID); # redundant? - $set->version_id(0); - } + # To check for a proctored test, the set version is needed, not the template. So get that. + if ($requestedVersion) { + $set = $db->getMergedSetVersion($effectiveUserID, $setID, $requestedVersion); + } elsif ($latestVersion) { + $set = $db->getMergedSetVersion($effectiveUserID, $setID, $latestVersion); + } else { + # If there is not a requested version or a latest version, then create dummy set to proceed. + # FIXME RETURN TO: should this be global2version? + $set = global2user($db->{set_version}{record}, $db->getGlobalSet($setID)); + $set->user_id($effectiveUserID); + $set->psvn('000'); + $set->set_id($setID); # redundant? + $set->version_id(0); + } - # If there is a cap on problems per page, make sure that is respected in case something higher snuck in. - if ($ce->{test}{maxProblemsPerPage} - && ($set->problems_per_page == 0 || $set->problems_per_page > $ce->{test}{maxProblemsPerPage})) - { - $set->problems_per_page($ce->{test}{maxProblemsPerPage}); - } + # If there is a cap on problems per page, make sure that is respected in case something higher snuck in. + if ($ce->{test}{maxProblemsPerPage} + && ($set->problems_per_page == 0 || $set->problems_per_page > $ce->{test}{maxProblemsPerPage})) + { + $set->problems_per_page($ce->{test}{maxProblemsPerPage}); } - my $setVersionNumber = $set ? $set->version_id : 0; + my $setVersionNumber = $set->version_id; # Assemble gateway parameters @@ -498,16 +422,14 @@ async sub initialize ($c) { # date. If a specific version has not been requested and conditional release is enabled, then this also checks to # see if the conditions have been met for a conditional release. my $isOpen = ( - $requestedVersion ? ($set && $set->open_date && after($set->open_date, $c->submitTime)) : ($tmplSet - && $tmplSet->open_date + $requestedVersion ? ($set->open_date && after($set->open_date, $c->submitTime)) : ($tmplSet->open_date && after($tmplSet->open_date, $c->submitTime) && !($ce->{options}{enableConditionalRelease} && is_restricted($db, $tmplSet, $effectiveUserID))) ) || $authz->hasPermissions($userID, 'view_unopened_sets'); my $isClosed = - $tmplSet - && $tmplSet->due_date + $tmplSet->due_date && after($tmplSet->due_date, $c->submitTime) && !$authz->hasPermissions($userID, 'record_answers_after_due_date'); @@ -515,17 +437,14 @@ async sub initialize ($c) { # version. Among other things, the number of attempts is a property of the problem, so get a problem to check that. # Note that for a gateway quiz all problems will have the same number of attempts. This means that if the set # doesn't have any problems we're up a creek, so check for that here and bail if that is the case. - my @setPNum = $setID eq 'Undefined_Set' ? (1) : $db->listUserProblems($effectiveUser->user_id, $setID); - die("Set $setID contains no problems.") if (!@setPNum); - - # If we assigned a fake problem above, $problem is already defined. Otherwise, get the problem, or define it to be - # undefined if the set hasn't been versioned to the user yet. This is fixed when we assign the setVersion. - if (!$problem) { - $problem = - $setVersionNumber - ? $db->getMergedProblemVersion($effectiveUser->user_id, $setID, $setVersionNumber, $setPNum[0]) - : undef; - } + my @setPNum = $db->listUserProblems($effectiveUser->user_id, $setID); + die("Set $setID contains no problems.") if !@setPNum; + + # Get the problem if there is a versioned set, otherwise a problem will be defined when creating a versioned test. + $problem = + $setVersionNumber + ? $db->getMergedProblemVersion($effectiveUser->user_id, $setID, $setVersionNumber, $setPNum[0]) + : undef; my $maxAttemptsPerVersion = $tmplSet->attempts_per_version || 0; my $timeInterval = $tmplSet->time_interval || 0; @@ -549,7 +468,7 @@ async sub initialize ($c) { my $currentNumVersions = 0; # this is the number of versions in the time interval my $totalNumVersions = 0; - if ($setVersionNumber && !$c->{invalidSet} && $setID ne 'Undefined_Set') { + if ($setVersionNumber) { my @setVersionIDs = $db->listSetVersions($effectiveUserID, $setID); my @setVersions = $db->getSetVersions(map { [ $effectiveUserID, $setID, $_ ] } @setVersionIDs); for (@setVersions) { @@ -563,7 +482,7 @@ async sub initialize ($c) { my $versionIsOpen = 0; - if ($isOpen && !$isClosed && !$c->{invalidSet}) { + if ($isOpen && !$isClosed) { # If specific version was not requested, then create a new one if needed. if (!$requestedVersion) { if ( @@ -741,7 +660,7 @@ async sub initialize ($c) { } } - } elsif (!$c->{invalidSet} && !$requestedVersion) { + } elsif (!$requestedVersion) { $c->{invalidSet} = $c->maketext('This test is closed. No new test versions may be taken.'); } @@ -867,12 +786,7 @@ async sub initialize ($c) { # Set up problem numbering and multipage variables. - my @problemNumbers; - if ($setID eq 'Undefined_Set') { - @problemNumbers = (1); - } else { - @problemNumbers = $db->listProblemVersions($effectiveUserID, $setID, $setVersionNumber); - } + my @problemNumbers = $db->listProblemVersions($effectiveUserID, $setID, $setVersionNumber); # To speed up processing of long (multi-page) tests, we want to only translate those problems that are being # submitted or are currently being displayed. So determine which problems are on the current page. @@ -924,12 +838,7 @@ async sub initialize ($c) { my @pg_results; # Process the problems as needed. - my @mergedProblems; - if ($setID eq 'Undefined_Set') { - @mergedProblems = ($problem); - } else { - @mergedProblems = $db->getAllMergedProblemVersions($effectiveUserID, $setID, $setVersionNumber); - } + my @mergedProblems = $db->getAllMergedProblemVersions($effectiveUserID, $setID, $setVersionNumber); my @renderPromises; @@ -1299,8 +1208,7 @@ sub path ($c, $args) { $args, 'WeBWorK' => $navigation_allowed ? $c->url_for('root') : '', $courseName => $navigation_allowed ? $c->url_for('set_list') : '', - $setID eq 'Undefined_Set' - || $c->{invalidSet} || $c->{actingCreationError} || $c->stash->{actingConfirmation} + $c->{invalidSet} || $c->{actingCreationError} || $c->stash->{actingConfirmation} ? ( defined $c->stash('versionID') ? ($setID => $c->url_for('problem_list', setID => $setID), 'v' . $c->stash('versionID') => '') @@ -1321,10 +1229,7 @@ sub nav ($c, $args) { # Set up and display a student navigation for those that have permission to act as a student. if ($c->authz->hasPermissions($userID, 'become_student')) { - my $setID = $c->{set}->set_id; - - return '' if $setID eq 'Undefined_Set'; - + my $setID = $c->{set}->set_id; my $setVersion = $c->{set}->version_id; # Find all versions of this set that have been taken (excluding those taken by the current user). diff --git a/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm b/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm index 59c1e9ce1d..a9831edf14 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm @@ -18,15 +18,6 @@ The course information and problems are located in the course templates directory. Course information has the name defined by $ce->{courseFiles}{course_info} -editMode = temporaryFile | savedFile - -This flag is read by Problem.pm and ProblemSet.pm (perhaps others). - -The value of temporaryFile means view the temp file defined by -fname.user_name.tmp instead of the file fname. - -The value of savedFile means to use fname directly. - The suffix for a temporary file is "user_name.tmp" by default. =head2 File types (file_type) which can be edited. @@ -182,8 +173,11 @@ sub initialize ($c) { return; } - # Clean up sourceFilePath and check that sourceFilePath is relative to the templates folder - if ($c->{file_type} eq 'source_path_for_problem_file') { + # Clean up sourceFilePath and check that sourceFilePath is relative to the templates folder. + # This is also needed for header files not attached to a set. + if ($c->{file_type} eq 'source_path_for_problem_file' + || ($c->{file_type} =~ /header/ && !defined $c->{setID} && ($c->param('sourceFilePath') // '') =~ /\S/)) + { $c->{sourceFilePath} = $c->getRelativeSourceFilePath($c->param('sourceFilePath')); } @@ -330,8 +324,15 @@ sub page_title ($c) { return $c->maketext('Editor') unless $c->{file_type}; - return $c->maketext('Set Header for set [_1]', format_set_name_display($setID)) if $c->{file_type} eq 'set_header'; - return $c->maketext('Hardcopy Header for set [_1]', format_set_name_display($setID)) + return + defined $setID + ? $c->maketext('Set Header for set [_1]', format_set_name_display($setID)) + : $c->maketext('Set Header') + if $c->{file_type} eq 'set_header'; + return + defined $setID + ? $c->maketext('Hardcopy Header for set [_1]', format_set_name_display($setID)) + : $c->maketext('Hardcopy Header') if $c->{file_type} eq 'hardcopy_header'; return $c->maketext('Hardcopy Theme') if $c->{file_type} eq 'hardcopy_theme'; return $c->maketext('Course Information for course [_1]', $c->stash('courseID')) @@ -374,6 +375,8 @@ sub getRelativeSourceFilePath ($c, $sourceFilePath) { # This does not create the directories in the path to the file. # It returns an absolute path to the file. # $path should be an absolute path to the original file. +# FIXME: With the removal of Undefined_Set handling, should +# problems without a setID use something else in their path? sub determineTempEditFilePath ($c, $path) { my $user = $c->authen->{user_id}; my $setID = $c->{setID} // 'Undefined_Set'; @@ -487,9 +490,14 @@ sub getFilePaths ($c) { $editFilePath = "$ce->{webworkDirs}{hardcopyThemes}/" . $c->param('hardcopy_theme'); } } elsif ($c->{file_type} eq 'set_header' || $c->{file_type} eq 'hardcopy_header') { - my $set_record = $db->getGlobalSet($c->{setID}); + my $header_file = ''; + if (defined $c->{setID}) { + my $set_record = $db->getGlobalSet($c->{setID}); + $header_file = $set_record->{ $c->{file_type} } if $set_record; + } elsif (($c->param('sourceFilePath') // '') =~ /\S/) { + $header_file = $c->getRelativeSourceFilePath($c->param('sourceFilePath')); + } - my $header_file = defined $set_record ? $set_record->{ $c->{file_type} } : ''; if ($header_file && $header_file ne 'defaultHeader') { if ($header_file =~ m|^/|) { # Absolute address @@ -730,92 +738,13 @@ sub fixProblemContents { return $problemContents =~ s/(\r\n)|(\r)/\n/gr; } +# The view action is handled by javascript. If this is called, save the +# editor contents to the temporary file so the changes are not lost. sub view_handler ($c) { - my $problemSeed = $c->param('action.view.seed') // DEFAULT_SEED(); - my $displayMode = $c->param('action.view.displayMode') // 'MathJax'; - - # Grab the problemContents from the form in order to save it to the tmp file. + return unless defined $c->param('problemContents'); $c->{r_problemContents} = \(fixProblemContents($c->param('problemContents'))); - $c->saveFileChanges($c->{tempFilePath}); - - my $relativeTempFilePath = $c->getRelativeSourceFilePath($c->{tempFilePath}); - - # Construct redirect URL and redirect to it. - if ($c->{file_type} eq 'problem' || $c->{file_type} eq 'source_path_for_problem_file') { - # Redirect to Problem.pm or GatewayQuiz.pm. - # We need to know if the set is a gateway set to determine the redirect. - my $globalSet = defined $c->{setID} ? $c->db->getGlobalSet($c->{setID}) : undef; - - $c->authen->flash(status_message => $c->{status_message}->join('')); - $c->reply_with_redirect($c->systemLink( - defined $globalSet && $globalSet->assignment_type =~ /gateway/ - ? $c->url_for('gateway_quiz', setID => 'Undefined_Set') - : $c->url_for( - 'problem_detail', - setID => $c->{setID} // 'Undefined_Set', - problemID => $c->{problemID} // 1 - ), - params => { - displayMode => $displayMode, - problemSeed => $problemSeed, - editMode => 'temporaryFile', - sourceFilePath => $relativeTempFilePath - } - )); - } elsif ($c->{file_type} eq 'blank_problem' || $c->{file_type} eq 'sample_problem') { - # Redirect to Problem.pm. - $c->authen->flash(status_message => $c->{status_message}->join('')); - $c->reply_with_redirect($c->systemLink( - $c->url_for('problem_detail', setID => 'Undefined_Set', problemID => 1), - params => { - displayMode => $displayMode, - problemSeed => $problemSeed, - editMode => 'temporaryFile', - sourceFilePath => $relativeTempFilePath - } - )); - } elsif ($c->{file_type} eq 'set_header') { - # Redirect to ProblemSet - $c->authen->flash(status_message => $c->{status_message}->join('')); - $c->reply_with_redirect($c->systemLink( - $c->url_for('problem_list', setID => $c->{setID}), - params => { - set_header => $c->{tempFilePath}, - displayMode => $displayMode, - problemSeed => $problemSeed, - editMode => 'temporaryFile', - sourceFilePath => $relativeTempFilePath - } - )); - } elsif ($c->{file_type} eq 'hardcopy_header') { - # Redirect to ProblemSet?? It's difficult to view temporary changes for hardcopy headers. - $c->authen->flash(status_message => $c->{status_message}->join('')); - $c->reply_with_redirect($c->systemLink( - $c->url_for('problem_list', setID => $c->{setID}), - params => { - set_header => $c->{tempFilePath}, - displayMode => $displayMode, - problemSeed => $problemSeed, - editMode => 'temporaryFile', - sourceFilePath => $relativeTempFilePath - } - )); - } elsif ($c->{file_type} eq 'course_info') { - # Redirect to ProblemSets.pm. - $c->authen->flash(status_message => $c->{status_message}->join('')); - $c->reply_with_redirect($c->systemLink( - $c->url_for('set_list'), - params => { - course_info => $c->{tempFilePath}, - editMode => 'temporaryFile', - sourceFilePath => $relativeTempFilePath - } - )); - } else { - die "I don't know how to redirect this file type $c->{file_type}."; - } - + $c->{inputFilePath} = $c->{tempFilePath} if -r $c->{tempFilePath}; return; } @@ -908,7 +837,6 @@ sub add_problem_handler ($c) { params => { displayMode => $c->{displayMode}, problemSeed => $c->{problemSeed}, - editMode => 'savedFile', sourceFilePath => $c->getRelativeSourceFilePath($sourceFilePath), file_type => 'problem', } @@ -937,7 +865,7 @@ sub add_problem_handler ($c) { $c->authen->flash(status_message => $c->{status_message}->join('')); $c->reply_with_redirect($c->systemLink( $c->url_for('problem_list', setID => $targetSetName), - params => { displayMode => $c->{displayMode}, editMode => 'savedFile' } + params => { displayMode => $c->{displayMode} } )); } elsif ($targetFileType eq 'hardcopy_header') { # Update set record @@ -963,7 +891,7 @@ sub add_problem_handler ($c) { $c->authen->flash(status_message => $c->{status_message}->join('')); $c->reply_with_redirect($c->systemLink( $c->url_for('hardcopy_preselect_set', setID => $targetSetName), - params => { displayMode => $c->{displayMode}, editMode => 'savedFile' } + params => { displayMode => $c->{displayMode} } )); } else { die "Unsupported target file type $targetFileType"; @@ -988,90 +916,6 @@ sub save_handler ($c) { $c->saveFileChanges($c->{editFilePath}, scalar($c->param('backupFile'))); } - # Don't redirect unless it was requested to open in a new window. - return unless $c->param('newWindowSave'); - - if ($c->{file_type} eq 'problem' || $c->{file_type} eq 'source_path_for_problem_file') { - # Redirect to Problem.pm or GatewayQuiz.pm. - # We need to know if the set is a gateway set to determine the redirect. - my $globalSet = defined $c->{setID} ? $c->db->getGlobalSet($c->{setID}) : undef; - - $c->authen->flash(status_message => $c->{status_message}->join('')); - $c->reply_with_redirect($c->systemLink( - defined $globalSet && $globalSet->assignment_type =~ /gateway/ - ? $c->url_for('gateway_quiz', setID => 'Undefined_Set') - : $c->url_for( - 'problem_detail', - setID => $c->{setID} // 'Undefined_Set', - problemID => $c->{problemID} // 1 - ), - params => { - displayMode => $c->{displayMode}, - problemSeed => $c->{problemSeed}, - editMode => 'savedFile', - sourceFilePath => $c->getRelativeSourceFilePath($c->{editFilePath}) - } - )); - } elsif ($c->{file_type} eq 'set_header') { - # Redirect to ProblemSet.pm - $c->authen->flash(status_message => $c->{status_message}->join('')); - $c->reply_with_redirect($c->systemLink( - $c->url_for('problem_list', setID => $c->{setID}), - params => { - displayMode => $c->{displayMode}, - problemSeed => $c->{problemSeed}, - editMode => 'savedFile' - } - )); - } elsif ($c->{file_type} eq 'hardcopy_header') { - # Redirect to Hardcopy.pm - $c->authen->flash(status_message => $c->{status_message}->join('')); - $c->reply_with_redirect($c->systemLink( - $c->url_for('hardcopy_preselect_set', setID => $c->{setID}), - params => { - displayMode => $c->{displayMode}, - problemSeed => $c->{problemSeed}, - editMode => 'savedFile' - } - )); - } elsif ($c->{file_type} eq 'hardcopy_theme') { - # Redirect to PGProblemEditor.pm - $c->authen->flash(status_message => $c->{status_message}->join('')); - $c->reply_with_redirect($c->systemLink( - $c->url_for('instructor_problem_editor'), - params => { - editMode => 'savedFile', - hardcopy_theme => $c->{hardcopy_theme}, - file_type => 'hardcopy_theme', - sourceFilePath => $c->getRelativeSourceFilePath($c->{editFilePath}), - } - )); - } elsif ($c->{file_type} eq 'course_info') { - # Redirect to ProblemSets.pm - $c->authen->flash(status_message => $c->{status_message}->join('')); - $c->reply_with_redirect($c->systemLink($c->url_for('set_list'), params => { editMode => 'savedFile' })); - } elsif ($c->{file_type} eq 'source_path_for_problem_file') { - # Redirect to PGProblemEditor.pm - $c->authen->flash(status_message => $c->{status_message}->join('')); - $c->reply_with_redirect($c->systemLink( - $c->url_for( - 'instructor_problem_editor_withset_withproblem', - setID => $c->{setID}, - problemID => $c->{problemID} - ), - params => { - displayMode => $c->{displayMode}, - problemSeed => $c->{problemSeed}, - editMode => 'savedFile', - # The path relative to the templates directory is required. - sourceFilePath => $c->{editFilePath}, - file_type => 'source_path_for_problem_file' - } - )); - } else { - die "Unsupported save file type $c->{file_type}."; - } - return; } @@ -1127,9 +971,8 @@ sub save_as_handler ($c) { )); # If the save mode is 'add_to_set_as_new_problem', but this problem is not in a set (for example for a sample - # problem), then the redirect for this save mode will fail since there is no set or problem id. So switch to the - # 'new_independent_file' save mode. That will work since it fills in the 'Undefined_Set' for the set id and 1 - # for the problem id. + # problem), then the redirect for this save mode will fail since there is no set or problem id. So switch to + # the 'new_independent_file' save mode which does not need a set or problem id. $saveMode = 'new_independent_file' if $saveMode eq 'add_to_set_as_new_problem' && !defined $c->{setID}; } else { $c->{editFilePath} = $outputFilePath; @@ -1261,7 +1104,7 @@ sub save_as_handler ($c) { $c->shortPath($outputFilePath) )); $c->addgoodmessage($c->maketext('No changes have been made to set [_1]', $c->{setID})) - if defined $c->{setID} && $c->{setID} ne 'Undefined_Set'; + if defined $c->{setID}; } else { $c->addbadmessage($c->maketext('Unknown saveMode: [_1].', $saveMode)); return; @@ -1277,8 +1120,7 @@ sub save_as_handler ($c) { $problemPage = $c->url_for('instructor_problem_editor'); $new_file_type = 'course_info'; } elsif ($saveMode eq 'new_independent_file') { - $problemPage = - $c->url_for('instructor_problem_editor_withset_withproblem', setID => 'Undefined_Set', problemID => 1); + $problemPage = $c->url_for('instructor_problem_editor'); $new_file_type = $file_type =~ /header/ ? $file_type : 'source_path_for_problem_file'; } elsif ($saveMode eq 'new_hardcopy_theme') { $problemPage = $c->url_for('instructor_problem_editor'); diff --git a/lib/WeBWorK/ContentGenerator/Problem.pm b/lib/WeBWorK/ContentGenerator/Problem.pm index 36dcdbff64..92673a772d 100644 --- a/lib/WeBWorK/ContentGenerator/Problem.pm +++ b/lib/WeBWorK/ContentGenerator/Problem.pm @@ -10,7 +10,6 @@ WeBWorK::ContentGenerator::Problem - Allow a student to interact with a problem. use WeBWorK::Debug qw(debug); use WeBWorK::Utils qw(decodeAnswers wwRound); use WeBWorK::Utils::DateTime qw(before between after); -use WeBWorK::Utils::Files qw(path_is_subdir); use WeBWorK::Utils::JITAR qw(seq_to_jitar_id jitar_id_to_seq is_jitar_problem_hidden is_jitar_problem_closed jitar_problem_finished jitar_problem_adjusted_status); use WeBWorK::Utils::LanguageAndDirection qw(get_problem_lang_and_dir); @@ -19,7 +18,6 @@ use WeBWorK::Utils::ProblemProcessing qw(process_and_log_answer jitar_send_wa use WeBWorK::Utils::Rendering qw(getTranslatorDebuggingOptions renderPG); use WeBWorK::Utils::Sets qw(is_restricted format_set_name_display); use WeBWorK::AchievementEvaluator qw(checkForAchievements); -use WeBWorK::DB::Utils qw(global2user fake_set fake_problem); use WeBWorK::Localize; use WeBWorK::AchievementEvaluator; use WeBWorK::HTML::SingleProblemGrader; @@ -32,16 +30,6 @@ use WeBWorK::HTML::StudentNav qw(studentNav); # user - user ID of real user # effectiveUser - user ID of effective user # -# Integration with PGProblemEditor: -# -# editMode - if set, indicates alternate problem source location. -# can be "temporaryFile" or "savedFile". -# -# sourceFilePath - path to file to be edited -# problemSeed - force problem seed to value -# success - success message to display -# failure - failure message to display -# # Rendering options: # # showOldAnswers - request that last entered answer be shown (if allowed) @@ -81,7 +69,6 @@ sub can_showProblemGrader ($c, $user, $effectiveUser, $set, $problem) { return ($authz->hasPermissions($user->user_id, 'access_instructor_tools') && $authz->hasPermissions($user->user_id, 'problem_grader') - && $set->set_id ne 'Undefined_Set' && !$c->{invalidSet}); } @@ -247,7 +234,6 @@ async sub initialize ($c) { my $problemID = $c->stash('problemID'); my $userID = $c->authen->{user_id}; my $effectiveUserID = $c->param('effectiveUser'); - $c->{editMode} = $c->param('editMode'); my $user = $c->authen->{user}; my $effectiveUser = $c->authen->{effective_user}; @@ -258,12 +244,10 @@ async sub initialize ($c) { # Obtain the merged set for $effectiveUser $c->{set} = $db->getMergedSet($effectiveUserID, $setID); - # Determine if the set should be considered open. - # It is open if the user can view unopened sets or is an instructor editing a problem from the problem editor, - # or it is after the set open date and is not conditionally restricted and is not jitar hidden or closed. + # Determine if the set should be considered open. It is open if the user can view unopened sets, or it + # is after the set open date and is not conditionally restricted and is not jitar hidden or closed. return unless $authz->hasPermissions($userID, 'view_unopened_sets') - || $setID eq 'Undefined_Set' || ( after($c->{set}->open_date, $c->submitTime) && !( @@ -277,7 +261,7 @@ async sub initialize ($c) { ); # When a set is created enable_reduced_scoring is null, so we have to set it - if ($c->{set} && $c->{set}->enable_reduced_scoring ne '0' && $c->{set}->enable_reduced_scoring ne '1') { + if ($c->{set}->enable_reduced_scoring ne '0' && $c->{set}->enable_reduced_scoring ne '1') { my $globalSet = $db->getGlobalSet($c->{set}->set_id); $globalSet->enable_reduced_scoring('0'); $db->putGlobalSet($globalSet); @@ -287,88 +271,16 @@ async sub initialize ($c) { # Obtain the merged problem for the effective user. my $problem = $db->getMergedProblem($effectiveUserID, $setID, $problemID); - if ($authz->hasPermissions($userID, 'modify_problem_sets')) { - # This is the case of the problem editor for a user that can modify problem sets. - - # If a user set does not exist for this user and this set, then check - # the global set. If that does not exist, then create a fake set. If it does, then add fake user data. - unless (defined $c->{set}) { - my $userSetClass = $db->{set_user}->{record}; - my $globalSet = $db->getGlobalSet($setID); - - if (not defined $globalSet) { - $c->{set} = fake_set($db); - } else { - $c->{set} = global2user($userSetClass, $globalSet); - $c->{set}->psvn(0); - } - } - - # If a problem is not defined obtain the global problem, convert it to a user problem, and add fake user data. - unless (defined $problem) { - my $globalProblem = $db->getGlobalProblem($setID, $problemID); - - # If the global problem doesn't exist either, bail! - if (!defined $globalProblem) { - my $sourceFilePath = $c->param('sourceFilePath'); - - # These are problems from setmaker. If declared invalid, they won't come up. - if (defined $sourceFilePath) { - die 'sourceFilePath is unsafe!' - unless path_is_subdir($sourceFilePath, $ce->{courseDirs}{templates}, 1); - } else { - $c->{invalidProblem} = $c->{invalidSet} = 1; - return; - } - - $problem = fake_problem($db); - $problem->problem_id(1); - $problem->source_file($sourceFilePath); - $problem->user_id($effectiveUserID); - } else { - $problem = global2user($db->{problem_user}{record}, $globalProblem); - $problem->user_id($effectiveUserID); - $problem->problem_seed(0); - $problem->status(0); - $problem->attempted(0); - $problem->last_answer(''); - $problem->num_correct(0); - $problem->num_incorrect(0); - } - } - - # Now we're sure we have valid UserSet and UserProblem objects - - # Deal with possible editor overrides. - # If the caller is asking to override the source file, and editMode calls for a temporary file, do so. - my $sourceFilePath = $c->param('sourceFilePath'); - if (defined $c->{editMode} && $c->{editMode} eq 'temporaryFile' && defined $sourceFilePath) { - die 'sourceFilePath is unsafe!' - unless path_is_subdir($sourceFilePath, $ce->{courseDirs}->{templates}, 1); - $problem->source_file($sourceFilePath); - } - - # If the problem does not have a source file or no source file has been passed in - # then this is really an invalid problem (probably from a bad URL). - $c->{invalidProblem} = !(defined $sourceFilePath || $problem->source_file); - - # If the caller is asking to override the problem seed, do so. - my $problemSeed = $c->param('problemSeed'); - if (defined $problemSeed && $problemSeed =~ /^[+-]?\d+$/) { - $problem->problem_seed($problemSeed); - } + # Set the problem as invalid if it was not assigned to the effective user or is hidden. + $c->{invalidProblem} = + !(defined $problem && ($c->{set}->visible || $authz->hasPermissions($userID, 'view_hidden_sets'))); + $c->addbadmessage($c->maketext('This problem will not count toward your grade.')) + if !$c->{invalidProblem} && !$problem->value; + if ($authz->hasPermissions($userID, 'modify_problem_sets')) { $c->addmessage($c->{set}->visible ? $c->tag('p', class => 'font-visible m-0', $c->maketext('This set is visible to students.')) : $c->tag('p', class => 'font-hidden m-0', $c->maketext('This set is hidden from students.'))); - - } else { - # Test for additional problem validity if it's not already invalid. - $c->{invalidProblem} = - !(defined $problem && ($c->{set}->visible || $authz->hasPermissions($userID, 'view_hidden_sets'))); - - $c->addbadmessage($c->maketext('This problem will not count toward your grade.')) - if $problem && !$problem->value && !$c->{invalidProblem}; } $c->{userID} = $userID; @@ -391,8 +303,7 @@ async sub initialize ($c) { # Check for a page refresh which causes a cached form resubmission. In that case this is # not a valid submission of answers. if ( - $c->{set}->set_id ne 'Undefined_Set' - && $c->{submitAnswers} + $c->{submitAnswers} && ( !defined $formFields->{num_attempts} || (defined $formFields->{num_attempts} @@ -472,12 +383,12 @@ async sub initialize ($c) { my $rerandomizePeriod = $ce->{pg}{options}{periodicRandomizationPeriod} // 0; $problem->{prPeriod} = $ce->{problemDefaults}{prPeriod} - if (defined $problem->{prPeriod} && $problem->{prPeriod} =~ /^\s*$/); + if defined $problem->{prPeriod} && $problem->{prPeriod} =~ /^\s*$/; $rerandomizePeriod = $problem->{prPeriod} - if (defined $problem->{prPeriod} && $problem->{prPeriod} > -1); + if defined $problem->{prPeriod} && $problem->{prPeriod} > -1; - $prEnabled = 0 if ($rerandomizePeriod < 1 || $c->{editMode}); + $prEnabled = 0 if $rerandomizePeriod < 1; my $canUpdateProblemRandomization = $userID eq $effectiveUserID || $authz->hasPermissions($userID, 'record_answers_when_acting_as_student'); @@ -637,7 +548,7 @@ sub siblings ($c) { my @problemRecords = $db->getMergedProblemsWhere({ user_id => $eUserID, set_id => $setID }, 'problem_id'); my @problemIDs = map { $_->problem_id } @problemRecords; - my $isJitarSet = $setID ne 'Undefined_Set' && $c->{set}->assignment_type eq 'jitar' ? 1 : 0; + my $isJitarSet = $c->{set}->assignment_type eq 'jitar' ? 1 : 0; # Variables for the progress bar my $num_of_problems = 0; @@ -761,48 +672,42 @@ sub nav ($c, $args) { my $ce = $c->ce; my $authz = $c->authz; - my $setID = $c->{set}->set_id; - my $problemID = $c->{problem}->problem_id; - my $userID = $c->authen->{user_id}; - my $eUserID = $c->param('effectiveUser'); - - my $mergedSet = $db->getMergedSet($eUserID, $setID); - return '' if !$mergedSet; - - my $isJitarSet = $mergedSet->assignment_type eq 'jitar'; + my $setID = $c->{set}->set_id; + my $problemID = $c->{problem}->problem_id; + my $userID = $c->authen->{user_id}; + my $eUserID = $c->param('effectiveUser'); + my $isJitarSet = $c->{set}->assignment_type eq 'jitar'; my ($prevID, $nextID); # Find the next or previous problem, and determine if it is actually open for a jitar set. - if (!$c->{invalidProblem}) { - my @problemIDs = - map { $_->[3] } $db->listUserProblemsWhere({ user_id => $eUserID, set_id => $setID }, 'problem_id'); - - if ($isJitarSet) { - my @processedProblemIDs; - for my $id (@problemIDs) { - push @processedProblemIDs, $id - unless !$authz->hasPermissions($eUserID, 'view_unopened_sets') - && is_jitar_problem_hidden($db, $eUserID, $setID, $id); - } - @problemIDs = @processedProblemIDs; + my @problemIDs = + map { $_->[3] } $db->listUserProblemsWhere({ user_id => $eUserID, set_id => $setID }, 'problem_id'); + + if ($isJitarSet) { + my @processedProblemIDs; + for my $id (@problemIDs) { + push @processedProblemIDs, $id + unless !$authz->hasPermissions($eUserID, 'view_unopened_sets') + && is_jitar_problem_hidden($db, $eUserID, $setID, $id); } + @problemIDs = @processedProblemIDs; + } - my $curr_index = 0; - - for (my $i = 0; $i <= $#problemIDs; $i++) { - $curr_index = $i if $problemIDs[$i] == $problemID; - } + my $curr_index = 0; - $prevID = $problemIDs[ $curr_index - 1 ] if $curr_index - 1 >= 0; - $nextID = $problemIDs[ $curr_index + 1 ] if $curr_index + 1 <= $#problemIDs; - $nextID = '' - if ($isJitarSet - && $nextID - && !$authz->hasPermissions($eUserID, 'view_unopened_sets') - && is_jitar_problem_closed($db, $ce, $eUserID, $setID, $nextID)); + for (my $i = 0; $i <= $#problemIDs; $i++) { + $curr_index = $i if $problemIDs[$i] == $problemID; } + $prevID = $problemIDs[ $curr_index - 1 ] if $curr_index - 1 >= 0; + $nextID = $problemIDs[ $curr_index + 1 ] if $curr_index + 1 <= $#problemIDs; + $nextID = '' + if ($isJitarSet + && $nextID + && !$authz->hasPermissions($eUserID, 'view_unopened_sets') + && is_jitar_problem_closed($db, $ce, $eUserID, $setID, $nextID)); + my @links; if ($prevID) { @@ -813,12 +718,8 @@ sub nav ($c, $args) { push @links, $c->maketext('Previous Problem'), '', $c->maketext('Previous Problem'); } - if (defined $setID && $setID ne 'Undefined_Set') { - push @links, $c->maketext('Problem List'), $c->url_for('problem_list', setID => $setID), - $c->maketext('Problem List'); - } else { - push @links, $c->maketext('Problem List'), '', $c->maketext('Problem List'); - } + push @links, $c->maketext('Problem List'), $c->url_for('problem_list', setID => $setID), + $c->maketext('Problem List'); if ($nextID) { push @links, $c->maketext('Next Problem'), @@ -910,17 +811,11 @@ sub page_title ($c) { 'span', class => 'ms-2', $c->link_to( - $c->maketext('Edit') => $c->systemLink( - $c->url_for( - 'instructor_problem_editor_withset_withproblem', - setID => $c->{set}->set_id, - problemID => $c->{problem}->problem_id - ), - # If we are here without a real homework set, carry that through. - $c->{set}->set_id eq 'Undefined_Set' - ? (params => [ 'sourceFilePath' => $c->param('sourceFilePath') ]) - : () - ), + $c->maketext('Edit') => $c->systemLink($c->url_for( + 'instructor_problem_editor_withset_withproblem', + setID => $c->{set}->set_id, + problemID => $c->{problem}->problem_id + )), target => 'WW_Editor', class => 'btn btn-sm btn-secondary' ) @@ -1084,7 +979,7 @@ sub output_score_summary ($c) { } # Print jitar specific information for students (and notify instructor if necessary). - if ($set->set_id ne 'Undefined_Set' && $set->assignment_type() eq 'jitar') { + if ($set->assignment_type eq 'jitar') { my @problemIDs = map { $_->[3] } $db->listUserProblemsWhere({ user_id => $effectiveUser, set_id => $set->set_id }, 'problem_id'); @@ -1228,15 +1123,6 @@ sub output_score_summary ($c) { sub output_misc ($c) { my $output = $c->c; - # Only allow file editing for users that have the permission to modify problem sets. - if ($c->authz->hasPermissions($c->authen->{user_id}, 'modify_problem_sets')) { - push(@$output, $c->hidden_field(editMode => $c->{editMode})) - if defined $c->{editMode} && $c->{editMode} eq 'temporaryFile'; - push(@$output, $c->hidden_field(sourceFilePath => $c->{problem}{source_file})) - if defined $c->{problem}{source_file}; - push(@$output, $c->hidden_field(problemSeed => $c->param('problemSeed'))) if defined $c->param('problemSeed'); - } - # Make sure the student nav filter setting is preserved when the problem form is submitted. push(@$output, $c->hidden_field(studentNavFilter => $c->param('studentNavFilter'))) if $c->param('studentNavFilter'); @@ -1325,7 +1211,7 @@ sub output_summary ($c) { ) ) if $c->{resubmitDetected}; - if ($c->{set}->set_id ne 'Undefined_Set' && $c->{set}->assignment_type eq 'jitar') { + if ($c->{set}->assignment_type eq 'jitar') { my $hasChildren = 0; my @problemIDs = @@ -1387,8 +1273,7 @@ sub output_achievement_message ($c) { # check to see if there are new achievements and output them. if ($c->ce->{achievementsEnabled} && $c->{will}{recordAnswers} - && $c->{submitAnswers} - && $c->{problem}->set_id ne 'Undefined_Set') + && $c->{submitAnswers}) { return checkForAchievements($c->{problem}, $c); } @@ -1396,22 +1281,6 @@ sub output_achievement_message ($c) { return ''; } -# Output a custom edit message -sub output_custom_edit_message ($c) { - if ($c->authz->hasPermissions($c->authen->{user_id}, 'modify_problem_sets') - && $c->{editMode} - && $c->{editMode} eq 'temporaryFile') - { - return $c->tag( - 'p', - class => 'temporaryFile', - $c->maketext('Viewing temporary file: [_1]', $c->{problem}->source_file) - ); - } - - return ''; -} - # Output the "Show Past Answers" button sub output_past_answer_button ($c) { if ($c->authz->hasPermissions($c->authen->{user_id}, 'view_answers')) { diff --git a/lib/WeBWorK/ContentGenerator/ProblemSet.pm b/lib/WeBWorK/ContentGenerator/ProblemSet.pm index 0dc9c44016..373b882cc9 100644 --- a/lib/WeBWorK/ContentGenerator/ProblemSet.pm +++ b/lib/WeBWorK/ContentGenerator/ProblemSet.pm @@ -11,7 +11,6 @@ problem set. use WeBWorK::Debug qw(debug); use WeBWorK::Utils qw(wwRound); use WeBWorK::Utils::DateTime qw(after); -use WeBWorK::Utils::Files qw(path_is_subdir); use WeBWorK::Utils::Rendering qw(renderPG); use WeBWorK::Utils::Sets qw(is_restricted grade_set format_set_name_display test_version_is_complete); use WeBWorK::Localize; @@ -116,19 +115,6 @@ async sub initialize ($c) { ? $ce->{webworkFiles}{screenSnippets}{setHeader} : $c->{set}->set_header; - if ($authz->hasPermissions($userID, 'modify_problem_sets')) { - if (defined $c->param('editMode') && $c->param('editMode') eq 'temporaryFile') { - $screenSetHeader = $c->param('sourceFilePath'); - $screenSetHeader = "$ce->{courseDirs}{templates}/$screenSetHeader" unless $screenSetHeader =~ m!^/!; - die 'sourceFilePath is unsafe!' unless path_is_subdir($screenSetHeader, $ce->{courseDirs}{templates}); - $c->addmessage($c->tag( - 'p', - class => 'temporaryFile', - $c->maketext('Viewing temporary file: [_1]', $screenSetHeader) - )); - } - } - return unless $screenSetHeader; my $problem = WeBWorK::DB::Record::UserProblem->new( diff --git a/lib/WeBWorK/ContentGenerator/ProblemSets.pm b/lib/WeBWorK/ContentGenerator/ProblemSets.pm index 1e24ab1aad..22a956d8bb 100644 --- a/lib/WeBWorK/ContentGenerator/ProblemSets.pm +++ b/lib/WeBWorK/ContentGenerator/ProblemSets.pm @@ -10,7 +10,7 @@ WeBWorK::ContentGenerator::ProblemSets - Display a list of built problem sets. use WeBWorK::Debug qw(debug); use WeBWorK::Utils qw(sortByName); use WeBWorK::Utils::DateTime qw(after); -use WeBWorK::Utils::Files qw(readFile path_is_subdir); +use WeBWorK::Utils::Files qw(readFile); use WeBWorK::Utils::Sets qw(restricted_set_message); use WeBWorK::Localize; @@ -82,25 +82,6 @@ sub initialize ($c) { my $course_info_path = "$ce->{courseDirs}{templates}/$ce->{courseFiles}{course_info}"; - if ($authz->hasPermissions($user, 'access_instructor_tools')) { - if (defined $c->param('editMode') && $c->param('editMode') eq 'temporaryFile') { - $course_info_path = $c->param('sourceFilePath'); - $course_info_path = "$ce->{courseDirs}{templates}/$course_info_path" - unless $course_info_path =~ m!^/!; - - unless (path_is_subdir($course_info_path, $ce->{courseDirs}{templates})) { - $c->addbadmessage('sourceFilePath is unsafe!'); - return ''; - } - - $c->addmessage($c->tag( - 'p', - class => 'temporaryFile', - $c->maketext('Viewing temporary file: [_1]', $course_info_path) - )); - } - } - if (-f $course_info_path) { $c->stash->{course_info_contents} = eval { readFile($course_info_path) }; $c->stash->{course_info_error} = $@ if $@; diff --git a/templates/ContentGenerator/GatewayQuiz.html.ep b/templates/ContentGenerator/GatewayQuiz.html.ep index 2647fb4338..21248b4678 100644 --- a/templates/ContentGenerator/GatewayQuiz.html.ep +++ b/templates/ContentGenerator/GatewayQuiz.html.ep @@ -366,7 +366,7 @@ % } % - % if ($c->{can}{showWork} && $setID ne 'Undefined_Set') { + % if ($c->{can}{showWork}) {
@@ -853,6 +853,6 @@ % % # If achievements enabled, check to see if there are new ones and output them. Use the first % # problem to seed the data. However, all of the problems will be provided to the achievement evaluator. -% if ($ce->{achievementsEnabled} && $c->{will}{recordAnswers} && $c->{submitAnswers} && $setID ne 'Undefined_Set') { +% if ($ce->{achievementsEnabled} && $c->{will}{recordAnswers} && $c->{submitAnswers}) { <%= checkForAchievements($problems->[0], $c, setVersion => $setVersionID) =%> % } diff --git a/templates/ContentGenerator/Instructor/PGProblemEditor/save_as_form.html.ep b/templates/ContentGenerator/Instructor/PGProblemEditor/save_as_form.html.ep index 0d1a7ffc49..c47b441bc4 100644 --- a/templates/ContentGenerator/Instructor/PGProblemEditor/save_as_form.html.ep +++ b/templates/ContentGenerator/Instructor/PGProblemEditor/save_as_form.html.ep @@ -20,9 +20,9 @@ % # If it is an absolute path make it relative. % $shortFilePath =~ s|^/*|| if $shortFilePath =~ m|^/|; % -% # Don't add to or replace problems in sets or replace set/hardcopy headers if the set is the Undefined_Set or +% # Don't add to or replace problems in sets or replace set/hardcopy headers if the set is undefined or % # if the problem is the blank_problem or a sample problem. -% my $fileIsAssociatedWithSet = defined $c->{setID} && $c->{setID} ne 'Undefined_Set' && !$isTemplate; +% my $fileIsAssociatedWithSet = defined $c->{setID} && !$isTemplate; %
diff --git a/templates/ContentGenerator/Instructor/PGProblemEditor/save_form.html.ep b/templates/ContentGenerator/Instructor/PGProblemEditor/save_form.html.ep index fc4d746893..c628b0128d 100644 --- a/templates/ContentGenerator/Instructor/PGProblemEditor/save_form.html.ep +++ b/templates/ContentGenerator/Instructor/PGProblemEditor/save_form.html.ep @@ -6,16 +6,6 @@
<%== maketext('Save to [_1] and View', tag('b', dir => 'ltr', $c->shortPath($c->{editFilePath}))) =%>
- % unless ($c->{file_type} eq 'hardcopy_theme') { -
- <%= check_box - newWindowSave => 1, - id => 'newWindowSave', - class => 'form-check-input', - $c->{file_type} eq 'hardcopy_header' ? (checked => undef) : () =%> - <%= label_for newWindowSave => maketext('Open in new window'), class => 'form-check-label' =%> -
- % } <%= hidden_field 'action.save.source_file' => $c->{editFilePath} =%> % # Backup options diff --git a/templates/ContentGenerator/Instructor/PGProblemEditor/view_form.html.ep b/templates/ContentGenerator/Instructor/PGProblemEditor/view_form.html.ep index 86044cb581..a5208323f5 100644 --- a/templates/ContentGenerator/Instructor/PGProblemEditor/view_form.html.ep +++ b/templates/ContentGenerator/Instructor/PGProblemEditor/view_form.html.ep @@ -35,12 +35,4 @@
% } - % unless ($c->{file_type} eq 'hardcopy_theme') { -
-
- <%= check_box 'newWindowView' => 1, id => 'newWindowView', class => 'form-check-input' =%> - <%= label_for newWindowView => maketext('Open in new window'), class => 'form-check-label' =%> -
-
- % }
diff --git a/templates/ContentGenerator/Instructor/ProblemSetDetail.html.ep b/templates/ContentGenerator/Instructor/ProblemSetDetail.html.ep index 81d5824dc6..f30c026e6a 100644 --- a/templates/ContentGenerator/Instructor/ProblemSetDetail.html.ep +++ b/templates/ContentGenerator/Instructor/ProblemSetDetail.html.ep @@ -579,8 +579,7 @@ <% end =%> % my $problemLink; % if ($isGatewaySet) { - % # If editing a set version, then open the actual set version. - % # Otherwise, the best that can be done is to use an undefined set. + % # Only link to a test when editing a test version. % if ($editingSetVersion) { % my ($problemNumber, $pageNumber) = % get_test_problem_position($db, $problemToShow); @@ -596,25 +595,6 @@ % currentPage => $pageNumber % } % )->fragment("prob$problemNumber"); - % } else { - % $problemLink = $c->systemLink( - % url_for( - % 'gateway_quiz', - % setID => 'Undefined_Set', - % problemID => '1' - % ), - % params => { - % effectiveUser => @editForUser == 1 ? $editForUser[0] - % : $authen->{user_id}, - % problemSeed => $problemToShow - % ? $problemToShow->problem_seed - % : '', - % sourceFilePath => $problemToShow - % && $problemToShow->source_file - % ? $problemToShow->source_file - % : $globalProblems->{$problemID}->source_file - % } - % ); % } % } else { % $problemLink = $c->systemLink( @@ -630,19 +610,21 @@ % } % ); % } - <%= link_to $problemLink, - class => 'psd_view btn btn-secondary btn-sm' - . (@editForUser ? '' : ' px-1 px-sm-2'), - target => 'WW_View', - data => { - bs_toggle => 'tooltip', - bs_placement => 'top', - bs_title => maketext('Open in New Window') - }, - begin =%> - - - <% end =%> + % if ($problemLink) { + <%= link_to $problemLink, + class => 'psd_view btn btn-secondary btn-sm' + . (@editForUser ? '' : ' px-1 px-sm-2'), + target => 'WW_View', + data => { + bs_toggle => 'tooltip', + bs_placement => 'top', + bs_title => maketext('Open in New Window') + }, + begin =%> + + + <% end =%> + % } % } % if ($authz->hasPermissions($authen->{user_id}, 'problem_grader')) { <%= link_to $c->systemLink(url_for( diff --git a/templates/ContentGenerator/Instructor/SetMaker/problem_row.html.ep b/templates/ContentGenerator/Instructor/SetMaker/problem_row.html.ep index 33d5822c4e..ed4ba0ceed 100644 --- a/templates/ContentGenerator/Instructor/SetMaker/problem_row.html.ep +++ b/templates/ContentGenerator/Instructor/SetMaker/problem_row.html.ep @@ -8,11 +8,6 @@ % ($isMO, $isstatic) = $c->getDBextras($sourceFileName); % } % -% # Determine if the target set is a gateway assignment. -% my $setRecord = - % (defined param('local_sets') && param('local_sets') ne '') ? $db->getGlobalSet(param('local_sets')) : undef; -% my $isGatewaySet = defined $setRecord && $setRecord->assignment_type =~ /gateway/; -% % my $noshowclass = $sourceFileData->{morelt} ? "MLT$sourceFileData->{morelt}" : "NS$cnt"; % if ($sourceFileData->{children}) { % my $numchild = @{ $sourceFileData->{children} }; @@ -278,11 +273,7 @@ % } <%= link_to $c->systemLink( - url_for( - 'instructor_problem_editor_withset_withproblem', - setID => 'Undefined_Set', - problemID => '1' - ), + url_for('instructor_problem_editor'), params => { sourceFilePath => $sourceFileName, problemSeed => $c->{problem_seed} @@ -300,30 +291,6 @@ <% end =%> - <%= link_to $c->systemLink( - url_for( - $isGatewaySet ? 'gateway_quiz' : 'problem_detail', - setID => 'Undefined_Set', - $isGatewaySet ? () : (problemID => '1') - ), - params => { - effectiveUser => $authen->{user_id}, - editMode => 'SetMaker', - problemSeed => $c->{problem_seed}, - sourceFilePath => $sourceFileName, - } - ), - id => "tryit$cnt", - class => 'text-decoration-none btn btn-sm btn-secondary', - target => 'WW_View', - data => { - bs_title => maketext('Try it'), - bs_toggle => 'tooltip', - bs_placement => 'top' - }, - begin =%> - - <% end =%> <%== maketext('Debugging information is shown below.') %>
% } -
<%= $c->output_custom_edit_message %>
<%= $c->output_summary %>
<%= $c->output_achievement_message %>
diff --git a/templates/HelpFiles/InstructorPGProblemEditor.html.ep b/templates/HelpFiles/InstructorPGProblemEditor.html.ep index 38cad5850f..6ea0c98639 100644 --- a/templates/HelpFiles/InstructorPGProblemEditor.html.ep +++ b/templates/HelpFiles/InstructorPGProblemEditor.html.ep @@ -151,9 +151,7 @@

<%= maketext('Render or re-render the problem, course info file, or set header being edited. This does not ' . 'change the permanent file on the disk. You can view different versions of the same problem by ' - . 'changing the seed. You can also change the manner in which the mathematics is typeset. If "Open ' - . 'in new window" is checked, then the problem will open in a new tab or window showing the ' - . 'problem as it will be rendered for students in the set.') =%> + . 'changing the seed. You can also change the manner in which the mathematics is typeset.') =%>

<%= maketext('This tab is available for all file types except hardcopy header files.') =%> @@ -205,12 +203,11 @@

<%= maketext('Save the contents of the editor window to the file on disk and re-render the problem. If ' - . '"Open in new window" is checked, then the problem will open in a new tab or window showing the ' - . 'problem as it will be rendered for students in the set. If "Create backup" is checked then a ' - . 'backup of the current file on disk will be created before overwriting it with the contents of ' - . 'the editor window. If a backup already exists, then a "Delete oldest backup" check box will be ' - . 'present. If that is checked, then the oldest backup will be deleted when "Save" is clicked. ' - . 'These backups can be reverted to and managed on the "Revert" tab.') =%> + . '"Create backup" is checked then a backup of the current file on disk will be created before ' + . 'overwriting it with the contents of the editor window. If a backup already exists, then a ' + . '"Delete oldest backup" check box will be present. If that is checked, then the oldest backup ' + . 'will be deleted when "Save" is clicked. These backups can be reverted to and managed on the ' + . '"Revert" tab.') =%>

<%= maketext('This tab is available for all files, but is only active for files in the course templates '