Allow viewing ProblemSet page for any valid set.#2869
Allow viewing ProblemSet page for any valid set.#2869somiaj wants to merge 1 commit intoopenwebwork:developfrom
Conversation
ba53b4d to
1bc4f49
Compare
98dab46 to
e5ef385
Compare
drgrice1
left a comment
There was a problem hiding this comment.
There is another problem with this. Currently if $LTIGradeMod is "homework" and $permissionLevels{navigation_allowed} is set to "student" or lower, then when the student views the assignments page they are shown:

However, now the message stating that you need to log in to the set via the LMS is not shown and the links for those sets are active. Furthermore, if you click on one of those sets to open it you get:

That isn't good, and needs to be rethought with this pull request.
e5ef385 to
89f1867
Compare
|
@drgrice1 I updated it to show a warning message about accessing the set via the LMS in the case you described. I don't have a way to test LTI in my development setup, so I'm mostly 'guessing' it works (sorry about that, I just don't have time to setup a way to test that at the moment). |
|
I am not quite sure if the links should be active and/or the message should be shown on the |
drgrice1
left a comment
There was a problem hiding this comment.
I still really don't like this pull request as it is. I guess I just really don't like that this is a permission at all. I think we need to decide if set headers really need such a permission. I really don't think there is such a need, and I think this is just yet another rather confusing permission.
The addition of this permission also adds a bunch of redundancy and now out of sync wording for text shown in different cases. The wording on the problem sets page if a student is missing the requisite grade pass back data and the user does not have this new permission is You must log into this set via your Learning Management System ([_1])., but the new wording if the user does have this new permission is You must access this set via your Learning Management System ([_1]) before starting. Those should use the same wording. Then there is also the message that is in the Authz::checkSet method that says You must use your Learning Management System ([_1]) to access this set. Try logging in to the Learning Management System and visiting the set from there. and is shown if the user finds and uses a link that directly enters the set, but doesn't have the new permission.
Ultimately we need to decide if this needs a permission at all. I vote no. I don't see the need for the security for set headers.
|
I thought I had posted to this thread but can't find it now. I wonder if I
drafted something and moved away before clicking to post.
No matter. The gist of what I wrote was why not just always show set
descriptions and headers, even when the user cannot access the set? If an
instructor wants to hide something, just don't put in in the header in the
first place ppl. Or if they need to, get fancy and code pg within the
header to conditionally reveal certain things.
Even if there were a global permission, an instructor might want one set to
reveal the header always, and another only under the right conditions.
On Tue, Jan 13, 2026, 6:48 PM Glenn Rice ***@***.***> wrote:
***@***.**** commented on this pull request.
I still really don't like this pull request as it is. I guess I just
really don't like that this is a permission at all. I think we need to
decide if set headers really need such a permission. I really don't think
there is such a need, and I think this is just yet another rather confusing
permission.
The addition of this permission also adds a bunch of redundancy and now
out of sync wording for text shown in different cases. The wording on the
problem sets page if a student is missing the requisite grade pass back
data and the user does not have this new permission is You must log into
this set via your Learning Management System ([_1])., but the new wording
if the user does have this new permission is You must access this set via
your Learning Management System ([_1]) before starting. Those should use
the same wording. Then there is also the message that is in the
Authz::checkSet method that says You must use your Learning Management
System ([_1]) to access this set. Try logging in to the Learning Management
System and visiting the set from there. and is shown if the user finds
and uses a link that directly enters the set, but doesn't have the new
permission.
Ultimately we need to decide if this needs a permission at all. I vote no.
I don't see the need for the security for set headers.
------------------------------
In templates/ContentGenerator/ProblemSet.html.ep
<https://urldefense.com/v3/__https://github.com/openwebwork/webwork2/pull/2869*discussion_r2688712338__;Iw!!Ka_JY85zDv0FFw!k5pH44OTON2tqfqraCMeu-PhGhvwC6NB1lKo_2ZWynBJ8zENbNMjPgRRzRMFdWALexDFxfKstEMAIplpsTBi7-03QBY$>
:
> % }
%
% my $set = $c->{set};
%
% # Stats message displays the current status of the set and states the next important date.
<%= include 'ContentGenerator/Base/set_status', set => $set =%>
%
+% # Shows warning about restricted IP settings.
+% if ($c->{viewSetCheck} && $c->{viewSetCheck} eq 'restricted') {
+ <div class="alert alert-warning"><%= $c->{invalidSet} %></div>
+% }
+% # Show a warning that the user must access the set via their LMS before they can start.
+% if ($c->{viewSetCheck} && $c->{viewSetCheck} eq 'lti_restricted') {
+ <div class="alert alert-warning">
+ <%= maketext('You must access this set via your Learning Management System ([_1]) before starting.',
This needs to be
⬇️ Suggested change
- <%= maketext('You must access this set via your Learning Management System ([_1]) before starting.',
+ <%== maketext('You must access this set via your Learning Management System ([_1]) before starting.',
Otherwise the link is displayed literally as <a href="
https://myschool.edu/lms/
<https://urldefense.com/v3/__https://myschool.edu/lms/__;!!Ka_JY85zDv0FFw!k5pH44OTON2tqfqraCMeu-PhGhvwC6NB1lKo_2ZWynBJ8zENbNMjPgRRzRMFdWALexDFxfKstEMAIplpsTBiaAGszrI$>">the
LMS</a> instead of being a link.
—
Reply to this email directly, view it on GitHub
<https://urldefense.com/v3/__https://github.com/openwebwork/webwork2/pull/2869*pullrequestreview-3658593873__;Iw!!Ka_JY85zDv0FFw!k5pH44OTON2tqfqraCMeu-PhGhvwC6NB1lKo_2ZWynBJ8zENbNMjPgRRzRMFdWALexDFxfKstEMAIplpsTBi92ZqezY$>,
or unsubscribe
<https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ABEDOAG6LJEQCRGEW776E2D4GWVBPAVCNFSM6AAAAACP6ZZAKOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTMNJYGU4TGOBXGM__;!!Ka_JY85zDv0FFw!k5pH44OTON2tqfqraCMeu-PhGhvwC6NB1lKo_2ZWynBJ8zENbNMjPgRRzRMFdWALexDFxfKstEMAIplpsTBi_41jQFM$>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
Alex Jordan
Mathematics Instructor
Portland Community College
|
|
All that is to say the goal of making the situation friendlier is good. And
I see it as an improvement just to always show headers.
Alex Jordan
Mathematics Instructor
Portland Community College
…On Tue, Jan 13, 2026, 6:58 PM Alex Jordan ***@***.***> wrote:
I thought I had posted to this thread but can't find it now. I wonder if I
drafted something and moved away before clicking to post.
No matter. The gist of what I wrote was why not just always show set
descriptions and headers, even when the user cannot access the set? If an
instructor wants to hide something, just don't put in in the header in the
first place ppl. Or if they need to, get fancy and code pg within the
header to conditionally reveal certain things.
Even if there were a global permission, an instructor might want one set
to reveal the header always, and another only under the right conditions.
On Tue, Jan 13, 2026, 6:48 PM Glenn Rice ***@***.***> wrote:
> ***@***.**** commented on this pull request.
>
> I still really don't like this pull request as it is. I guess I just
> really don't like that this is a permission at all. I think we need to
> decide if set headers really need such a permission. I really don't think
> there is such a need, and I think this is just yet another rather confusing
> permission.
>
> The addition of this permission also adds a bunch of redundancy and now
> out of sync wording for text shown in different cases. The wording on the
> problem sets page if a student is missing the requisite grade pass back
> data and the user does not have this new permission is You must log into
> this set via your Learning Management System ([_1])., but the new
> wording if the user does have this new permission is You must access
> this set via your Learning Management System ([_1]) before starting.
> Those should use the same wording. Then there is also the message that is
> in the Authz::checkSet method that says You must use your Learning
> Management System ([_1]) to access this set. Try logging in to the Learning
> Management System and visiting the set from there. and is shown if the
> user finds and uses a link that directly enters the set, but doesn't have
> the new permission.
>
> Ultimately we need to decide if this needs a permission at all. I vote
> no. I don't see the need for the security for set headers.
> ------------------------------
>
> In templates/ContentGenerator/ProblemSet.html.ep
> <https://urldefense.com/v3/__https://github.com/openwebwork/webwork2/pull/2869*discussion_r2688712338__;Iw!!Ka_JY85zDv0FFw!k5pH44OTON2tqfqraCMeu-PhGhvwC6NB1lKo_2ZWynBJ8zENbNMjPgRRzRMFdWALexDFxfKstEMAIplpsTBi7-03QBY$>
> :
>
> > % }
> %
> % my $set = $c->{set};
> %
> % # Stats message displays the current status of the set and states the next important date.
> <%= include 'ContentGenerator/Base/set_status', set => $set =%>
> %
> +% # Shows warning about restricted IP settings.
> +% if ($c->{viewSetCheck} && $c->{viewSetCheck} eq 'restricted') {
> + <div class="alert alert-warning"><%= $c->{invalidSet} %></div>
> +% }
> +% # Show a warning that the user must access the set via their LMS before they can start.
> +% if ($c->{viewSetCheck} && $c->{viewSetCheck} eq 'lti_restricted') {
> + <div class="alert alert-warning">
> + <%= maketext('You must access this set via your Learning Management System ([_1]) before starting.',
>
> This needs to be
> ⬇️ Suggested change
>
> - <%= maketext('You must access this set via your Learning Management System ([_1]) before starting.',
> + <%== maketext('You must access this set via your Learning Management System ([_1]) before starting.',
>
> Otherwise the link is displayed literally as <a href="
> https://myschool.edu/lms/
> <https://urldefense.com/v3/__https://myschool.edu/lms/__;!!Ka_JY85zDv0FFw!k5pH44OTON2tqfqraCMeu-PhGhvwC6NB1lKo_2ZWynBJ8zENbNMjPgRRzRMFdWALexDFxfKstEMAIplpsTBiaAGszrI$>">the
> LMS</a> instead of being a link.
>
> —
> Reply to this email directly, view it on GitHub
> <https://urldefense.com/v3/__https://github.com/openwebwork/webwork2/pull/2869*pullrequestreview-3658593873__;Iw!!Ka_JY85zDv0FFw!k5pH44OTON2tqfqraCMeu-PhGhvwC6NB1lKo_2ZWynBJ8zENbNMjPgRRzRMFdWALexDFxfKstEMAIplpsTBi92ZqezY$>,
> or unsubscribe
> <https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ABEDOAG6LJEQCRGEW776E2D4GWVBPAVCNFSM6AAAAACP6ZZAKOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTMNJYGU4TGOBXGM__;!!Ka_JY85zDv0FFw!k5pH44OTON2tqfqraCMeu-PhGhvwC6NB1lKo_2ZWynBJ8zENbNMjPgRRzRMFdWALexDFxfKstEMAIplpsTBi_41jQFM$>
> .
> You are receiving this because you are subscribed to this thread.Message
> ID: ***@***.***>
>
Alex Jordan
Mathematics Instructor
Portland Community College
|
|
So it sounds like you are on the same page as me then. |
|
I agree with you on that this shouldn't be a permission. When I was originally thinking about adding this I was just going to give access to the set info. I ultimately went with a permission so if someone didn't like this change they could just disable it by setting the permission, as I wasn't sure if my use case was the only use case to consider. But it seems you two would also rather just always show the |
|
@somiaj: By the way, you don't need actual LTI authentication to test this for the LTI setup. Just set up the course so that it uses LTI authentication with homework grade passback, but allow direct login. That will still show the behavior that needs to be tested here. |
drgrice1
left a comment
There was a problem hiding this comment.
There is another case that is not working right with this pull request. That is the case of restricted release sets. If a set has a release condition on another set, the with the develop branch you see

on the problem sets page.
With this pull request, that message telling the student that the condition that needs to be met is still shown, and the link to the set is active. When you click on the link, then you see

That also needs to be done differently.
89f1867 to
6944e57
Compare
ada5806 to
7e35aae
Compare
|
This has been updated so the ProblemSet page can always be shown if the set is a valid set for the user. In addition set messages should be consistent between the ProblemSets and ProblemSet page. I added a new I updated the PR name and initial comment description stating what is done. |
929647a to
74fe4b2
Compare
25e621a to
11c98fb
Compare
11c98fb to
ccfb529
Compare
|
There is something different with restricted release sets that I still don't think is working right. Previously although an instructor could access a restricted release set without satisfying the prerequisites, it would still state on the "Assignments" page that in order to access the set you must score at least ... in order to access this set (as in the screenshot in #2869 (review)). That is still shown in the "Assignments" page for students with this pull request, but not for instructors. If the instructor enters the set, they also don't see the message there, so now they don't see it anywhere, and have no indication that the set is any different than a regular set. That is without inspecting the settings for the set in the sets manager. |
|
I see, and I have a way to adjust for that. But some thoughts. First that message would only show when the student hadn't completed the the prerequisite assignment(s). In order to help inform instructors about the prerequisites, should that message always show to instructors even if met by the student? Also if this is to help instructors see what possible issues is, think we should also show instructors the LTI message as well, so they can see why a student might have trouble access a set in that case as well. I can make it like it was, but thinking this should be updated or adjusted in any way. |
|
Yeah, I don't think that this was handled the best before, but is how it has been since restricted release sets were implemented. I think that the important thing is that there are indicators letting the instructor know the restrictions that apply for the set. |
ccfb529 to
2f46926
Compare
|
I updated it so restricted set messages are not hidden from instructors. I put the message on both the ProblemSets and ProblemSet page. This includes showing any LTI message (only when acting as a student) that way a instructor can see and inform a student they need to access the set via their LMS first if a student contacts them. The conditional release set message is also shown again (Still requires that the conditions are not met, I didn't change that, but could, though when acting as a student the message should be what a student would see, and most likely when not acting the conditions have not been met so it will be there). Also instructors will see IP restriction messages on sets that have IP restrictions on the ProblemSet page, but not the ProblemSets page. |
2f46926 to
b52572e
Compare
lib/WeBWorK/Utils/Sets.pm
Outdated
| @@ -1,3 +1,4 @@ | |||
| ## Please see file perltidy.ERR | |||
There was a problem hiding this comment.
Is this comment essential to understanding the code in this file? 🤪
There was a problem hiding this comment.
Unsure how that got in there. What I get for deviating from my usual approach to manually running perltidy.
lib/WeBWorK/Utils/Sets.pm
Outdated
| && !$set->lis_source_did) | ||
| { | ||
| return $c->maketext( | ||
| 'You must access this set from your Learning Management System ([_1]) before starting.', |
There was a problem hiding this comment.
I don't like this wording. Something doesn't seem right with the "before starting" part to me.
There was a problem hiding this comment.
Any suggestions, I felt just saying they must access from the LMS seemed incomplete since they are in the set. They only need to access from the LMS before they work on the homework problems or before they start a test version, and if they are seeing this before the open date, nothing will appear to change (except the message go away) if they access from the LMS. Maybe: "before working on the assignment", though for a test, "before starting a test version", then the message would depend on assignment type? I think telling them about the grade link might be too technical, "to link your grade with your LMS".
I'll try to think of something, I just don't have any ideas at the moment.
There was a problem hiding this comment.
Yeah, I am not sure either. This is why we all try to avoid having our students see this message.
I use the navigation_allowed permission so that my students never even see the "Assignments" page.
I think @Alex-Jordan uses LTI 1.3 and accesses all of the sets himself at the beginning of each term. D2L sends the pass back data for instructors, so a student just has to enter one set and then all sets have what are needed for grade pass back.
There was a problem hiding this comment.
@drgrice1 @Alex-Jordan What about this? It does make the default 'the LMS' not as good, but I think makes any configuration useful:
"You must access this assignment from [_1] once before you can start it."
There was a problem hiding this comment.
So using 'the LMS' seems odd there, but 'Canvas', 'Blackboard', 'D2L Brightspace' would all seem reasonable there.
There was a problem hiding this comment.
That seems okay to me.
Perhaps you could use
"You must access this assignment from the external authentication system ([_1]) once before you can start it."
like in the other places where the $LTI{v1p1}{LMS_name} is used. Although that seems to be a little long for in the list on the "Assignments" page.
There was a problem hiding this comment.
I went with, "You must access this assignment from [_1] before you can start.", a little shorter than my original suggestion and gets the same point across in my view.
There was a problem hiding this comment.
I can change it if anyone has a better suggestion, though as @drgrice1 mentioned, best to try to find ways to keep students from seeing this message, so probably only going to be an issue for us still using lti 1.1, as lti 1.3, only takes a single student or instructor to access, or you could even disable it altogether.
There was a problem hiding this comment.
The navigation_allowed permission works with LTI 1.1. As I mentioned, that is what I use.
Also, LTI 1.3 may not fix anything for Canvas in regard to this. The issue is that Canvas does not send LTI grade pass back data for an instructor. I believe that is true even for LTI 1.3. So at least one student has to enter each set from the LMS. Perhaps the Canvas test student would work though. Not sure.
There was a problem hiding this comment.
I would assume the test student would work. in LTI 1.1 it sends enough info that grade pass back works just fine for the test student, and I would be surprised if that changed in 1.3.
drgrice1
left a comment
There was a problem hiding this comment.
Other the the perltidy comment and the wording of the LTI access message, I think this pull request is looking good.
b52572e to
927bb4b
Compare
Rework the ProblemSet page so users can always see the set header and additional set information for any set assigned to them. Instead of giving an error message when a user tries to access a set that is not open, lti restricted, conditional release, or ip restricted, checkSet will set a flag which is used to determine what information a user can see. If a set not visible and a user cannot see hidden sets, only a warning message is shown. In all other cases the set header and any date information, such as when the set opens, closes, etc, is shown. If the set is restricted due to a conditional release, ip restrictions, or lti restrictions, a warning message is shown informing the user of the restriction. When a set is restricted, never show the set problems and always show any previous taken set versions. This way users can still access set versions they have taken for a restricted set from a non restricted location. Note, this is consistent with the permission `view_unoppend_sets` description, which states it only configures if a set problems can be seen. So this permission is no longer used to see if a user can see set information such as open date and set header. This also makes it so the right info panel div on the ProblemSet page is only shown if the set header exists, and is not empty. Translations were added to messages about IP restrictions.
927bb4b to
6a218bb
Compare
Rework the ProblemSet page so users can always see the set header and additional set information for any set assigned to them. Instead of giving an error message when a user tries to access a set that is not open, lti restricted, conditional release, or ip restricted, checkSet will set a flag which is used to determine what information a user can see.
If a set not visible and a user cannot see hidden sets, only a warning message is shown. In all other cases the set header and any date information, such as when the set opens, closes, etc, is shown. If the set is restricted due to a conditional release, ip restrictions, or lti restrictions, a warning message is shown informing the user of the restriction.
When a set is restricted, never show the set problems and always show any previous taken set versions. This way users can still access set versions they have taken for a restricted set from a non restricted location.
Note, this is consistent with the permission
view_unoppend_setsdescription, which states it only configures if a set problems can be seen. So this permission is no longer used to see if a user can see set information such as open date and set header.This also makes it so the right info panel div on the ProblemSet page is only shown if the set header exists, and is not empty.
Translations were added to messages about IP restrictions.