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

New: network connection test #270

Merged
merged 8 commits into from
Apr 24, 2023
Merged

New: network connection test #270

merged 8 commits into from
Apr 24, 2023

Conversation

danielghost
Copy link
Contributor

Fixes #268

New

  • Added feature to test the network connection when transferring data to the LMS API.
  • Raises an error if the connection is lost.

Further error handling can be utlised with adapt-contrib-trackingErrors.

@danielghost danielghost self-assigned this Mar 15, 2023
@danielghost danielghost requested a review from joe-replin March 15, 2023 18:14
@joe-replin
Copy link

joe-replin commented Mar 23, 2023

This is great. The connection test is triggering appropriately in my LMS testing. While I have previously seen the data error in a live setting triggering appropriately, I'm not entirely sure how to appropriately test an initialization, data or termination error without triggering the events in the console.

Lastly, I am getting a slight warning in logger.js to change the four instances of 'str: str' to a simple 'str' shorthand.

this.logArr[this.logArr.length] = { str: str, type: Logger.LOG_TYPE_INFO, time: Date.now() };

@joe-replin
Copy link

@danielghost, I also tested this by disabling adapt-contrib-trackingErrors. I haven't been given any indication that I do have a connection error even after attempting to adjust my level of logging. Am I to expect a console or alert message without it?

I understand that some LMSs (e.g. SCORM Cloud) do a good job of notifying users of a network issue via their own popups, so we aren't going to use this feature everywhere. But I suppose a console log would be sufficient for when adapt-contrib-trackingErrors is not attached to the framework.

This schema could probably be deleted altogether as we move passed legacy AAT.
Unable to test at this time in the AAT.
@joe-replin
Copy link

I've committed schemas for the connection test feature. I'm not sure if we'll need the legacy schema anymore and this plug-in is currently not compatible with our legacy testing site. I'm also unable to test on a v1 testing site at the moment, but will test these as son as I have an instance available to use.

@joe-replin
Copy link

joe-replin commented Mar 23, 2023

@danielghost, one more thing I discovered in testing is that the framework won't build unless "_connectionTest" is part of config. I think you just need to add a simple return check to see if it exists wherever the check is made.

@danielghost
Copy link
Contributor Author

@joe-replin - please see comments below:

@danielghost, I also tested this by disabling adapt-contrib-trackingErrors. I haven't been given any indication that I do have a connection error even after attempting to adjust my level of logging. Am I to expect a console or alert message without it?

Good point. Have included this in 4dd09c5.

Lastly, I am getting a slight warning in logger.js to change the four instances of 'str: str' to a simple 'str' shorthand.

Not seeing this, but it shouldn't be linked to the connection test changes, so you may need to investigate this further and raise as a separate issue if it can be replicated.

@danielghost, one more thing I discovered in testing is that the framework won't build unless "_connectionTest" is part of config. I think you just need to add a simple return check to see if it exists wherever the check is made.

Not seeing this issue myself. If the config is missing it will use the defaults, otherwise it merges them together.

Copy link

@joe-replin joe-replin left a comment

Choose a reason for hiding this comment

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

This looks great. :)

Copy link
Member

@oliverfoster oliverfoster left a comment

Choose a reason for hiding this comment

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

See #273 for suggested amends

js/scorm/Connection.js Outdated Show resolved Hide resolved
js/scorm/wrapper.js Outdated Show resolved Hide resolved
js/adapt-stateful-session.js Outdated Show resolved Hide resolved
js/scorm/wrapper.js Outdated Show resolved Hide resolved
@oliverfoster oliverfoster mentioned this pull request Apr 17, 2023
* Slight refactor

---------

Co-authored-by: Dan Ghost <daniel.ghost@kineo.com>
@oliverfoster oliverfoster merged commit ec5ef8a into master Apr 24, 2023
github-actions bot pushed a commit that referenced this pull request Apr 24, 2023
# [5.6.0](v5.5.9...v5.6.0) (2023-04-24)

### New

