Skip to content

fix: FM-901 Option click sfx does not play in offline mode in Assessment embedded mode.#1962

Merged
ashwinnair-chimple merged 2 commits into
developfrom
FM-901
May 14, 2026
Merged

fix: FM-901 Option click sfx does not play in offline mode in Assessment embedded mode.#1962
ashwinnair-chimple merged 2 commits into
developfrom
FM-901

Conversation

@ashwinnair-chimple
Copy link
Copy Markdown
Collaborator

@ashwinnair-chimple ashwinnair-chimple commented May 13, 2026

Changes

  • Option click sfx does not play in offline mode in Assessm… …ent embedded mode.
  • made changes to workbox and service worker

How to test

  • play assessment survey in offline mode

Ref: FM-901

Summary by CodeRabbit

  • Improvements
    • Enhanced offline caching of assessment audio files to improve availability when taking assessments
    • Improved error resilience—app now continues caching available audio resources even when some assets fail to load
    • Optimized audio asset preloading for faster assessment access

Review Change Stack

@ashwinnair-chimple ashwinnair-chimple changed the title fix: FM-901 Option click sfx does not play in offline mode in Assessm… …ent embedded mode. fix: FM-901 Option click sfx does not play in offline mode in Assessment embedded mode. May 13, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 13, 2026

📝 Walkthrough

Walkthrough

The PR improves service worker assessment audio caching by refactoring cacheAssessmentLanguage() to unconditionally cache shared audio assets and handle JSON fetch failures gracefully, adds four additional audio files to the preload list, and narrows the workbox build config to exclude only mp3 files instead of all audio assets.

Changes

Assessment Audio Offline Caching

Layer / File(s) Summary
Refactored assessment audio caching with shared assets
src/sw-src.js
cacheAssessmentLanguage() introduces sharedAudioAssets collection added unconditionally to urlsToCache. JSON fetch response is conditionally parsed only when ok; on failure, logs warning without early return and caches available URLs (dataUrl and shared assets at minimum).
Preload additional assessment audio files
src/sw-src.js
preloadAdditionalAssets() appends four assessment audio files to assetsToCache using getAssessmentAssetPath to compute their install-time cache URLs.
Update workbox build configuration
workbox-config.js
globIgnores pattern narrowed from assessment-survey/audio/**/* to assessment-survey/audio/**/*.mp3, allowing non-mp3 audio files to be included in precache manifest.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • curiouslearning/FeedTheMonsterJS#1928: Updates assessment asset URL resolution via getAssessmentAssetPath in src/sw-src.js, which directly supports the audio preload and caching improvements in this PR.
  • curiouslearning/FeedTheMonsterJS#1935: Modifies filename normalization for assessment mp3 URLs in src/sw-src.js, affecting how the audio URLs are generated for caching.

Suggested reviewers

  • bernhardccodev
  • janfb-codev

Poem

🐰 Four assets now preload with grace,
Shared audio finds its place,
When JSON falls, we don't retreat,
Graceful caching stays complete! 🎵

🚥 Pre-merge checks | ✅ 4 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Tests & Lint & Coverage ⚠️ Warning No automated tests added/updated. PR modifies service worker caching logic without corresponding tests. Modified .js files are outside ESLint/Jest scope (configured for .ts only). Add tests for cacheAssessmentLanguage() function in src/sw-src.js verifying shared audio asset caching and offline playback functionality.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: fixing option click sound effects in offline Assessment embedded mode, and follows the team's convention with 'fix:' prefix and JIRA key FM-901.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch FM-901

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/sw-src.js (1)

71-74: ⚡ Quick win

Use one cache strategy for the shared assessment SFX.

These four .wavs are now cached in three places: install-time dynamic-cache, every assessment-${dataKey} cache, and the Workbox precache after the globIgnores change. That adds duplicate fetch/storage work and increases eviction risk on low-storage devices. I'd keep the shared sounds in one shared cache path only—preferably the Workbox precache—and reserve assessment-${dataKey} for the dataKey-specific mp3s.

Also applies to: 302-315

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/sw-src.js` around lines 71 - 74, The shared SFX .wav files returned by
getAssessmentAssetPath("audio/...") are being cached in three places; remove
them from the runtime caching and assessment-specific caches and rely solely on
the Workbox precache: update the code that populates "dynamic-cache" and the
assessment-${dataKey} cache to skip any paths that match the shared SFX
filenames (or check against a small sharedSfx set), ensuring these four entries
remain only in the precache registration, and apply the same exclusion logic to
the other similar block (the later loop around the 302-315 area) so
assessment-${dataKey} only stores dataKey-specific mp3s.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/sw-src.js`:
- Around line 71-74: The shared SFX .wav files returned by
getAssessmentAssetPath("audio/...") are being cached in three places; remove
them from the runtime caching and assessment-specific caches and rely solely on
the Workbox precache: update the code that populates "dynamic-cache" and the
assessment-${dataKey} cache to skip any paths that match the shared SFX
filenames (or check against a small sharedSfx set), ensuring these four entries
remain only in the precache registration, and apply the same exclusion logic to
the other similar block (the later loop around the 302-315 area) so
assessment-${dataKey} only stores dataKey-specific mp3s.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1c13ab5f-ac79-4acf-b654-8f6e244f4541

📥 Commits

Reviewing files that changed from the base of the PR and between ecd5a13 and 89cdbb2.

📒 Files selected for processing (2)
  • src/sw-src.js
  • workbox-config.js

@ashwinnair-chimple ashwinnair-chimple merged commit fac3b09 into develop May 14, 2026
3 checks passed
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.

3 participants