feat: show 4xx status codes as warning (orange) in Functions and Sites #2351
Conversation
ConsoleProject ID: Sites (2)
Note Appwrite has a Discord community with over 16 000 members. |
WalkthroughThe PR updates status-to-badge mappings across four Svelte components:
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/routes/(console)/project-[region]-[project]/sites/site-[site]/logs/sheet.svelte (2)
102-106: Remove redundant optional chaining in guarded blockInside
{#if selectedLog},selectedLogis non-null. Drop?.for consistency.- type={selectedLog?.responseStatusCode >= 500 + type={selectedLog.responseStatusCode >= 500
102-106: Handle status code 0 consistently (neutral/no type) or confirm it can't occur for SitesFunctions executions views treat
0as neutral (typeundefined). Sites views currently map it to'success'. Align or confirm the product difference is intentional.- : 'success'} /> + : selectedLog.responseStatusCode === 0 + ? undefined + : 'success'} />src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/table.svelte (1)
99-105: De-duplicate status→badge mapping via a small helperThis ternary appears across 4 files. Centralize to avoid drift.
Add once (e.g.,
src/lib/helpers/httpStatus.ts):export type BadgeType = 'success' | 'warning' | 'error' | undefined; export const statusCodeToBadgeType = (code?: number): BadgeType => code == null || code === 0 ? undefined : code >= 500 ? 'error' : code >= 400 ? 'warning' : 'success';Then apply here:
- type={log.responseStatusCode >= 500 - ? 'error' - : log.responseStatusCode >= 400 - ? 'warning' - : log.responseStatusCode === 0 - ? undefined - : 'success'} + type={statusCodeToBadgeType(log.responseStatusCode)}And import:
import { statusCodeToBadgeType } from '$lib/helpers/httpStatus';src/routes/(console)/project-[region]-[project]/sites/site-[site]/logs/table.svelte (2)
90-94: Add 0→neutral case for parity with Functions, or confirm 0 never appears for SitesWithout this,
0shows as'success'here but neutral elsewhere.- : log.responseStatusCode >= 400 - ? 'warning' - : 'success'} + : log.responseStatusCode >= 400 + ? 'warning' + : log.responseStatusCode === 0 + ? undefined + : 'success'}
90-94: Prefer shared helper to keep mappings consistentIf you add
statusCodeToBadgeType, replace the ternary here too:- type={log.responseStatusCode >= 500 ? 'error' : log.responseStatusCode >= 400 ? 'warning' : 'success'} + type={statusCodeToBadgeType(log.responseStatusCode)}src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/sheet.svelte (1)
101-107: Drop optional chaining; the block already guardsselectedLogKeeps the condition set consistent and slightly cleaner.
- type={selectedLog?.responseStatusCode >= 500 + type={selectedLog.responseStatusCode >= 500
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/sheet.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/table.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/logs/sheet.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/sites/site-[site]/logs/table.svelte(1 hunks)
🔇 Additional comments (3)
src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/table.svelte (2)
99-105: LGTM: Correct 5xx→error, 4xx→warning, 0→neutral mappingMatches the PR intent and improves UX differentiation.
99-105: Confirm Badge.type supports 'warning' and undefinedVerified — Pink's Tag/Badge supports a "warning" state and the
typeprop is optional (omitting it renders a neutral badge).src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/sheet.svelte (1)
101-107: LGTM: Mapping matches table view and PR intent5xx→error, 4xx→warning, 0→neutral, else→success looks good.
There was a problem hiding this comment.
we should just make a helper function in order to handle this. too may ternary ops and readability issues.
What does this PR do?
Update status code badge coloring so client errors (4xx) are orange and server errors (5xx) remain red across Functions Executions and Sites Logs (tables and sheets).
Test Plan
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
yes
Summary by CodeRabbit