fix(audio): verse recitation URLs should return absolute paths, not relative#45
fix(audio): verse recitation URLs should return absolute paths, not relative#45Cyberistic wants to merge 1 commit intoquran:mainfrom
Conversation
…elative - Added audioUrl field to VerseRecitation type for absolute URLs - Added normalizeVerseRecitations() helper to convert relative paths to absolute - Updated mocks to include relative URL examples - Added tests verifying URL normalization Verse recitation methods now return both url (relative) and audioUrl (absolute) for consistency with chapter recitations and immediate usability.
There was a problem hiding this comment.
Pull request overview
This PR updates the Quran Audio SDK to return absolute, immediately usable URLs for verse recitations, aligning behavior with existing chapter recitation URL handling while preserving the original relative url value.
Changes:
- Adds a new
audioUrlfield toVerseRecitationto provide an absolute URL alongside the existing relativeurl. - Normalizes verse recitation responses in
findVerseRecitationsByChapter()andfindVerseRecitationsByKey()by prependinghttps://verses.quran.com/when needed. - Updates mocks and adds tests to validate both relative (
url) and absolute (audioUrl) fields.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/api/src/sdk/audio.ts | Adds URL normalization helper and applies it to verse-recitation endpoints. |
| packages/api/src/types/api/AudioData.ts | Extends VerseRecitation with a new audioUrl field and JSDoc describing URL semantics. |
| packages/api/mocks/handlers.ts | Updates mock verse recitation payloads to include the relative url field used for normalization. |
| packages/api/test/audio.test.ts | Adds test coverage asserting audioUrl is correctly derived from url. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| verseKey: VerseKey; | ||
| /** Relative URL path (e.g., "AbdulBaset/Murattal/mp3/002255.mp3") */ | ||
| url: string; | ||
| /** Absolute URL (e.g., "https://download.quranicaudio.com/qdc/AbdulBaset/Murattal/mp3/002255.mp3") */ |
There was a problem hiding this comment.
audioUrl JSDoc example uses the chapter-recitation host (download.quranicaudio.com/qdc/...), but this PR normalizes verse URLs to https://verses.quran.com/.... Update the example (and any wording) so it matches the actual audioUrl being returned for verse recitations.
| /** Absolute URL (e.g., "https://download.quranicaudio.com/qdc/AbdulBaset/Murattal/mp3/002255.mp3") */ | |
| /** Absolute URL (e.g., "https://verses.quran.com/AbdulBaset/Murattal/mp3/002255.mp3") */ |
| function normalizeVerseRecitations( | ||
| audioFiles: VerseRecitation[], | ||
| ): VerseRecitation[] { | ||
| return audioFiles.map((file) => { | ||
| // If url is already absolute, use it; otherwise prepend the base URL | ||
| const absoluteUrl = file.url.startsWith("http") | ||
| ? file.url | ||
| : `${AUDIO_BASE_URL}/${file.url}`; | ||
|
|
There was a problem hiding this comment.
normalizeVerseRecitations is typed as taking/returning VerseRecitation[], but VerseRecitation now requires audioUrl even though the upstream API payload only provides the relative url. This makes the intermediate data.audioFiles type inaccurate and can mislead future changes. Consider introducing a separate "raw" type (e.g., type RawVerseRecitation = Omit<VerseRecitation, "audioUrl">) for the fetch response and keep VerseRecitation for the normalized return type (or make audioUrl optional on the interface if it can be absent).
| // If url is already absolute, use it; otherwise prepend the base URL | ||
| const absoluteUrl = file.url.startsWith("http") | ||
| ? file.url | ||
| : `${AUDIO_BASE_URL}/${file.url}`; |
There was a problem hiding this comment.
The normalization logic has an untested branch when file.url is already absolute (startsWith("http")). Adding a test case for an already-absolute url would help prevent regressions and clarify expected behavior (e.g., whether audioUrl should equal url in that case).
Description
Currently, the
findVerseRecitationsByChapter()andfindVerseRecitationsByKey()methods in the Quran Audio API return relative URL paths (e.g.,AbdulBaset/Murattal/mp3/002255.mp3) for verse recitations, whilefindAllChapterRecitations()returns absolute URLs (e.g.,https://download.quranicaudio.com/qdc/...).The Problem:
AbdulBaset/Murattal/mp3/002255.mp3, It doesn't even tell us what the base URL is! Furthermore, the chapter recitation base URL (https://download.quranicaudio.com/qdc/) does not work with verse recitations. Users have no way to know the correct base URL without external documentation or trial and errorThe Solution:
This PR normalizes verse recitation URLs to absolute paths by prepending the correct base URL (
https://verses.quran.com/), ensuring consistent behavior across all audio methods.Changes Made
Updated
VerseRecitationtype (src/types/api/AudioData.ts)audioUrl: stringfield alongside existingurlfieldAdded URL normalization (
src/sdk/audio.ts)AUDIO_BASE_URL = "https://verses.quran.com"normalizeVerseRecitations()helper functionfindVerseRecitationsByChapter()andfindVerseRecitationsByKey()Updated mocks (
mocks/handlers.ts)urlfield with relative paths to verse recitation mock responsesAdded tests (
test/audio.test.ts)url(relative) andaudioUrl(absolute) fieldsAPI Changes
Before:
After:
Backward Compatibility
Fully backward compatible - The original
urlfield is preserved with the relative path, and the newaudioUrlfield provides the normalized absolute URL. Eventually, I believe only one of the fields should be kept. Also, need to figure out what to do when baseURL is manually overridden in the client config.I'm not sure how this function ever got past review when the relative url doesn't even work, was I missing something?