search

openwebwork / webwork2

Course management front end for WeBWorK
http://webwork.maa.org/wiki/Main_Page
Other
141 stars 164 forks source link

Courses with names that end with "modelCourse" not appearing on develop. #2384

Open Alex-Jordan opened 3 months ago

Alex-Jordan commented 3 months ago

Recently I moved our production server from 2.18 to develop. Prior to the move, we had model courses for classes with 0- at the start of the course name. For example 0-mth060-modelCourse. This was just a hack to make them appear at the top of the list.

Since the upgrade, these courses do not appear in the admin course. They don't appear in Course Listings, and they don't appear in the dropdown list of courses to copy from. But their folders are still there in the courses/ folder, and their tables are still there in the database. So something is preventing the admin course from "seeing" them, and I guess it has to do with the "0-" at the start of the course name.

drgrice1 commented 3 months ago

I just tested with the develop branch. I created the course 0-course, and it shows up in all of the places that you list. I also tested with the lti-content-item-request branch for #2322, and this issue does not happen for me with that branch either.

Then I tested with the course 0. I can't create a course with that ID. Trying to do so gives "You must specify a course ID." This is probably identical to the old set "0" bug. That is, a if ($courseID) instead of if (defined $courseID && $courseID ne ''). So if we want to allow the 0 course, then that will need to be fixed.

Alex-Jordan commented 3 months ago

I changed the title to describe what is actually going on. Line 117 of lib/WeBWorK/Utils/CourseManagement.pm is excluding courses that end with modelCourse from showing up in a course list. Is there any reason not to make that check be an exact match on 'modelCourse'?

Alex-Jordan commented 3 months ago

Now that I can list those courses after changing that one line, I renamed them to end in model-course instead of modelCourse. I will revert the edited line in case there is a reason to leave it the way it is.

drgrice1 commented 3 months ago

Yeah, that is a mistake. That should be $_->basename ne 'modelCourse'. Note that $_ will be the full path, and not just the directory name. So for the model course it will be /opt/webwork/courses/modelCourse.

somiaj commented 3 months ago

Are we going to want the model course to always have the exact name of modelCourse? What if an institution wants multiple model courses they can copy from when creating new courses but not want them to appear in course lists? Would it be over kill to add a configuration option (in localOverrides.conf or another appropriate place) that can list what the names of the model (or other courses) to skip are? Maybe even allow it to support regex/globbing? With the default being what is done here, skip only the course named modelCourse.

drgrice1 commented 3 months ago

There is no need for such a thing. Those courses can be hidden if you do not want them to appear on the WeBWorK home page, and they absolutely should appear in other lists in the admin course. So I say no to such an option.

Alex-Jordan commented 3 months ago

I see, so that's why it was using modelCourse$.

Perhaps we should evolve the model course to become an actual course? There could be a course archive file that comes with the distribution, and it could even have the student orientation pre-loaded.

drgrice1 commented 3 months ago

Perhaps we should evolve the model course to become an actual course? There could be a course archive file that comes with the distribution, and it could even have the student orientation pre-loaded.

I don't think that having the model course be an actual course is such a good idea. I don't like the idea of including a course archive in the GitHub repository for one thing. Binary files are always a mess in a git repository. Every time that we change the model course archive, the entire binary file has to be added to the repository, and binary files don't diff so nicely as text files. That results in the repository size growing much faster than for changes to text files.

I think that a better idea is to discontinue putting the model course in the courses directory, and instead use it directly from the webwork2 directory. Now that any course can be copied, there really is no reason for the model course to be customizable. A webwork server administrator can create their own actual course (or several of them even) to use for their model course.

For someone that is starting from scratch with webwork, the model course is there for them to get started with. Then in future semesters they can start using previous courses as models.

There really is no need for the model course to be an actual course. Even if it were a model course the student orientation would not be "pre-loaded" unless the administrator chooses to copy assignments/sets. I don't like the idea of forcing that set to be pre-loaded either. Then if someone doesn't want to use it, they have to delete it. I think there are many like me that find much of the student orientation set not particularly useful or relevant for the way that their courses work. The way it is now is fine. If an instructor wants to use the set it is there for them to import.

@somiaj: Sorry for being a bit abrupt with my reply to your idea of a configuration option with a list of courses to not show in the admin course. I just think that having actual courses (with a directory and all database tables) hidden is not a good idea. It makes it difficult to manage those courses, and those courses will still need to be upgraded when webwork is upgraded in addition to updates to course content and such.

In addition we have so many things in localOverrides.conf that have been added over the years that are similar to this, and I think in most cases these things are only used by very few. I think that if the admin course had a user interface for editing site settings like this, rather than a system administrator needing to edit a configuration file, then these things would get used more.