Added enrollment question check to CourseModulePermissionBase #1080
Conversation
EerikSaksi
force-pushed
the
enrollment-questionaire-module-opening-time
branch
from
ea2a751 to
9943e44
Compare
|
Changed the formatting and added LearningObject import @PasiSa |
There was a problem hiding this comment.
Good start! When I tested this, I noticed that it really does not work yet because there is also another permission (ExerciseVisiblePermissionBase) that checks the module opening time. Fixing that other permission is easy. I also thought that some details could be formatted better. CourseModulePermissionBase.has_object_permission() has the part module.requirements.exists() in the end that should not be used in enrollment questionnaires.
I made some changes myself and pushed them to git so that you may finalize this PR.
git commit message
- commit message heading: use the imperative mood
- line length: at most 80 characters
- the heading could be reworded better. Emphasize the goal ("enrollment questionnaires should not be affected by the module opening time") instead of the implementation details on the code lines ("check in CourseModulePermissionBase").
|
Thanks @Mankro for the feedback, I will get to it! |
EerikSaksi
force-pushed
the
enrollment-questionaire-module-opening-time
branch
from
8178f40 to
da30fda
Compare
|
I tested the new changes, it seems like the enrolment questionnaire is accessible before the course is open, but the course is not. I am not sure if this is the desired behaviour. 2022-09-23.16-49-36.mp4 |
I haven’t studied this issue / PR properly at all, but I’ll just mention that AFAIC, it shouldn’t be possible to fill in or even see the enrollment questionnaire before the course is actually open. |
If I remember correctly, the enrollment questionnaire is supposed to be open if the course settings (enrollment starting time and enrollment ending time) define so. If the teacher wants to set enrollment starting time before the course opening time, then it is possible. If the enrollment opening times are not defined, they default to the course opening times. If the enrollments are fetched from Sisu, we should check what happens. Is access restricted by the course opening times? |
|
I added a test for accessing the enrollment exercise which tests combinations of starting times and enrollment starting times which passes. I might have misunderstood what the expected behavior is but if the test describes the expected behavior then this should work. |
Enrollment questionnaires should not be affected by module opening times because there are separate course settings for the enrollment start and end times. Fixes apluslms#679 Co-authored-by: Markku Riekkinen <markku.riekkinen@aalto.fi>
markkuriekkinen
force-pushed
the
enrollment-questionaire-module-opening-time
branch
from
0067ae0 to
4e9f956
Compare
Before, the permission check would work by simply checking whether the module had opened yet. With this check, the module opening time is only checked if the exercise is not an enrollment question
Fixes #679
Description
What?
This change adds a check for if the exercise is an enrolment question to CourseModulePermissionBase.
Why?
Enrolment questions should be accessible before the module is open
How?
Before the check for if the module is open, we check whether the exercise is an enrolment question. If this is the case, then we do not care if the module is open.
Fixes #679
Testing
Remember to add or update unit tests for new features and changes.
What type of test did you run?
Ensuring that students can open the URL to enrolment exercises in modules which have not opened yet, making sure that no error is thrown when the view supplied to CourseModulePermissionBase does not have a exercise attribute (when called from ModuleView and not ExerciseView)
Did you test the changes in
Think of what is affected by these changes and could become broken
Translation
Programming style
Have you updated the README or other relevant documentation?
Is it Done?
Clean up your git commit history before submitting the pull request!