Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve GetValue API #69

Merged
merged 3 commits into from
Apr 29, 2024

Conversation

ahmed-arb
Copy link
Contributor

After this change, scorm_get_values was exclusively used for getting uncached_values.

This commit:

  • removes student information and other scorm data from get value func and sends it as part of scorm_data in student view,
  • adds cmi.score.scaled to uncached_values,
  • and removes old test cases.

@ahmed-arb ahmed-arb requested a review from ziafazal March 21, 2024 08:29
Copy link
Collaborator

@ziafazal ziafazal left a comment

Choose a reason for hiding this comment

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

@ahmed-arb could you please add few tests to support you changes?

openedxscorm/tests.py Show resolved Hide resolved
openedxscorm/scormxblock.py Outdated Show resolved Hide resolved
@ahmed-arb ahmed-arb force-pushed the ahmed-arb/update-scorm-get-value branch from 45db411 to fa3454b Compare April 2, 2024 10:29
@ahmed-arb ahmed-arb requested a review from ziafazal April 3, 2024 05:41
Copy link
Collaborator

@ziafazal ziafazal left a comment

Choose a reason for hiding this comment

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

👍

@ziafazal ziafazal merged commit 1c2f6da into overhangio:master Apr 29, 2024
ziafazal pushed a commit that referenced this pull request May 27, 2024
* fix: inaccissible info in scorm_get_value

* chore: code refacorting and adds a test for user info

* chore: update test instructions in readme
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.

2 participants