-
Notifications
You must be signed in to change notification settings - Fork 297
Fix 2283 Updated cloud references to use latest specs #2697
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
base: main
Are you sure you want to change the base?
Fix 2283 Updated cloud references to use latest specs #2697
Conversation
WalkthroughThis pull request refactors version alias handling and content type normalization across the documentation reference routes. It introduces a "latest" alias in the examples lookup mapping to 1.7.x, adds a content type normalization helper Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/routes/docs/references/`[version]/[platform]/[service]/+page.server.ts:
- Around line 22-28: The validation currently rejects params.version values not
in versions unless it's 'cloud', so requests using the 'latest' alias will 404;
update the guard in +page.server.ts to accept 'latest' as valid by allowing
params.version === 'latest' (or treat it the same as 'cloud') when checking
against versions before converting params.version into the local const version
(used later), i.e., adjust the first validation that references params.version
and versions so it permits 'cloud' OR 'latest' (while keeping subsequent
conversion: const version = params.version === 'cloud' ? 'latest' :
params.version).
🧹 Nitpick comments (1)
src/lib/utils/specs.ts (1)
344-349: Optional: centralize thelatest→ concrete version mapping.You now have
latestnormalization in multiple places (getExamples,exampleVersion, and routes). A small helper/constant would reduce drift when the real latest version changes.
| // Validate original params.version | ||
| if (params.version !== 'cloud' && !versions.includes(params.version)) error(404, 'Invalid version'); | ||
| if (!platforms.includes(platform as Platform)) error(404, 'Invalid platform'); | ||
| if (!services.includes(service as ServiceValue)) error(404, 'Invalid service'); | ||
|
|
||
| // Convert cloud to latest after validation | ||
| const version = params.version === 'cloud' ? 'latest' : params.version; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
latest alias will 404 with current validation.
Validation rejects anything not in versions unless it’s cloud. If links now resolve to /latest/… (see models route), this will 404. Either allow latest as an alias here or keep URLs on /cloud/.
🔧 Example fix (accept `latest`)
- if (params.version !== 'cloud' && !versions.includes(params.version)) error(404, 'Invalid version');
+ const isAlias = params.version === 'cloud' || params.version === 'latest';
+ if (!isAlias && !versions.includes(params.version)) error(404, 'Invalid version');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Validate original params.version | |
| if (params.version !== 'cloud' && !versions.includes(params.version)) error(404, 'Invalid version'); | |
| if (!platforms.includes(platform as Platform)) error(404, 'Invalid platform'); | |
| if (!services.includes(service as ServiceValue)) error(404, 'Invalid service'); | |
| // Convert cloud to latest after validation | |
| const version = params.version === 'cloud' ? 'latest' : params.version; | |
| // Validate original params.version | |
| const isAlias = params.version === 'cloud' || params.version === 'latest'; | |
| if (!isAlias && !versions.includes(params.version)) error(404, 'Invalid version'); | |
| if (!platforms.includes(platform as Platform)) error(404, 'Invalid platform'); | |
| if (!services.includes(service as ServiceValue)) error(404, 'Invalid service'); | |
| // Convert cloud to latest after validation | |
| const version = params.version === 'cloud' ? 'latest' : params.version; |
🤖 Prompt for AI Agents
In `@src/routes/docs/references/`[version]/[platform]/[service]/+page.server.ts
around lines 22 - 28, The validation currently rejects params.version values not
in versions unless it's 'cloud', so requests using the 'latest' alias will 404;
update the guard in +page.server.ts to accept 'latest' as valid by allowing
params.version === 'latest' (or treat it the same as 'cloud') when checking
against versions before converting params.version into the local const version
(used later), i.e., adjust the first validation that references params.version
and versions so it permits 'cloud' OR 'latest' (while keeping subsequent
conversion: const version = params.version === 'cloud' ? 'latest' :
params.version).
…for-Download-Deployment-Response-is-Wrong
What does this PR do?
This PR solves issue #2283. page.server.ts files referenced cloud as 1.7.x specs. This PR solves the issue by changing cloud to be a reference to latest specs (most updated).
Specs generated by the backend included content with a response mapped to "/" instead of "application/gzip". This PR implements a function that converts the "/" to "application/gzip". This is a lightweight workaround for the mapping in the backend. Backend solutions to this issue are detailed in the original spec generator fix PR 10277 and can be implemented if requested as a replacement to the conversion function in specs.ts
Test Plan
Run the instructions detailed in PR 10277 to get updated latest specs. Replace latest specs with the updated ones at this location: app\config\specs. Build and run Appwrite/Website. Then at these locations: Functions and Sites the Download Deployment documentation will show a response of "200 application/gzip".
Related PRs and Issues
This PR is directly related to Issues: #810, #2283. As well as the following PR: 10277
Have you read the Contributing Guidelines on issues?
Yes
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.