WikiEduDashboard
WikiEduDashboard copied to clipboard
WikiEducationFoundation
add is_monday_start option to course creation and cloning.
NOTE: Please review the pull request process before opening your first PR: https://github.com/WikiEducationFoundation/WikiEduDashboard/blob/master/CONTRIBUTING.md#pull-request-process
#5689
What this PR does
This PR adds the is_monday_start flag to the course model, allowing the system to track whether a course follows a Monday–Sunday or a Sunday–Saturday schedule.
Screenshots
Before: https://github.com/user-attachments/assets/eb5b308f-9f44-424d-8c24-6d05ed832123 After:
course creation https://github.com/user-attachments/assets/c972ef0b-ba9b-4ab9-beb6-b8afea4e3a86
course cloning
https://github.com/user-attachments/assets/d966ed36-bb5a-4a1f-bc05-4882749bcb97
Hey @ragesoss, just following up on this PR. Let me know if you need any changes. Thanks!
Thanks! It's still on my todo list, but I'm further behind than I'd like to be on some other projects. Hoping to get to your open PRs next week.
The code here looks reasonable. I definitely want tests to be included as well. I was expecting this to require some backend changes related to how dates are handled, like due dates for training modules in BlockDateManager. Did you check the behavior of that to make sure it doesn't require any changes?
When you mention tests, are you referring to the frontend select button tests? I haven’t looked into BlockDateManager yet, but I will check it out and work on migrating the handling of weekday dates to the backend."
Yes, I think at minimum this should include a feature spec that uses the feature from an instructor perspective.
I tried writing test cases for this, but I'm not sure why they keep failing with the error "Course already exists." Even after changing the course name, term, and other details, the issue persists. No matter what I try, it doesn't seem to work. Could you help me with this?
@shishiro26 can you post the test code that fails with the screenshot you posted?
I initially added some of my code, but it failed. Later, I removed it and tested again, but it still failed.
@shishiro26 is this happening for the test case you wrote, or is that happening on another unrelated test? Without seeing what code was running for the test, it's hard to guess what might be wrong.
I haven't written any new test code. I just ran the existing code, and it started failing in my local environment right away.
This is the latest run I performed with the existing code in course_creation.spec.rb.
Can you post the stack traces from the failures? That will show exactly which line of the test it failed at.
Ah, so it looks like there are javascript errors causing the problem. Try running yarn build to use the production version of the assets and see if the errors change.
The large number of places that switch behavior on the is_monday_start value sets off alarms for me. Can this be implemented in a way that has a narrowing footprint in terms of where there are branching code paths?
I think it's also worth considering how easy it will be to extend this to support arbitrary week boundaries. I only want to add Monday support at this point, but implementation-wise, it might be worth doing in a way that is more general and easy to extend.
Per my comment above, I also still assume that fully implementing this feature will require backend changes related due dates (but I have not studied it in detail, so I might be mistaken about that). If you don't think any backend changes for that are needed, please walk me through the logic and how you tested for problems.