From 6334dd3237acec02e418d7120c3eebf1ea11241b Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Tue, 3 Nov 2026 21:26:36 -0700 Subject: [PATCH 1/2] Fix some issues when viewing a Problem with editMode. When viewing a problem in editMode (such as viewing a problem from the problem editor with the "Open in new window" checked) don't use the past answers from the effectiveUser if the seeds do not match. This prevents showing past answers if the seed is different, but past answers could be shown if the seeds match for debugging a problem. Hide the student navigation bar and problem grader when in editMode. It could be the wrong seed or even a temporary edit that hasn't been saved, and could be confusing to have the problem grader shown in this case. Prevent submitting/recording answers when in edit mode. This is to prevent submitting answers for wrong seeds for you or for students if that permission is configured. Always include the editMode hidden input when in editMode (not just for temporary files) so a check/preview/show correct form submit does not loose track of being in editMode. Co-Authored-By: Claude Opus 5.5 --- lib/WeBWorK/ContentGenerator/Problem.pm | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/Problem.pm b/lib/WeBWorK/ContentGenerator/Problem.pm index 36dcdbff64..288b8ad0d9 100644 --- a/lib/WeBWorK/ContentGenerator/Problem.pm +++ b/lib/WeBWorK/ContentGenerator/Problem.pm @@ -82,7 +82,8 @@ 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}); + && !$c->{invalidSet} + && !$c->{editMode}); } sub can_showAnsGroupInfo ($c, $user, $effectiveUser, $set, $problem) { @@ -125,6 +126,8 @@ sub can_showSolutions ($c, $user, $effectiveUser, $set, $problem) { sub can_recordAnswers ($c, $user, $effectiveUser, $set, $problem, $submitAnswers = 0) { my $authz = $c->authz; + return 0 if $c->{editMode}; + if ($user->user_id ne $effectiveUser->user_id) { return $authz->hasPermissions($user->user_id, 'record_answers_when_acting_as_student'); } @@ -355,6 +358,7 @@ async sub initialize ($c) { # If the caller is asking to override the problem seed, do so. my $problemSeed = $c->param('problemSeed'); if (defined $problemSeed && $problemSeed =~ /^[+-]?\d+$/) { + $c->{seedChanged} = $problemSeed != ($problem->problem_seed || 0); $problem->problem_seed($problemSeed); } @@ -381,7 +385,7 @@ async sub initialize ($c) { # Set options from form fields (see comment at top of file for form fields). my $redisplay = $c->param('redisplay'); - $c->{submitAnswers} = $c->param('submitAnswers'); + $c->{submitAnswers} = $c->{editMode} ? 0 : $c->param('submitAnswers'); my $checkAnswers = $c->param('checkAnswers'); my $previewAnswers = $c->param('previewAnswers') && $ce->{pg}{options}{showPreviewButton} ? 1 : 0; my $requestNewSeed = $c->param('requestNewSeed') // 0; @@ -531,10 +535,14 @@ async sub initialize ($c) { # requiring another answer submission. my $showReturningFeedback = 0; + # Do not use answers/persistent data when editing a problem with a different seed. + my $useSavedData = !($c->{editMode} && $c->{seedChanged}); + # Reinsert sticky answers. Do this only if new answers are NOT being submitted, # and a new problem version is NOT being opened. if (!($prEnabled && !$problem->{prCount}) && !($c->{submitAnswers} || $previewAnswers || $checkAnswers) + && $useSavedData && $will{showOldAnswers}) { my %oldAnswers = decodeAnswers($problem->last_answer); @@ -586,7 +594,7 @@ async sub initialize ($c) { : 0 ), debuggingOptions => getTranslatorDebuggingOptions($authz, $userID), - $prEnabled && !$problem->{prCount} + ($prEnabled && !$problem->{prCount}) || (!$useSavedData && !defined $formFields->{problem_data}) ? (problemData => '{}') : ($can{checkAnswers} && defined $formFields->{problem_data}) ? (problemData => $formFields->{problem_data}) @@ -837,7 +845,7 @@ sub nav ($c, $args) { role => 'navigation', 'aria-label' => 'problem navigation', $c->c($c->tag('div', class => 'd-flex submit-buttons-container', $c->navMacro($args, \%tail, @links)), - studentNav($c, $setID))->join('') + $c->{editMode} ? '' : studentNav($c, $setID))->join('') ); } @@ -1230,8 +1238,7 @@ sub output_misc ($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(editMode => $c->{editMode})) if $c->{editMode}; 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'); From ddfb9ed3fe7523db2520739445115fafe047379e Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Sat, 7 Nov 2026 13:35:50 -0700 Subject: [PATCH 2/2] Better deal with viewing a Problem in editMode. First, there is an initial test that requires editMode be set in combination with Undefined_Set along with testing permissions. Also reject invalid editMode settings. This makes it so using Undefined_Set in a url will fail with an invalid set unless editMode is set along with correct permissions. This also simplifies logic, so one only needs to test for editMode when deciding if to show/do something and not have to check the setID against Undefined_Set. In editMode, prevent submits, and other features such as show me another that could edit the database. Hide the nav bar completely, the siblings links, and other page features that are not useful when viewing/testing a problem edit in its own window. Hide buttons like "Show Me Another", "Show Past Answers", "Email Instructor", "Edit", "Edit Tags" buttons when in editMode. Fix a bunch of existing issues around use of Undefined_Set. A TA could try to view an Undefined_Set and get some bugs/crashes, breadcrumb links to an Undefined_Set caused a crash on the sets page (so don't include links in that case). Prevent a user from creating a set with ID Undefined_Set as this will not interact correctly with editMode. Update checkSet to only allow `Undefined_Set` for problem or test routes. This way there is no longer an error when trying to link to the ProblemSet `Undefined_Set`. Don't show the problem summary message in editMode (saved score, number of attempts used, number of attempts available, and so on). Only show the score of the recently checked answers when checking answers, so partial credit can be tested. Note this does hide any `state_summary_msg` created by custom macros/graders, so this isn't being tested. Add show_me_another to the checkSet test. This should have been done already, to avoid showing sets in show_me_another that have restrictions. This also simplifies tests in Problem.pm to ensure that both routes have already tested things with checkSet. Don't force show initial feedback after the answer date when in editMode. This didn't work well with Undefined_Set. Messages in editMode are still shown when editing a live problem (Such as this set is hidden/visible to students), but they are not shown when looking at an Undefined_Set. I'm unsure exactly what messages should be shown, but leaving these is probably still useful when viewing problems in a live set. Co-Authored-By: Claude Opus 5.5 --- lib/WeBWorK/Authz.pm | 23 +- .../ContentGenerator/API/SetActions.pm | 2 + .../Instructor/PGProblemEditor.pm | 9 +- .../Instructor/ProblemSetList.pm | 2 + .../ContentGenerator/Instructor/SetMaker.pm | 2 + lib/WeBWorK/ContentGenerator/Problem.pm | 281 +++++++++--------- lib/WeBWorK/File/SetDef.pm | 6 + .../Instructor/SetMaker/problem_row.html.ep | 2 +- .../Problem/submit_buttons.html.ep | 2 +- 9 files changed, 177 insertions(+), 152 deletions(-) diff --git a/lib/WeBWorK/Authz.pm b/lib/WeBWorK/Authz.pm index 6005d7e1c7..7d416298c9 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 @@ -208,6 +209,12 @@ sub checkSet ($self) { # If there is no input userName, then the content generator will be forcing a login, so just bail. return 0 if !$userName || !$effectiveUserName; + # The Undefined_Set is only valid for instructors viewing problems in a problem set or a test. + my $allowUndefinedSet = + $setName eq 'Undefined_Set' + && (grep { $node_name eq $_ } qw(problem_detail gateway_quiz gateway_quiz_version)) + && $self->hasPermissions($userName, 'access_instructor_tools'); + # Do we have a cached set that we can use? my $set = $self->{merged_set}; @@ -215,8 +222,8 @@ 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. + } elsif ($allowUndefinedSet) { + # This is the case of previewing a problem from the problem editor or library browser. return 0; } else { if ($db->existsSetVersion($effectiveUserName, $setName, $setVersion)) { @@ -241,9 +248,8 @@ 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. + } elsif ($allowUndefinedSet) { + # This is the case of viewing a problem from the problem editor or library browser. return 0; } else { return $c->maketext('Requested set "[_1]" is not assigned to user [_2].', $setName, $effectiveUserName); @@ -301,7 +307,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/API/SetActions.pm b/lib/WeBWorK/ContentGenerator/API/SetActions.pm index 10008da41b..6ed8047126 100644 --- a/lib/WeBWorK/ContentGenerator/API/SetActions.pm +++ b/lib/WeBWorK/ContentGenerator/API/SetActions.pm @@ -165,6 +165,8 @@ sub createNewSet ($c) { $newSetName =~ s/\s/_/g; + return $c->renderError('The set name "Undefined_Set" is reserved.') if $newSetName eq 'Undefined_Set'; + my $db = $c->db; return $c->renderError("The set name '$newSetName' already exists. " diff --git a/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm b/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm index 59c1e9ce1d..c2c93c50f7 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/PGProblemEditor.pm @@ -313,13 +313,16 @@ sub initialize ($c) { sub path ($c, $args) { # We need to build a path to the problem being edited by hand, since it is not the same as the url path for this # page. The bread crumb path leads back to the problem being edited, not to the Instructor tool. + # Don't link to the set or problem for the Undefined_Set, since neither exist. + my $isUndefinedSet = ($c->stash('setID') // '') eq 'Undefined_Set'; return $c->pathMacro( $args, 'WeBWorK' => $c->url_for('root'), $c->stash('courseID') => $c->url_for('set_list'), - ($c->stash('setID') // '') => $c->url_for('problem_list'), - ($c->{prettyProblemNumber} // '') => - $c->url_for('problem_detail', problemID => $c->stash('problemID') || ''), + ($c->stash('setID') // '') => $isUndefinedSet ? '' : $c->url_for('problem_list'), + ($c->{prettyProblemNumber} // '') => $isUndefinedSet + ? '' + : $c->url_for('problem_detail', problemID => $c->stash('problemID') || ''), $c->maketext('Editor') => '' ); } diff --git a/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetList.pm b/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetList.pm index b4dc785399..cc48d36403 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetList.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetList.pm @@ -429,6 +429,8 @@ sub create_handler ($c) { $newSetID =~ s/_/ /gr ) ) unless $newSetID =~ m/^[-a-zA-Z0-9_.]*$/; + return (0, $c->maketext('Failed to create new set: The set name "[_1]" is reserved.', 'Undefined_Set')) + if $newSetID eq 'Undefined_Set'; return ( 0, $c->maketext( diff --git a/lib/WeBWorK/ContentGenerator/Instructor/SetMaker.pm b/lib/WeBWorK/ContentGenerator/Instructor/SetMaker.pm index 7621b58c9a..f0b00cbe8d 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/SetMaker.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/SetMaker.pm @@ -561,6 +561,8 @@ sub initialize ($c) { if (!$newSetName) { $c->addbadmessage($c->maketext('Please specify a new set name.')); + } elsif ($newSetName eq 'Undefined_Set') { + $c->addbadmessage($c->maketext('The set name "[_1]" is reserved.', 'Undefined_Set')); } elsif (defined $db->getGlobalSet($newSetName)) { $c->addbadmessage($c->maketext('The set "[_1]" already exists.', $newSetName)); } else { diff --git a/lib/WeBWorK/ContentGenerator/Problem.pm b/lib/WeBWorK/ContentGenerator/Problem.pm index 288b8ad0d9..29fb2bef9c 100644 --- a/lib/WeBWorK/ContentGenerator/Problem.pm +++ b/lib/WeBWorK/ContentGenerator/Problem.pm @@ -81,7 +81,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} && !$c->{editMode}); } @@ -198,6 +197,8 @@ sub can_useMathQuill ($c) { sub can_showMeAnother ($c, $user, $effectiveUser, $set, $problem, $submitAnswers = 0) { my $ce = $c->ce; + return 0 if $c->{editMode}; + # If the showMeAnother button isn't enabled for the course, then it can't be used. return 0 unless $ce->{pg}{options}{enableShowMeAnother}; @@ -258,15 +259,27 @@ async sub initialize ($c) { # Check that the set is valid. $c->{invalidSet} is set in checkSet called by ContentGenerator.pm. return if $c->{invalidSet}; - # Obtain the merged set for $effectiveUser + # Undefined_Set requires editMode, editMode must be valid, and both require permission to modify problem sets. + if ( + ($c->{editMode} || $setID eq 'Undefined_Set') + && (($c->{editMode} // '') !~ /^(?:temporaryFile|savedFile)\z/ + || !$authz->hasPermissions($userID, 'modify_problem_sets')) + ) + { + $c->{invalidProblem} = $c->{invalidSet} = 1; + return; + } + + # Obtain the merged set for $effectiveUser, or create a fake set for the Undefined_Set in editMode. $c->{set} = $db->getMergedSet($effectiveUserID, $setID); + $c->{set} //= fake_set($db) if $c->{editMode}; # 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. return unless $authz->hasPermissions($userID, 'view_unopened_sets') - || $setID eq 'Undefined_Set' + || $c->{editMode} || ( after($c->{set}->open_date, $c->submitTime) && !( @@ -290,24 +303,10 @@ 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. + # Deal with creating fake sets/problems, updating problem source if needed when in editMode. + if ($c->{editMode}) { + # If the problem is not assigned to the effective user, use the global problem with fake user data. + # If there is no global problem, create a fake problem from sourceFilePath. unless (defined $problem) { my $globalProblem = $db->getGlobalProblem($setID, $problemID); @@ -340,39 +339,37 @@ async sub initialize ($c) { } } - # 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. + # Use the temporary file if one was passed in. my $sourceFilePath = $c->param('sourceFilePath'); - if (defined $c->{editMode} && $c->{editMode} eq 'temporaryFile' && defined $sourceFilePath) { + if ($c->{editMode} eq 'temporaryFile' && defined $sourceFilePath) { die 'sourceFilePath is unsafe!' - unless path_is_subdir($sourceFilePath, $ce->{courseDirs}->{templates}, 1); + 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. + # Override the problem seed if one was passed in. my $problemSeed = $c->param('problemSeed'); if (defined $problemSeed && $problemSeed =~ /^[+-]?\d+$/) { $c->{seedChanged} = $problemSeed != ($problem->problem_seed || 0); $problem->problem_seed($problemSeed); } + } + # The problem is invalid if it does not exist, has no source file, + # or is in a hidden set that the user cannot view (hidden sets are allowed in editMode). + $c->{invalidProblem} = + !(defined $problem + && $problem->source_file + && ($c->{editMode} || $c->{set}->visible || $authz->hasPermissions($userID, 'view_hidden_sets'))); + + if ($c->{editMode} || $authz->hasPermissions($userID, 'modify_problem_sets')) { + # Show the set visibility, except for the Undefined_Set which is not a real set. $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->tag('p', class => 'font-hidden m-0', $c->maketext('This set is hidden from students.'))) + if $setID ne 'Undefined_Set'; + } elsif (!$c->{invalidProblem} && !$problem->value) { + $c->addbadmessage($c->maketext('This problem will not count toward your grade.')); } $c->{userID} = $userID; @@ -395,8 +392,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} @@ -553,6 +549,10 @@ async sub initialize ($c) { my $showOnlyCorrectAnswers = $c->param('showCorrectAnswers') && $will{showCorrectAnswers}; + # Show feedback after the answer date unless this is a preview or editMode. + my $useAnswerDateFeedback = + !$c->{previewAnswers} && !$c->{editMode} && after($c->{set}->answer_date, $c->submitTime); + # Translation debug('begin pg processing'); my $pg = await renderPG( @@ -582,19 +582,17 @@ async sub initialize ($c) { forceShowAttemptResults => $showOnlyCorrectAnswers || $will{checkAnswers} || $will{showProblemGrader} - || ($ce->{pg}{options}{automaticAnswerFeedback} - && !$c->{previewAnswers} - && after($c->{set}->answer_date, $c->submitTime)), + || ($ce->{pg}{options}{automaticAnswerFeedback} && $useAnswerDateFeedback), showMessages => !$showOnlyCorrectAnswers, showCorrectAnswers => ( $c->{submitAnswers} && $c->{showCorrectOnRandomize} ? 2 - : !$c->{previewAnswers} && after($c->{set}->answer_date, $c->submitTime) - ? ($ce->{pg}{options}{correctRevealBtnAlways} ? 1 : 2) + : $useAnswerDateFeedback ? ($ce->{pg}{options}{correctRevealBtnAlways} ? 1 : 2) : $will{showProblemGrader} || (!$c->{previewAnswers} && $will{showCorrectAnswers}) ? 1 : 0 ), debuggingOptions => getTranslatorDebuggingOptions($authz, $userID), - ($prEnabled && !$problem->{prCount}) || (!$useSavedData && !defined $formFields->{problem_data}) + ($prEnabled && !$problem->{prCount}) + || (!$useSavedData && !defined $formFields->{problem_data}) ? (problemData => '{}') : ($can{checkAnswers} && defined $formFields->{problem_data}) ? (problemData => $formFields->{problem_data}) @@ -637,7 +635,7 @@ sub siblings ($c) { my $authz = $c->authz; # Can't show sibling problems if the set is invalid. - return '' if $c->{invalidSet}; + return '' if $c->{invalidSet} || $c->{editMode}; my $setID = $c->{set}->set_id; my $eUserID = $c->param('effectiveUser'); @@ -645,7 +643,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,7 +759,7 @@ sub siblings ($c) { } sub nav ($c, $args) { - return '' if $c->{invalidProblem} || $c->{invalidSet}; + return '' if $c->{invalidProblem} || $c->{invalidSet} || $c->{editMode}; my %can = %{ $c->{can} }; @@ -782,35 +780,33 @@ sub nav ($c, $args) { 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; + my $curr_index = 0; - 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)); + 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) { @@ -821,12 +817,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'), @@ -845,7 +837,7 @@ sub nav ($c, $args) { role => 'navigation', 'aria-label' => 'problem navigation', $c->c($c->tag('div', class => 'd-flex submit-buttons-container', $c->navMacro($args, \%tail, @links)), - $c->{editMode} ? '' : studentNav($c, $setID))->join('') + studentNav($c, $setID))->join('') ); } @@ -862,7 +854,8 @@ sub path ($c, $args) { my @path = ( WeBWorK => $navigation_allowed ? $c->url_for('root') : '', $c->stash('courseID') => $navigation_allowed ? $c->url_for('set_list') : '', - $c->stash('setID') => $c->url_for('problem_list'), + # Don't link to the Undefined_Set, since it does not exist. + $c->stash('setID') => $c->stash('setID') eq 'Undefined_Set' ? '' : $c->url_for('problem_list'), ); if ($c->current_route eq 'show_me_another') { @@ -892,7 +885,7 @@ sub page_title ($c) { $c->maketext('[_1]: Problem [_2]', $c->tag('span', dir => 'ltr', format_set_name_display($setID)), $problemID); # Return here if we don't have the requisite information. - return $header if ($c->{invalidSet} || $c->{invalidProblem}); + return $header if $c->{invalidSet} || $c->{invalidProblem}; my $ce = $c->ce; my $problem = $c->{problem}; @@ -911,24 +904,18 @@ sub page_title ($c) { } # Add the edit link to the sub header if the user has the permissions to edit problems. - if ($c->authz->hasPermissions($c->authen->{user_id}, 'modify_problem_sets')) { + if (!$c->{editMode} && $c->authz->hasPermissions($c->authen->{user_id}, 'modify_problem_sets')) { $subheader = $c->c( $subheader, $c->tag( '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' ) @@ -936,8 +923,8 @@ sub page_title ($c) { )->join(''); } - # Add the tag edit button to the sub header if the user has permission to edit tags. - if ($c->authz->hasPermissions($c->authen->{user_id}, 'modify_tags')) { + # Add the tag edit button to the sub header if the user has permission to edit tags outside of editMode. + if ($c->authz->hasPermissions($c->authen->{user_id}, 'modify_tags') && !$c->{editMode}) { $subheader = $c->c( $subheader, $c->tag( @@ -1054,45 +1041,58 @@ sub output_score_summary ($c) { my $output = $c->c; - unless (defined $pg->{state}{state_summary_msg} && $pg->{state}{state_summary_msg} =~ /\S/) { + if ($c->{editMode}) { + # Only show the score for the answers that were checked in editMode. push( @$output, - $c->{submitAnswers} ? $c->{scoreRecordedMessage} . $c->tag('br') : '', - $c->maketext('You have attempted this problem [quant,_1,time,times].', $attempts), - $prMessage, - $c->tag('br'), - $c->{submitAnswers} - ? ( - $c->maketext( - 'You received a score of [_1] for this attempt.', - wwRound(0, - compute_reduced_score($ce, $problem, $set, $pg->{result}{score}, $c->submitTime) * 100) - . '%' - ), - $c->tag('br') - ) - : '', - $problem->attempted - ? ( - $c->maketext( - 'Your overall recorded score is [_1]. [_2]', - wwRound(0, $problem->status * 100) . '%', - $problem->value ? '' : $c->maketext('(This problem will not count toward your grade.)') - ), - $c->tag('br') - ) - : '', - $setClosed ? $setClosedMessage : $c->maketext( - 'You have [negquant,_1,unlimited attempts,attempt,attempts] remaining.', - $problem->max_attempts - $attempts + $c->maketext( + 'You received a score of [_1] for this attempt.', + wwRound(0, $pg->{result}{score} * 100) . '%' ) - ); + ) if $c->{will}{checkAnswers}; } else { - push(@$output, $pg->{state}{state_summary_msg}); + unless (defined $pg->{state}{state_summary_msg} && $pg->{state}{state_summary_msg} =~ /\S/) { + push( + @$output, + $c->{submitAnswers} ? $c->{scoreRecordedMessage} . $c->tag('br') : '', + $c->maketext('You have attempted this problem [quant,_1,time,times].', $attempts), + $prMessage, + $c->tag('br'), + $c->{submitAnswers} + ? ( + $c->maketext( + 'You received a score of [_1] for this attempt.', + wwRound( + 0, + compute_reduced_score($ce, $problem, $set, $pg->{result}{score}, $c->submitTime) * 100 + ) + . '%' + ), + $c->tag('br') + ) + : '', + $problem->attempted + ? ( + $c->maketext( + 'Your overall recorded score is [_1]. [_2]', + wwRound(0, $problem->status * 100) . '%', + $problem->value ? '' : $c->maketext('(This problem will not count toward your grade.)') + ), + $c->tag('br') + ) + : '', + $setClosed ? $setClosedMessage : $c->maketext( + 'You have [negquant,_1,unlimited attempts,attempt,attempts] remaining.', + $problem->max_attempts - $attempts + ) + ); + } else { + push(@$output, $pg->{state}{state_summary_msg}); + } } # Print jitar specific information for students (and notify instructor if necessary). - if ($set->set_id ne 'Undefined_Set' && $set->assignment_type() eq 'jitar') { + if (!$c->{editMode} && $set->assignment_type eq 'jitar') { my @problemIDs = map { $_->[3] } $db->listUserProblemsWhere({ user_id => $effectiveUser, set_id => $set->set_id }, 'problem_id'); @@ -1236,9 +1236,9 @@ 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 $c->{editMode}; + # Add hidden fields when in editMode. + if ($c->{editMode}) { + push(@$output, $c->hidden_field(editMode => $c->{editMode})); 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'); @@ -1332,7 +1332,7 @@ sub output_summary ($c) { ) ) if $c->{resubmitDetected}; - if ($c->{set}->set_id ne 'Undefined_Set' && $c->{set}->assignment_type eq 'jitar') { + if (!$c->{editMode} && $c->{set}->assignment_type eq 'jitar') { my $hasChildren = 0; my @problemIDs = @@ -1390,12 +1390,11 @@ sub output_summary ($c) { # Output the achievement message if there is one. sub output_achievement_message ($c) { - # If achievements are enabled and this is not an undefined set, - # check to see if there are new achievements and output them. + # If achievements are enabled check to see if there are new achievements and output them. + # Note, submitAnswers will always be false when in editMode, so this only works on real problems. if ($c->ce->{achievementsEnabled} && $c->{will}{recordAnswers} - && $c->{submitAnswers} - && $c->{problem}->set_id ne 'Undefined_Set') + && $c->{submitAnswers}) { return checkForAchievements($c->{problem}, $c); } @@ -1421,7 +1420,7 @@ sub output_custom_edit_message ($c) { # Output the "Show Past Answers" button sub output_past_answer_button ($c) { - if ($c->authz->hasPermissions($c->authen->{user_id}, 'view_answers')) { + if (!$c->{editMode} && $c->authz->hasPermissions($c->authen->{user_id}, 'view_answers')) { my $problemID = $c->{problem}->problem_id; my $setRecord = $c->db->getGlobalSet($c->{problem}->set_id); if (defined $setRecord && $setRecord->assignment_type eq 'jitar') { @@ -1456,6 +1455,8 @@ sub output_past_answer_button ($c) { # Output the "Email Instructor" button sub output_email_instructor ($c) { + return '' if $c->{editMode}; + my $user = $c->authen->{user}; # FIXME: Most of what is passed here is only needed by the feedback form, and should be extracted in the diff --git a/lib/WeBWorK/File/SetDef.pm b/lib/WeBWorK/File/SetDef.pm index b5923eeafb..b898c41191 100644 --- a/lib/WeBWorK/File/SetDef.pm +++ b/lib/WeBWorK/File/SetDef.pm @@ -84,6 +84,12 @@ sub importSetsFromDef ($ce, $db, $setDefFiles, $existingSets = undef, $assign = my $prettySetID = format_set_name_display($setData->{setID}); + if ($setData->{setID} eq 'Undefined_Set') { + push @skipped, $setData->{setID}; + push @errors, [ x('The set name "[_1]" is reserved.'), 'Undefined_Set' ]; + next; + } + if ($allSets{ $setData->{setID} }) { # This set already exists! push @skipped, $setData->{setID}; diff --git a/templates/ContentGenerator/Instructor/SetMaker/problem_row.html.ep b/templates/ContentGenerator/Instructor/SetMaker/problem_row.html.ep index 33d5822c4e..c0a2f8ead8 100644 --- a/templates/ContentGenerator/Instructor/SetMaker/problem_row.html.ep +++ b/templates/ContentGenerator/Instructor/SetMaker/problem_row.html.ep @@ -308,7 +308,7 @@ ), params => { effectiveUser => $authen->{user_id}, - editMode => 'SetMaker', + editMode => 'savedFile', problemSeed => $c->{problem_seed}, sourceFilePath => $sourceFileName, } diff --git a/templates/ContentGenerator/Problem/submit_buttons.html.ep b/templates/ContentGenerator/Problem/submit_buttons.html.ep index e676234e42..00728cf5c3 100644 --- a/templates/ContentGenerator/Problem/submit_buttons.html.ep +++ b/templates/ContentGenerator/Problem/submit_buttons.html.ep @@ -102,7 +102,7 @@ : '' ) } =%> - % } else { + % } elsif (!$c->{editMode}) { % # If showMeAnother is available for the course, and for the current problem and is either not yet % # available because the student hasn't tried enough times or is no longer available because the user has used % # show me another the maximum allowed number of times then gray it out. Otherwise display nothing.