Merge Numbas Integration feature into 8.0.x #443
Conversation
first commit containing endpoints and DB changes new faeture
first commit containing endpoints and DB changes new faeture
- Implement methods in task_definition model for numbas data management - Implement routes in task_definition_api for numbas data managemnt - Remove unused upload API in numbas_api
Rubocop offenses are fixed as well
app/api/authentication_api.rb
Outdated
| error!({ error: 'You cannot get SCORM tokens' }, 403) | ||
| end | ||
|
|
||
| token = current_user.auth_tokens.find_by(token_type: 'scorm') |
There was a problem hiding this comment.
What can a scorm token access? It looks like there is not scope applied to these at the moment... where are they used in the auth process?
| params do | ||
| requires :task_def_id, type: Integer, desc: 'Task Definition ID to get SCORM test data for' | ||
| end | ||
| get '/scorm/:task_def_id/:username/:auth_token/*file_path' do |
There was a problem hiding this comment.
Should any of the api calls here be limited to scorm tokens?
| @@ -0,0 +1,5 @@ | |||
| class AddAuthTokenType < ActiveRecord::Migration[7.1] | |||
| def change | |||
| add_column :auth_tokens, :token_type, :string, null: false | |||
There was a problem hiding this comment.
We need a default here - null is not allowed... so all current tokens should have what value?
| "cmi.learner_name": learner_name, | ||
| "cmi.learner_id": learner_id | ||
| } | ||
| self.cmi_datamodel = init_state.to_json |
There was a problem hiding this comment.
We can change this to use the serialize :cmi_datamodel, coder: JSON
| end | ||
|
|
||
| def review | ||
| dm = JSON.parse(self.cmi_datamodel) |
There was a problem hiding this comment.
serializer can do this JSON.parse
|
|
||
| # when review is requested change the mode to review | ||
| dm['cmi.mode'] = 'review' | ||
| self[:cmi_datamodel] = dm.to_json |
| end | ||
|
|
||
| def override_success_status(new_success_status) | ||
| dm = JSON.parse(self.cmi_datamodel) |
There was a problem hiding this comment.
serializer can do this JSON.parse
| def override_success_status(new_success_status) | ||
| dm = JSON.parse(self.cmi_datamodel) | ||
| dm['cmi.success_status'] = (new_success_status ? 'passed' : 'failed') | ||
| self[:cmi_datamodel] = dm.to_json |
|
|
||
| # Add a scorm extension to the task | ||
| def grant_scorm_extension(by_user) | ||
| if update(scorm_extensions: self.scorm_extensions + task_definition.scorm_attempt_limit) |
There was a problem hiding this comment.
Can just return response from update
|
|
||
| if task_definition.scorm_attempt_limit == 0 | ||
| error!({ message: 'This task allows unlimited attempts to complete the test' }, 400) | ||
| return |
There was a problem hiding this comment.
error! throws an exception - so this is unreachable... we should look and remove these unnecessary return statements
app/api/test_attempts_api.rb
Outdated
| end | ||
|
|
||
| # Handle common exceptions | ||
| rescue_from :all do |e| |
There was a problem hiding this comment.
These are handled by the API at the moment... so we should be able to remove these
Issues occurred when new to done fails, but the zip is already gone. Add checks to avoid double processing, and delay delete of zip until the files are checked
… satikaj-8.0.x
Description
This pull request contains the Numbas (SCORM) API and related backend changes.
Summary of key features:
Type of change
How Has This Been Tested?
Corresponding unit tests have been added.
In order to test the full feature set (e.g. answering and saving tests), the frontend must be used with the related
doubtfire-web/add-numbas-integrationchanges. Make sure to disable browser web security or CORS for the test viewing functionality to work.Login as convenor to configure tests and manage test attempts. Login as student to launch and view tests and make test attempts.
Checklist:
This is ready for review, @macite @maddernd