Skip to content

Move system achievements out of modelCourse. - #3215

Open
somiaj wants to merge 1 commit into
openwebwork:developfrom
somiaj:move-achievements-out-of-course
Open

somiaj wants to merge 1 commit into
openwebwork:developfrom
somiaj:move-achievements-out-of-course

Conversation

@somiaj

@somiaj somiaj commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

System achievements should not be placed in a course. This way any changes to them will propagate to all courses using them. Instead courses should be able to create local overrides of system achievements as needed.

This moves all of the default achievement evaluators and icons out of the modelCourse and places them in a system location. Then logic is added to determine if an achievement asset, either an icon or an evaluator, has a course override, or the system default should be used.

In order to better validate path names, icons, evaluators, and notification templates are no longer allowed to have a slash in their name and be an absolute or relative path. In addition all paths are checked they are actual files and not directories. All course achievement assets must be placed in their appropriate directory without any subdirectories.

The achievement list editor will add a message if the name is invalid (contains a slash) or is overriding a system asset, and now has javascript validation to not allow saving new data unless the evaluator file and icon file do not contain a slash. The achievement evaluator editor has been updated to inform the user if they are editing a system achievement and will create a course override, and provide a tab to revert to the system achievement.

In addition JavaScript validation has been added to the achievement evaluator editor and achievement notification editor.

Some icons were updated in this change to better work with dark mode. This includes adding a default achievement which is white circle with a black outline and a "?" in it to match other badges, a white background inside two icons in which this was not done, and there was a small white boundary around some of the level achievements that was removed.

This patch was designed and mostly written by me, but Claude helped test it and provided some feedback, along with Claude mostly wrote the achievement notifications editor code based on what I already did to the achievement editor and help write the javascript validation for the slash in filenames on the AchivementList editor. One thing Claude pointed out was I didn't consider if an asset name was "." or "..", which would not lead to accessing the directory so wasn't technically needed. This lead me to change the tests from '-e' to '-f' to ensure a file exists that isn't a directory.

This is a breaking change, and will require a bit of manual work on instructors to delete any overrides they don't want. I am unsure if an upgrade course path should be added. I suspect deleting overridden icons is safe, but it won't be safe for achievement evaluators, specifically level achievement evaluators I suspect are overridden a lot. The AchievementList when editing all achievements can inform a user what is being overridden, but they need to manually delete icons. The editor can be used to revert evaluators.

I don't know how bad the restricting file names to not contain a slash is going to be. My guess is it would be rare to manage achievements in their own sub-directories. Maybe @Alex-Jordan or @drdrew42 can comment on this, as they have access to lots of courses. That can be changed, but I don't think it is worth allowing sub-directories (but my use case maybe limited).

@somiaj
somiaj force-pushed the move-achievements-out-of-course branch 2 times, most recently from 813e73c to 31f20e5 Compare September 22, 2026 02:04
System achievements should not be placed in a course. This way any
changes to them will propagate to all courses using them. Instead
courses should be able to create local overrides of system achievements
as needed.

This moves all of the default achievement evaluators and icons out
of the modelCourse and places them in a system location. Then logic
is added to determine if an achievement asset, either an icon or
an evaluator, has a course override, or the system default should
be used.

In order to better validate path names, icons, evaluators, and
notification templates are no longer allowed to have a slash in their
name and be an absolute or relative path. In addition all paths are
checked they are actual files and not directories. All course achievement
assets must be placed in their appropriate directory without any
subdirectories.

The achievement list editor will add a message if the name is
invalid (contains a slash) or is overriding a system asset, and
now has javascript validation to not allow saving new data unless
the evaluator file and icon file do not contain a slash. The
achievement evaluator editor has been updated to inform the user
if they are editing a system achievement and will create a course
override, and provide a tab to revert to the system achievement.

In addition JavaScript validation has been added to the achievement
evaluator editor and achievement notification editor.

Some icons were updated in this change to better work with dark mode.
This includes adding a default achievement which is white circle
with a black outline and a "?" in it to match other badges, a white
background inside two icons in which this was not done, and there was
a small white boundary around some of the level achievements that was
removed.

This patch was designed and mostly written by me, but Claude helped
test it and provided some feedback, along with Claude mostly wrote
the achievement notifications editor code based on what I already
did to the achievement editor and help write the javascript validation
for the slash in filenames on the AchivementList editor. One thing
Claude pointed out was I didn't consider if an asset name was "." or
"..", which would not lead to accessing the directory so wasn't
technically needed. This lead me to change the tests from '-e' to
'-f' to ensure a file exists that isn't a directory.
@somiaj
somiaj force-pushed the move-achievements-out-of-course branch from 31f20e5 to 11c2115 Compare September 22, 2026 14:35
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.

1 participant