Skip to content

fix: set completion_status to completed when lesson_status is passed#104

Merged
DawoudSheraz merged 4 commits into
overhangio:masterfrom
so-jd:fix-scrom1_2-completion
Aug 25, 2025
Merged

fix: set completion_status to completed when lesson_status is passed#104
DawoudSheraz merged 4 commits into
overhangio:masterfrom
so-jd:fix-scrom1_2-completion

Conversation

@so-jd
Copy link
Copy Markdown
Contributor

@so-jd so-jd commented Aug 7, 2025

I found a logic bug, relating to this issue #90 (was experiencing it as well).

In SCORM 2004 you have:
"cmi.completion_status" = "completed" / "incomplete" / "not attempted" / "unknown" - this event Indicates whether the learner has completed the SCO
"cmi.success_status" = "passed" / "failed" - Indicates whether the learner has mastered the SCO

but in SCORM 1.2:
"cmi.core.lesson_status" = “passed”/ “completed”/ “failed”/ “incomplete”/ “browsed”/ “not attempted” - Indicates whether the learner has completed AND satisfied the requirements for the SCO

In the code when evaluating "cmi.core.lesson_status" == passedwe need to also set the complete status otherwise SCROM 1.2 courses will not end successfully

ref: https://scorm.com/scorm-explained/technical-scorm/run-time/run-time-reference/#section-3

@Danyal-Faheem
Copy link
Copy Markdown
Contributor

Hi @so-jd, thank you for your contribution!

This change LGTM but can I also request you to add a changelog entry to your PR?

You can do this by installing scriv in a virtual environment using pip install scriv and then running scriv create in the root directory of this repository.

@so-jd
Copy link
Copy Markdown
Contributor Author

so-jd commented Aug 8, 2025

No problem at all thank you for reviewing it quickly, I will push the change

@@ -0,0 +1,12 @@

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also edit this file as per the instructions in this file?

Sorry, this one is probably on me as I provided incomplete instructions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah that one is on me, i should have noticed, anyway pushed an update

Comment thread changelog.d/20250808_091751_sorlanczyk_fix_scrom1_2_completion.md Outdated
@so-jd
Copy link
Copy Markdown
Contributor Author

so-jd commented Aug 8, 2025

@Danyal-Faheem I updated to change log to your request, also want to bring to your attention that I made a mistake in my original branch and I fixed it, failed state is not considered complete, but passed should so I added another if statement for it

@DawoudSheraz DawoudSheraz moved this from Pending Triage to In review in Tutor project management Aug 11, 2025
@so-jd
Copy link
Copy Markdown
Contributor Author

so-jd commented Aug 11, 2025

Hi @Danyal-Faheem thanks for approving, is there another step needed for merging? I don't have permission to merge.

@Danyal-Faheem
Copy link
Copy Markdown
Contributor

Hi @so-jd, can I also request you to squash the commits and update the commit message and PR title to follow conventional commits standards as well as make it consistent with the changelog entry?

@so-jd so-jd changed the title fix scorm 1-2 fix: fix scorm 1.2 completion status Aug 12, 2025
@so-jd so-jd changed the title fix: fix scorm 1.2 completion status fix: scorm 1.2 completion status Aug 12, 2025
@so-jd so-jd changed the title fix: scorm 1.2 completion status fix: Set completion_status to completed when lesson_status is passed Aug 12, 2025
@so-jd so-jd changed the title fix: Set completion_status to completed when lesson_status is passed fix: set completion_status to completed when lesson_status is passed Aug 12, 2025
@so-jd
Copy link
Copy Markdown
Contributor Author

so-jd commented Aug 12, 2025

Hi @Danyal-Faheem I changed the PR name to fix: set completion_status to completed when lesson_status is passed, but what do you mean by squash the commits? in my branch? cause you can set it to squash when you merge the PR.

@DawoudSheraz DawoudSheraz merged commit ccef54c into overhangio:master Aug 25, 2025
@github-project-automation github-project-automation Bot moved this from In review to Done in Tutor project management Aug 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

4 participants