Skip to content

Make renaming a course or unarchiving a course to a new name work again.#1994

Merged
drdrew42 merged 2 commits into
openwebwork:WeBWorK-2.18from
drgrice1:bugfix/course-rename
May 25, 2023
Merged

Make renaming a course or unarchiving a course to a new name work again.#1994
drdrew42 merged 2 commits into
openwebwork:WeBWorK-2.18from
drgrice1:bugfix/course-rename

Conversation

@drgrice1

Copy link
Copy Markdown
Member

This was using the old four argument form of the CourseEnvironment constructor. I missed that in #1985.

To test this rename a course or unarchive a course to a new name. On the current release candidate branch you will get an error. With this pull request it will work.

Note that if you test this on the release candidate branch and rename a course, the course directory will be renamed. So you will need to manually rename that back to what it was. The database tables will still be under the old course name. If you attempt to unarchive to a new name, then the course directory will be created with the new name, but the database tables will not be. So you will need to manually delete that new course directory.

This was using the old four argument form of the CourseEnvironment
constructor.  I missed that in openwebwork#1985.

To test this rename a course or unarchive a course to a new name.  On
the current release candidate branch you will get an error.  With this
pull request it will work.

Note that if you test this on the release candidate branch and rename a
course, the course directory will be renamed.  So you will need to
manually rename that back to what it was.  The database tables will
still be under the old course name.  If you attempt to unarchive to a
new name, then the course directory will be created with the new name,
but the database tables will not be.  So you will need to manually
delete that new course directory.
@pstaabp

pstaabp commented May 23, 2023

Copy link
Copy Markdown
Member

This works. However, I tested with a space in the new course name which had no errors. However that space is in the database tables and the directory name. Then there were errors.

This might be a good opportunity to replace spaces with _.

@drgrice1

Copy link
Copy Markdown
Member Author

I think that this pull request is not the place for that type of change. This is a straight forward bugfix pull request. It fixes something that is broken, and is a critical issue for the release. Making spaces work in course names is a new feature.

@pstaabp

pstaabp commented May 23, 2023

Copy link
Copy Markdown
Member

We'll leave this as is then. I would say that fixing the space problem would be a bug fix since not checking for them results in a non-functional course. I'll add an issue to flag it.

@drgrice1

Copy link
Copy Markdown
Member Author

I am not sure that you can classify behavior that has been the norm for 20 years (since the inception of webwork2) as a bug fix. I am not opposed to making the change though in another pull request.

@drgrice1

Copy link
Copy Markdown
Member Author

So I added a check for course name validity when unarchiving a course to a new name now.

@pstaabp

pstaabp commented May 25, 2023

Copy link
Copy Markdown
Member

Thanks for adding the check @drgrice1. Looks good.

@drdrew42 drdrew42 merged commit 3a710a9 into openwebwork:WeBWorK-2.18 May 25, 2023
@drgrice1 drgrice1 deleted the bugfix/course-rename branch May 25, 2023 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants