Prevent a NULL title from being saved in the database when a course is created/copied. #2516
Conversation
drgrice1
force-pushed
the
bugfix/null-course-title
branch
from
7617190
to
6959880
Compare
drgrice1
force-pushed
the
bugfix/null-course-title
branch
from
6959880
to
5447ca6
Compare
drgrice1
force-pushed
the
bugfix/null-course-title
branch
from
5447ca6
to
7a89351
Compare
…s created/copied. Also don't use an undefined course title on the course home page. Note that a NULL setting in the database becomes undef when converted to a Perl scalar value.
drgrice1
force-pushed
the
bugfix/null-course-title
branch
from
7a89351
to
138ba78
Compare
|
By the way, with the approach I used a course title or institution of |
|
I think it should be allowed. Just to be consistent since we let other things be "0" (like set ids). |
|
Okay, so a course title or institution of |
drgrice1
force-pushed
the
bugfix/null-course-title
branch
from
6455278
to
49b45ed
Compare
| my $add_courseID = trim_spaces($c->param('new_courseID')) // ''; | ||
| my $add_courseTitle = trim_spaces($c->param('add_courseTitle')) // ''; | ||
| my $add_courseInstitution = trim_spaces($c->param('add_courseInstitution')) // ''; | ||
| my $add_courseID = trim_spaces($c->param('new_courseID')) // ''; |
There was a problem hiding this comment.
This brings to my attention that we also cannot have a course named "0". Is that what we want? I don't think I would ever name a course "0", but I could see someone doing that as some sort of orientation or training course. Maybe.
If you try to create a course named "0" right now using the admin course, you are met with "You must specify a course ID.". And for anyone who is not a perl cognoscente, you would think "But I did!".
I feel like trim_spaces is part of what is broken here and it should not turn 0 into ``. That is not consistent with what the name of the macro suggests it does.
There was a problem hiding this comment.
I agree that trim_spaces is not doing the right thing. I thought about changing that, but if we do so we need to review all of its uses. There aren't so many of them, so that isn't that hard. Most of the uses are in this file, there is one usage in lib/WeBWorK/Utils.pm (for the password), and the rest are in lib/WeBWorK/ContentGenerator/Instructor/AddUsers.pm.
As to allowing a course id of "0", that may be problematic. I considered changing it when I did this, but then thought that there may be problems with that. The course title is only used in a few places, so it doesn't take much to make sure that is okay. The course institution isn't actually used at all, and is purely something saved in the database (not sure why we have it). The course id, on the other hand, is used everywhere. So it would take some time to verify that everything will work okay with it being "0".
There was a problem hiding this comment.
I recommend that any fix for trim_spaces or allowing the course id to be "0" not be done in this pull request since this is paired with a hot fix pull request. Keep the two the same. Those things can be done in another pull request.
Prevent a NULL title from being saved in the database when a course is created/copied (hotfix of #2516)
Also don't use an undefined course title on the course home page. Note that a NULL setting in the database becomes undef when converted to a Perl scalar value.
This fixes issue #2515 and probably #2497 as well.