refactor(dashboard): centralize app URL usage and enhance robots.txt …#369
Conversation
…rules - Replaced hardcoded URLs with APP_URL constant for consistency across layout, OG image generation, and status pages. - Updated robots.txt to allow access to the /public/ path in addition to /status/.
|
@mezotv is attempting to deploy a commit to the Databuddy OSS Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Greptile SummaryThis PR centralizes the hardcoded Key changes:
Confidence Score: 5/5Safe to merge — all findings are minor style and defensive-coding suggestions with no current breakage. All three comments are P2: the trailing-slash guard is a defensive best practice (not a current bug given the fallback default has no slash), the sitemap helper inconsistency is purely stylistic, and the canonical/openGraph URL format mismatch is functionally equivalent due to Next.js metadataBase resolution. No logic errors, security issues, or broken contracts were found. Minor improvements suggested for apps/dashboard/lib/app-url.ts, apps/dashboard/app/sitemap.ts, and apps/dashboard/app/status/[slug]/page.tsx. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
ENV["NEXT_PUBLIC_APP_URL env var\n(fallback: https://app.databuddy.cc)"]
APPURL["lib/app-url.ts\nAPP_URL constant\ngetStatusPageUrl(slug)"]
ENV --> APPURL
APPURL --> LAYOUT["app/layout.tsx\nmetadataBase, openGraph.url,\nalternates.canonical"]
APPURL --> OGPAGE["(dby)/dby/l/[slug]/page.tsx\nOG_IMAGE_BASE"]
APPURL --> NAVBAR["status/[slug]/_components/status-navbar.tsx\nLogin href"]
APPURL --> SITEMAP["app/sitemap.ts\nstatus page URLs"]
APPURL --> STATUSPAGE["status/[slug]/page.tsx\nopenGraph.url, JSON-LD url"]
ROBOTS["app/robots.ts\nallow: /status/, /public/"]
Reviews (1): Last reviewed commit: "refactor(dashboard): centralize app URL ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const statusPages = await getPublicStatusPages(); | ||
|
|
||
| return statusPages.map((statusPage) => ({ | ||
| url: `${APP_URL}/status/${statusPage.slug}`, |
There was a problem hiding this comment.
getStatusPageUrl helper not reused
The getStatusPageUrl helper was introduced in lib/app-url.ts specifically to build status page URLs, and it's correctly used in status/[slug]/page.tsx. However, the sitemap constructs the same URL manually instead of calling the helper, creating a subtle inconsistency.
| url: `${APP_URL}/status/${statusPage.slug}`, | |
| url: getStatusPageUrl(statusPage.slug), |
You'll also need to add the import at the top of the file:
import { APP_URL, getStatusPageUrl } from "@/lib/app-url";Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| export const APP_URL = | ||
| process.env.NEXT_PUBLIC_APP_URL || "https://app.databuddy.cc"; |
There was a problem hiding this comment.
NEXT_PUBLIC_APP_URL not stripped
If NEXT_PUBLIC_APP_URL is set with a trailing slash (e.g. https://app.databuddy.cc/), every constructed URL will contain a double slash (e.g. https://app.databuddy.cc//status/my-org). It's a good defensive practice to strip a trailing slash here so downstream usages remain correct regardless of how the env var is configured.
| export const APP_URL = | |
| process.env.NEXT_PUBLIC_APP_URL || "https://app.databuddy.cc"; | |
| export const APP_URL = ( | |
| process.env.NEXT_PUBLIC_APP_URL || "https://app.databuddy.cc" | |
| ).replace(/\/$/, ""); |
| alternates: { | ||
| canonical: `/status/${slug}`, | ||
| }, |
There was a problem hiding this comment.
openGraph.url is absolute
Within the same metadata block, alternates.canonical uses a root-relative path (/status/${slug}) while openGraph.url (line 72) uses the fully-qualified URL from getStatusPageUrl. Both resolve to the same address because metadataBase is set in layout.tsx, but the inconsistency can be confusing and could cause issues if metadataBase is ever changed or unset.
Aligning them to both use getStatusPageUrl(slug) would make the intent clearer:
| alternates: { | |
| canonical: `/status/${slug}`, | |
| }, | |
| alternates: { | |
| canonical: getStatusPageUrl(slug), | |
| }, |
2 participants
Checklist