* network connection test (#270) ([ec5ef8a](ec5ef8a)), closes [#270](#270)
@github-actions
Copy link

🎉 This PR is included in version 5.6.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

github-actions bot pushed a commit to nagyist/adapt-contrib-spoor that referenced this pull request Mar 19, 2024
# [2.2.0](v2.1.3...v2.2.0) (2024-03-19)

### Bug

* score logic (adaptlearning#267) ([898fa80](898fa80)), closes [adaptlearning#267](https://github.com/nagyist/adapt-contrib-spoor/issues/267)

### chore

* added package.json (adaptlearning#214) ([c940daf](c940daf)), closes [adaptlearning#214](https://github.com/nagyist/adapt-contrib-spoor/issues/214) [#3072](https://github.com/nagyist/adapt-contrib-spoor/issues/3072)

### Chore

* Readme updates for broken links, clarity, and linting errors (fixes adaptlearning#312) (adaptlearning#313) ([df90b8d](df90b8d)), closes [adaptlearning#312](https://github.com/nagyist/adapt-contrib-spoor/issues/312) [adaptlearning#313](https://github.com/nagyist/adapt-contrib-spoor/issues/313)

### fix

* Adding a pagehide listener to cover bases when unload becomes deprecated (adaptlearning#298) ([177de84](177de84)), closes [adaptlearning#298](https://github.com/nagyist/adapt-contrib-spoor/issues/298)

### Fix

* Add SCORM version selector to config JSON schema (fixes adaptlearning#249) ([a824a72](a824a72)), closes [adaptlearning#249](https://github.com/nagyist/adapt-contrib-spoor/issues/249)
* Added gitignore for release automation (adaptlearning#244) ([27decdb](27decdb)), closes [adaptlearning#244](https://github.com/nagyist/adapt-contrib-spoor/issues/244)
* Added release automation (adaptlearning#240) ([d2b21a4](d2b21a4)), closes [adaptlearning#240](https://github.com/nagyist/adapt-contrib-spoor/issues/240)
* Amend to default schema value for `_setCompletedWhenFailed` (fixes adaptlearning#256) (adaptlearning#258) ([cf29cb1](cf29cb1)), closes [adaptlearning#256](https://github.com/nagyist/adapt-contrib-spoor/issues/256) [adaptlearning#258](https://github.com/nagyist/adapt-contrib-spoor/issues/258)
* Amended logic to check for supported `_children` elements before attempting to set values (adaptlearning#306) ([4dddbee](4dddbee)), closes [adaptlearning#306](https://github.com/nagyist/adapt-contrib-spoor/issues/306)
* Bump http-cache-semantics from 4.1.0 to 4.1.1 (adaptlearning#263) ([aa3e7f4](aa3e7f4)), closes [adaptlearning#263](https://github.com/nagyist/adapt-contrib-spoor/issues/263)
* Changed logic for 401 error handling (adaptlearning#303) ([a71d7a0](a71d7a0)), closes [adaptlearning#303](https://github.com/nagyist/adapt-contrib-spoor/issues/303)
* Check LMS supports cmi.interactions._count (fixes adaptlearning#294) (adaptlearning#295) ([d73c7e4](d73c7e4)), closes [adaptlearning#294](https://github.com/nagyist/adapt-contrib-spoor/issues/294) [adaptlearning#295](https://github.com/nagyist/adapt-contrib-spoor/issues/295)
* Close loading screen via router.hideLoading() (fixes adaptlearning#291) (adaptlearning#292) ([0fe5539](0fe5539)), closes [adaptlearning#291](https://github.com/nagyist/adapt-contrib-spoor/issues/291) [adaptlearning#292](https://github.com/nagyist/adapt-contrib-spoor/issues/292)
* Correct printCompletionInformation string creation ([9e07fbf](9e07fbf))
* delay error messaging until all course data is ready (adaptlearning#301) ([62a2f30](62a2f30)), closes [adaptlearning#301](https://github.com/nagyist/adapt-contrib-spoor/issues/301)
* Dynamic course folders (fixes adaptlearning#264) (adaptlearning#265) ([a0b8813](a0b8813)), closes [adaptlearning#264](https://github.com/nagyist/adapt-contrib-spoor/issues/264) [adaptlearning#265](https://github.com/nagyist/adapt-contrib-spoor/issues/265)
* Ensure cmi.interactions.n.id is unique (fixes adaptlearning#234) (adaptlearning#246) ([ad5380a](ad5380a)), closes [adaptlearning#234](https://github.com/nagyist/adapt-contrib-spoor/issues/234) [adaptlearning#246](https://github.com/nagyist/adapt-contrib-spoor/issues/246) [adaptlearning#234](https://github.com/nagyist/adapt-contrib-spoor/issues/234) [adaptlearning#246](https://github.com/nagyist/adapt-contrib-spoor/issues/246)
* error logging order to match call execution (fixes adaptlearning#307). (adaptlearning#308) ([ea190bf](ea190bf)), closes [adaptlearning#307](https://github.com/nagyist/adapt-contrib-spoor/issues/307) [adaptlearning#308](https://github.com/nagyist/adapt-contrib-spoor/issues/308)
* escaped double quotes in _setCompletedWhenFailed help msg (fixes adaptlearning#254) ([333b58c](333b58c)), closes [adaptlearning#254](https://github.com/nagyist/adapt-contrib-spoor/issues/254)
* LZMAFactory error when Web Workers are not implemented - fixes adaptlearning#253. (adaptlearning#255) ([b790967](b790967)), closes [adaptlearning#253](https://github.com/nagyist/adapt-contrib-spoor/issues/253) [adaptlearning#255](https://github.com/nagyist/adapt-contrib-spoor/issues/255)
* Release job now uses v3 for checkout and setup-node actions ([0e3743d](0e3743d))
* Reworked and removed offline_API_wrapper.js (fixes adaptlearning#260) (adaptlearning#261) ([75f15a6](75f15a6)), closes [adaptlearning#260](https://github.com/nagyist/adapt-contrib-spoor/issues/260) [adaptlearning#261](https://github.com/nagyist/adapt-contrib-spoor/issues/261)
* Score logic ([c0d5a62](c0d5a62))
* set html lang and dir attributes from config.json (fixes #499) (adaptlearning#310) ([2642a5a](2642a5a)), closes [#499](https://github.com/nagyist/adapt-contrib-spoor/issues/499) [adaptlearning#310](https://github.com/nagyist/adapt-contrib-spoor/issues/310)
* Undefined settings error (adaptlearning#275) ([791c436](791c436)), closes [adaptlearning#275](https://github.com/nagyist/adapt-contrib-spoor/issues/275)
* version bump for pull 125 ([608e79f](608e79f))
* Version numbers removed from Readme files ([9f43ba6](9f43ba6))
* Version patch bump (adaptlearning#290) ([f801969](f801969)), closes [adaptlearning#290](https://github.com/nagyist/adapt-contrib-spoor/issues/290)
* wrapper.js & adapt-contrib-spoor updated to work with Jest testing. (fixes adaptlearning#284) (adaptlearning#285) ([cb3d93f](cb3d93f)), closes [adaptlearning#284](https://github.com/nagyist/adapt-contrib-spoor/issues/284) [adaptlearning#285](https://github.com/nagyist/adapt-contrib-spoor/issues/285)

### New

* Added _uniqueInteractionIds, testing support for cmi.interactions (adaptlearning#278) ([f88877c](f88877c)), closes [adaptlearning#278](https://github.com/nagyist/adapt-contrib-spoor/issues/278)
* Added `cmi.objectives` support (fixes adaptlearning#279). (adaptlearning#280) ([cf911c5](cf911c5)), closes [adaptlearning#279](https://github.com/nagyist/adapt-contrib-spoor/issues/279) [adaptlearning#280](https://github.com/nagyist/adapt-contrib-spoor/issues/280)
* Added ability to compile as SCORM 1.2 or SCORM 2004 (fixes adaptlearning#249) ([497637a](497637a)), closes [adaptlearning#249](https://github.com/nagyist/adapt-contrib-spoor/issues/249)
* Added ability to configure `_setCompletedWhenFailed` - fixes adaptlearning#242. (adaptlearning#251) ([f3a7b7a](f3a7b7a)), closes [adaptlearning#242](https://github.com/nagyist/adapt-contrib-spoor/issues/242) [adaptlearning#251](https://github.com/nagyist/adapt-contrib-spoor/issues/251)
* Improve use of scorm_test_harness.html (fixes adaptlearning#247) (adaptlearning#248) ([479c2d1](479c2d1)), closes [adaptlearning#247](https://github.com/nagyist/adapt-contrib-spoor/issues/247) [adaptlearning#248](https://github.com/nagyist/adapt-contrib-spoor/issues/248)
* Issue and pr project addition automation (refs adaptlearning/adapt_framework#3315) (adaptlearning#239) ([578f9b0](578f9b0)), closes [adaptlearning#239](https://github.com/nagyist/adapt-contrib-spoor/issues/239)
* network connection test (adaptlearning#270) ([ec5ef8a](ec5ef8a)), closes [adaptlearning#270](https://github.com/nagyist/adapt-contrib-spoor/issues/270)

### Update

* Added a new maxCharLimitOverride property to allow override o… (adaptlearning#287) ([68e7064](68e7064)), closes [adaptlearning#287](https://github.com/nagyist/adapt-contrib-spoor/issues/287)

### Upgrade

* Bump ip from 1.1.8 to 1.1.9 (adaptlearning#311) ([1728659](1728659)), closes [adaptlearning#311](https://github.com/nagyist/adapt-contrib-spoor/issues/311)
* Bump yaml and semantic-release (adaptlearning#276) ([9a46faa](9a46faa)), closes [adaptlearning#276](https://github.com/nagyist/adapt-contrib-spoor/issues/276)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Loss of Connection Notification feature
4 participants