Open
Conversation
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
aahiltn
force-pushed
the
aahil/invites-page-server
branch
from
8c61a4d to
2ee43a8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/internal/services/invite_page.go`:
- Around line 87-98: Replace the ad-hoc string concatenation in building deep
links and canonical URLs with proper URL construction and escaping: parse
deepLinkBase into a url.URL (use a safer production-friendly default instead of
"exp://localhost:8081/--" when DEEPLINK_BASE_URL is empty), append the invite
path with the invite code using url.URL.ResolveReference or constructing a Path
with path.Join and use url.PathEscape(code) for the path segment before
assigning data.DeepLink (wrap as template.URL only after building and validating
the URL); similarly parse APP_PUBLIC_URL into a url.URL, build the join URL by
adding "/join" and setting the "code" query parameter via url.Values or
u.Query().Set("code", url.QueryEscape(code)) then re-encode and assign to
data.CanonicalURL so codes with special characters and missing env values are
handled safely (refer to deepLinkBase, data.DeepLink, APP_PUBLIC_URL,
data.CanonicalURL, and code).
In `@backend/internal/templates/trip_invite.html`:
- Around line 274-292: The openApp function currently hardcodes appStoreURL to
"https://apps.apple.com"; update openApp to compute appStoreURL dynamically
based on the runtime platform (e.g., check navigator.userAgent for /android/i or
/iPhone|iPad|iPod|Macintosh/ to detect iOS) so Android users are sent to the
Google Play store (play.google.com) and iOS users to the App Store; keep the
existing deepLink, didNavigate logic and only replace the static appStoreURL
assignment and any comments so the setTimeout fallback uses the platform-aware
appStoreURL when window.location.href is set after 1500ms.
In `@backend/Makefile`:
- Around line 14-17: The run-prod Makefile target currently runs the built
binary with production Doppler config directly; add a safety gate that requires
an explicit opt-in variable (e.g., RUN_PROD=1 or PROD_CONFIRM=yes) before
executing the doppler run line and otherwise print a clear fatal message and
exit nonzero. Update the target that references run-prod, $(BINARY_NAME), and
the doppler run invocation to check the opt-in variable at the top of the target
and abort fast when it is not set to the expected value.
In `@frontend/app/`(app)/testing/index.tsx:
- Around line 35-64: The JWT debug UI currently renders the raw jwt and exposes
copyJwt; wrap the entire debug Box (the JSX that reads {jwt ?? "Not signed in"}
and the Copy JWT Button which calls copyJwt) behind a dev-only or internal
feature flag (e.g., __DEV__ || isInternalFeatureFlag()) so it is not rendered in
production builds; update the component to check that flag before returning or
rendering the Box and ensure the copyJwt handler remains unreachable when the
flag is false.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
backend/internal/templates/static/flight_path.svgis excluded by!**/*.svgbackend/internal/templates/static/logo.svgis excluded by!**/*.svgbackend/internal/templates/static/toggo_logo.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
backend/Makefilebackend/internal/controllers/invite_page.gobackend/internal/models/trip_invite_page.gobackend/internal/server/routers/invite_page.gobackend/internal/server/routers/routers.gobackend/internal/services/invite_page.gobackend/internal/services/trips.gobackend/internal/templates/join_enter_code.htmlbackend/internal/templates/renderer.gobackend/internal/templates/static/README.mdbackend/internal/templates/trip_invite.htmlbackend/internal/utilities/params.gofrontend/app/(app)/testing/index.tsxfrontend/app/(app)/testing/test-upload.tsx
Comment on lines
+87
to
+98
| deepLinkBase := os.Getenv("DEEPLINK_BASE_URL") | ||
| if deepLinkBase == "" { | ||
| deepLinkBase = "exp://localhost:8081/--" | ||
| } | ||
| data.DeepLink = template.URL(fmt.Sprintf("%s/invite/%s", strings.TrimRight(deepLinkBase, "/"), code)) | ||
|
|
||
| // Build canonical URL | ||
| baseURL := os.Getenv("APP_PUBLIC_URL") | ||
| if baseURL != "" { | ||
| trimmed := strings.TrimRight(baseURL, "/") | ||
| data.CanonicalURL = trimmed + "/join?code=" + code | ||
| } |
Contributor
There was a problem hiding this comment.
Use safe URL construction and a production-safe deeplink default.
Line 89 defaults to a local Expo URL, and Line 91/Line 97 build URLs by concatenation. This can break production deeplinks when env config is missing and can generate malformed URLs for non-alphanumeric codes.
Proposed fix
import (
"context"
"errors"
"fmt"
"html/template"
+ "net/url"
"os"
"strings"
"time"
@@
deepLinkBase := os.Getenv("DEEPLINK_BASE_URL")
if deepLinkBase == "" {
- deepLinkBase = "exp://localhost:8081/--"
+ deepLinkBase = "toggo://"
}
- data.DeepLink = template.URL(fmt.Sprintf("%s/invite/%s", strings.TrimRight(deepLinkBase, "/"), code))
+ data.DeepLink = template.URL(
+ fmt.Sprintf("%s/invite/%s", strings.TrimRight(deepLinkBase, "/"), url.PathEscape(code)),
+ )
@@
baseURL := os.Getenv("APP_PUBLIC_URL")
if baseURL != "" {
- trimmed := strings.TrimRight(baseURL, "/")
- data.CanonicalURL = trimmed + "/join?code=" + code
+ u, err := url.Parse(strings.TrimRight(baseURL, "/") + "/join")
+ if err == nil {
+ q := u.Query()
+ q.Set("code", code)
+ u.RawQuery = q.Encode()
+ data.CanonicalURL = u.String()
+ }
}📝 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.
Suggested change
| deepLinkBase := os.Getenv("DEEPLINK_BASE_URL") | |
| if deepLinkBase == "" { | |
| deepLinkBase = "exp://localhost:8081/--" | |
| } | |
| data.DeepLink = template.URL(fmt.Sprintf("%s/invite/%s", strings.TrimRight(deepLinkBase, "/"), code)) | |
| // Build canonical URL | |
| baseURL := os.Getenv("APP_PUBLIC_URL") | |
| if baseURL != "" { | |
| trimmed := strings.TrimRight(baseURL, "/") | |
| data.CanonicalURL = trimmed + "/join?code=" + code | |
| } | |
| deepLinkBase := os.Getenv("DEEPLINK_BASE_URL") | |
| if deepLinkBase == "" { | |
| deepLinkBase = "toggo://" | |
| } | |
| data.DeepLink = template.URL( | |
| fmt.Sprintf("%s/invite/%s", strings.TrimRight(deepLinkBase, "/"), url.PathEscape(code)), | |
| ) | |
| // Build canonical URL | |
| baseURL := os.Getenv("APP_PUBLIC_URL") | |
| if baseURL != "" { | |
| u, err := url.Parse(strings.TrimRight(baseURL, "/") + "/join") | |
| if err == nil { | |
| q := u.Query() | |
| q.Set("code", code) | |
| u.RawQuery = q.Encode() | |
| data.CanonicalURL = u.String() | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/internal/services/invite_page.go` around lines 87 - 98, Replace the
ad-hoc string concatenation in building deep links and canonical URLs with
proper URL construction and escaping: parse deepLinkBase into a url.URL (use a
safer production-friendly default instead of "exp://localhost:8081/--" when
DEEPLINK_BASE_URL is empty), append the invite path with the invite code using
url.URL.ResolveReference or constructing a Path with path.Join and use
url.PathEscape(code) for the path segment before assigning data.DeepLink (wrap
as template.URL only after building and validating the URL); similarly parse
APP_PUBLIC_URL into a url.URL, build the join URL by adding "/join" and setting
the "code" query parameter via url.Values or u.Query().Set("code",
url.QueryEscape(code)) then re-encode and assign to data.CanonicalURL so codes
with special characters and missing env values are handled safely (refer to
deepLinkBase, data.DeepLink, APP_PUBLIC_URL, data.CanonicalURL, and code).
Comment on lines
+274
to
+292
| function openApp() { | ||
| var deepLink = "{{ .Data.DeepLink }}"; | ||
| var appStoreURL = "https://apps.apple.com"; | ||
| var didNavigate = false; | ||
|
|
||
| // Listen for visibility change (app opened successfully) | ||
| document.addEventListener("visibilitychange", function() { | ||
| if (document.hidden) didNavigate = true; | ||
| }); | ||
|
|
||
| // Try to open the deep link | ||
| window.location.href = deepLink; | ||
|
|
||
| // If still here after 1.5s, app isn't installed — go to App Store | ||
| setTimeout(function() { | ||
| if (!didNavigate) { | ||
| window.location.href = appStoreURL; | ||
| } | ||
| }, 1500); |
Contributor
There was a problem hiding this comment.
Do not hardcode iOS App Store as the only deeplink fallback.
Line 276 and Line 290 always redirect to https://apps.apple.com when app open fails. Android users will be sent to the wrong store, which blocks onboarding on that platform.
Proposed platform-aware fallback
function openApp() {
var deepLink = "{{ .Data.DeepLink }}";
- var appStoreURL = "https://apps.apple.com";
+ var appStoreURL = "https://apps.apple.com";
+ var playStoreURL = "https://play.google.com/store/apps/details?id=com.toggo.app";
+ var isAndroid = /Android/i.test(navigator.userAgent);
+ var fallbackURL = isAndroid ? playStoreURL : appStoreURL;
var didNavigate = false;
@@
setTimeout(function() {
if (!didNavigate) {
- window.location.href = appStoreURL;
+ window.location.href = fallbackURL;
}
}, 1500);
}📝 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.
Suggested change
| function openApp() { | |
| var deepLink = "{{ .Data.DeepLink }}"; | |
| var appStoreURL = "https://apps.apple.com"; | |
| var didNavigate = false; | |
| // Listen for visibility change (app opened successfully) | |
| document.addEventListener("visibilitychange", function() { | |
| if (document.hidden) didNavigate = true; | |
| }); | |
| // Try to open the deep link | |
| window.location.href = deepLink; | |
| // If still here after 1.5s, app isn't installed — go to App Store | |
| setTimeout(function() { | |
| if (!didNavigate) { | |
| window.location.href = appStoreURL; | |
| } | |
| }, 1500); | |
| function openApp() { | |
| var deepLink = "{{ .Data.DeepLink }}"; | |
| var appStoreURL = "https://apps.apple.com"; | |
| var playStoreURL = "https://play.google.com/store/apps/details?id=com.toggo.app"; | |
| var isAndroid = /Android/i.test(navigator.userAgent); | |
| var fallbackURL = isAndroid ? playStoreURL : appStoreURL; | |
| var didNavigate = false; | |
| // Listen for visibility change (app opened successfully) | |
| document.addEventListener("visibilitychange", function() { | |
| if (document.hidden) didNavigate = true; | |
| }); | |
| // Try to open the deep link | |
| window.location.href = deepLink; | |
| // If still here after 1.5s, app isn't installed — go to App Store | |
| setTimeout(function() { | |
| if (!didNavigate) { | |
| window.location.href = fallbackURL; | |
| } | |
| }, 1500); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/internal/templates/trip_invite.html` around lines 274 - 292, The
openApp function currently hardcodes appStoreURL to "https://apps.apple.com";
update openApp to compute appStoreURL dynamically based on the runtime platform
(e.g., check navigator.userAgent for /android/i or /iPhone|iPad|iPod|Macintosh/
to detect iOS) so Android users are sent to the Google Play store
(play.google.com) and iOS users to the App Store; keep the existing deepLink,
didNavigate logic and only replace the static appStoreURL assignment and any
comments so the setTimeout fallback uses the platform-aware appStoreURL when
window.location.href is set after 1500ms.
Comment on lines
+14
to
+17
| run-prod: | ||
| $(MAKE) build | ||
| @echo "Running backend with prod config (Ctrl+C to stop)..." | ||
| doppler run --project backend --config prod -- ./$(BINARY_NAME) |
Contributor
There was a problem hiding this comment.
Add an explicit safety gate before allowing run-prod.
Line 14 introduces a direct local run path with production secrets/config. This is high-risk for accidental production writes during local testing. Require an explicit opt-in flag and fail fast otherwise.
Proposed hard-guard
run-prod:
+ `@test` "$(CONFIRM_PROD)" = "1" || (echo "Refusing to run prod config locally. Re-run with CONFIRM_PROD=1"; exit 1)
$(MAKE) build
`@echo` "Running backend with prod config (Ctrl+C to stop)..."
doppler run --project backend --config prod -- ./$(BINARY_NAME)📝 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.
Suggested change
| run-prod: | |
| $(MAKE) build | |
| @echo "Running backend with prod config (Ctrl+C to stop)..." | |
| doppler run --project backend --config prod -- ./$(BINARY_NAME) | |
| run-prod: | |
| `@test` "$(CONFIRM_PROD)" = "1" || (echo "Refusing to run prod config locally. Re-run with CONFIRM_PROD=1"; exit 1) | |
| $(MAKE) build | |
| `@echo` "Running backend with prod config (Ctrl+C to stop)..." | |
| doppler run --project backend --config prod -- ./$(BINARY_NAME) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/Makefile` around lines 14 - 17, The run-prod Makefile target
currently runs the built binary with production Doppler config directly; add a
safety gate that requires an explicit opt-in variable (e.g., RUN_PROD=1 or
PROD_CONFIRM=yes) before executing the doppler run line and otherwise print a
clear fatal message and exit nonzero. Update the target that references
run-prod, $(BINARY_NAME), and the doppler run invocation to check the opt-in
variable at the top of the target and abort fast when it is not set to the
expected value.
Comment on lines
+35
to
+64
| <Box | ||
| marginTop="md" | ||
| padding="md" | ||
| borderRadius="sm" | ||
| backgroundColor="surfaceCard" | ||
| alignSelf="stretch" | ||
| gap="xs" | ||
| > | ||
| <Text variant="smLabel" color="textQuaternary"> | ||
| Debug: JWT | ||
| </Text> | ||
| <Text | ||
| variant="smLabel" | ||
| color="textQuaternary" | ||
| numberOfLines={3} | ||
| style={{ fontFamily: "monospace" }} | ||
| > | ||
| {jwt ?? "Not signed in"} | ||
| </Text> | ||
| {jwt && ( | ||
| <Box alignSelf="flex-start"> | ||
| <Button | ||
| layout="textOnly" | ||
| label="Copy JWT" | ||
| variant="Secondary" | ||
| onPress={copyJwt} | ||
| /> | ||
| </Box> | ||
| )} | ||
| </Box> |
Contributor
There was a problem hiding this comment.
Do not expose access tokens outside dev-only flows
Lines 35-64 render the raw JWT and provide copy-to-clipboard. Even on a testing screen, this is a sensitive-token exposure risk if included in non-dev builds. Gate this block behind a dev flag (for example __DEV__) or a secure internal feature flag.
Proposed fix
export default function Home() {
const [jwt, setJwt] = useState<string | null>(null);
+ const showDebugAuth = __DEV__;
@@
- <Box
- marginTop="md"
- padding="md"
- borderRadius="sm"
- backgroundColor="surfaceCard"
- alignSelf="stretch"
- gap="xs"
- >
- <Text variant="smLabel" color="textQuaternary">
- Debug: JWT
- </Text>
- <Text
- variant="smLabel"
- color="textQuaternary"
- numberOfLines={3}
- style={{ fontFamily: "monospace" }}
- >
- {jwt ?? "Not signed in"}
- </Text>
- {jwt && (
- <Box alignSelf="flex-start">
- <Button
- layout="textOnly"
- label="Copy JWT"
- variant="Secondary"
- onPress={copyJwt}
- />
- </Box>
- )}
- </Box>
+ {showDebugAuth && (
+ <Box
+ marginTop="md"
+ padding="md"
+ borderRadius="sm"
+ backgroundColor="surfaceCard"
+ alignSelf="stretch"
+ gap="xs"
+ >
+ <Text variant="smLabel" color="textQuaternary">
+ Debug: JWT
+ </Text>
+ <Text
+ variant="smLabel"
+ color="textQuaternary"
+ numberOfLines={3}
+ style={{ fontFamily: "monospace" }}
+ >
+ {jwt ?? "Not signed in"}
+ </Text>
+ {jwt && (
+ <Box alignSelf="flex-start">
+ <Button
+ layout="textOnly"
+ label="Copy JWT"
+ variant="Secondary"
+ onPress={copyJwt}
+ />
+ </Box>
+ )}
+ </Box>
+ )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/`(app)/testing/index.tsx around lines 35 - 64, The JWT debug UI
currently renders the raw jwt and exposes copyJwt; wrap the entire debug Box
(the JSX that reads {jwt ?? "Not signed in"} and the Copy JWT Button which calls
copyJwt) behind a dev-only or internal feature flag (e.g., __DEV__ ||
isInternalFeatureFlag()) so it is not rendered in production builds; update the
component to check that flag before returning or rendering the Box and ensure
the copyJwt handler remains unreachable when the flag is false.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
backend/Makefile (1)
14-17:⚠️ Potential issue | 🟠 MajorAdd an explicit confirmation gate before
run-prodexecutes.Line 14 through Line 17 allows accidental local execution against production configuration with no hard opt-in.
Proposed fix
run-prod: + `@test` "$(CONFIRM_PROD)" = "1" || (echo "Refusing to run prod config locally. Re-run with CONFIRM_PROD=1"; exit 1) $(MAKE) build `@echo` "Running backend with prod config (Ctrl+C to stop)..." doppler run --project backend --config prod -- ./$(BINARY_NAME)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/Makefile` around lines 14 - 17, The run-prod Makefile target currently runs production config without any opt-in; modify the run-prod target in the Makefile to require an explicit confirmation gate before executing the build and doppler invocation—e.g. check for a confirmation environment variable (like RUN_PROD_CONFIRM or FORCE_RUN_PROD) or prompt interactively and abort unless the user supplied the explicit token (e.g., "YES" or RUN_PROD_CONFIRM=1); update the target that references $(BINARY_NAME) so it exits non-zero when confirmation is not present and only proceeds to call $(MAKE) build and doppler run when the confirmation check passes.backend/internal/services/invite_page.go (1)
87-98:⚠️ Potential issue | 🟠 MajorBuild deep links and canonical URLs with escaping-safe URL construction.
Line 89 defaults to a local Expo URL, and Lines 91/97 build URLs via string concatenation. This can break production deeplinks and mishandle non-alphanumeric codes.
Proposed fix
import ( "context" "errors" "fmt" "html/template" + "net/url" "os" "strings" "time" @@ deepLinkBase := os.Getenv("DEEPLINK_BASE_URL") if deepLinkBase == "" { - deepLinkBase = "exp://localhost:8081/--" + deepLinkBase = "toggo://" } - data.DeepLink = template.URL(fmt.Sprintf("%s/invite/%s", strings.TrimRight(deepLinkBase, "/"), code)) + data.DeepLink = template.URL( + fmt.Sprintf("%s/invite/%s", strings.TrimRight(deepLinkBase, "/"), url.PathEscape(code)), + ) baseURL := os.Getenv("APP_PUBLIC_URL") if baseURL != "" { - trimmed := strings.TrimRight(baseURL, "/") - data.CanonicalURL = trimmed + "/join?code=" + code + u, err := url.Parse(strings.TrimRight(baseURL, "/") + "/join") + if err == nil { + q := u.Query() + q.Set("code", code) + u.RawQuery = q.Encode() + data.CanonicalURL = u.String() + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/services/invite_page.go` around lines 87 - 98, The deep link and canonical URL are built via string concatenation which can mis-handle special characters in code and produce invalid URLs; update the logic that sets deepLinkBase, data.DeepLink and data.CanonicalURL to construct URLs using the net/url package (e.g., parse the base with url.Parse, use ResolveReference or manually set Path with path.Join-like behavior and use url.PathEscape for path segments and url.QueryEscape for query values) so the invite path uses an escaped code for the path (for data.DeepLink) and the join?code= value is query-escaped (for data.CanonicalURL); preserve the fallback default for deepLinkBase and continue to cast the final deep link to template.URL only after proper escaping/validation.backend/internal/templates/trip_invite.html (1)
274-292:⚠️ Potential issue | 🟠 MajorUse a platform-aware fallback URL in
openApp().Line 276 hardcodes the iOS App Store fallback. On Android, the 1.5s fallback sends users to the wrong store.
Proposed fix
function openApp() { var deepLink = "{{ .Data.DeepLink }}"; - var appStoreURL = "https://apps.apple.com"; + var appStoreURL = "https://apps.apple.com"; + var playStoreURL = "https://play.google.com/store/apps/details?id=com.toggo.app"; + var fallbackURL = /Android/i.test(navigator.userAgent) ? playStoreURL : appStoreURL; var didNavigate = false; @@ setTimeout(function() { if (!didNavigate) { - window.location.href = appStoreURL; + window.location.href = fallbackURL; } }, 1500); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/templates/trip_invite.html` around lines 274 - 292, The openApp() function currently hardcodes an iOS App Store fallback (appStoreURL) which misroutes Android users; update openApp() to detect the platform (e.g., check navigator.userAgent or navigator.userAgentData for Android vs iOS/Apple device) and set appStoreURL accordingly (Google Play URL for Android, App Store for iOS, and a sensible web fallback for other platforms), then keep the existing deepLink navigation and timeout logic so the platform-aware appStoreURL is used in the setTimeout fallback; refer to the openApp function, the deepLink variable, and the appStoreURL variable when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/internal/models/trip_invite_page.go`:
- Around line 3-10: Update the TripInvitePageData struct to change the DeepLink
field from template.URL to string (symbol: TripInvitePageData.DeepLink) so the
value is escaped normally, then validate the environment-provided deepLinkBase
before composing the deeplink (symbol: deepLinkBase or wherever
DEEPLINK_BASE_URL is read in invite_page.go) by parsing the URL and enforcing an
allowed scheme whitelist (e.g., "toggo" and "exp") and rejecting or erroring
when the scheme is missing or not allowed; ensure the code builds a properly
encoded string deeplink rather than storing a template.URL and propagate errors
when DEEPLINK_BASE_URL is invalid.
In `@backend/internal/templates/join_enter_code.html`:
- Around line 15-24: The template join_enter_code.html currently injects
Tailwind via CDN and runtime config; replace this by referencing a prebuilt
static CSS file produced during the backend build: add a build step (similar to
frontend PostCSS/Tailwind) that compiles Tailwind into a single CSS asset (e.g.,
public/css/invite.css), update the template to remove the tailwind CDN <script>
and config block and instead include a link to the compiled CSS, and ensure
invite_page.go (or the existing template rendering path) serves or references
the generated asset; if backend build infra doesn't exist yet, track this as a
follow-up task and avoid committing CDN usage to production templates.
---
Duplicate comments:
In `@backend/internal/services/invite_page.go`:
- Around line 87-98: The deep link and canonical URL are built via string
concatenation which can mis-handle special characters in code and produce
invalid URLs; update the logic that sets deepLinkBase, data.DeepLink and
data.CanonicalURL to construct URLs using the net/url package (e.g., parse the
base with url.Parse, use ResolveReference or manually set Path with
path.Join-like behavior and use url.PathEscape for path segments and
url.QueryEscape for query values) so the invite path uses an escaped code for
the path (for data.DeepLink) and the join?code= value is query-escaped (for
data.CanonicalURL); preserve the fallback default for deepLinkBase and continue
to cast the final deep link to template.URL only after proper
escaping/validation.
In `@backend/internal/templates/trip_invite.html`:
- Around line 274-292: The openApp() function currently hardcodes an iOS App
Store fallback (appStoreURL) which misroutes Android users; update openApp() to
detect the platform (e.g., check navigator.userAgent or navigator.userAgentData
for Android vs iOS/Apple device) and set appStoreURL accordingly (Google Play
URL for Android, App Store for iOS, and a sensible web fallback for other
platforms), then keep the existing deepLink navigation and timeout logic so the
platform-aware appStoreURL is used in the setTimeout fallback; refer to the
openApp function, the deepLink variable, and the appStoreURL variable when
making this change.
In `@backend/Makefile`:
- Around line 14-17: The run-prod Makefile target currently runs production
config without any opt-in; modify the run-prod target in the Makefile to require
an explicit confirmation gate before executing the build and doppler
invocation—e.g. check for a confirmation environment variable (like
RUN_PROD_CONFIRM or FORCE_RUN_PROD) or prompt interactively and abort unless the
user supplied the explicit token (e.g., "YES" or RUN_PROD_CONFIRM=1); update the
target that references $(BINARY_NAME) so it exits non-zero when confirmation is
not present and only proceeds to call $(MAKE) build and doppler run when the
confirmation check passes.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
backend/internal/templates/static/flight_path.svgis excluded by!**/*.svgbackend/internal/templates/static/logo.svgis excluded by!**/*.svgbackend/internal/templates/static/toggo_logo.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
backend/Makefilebackend/internal/controllers/invite_page.gobackend/internal/models/trip_invite_page.gobackend/internal/server/routers/invite_page.gobackend/internal/server/routers/routers.gobackend/internal/services/invite_page.gobackend/internal/services/trips.gobackend/internal/templates/join_enter_code.htmlbackend/internal/templates/renderer.gobackend/internal/templates/static/README.mdbackend/internal/templates/trip_invite.htmlbackend/internal/utilities/params.gofrontend/app/(app)/testing/index.tsxfrontend/app/(app)/testing/test-upload.tsx
Comment on lines
+3
to
+10
| import "html/template" | ||
|
|
||
| // TripInvitePageData contains data rendered by the invite landing page. | ||
| type TripInvitePageData struct { | ||
| TripName string | ||
| InviteCode string | ||
| DeepLink template.URL | ||
| CanonicalURL string |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the file structure and imports
echo "=== File content ==="
cat -n backend/internal/models/trip_invite_page.go
echo -e "\n=== DeepLink assignments ==="
rg -n 'DeepLink\s*=' backend/ --max-count 20
echo -e "\n=== URL validation patterns ==="
rg -n 'url\.Parse|scheme|ParseRequestURI|url\.URL' backend/internal --max-count 30
echo -e "\n=== Template rendering of DeepLink ==="
rg -n 'DeepLink|\.DeepLink' backend/internal/templates backend/ -A 2 --max-count 20Repository: GenerateNU/toggo
Length of output: 2090
🏁 Script executed:
#!/bin/bash
# Find deepLinkBase source
echo "=== deepLinkBase definition ==="
rg -n 'deepLinkBase\s*:?=' backend/internal/services/invite_page.go -B 5 -A 2
echo -e "\n=== Function signature for invite page service ==="
rg -n 'func.*InvitePage|deepLinkBase' backend/internal/services/invite_page.go -B 2 -A 8 | head -40
echo -e "\n=== Code/invite validation ==="
rg -n 'code|InviteCode' backend/internal/services/invite_page.go | head -20Repository: GenerateNU/toggo
Length of output: 2235
🏁 Script executed:
#!/bin/bash
# Check if DEEPLINK_BASE_URL is validated anywhere
echo "=== DEEPLINK_BASE_URL references ==="
rg -n 'DEEPLINK_BASE_URL' backend/
echo -e "\n=== Scheme validation in codebase ==="
rg -n 'scheme|protocol|toggo://|exp://' backend/internal --max-count 15Repository: GenerateNU/toggo
Length of output: 504
Avoid template.URL for deeplink data; the field should use string type.
Line 9 uses a trusted type that disables HTML escaping, but the DeepLink value is rendered in a JavaScript context where this bypasses appropriate escaping rules. Additionally, the deepLinkBase environment variable (line 87 of invite_page.go) lacks scheme validation, allowing any value to be interpolated into the deeplink URL.
Change DeepLink from template.URL to string and add scheme validation for the DEEPLINK_BASE_URL environment variable to enforce allowed schemes (e.g., toggo:// or exp://).
Proposed fix
-import "html/template"
-
// TripInvitePageData contains data rendered by the invite landing page.
type TripInvitePageData struct {
TripName string
InviteCode string
- DeepLink template.URL
+ DeepLink string
CanonicalURL string📝 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.
Suggested change
| import "html/template" | |
| // TripInvitePageData contains data rendered by the invite landing page. | |
| type TripInvitePageData struct { | |
| TripName string | |
| InviteCode string | |
| DeepLink template.URL | |
| CanonicalURL string | |
| // TripInvitePageData contains data rendered by the invite landing page. | |
| type TripInvitePageData struct { | |
| TripName string | |
| InviteCode string | |
| DeepLink string | |
| CanonicalURL string |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/internal/models/trip_invite_page.go` around lines 3 - 10, Update the
TripInvitePageData struct to change the DeepLink field from template.URL to
string (symbol: TripInvitePageData.DeepLink) so the value is escaped normally,
then validate the environment-provided deepLinkBase before composing the
deeplink (symbol: deepLinkBase or wherever DEEPLINK_BASE_URL is read in
invite_page.go) by parsing the URL and enforcing an allowed scheme whitelist
(e.g., "toggo" and "exp") and rejecting or erroring when the scheme is missing
or not allowed; ensure the code builds a properly encoded string deeplink rather
than storing a template.URL and propagate errors when DEEPLINK_BASE_URL is
invalid.
Comment on lines
+15
to
+24
| <script src="https://cdn.tailwindcss.com"></script> | ||
| <script> | ||
| tailwind.config = { | ||
| theme: { | ||
| extend: { | ||
| colors: { toggo: '#FF7E00' } | ||
| } | ||
| } | ||
| } | ||
| </script> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify CSP configuration and Tailwind CDN usage
rg -n -C3 'Content-Security-Policy|script-src|cdn\.tailwindcss\.com|tailwind\.config' backend/internalRepository: GenerateNU/toggo
Length of output: 1620
🏁 Script executed:
#!/bin/bash
# Check if static CSS files exist or if build process handles Tailwind
find backend -type f \( -name "*.css" -o -name "*.html" \) | head -20Repository: GenerateNU/toggo
Length of output: 151
🏁 Script executed:
#!/bin/bash
# Search for all Tailwind-related configuration in the project
rg -n 'tailwind|cdn' backend/ --type html --type js --type go -iRepository: GenerateNU/toggo
Length of output: 661
🏁 Script executed:
#!/bin/bash
# Check the build/static setup
ls -la backend/internal/templates/ 2>/dev/null || echo "templates dir not found"
git ls-files | grep -E '(static|dist|build|tailwind)' | head -20Repository: GenerateNU/toggo
Length of output: 699
🏁 Script executed:
#!/bin/bash
# Check for build configuration and package.json
find . -maxdepth 3 -type f \( -name "package.json" -o -name "webpack.config.*" -o -name "vite.config.*" -o -name "postcss.config.*" -o -name "tailwind.config.*" -o -name "Makefile" \) 2>/dev/nullRepository: GenerateNU/toggo
Length of output: 182
🏁 Script executed:
#!/bin/bash
# Examine the invite_page.go file to see actual CSP/COEP headers
cat -n backend/internal/server/routers/invite_page.go | head -50Repository: GenerateNU/toggo
Length of output: 1346
🏁 Script executed:
#!/bin/bash
# Search for any build or CSS generation comments/config
rg -n -A5 'CSS|stylesheet|static.*css|tailwind.*build' backend/ --type go --type js 2>/dev/null | head -30Repository: GenerateNU/toggo
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Check the backend Makefile to see if there's any CSS build process
cat -n backend/MakefileRepository: GenerateNU/toggo
Length of output: 9246
🏁 Script executed:
#!/bin/bash
# Check frontend/postcss.config.js and tailwind.config.js to understand the CSS build setup
echo "=== Frontend tailwind.config.js ===" && cat frontend/tailwind.config.js
echo -e "\n=== Frontend postcss.config.js ===" && cat frontend/postcss.config.jsRepository: GenerateNU/toggo
Length of output: 439
🏁 Script executed:
#!/bin/bash
# Check if there's any TODO or FIXME related to CDN or CSS
rg -n 'TODO|FIXME|HACK' backend/internal/server/routers/ backend/internal/templates/Repository: GenerateNU/toggo
Length of output: 42
Tailwind CDN dependency should be replaced with prebuilt CSS for production reliability.
Lines 15–24 load and configure Tailwind at request time from a third-party CDN. This introduces supply-chain risk and availability dependency for public-facing pages.
The current approach is intentional (documented in invite_page.go and enabled via relaxed COEP headers), but should be replaced with a static CSS build pipeline if production availability is a priority. This requires:
- Establishing a CSS compilation step in the backend build process (similar to the frontend's PostCSS/Tailwind setup)
- Generating a prebuilt CSS file for these templates
- Updating templates to reference the compiled CSS instead of the CDN script
Consider whether this trade-off is acceptable for these unauthenticated public pages, or track this as a separate refactoring task once backend CSS build infrastructure exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/internal/templates/join_enter_code.html` around lines 15 - 24, The
template join_enter_code.html currently injects Tailwind via CDN and runtime
config; replace this by referencing a prebuilt static CSS file produced during
the backend build: add a build step (similar to frontend PostCSS/Tailwind) that
compiles Tailwind into a single CSS asset (e.g., public/css/invite.css), update
the template to remove the tailwind CDN <script> and config block and instead
include a link to the compiled CSS, and ensure invite_page.go (or the existing
template rendering path) serves or references the generated asset; if backend
build infra doesn't exist yet, track this as a follow-up task and avoid
committing CDN usage to production templates.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
backend/Makefile (1)
14-17:⚠️ Potential issue | 🟠 MajorAdd a hard confirmation gate to
run-prod.Line 14 through Line 17 still allow direct execution with production Doppler config and no explicit opt-in. This can cause accidental production writes during local testing.
Suggested fix
run-prod: + `@test` "$(CONFIRM_PROD)" = "1" || (echo "Refusing to run prod config locally. Re-run with CONFIRM_PROD=1"; exit 1) $(MAKE) build `@echo` "Running backend with prod config (Ctrl+C to stop)..." doppler run --project backend --config prod -- ./$(BINARY_NAME)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/Makefile` around lines 14 - 17, The run-prod Makefile target currently invokes doppler run --config prod and the backend binary directly; add a hard confirmation gate in the run-prod target (referencing the target name run-prod, the BINARY_NAME variable, and the doppler run invocation) so the command only proceeds after explicit opt-in: implement an interactive prompt that requires typing "yes" (or require an explicit env var like CONFIRM_PROD=1) and aborts otherwise, and ensure the prompt runs before calling $(MAKE) build and before executing doppler run --project backend --config prod -- ./$(BINARY_NAME).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/internal/services/invite_page.go`:
- Around line 72-82: The inviter lookup currently swallows errors for
s.User.Find(ctx, invite.CreatedBy) and s.fileService.GetFile(...) while the
membership lookup intentionally returns errors for required data; add a brief
inline comment above the inviter block explaining that inviter information
(data.InviterName and data.InviterProfilePictureURL) is optional display data so
failures from s.User.Find and s.fileService.GetFile are intentionally ignored,
and explicitly state that required membership lookups still propagate
errors—this documents the design difference for future maintainers.
In `@backend/internal/templates/renderer.go`:
- Around line 22-25: The StaticFS function currently ignores the error returned
by fs.Sub which can hide initialization failures; update StaticFS to check the
error from fs.Sub(staticFS, "static") and either return a valid filesystem or
fail fast (panic with a clear message) so callers like ServeStatic() won't get
nil FS later—use the returned sub and err variables, and if err != nil then
panic or log/exit with the error (include the error text and the "static"
context) otherwise return sub.
In `@backend/internal/templates/static/README.md`:
- Line 5: Update the README example path to match the actual asset filename:
replace the example `/static/logo.svg` with `/static/toggo_logo.svg` so the
description for **toggo_logo.svg** and the suggested template usage (`<img
src="...">`) accurately reference the real file name.
In `@backend/internal/utilities/params.go`:
- Line 1: Replace the broad package-level "//nolint:revive" in package utilities
with a proper package comment describing the purpose of the utilities package,
and add doc comments for the exported functions ParseEntityTypeParam,
ParseAndValidateQueryParams, and ExtractLimitAndCursor that explain what each
does and their parameters/returns; if any revive rules still need suppressing,
use targeted inline directives (e.g. "//nolint:revive:exported") next to the
specific declarations instead of a blanket package suppression.
---
Duplicate comments:
In `@backend/Makefile`:
- Around line 14-17: The run-prod Makefile target currently invokes doppler run
--config prod and the backend binary directly; add a hard confirmation gate in
the run-prod target (referencing the target name run-prod, the BINARY_NAME
variable, and the doppler run invocation) so the command only proceeds after
explicit opt-in: implement an interactive prompt that requires typing "yes" (or
require an explicit env var like CONFIRM_PROD=1) and aborts otherwise, and
ensure the prompt runs before calling $(MAKE) build and before executing doppler
run --project backend --config prod -- ./$(BINARY_NAME).
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
backend/internal/templates/static/flight_path.svgis excluded by!**/*.svgbackend/internal/templates/static/logo.svgis excluded by!**/*.svgbackend/internal/templates/static/toggo_logo.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
backend/Makefilebackend/internal/controllers/invite_page.gobackend/internal/models/trip_invite_page.gobackend/internal/server/routers/invite_page.gobackend/internal/server/routers/routers.gobackend/internal/services/invite_page.gobackend/internal/services/trips.gobackend/internal/templates/join_enter_code.htmlbackend/internal/templates/renderer.gobackend/internal/templates/static/README.mdbackend/internal/templates/trip_invite.htmlbackend/internal/utilities/params.gofrontend/app/(app)/testing/index.tsxfrontend/app/(app)/testing/test-upload.tsx
Comment on lines
+72
to
+82
| // Look up the inviter's name and profile picture | ||
| inviter, err := s.User.Find(ctx, invite.CreatedBy) | ||
| if err == nil { | ||
| data.InviterName = inviter.Name | ||
| if inviter.ProfilePicture != nil { | ||
| fileResp, err := s.fileService.GetFile(ctx, *inviter.ProfilePicture, models.ImageSizeSmall) | ||
| if err == nil { | ||
| data.InviterProfilePictureURL = &fileResp.URL | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inconsistent error handling between optional and required data.
The inviter lookup (lines 73-82) silently ignores errors while the membership lookup (lines 109-111) propagates them. This is likely intentional since inviter info is optional display data while members are required. Consider adding a brief comment explaining this distinction for future maintainers.
Also applies to: 108-112
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/internal/services/invite_page.go` around lines 72 - 82, The inviter
lookup currently swallows errors for s.User.Find(ctx, invite.CreatedBy) and
s.fileService.GetFile(...) while the membership lookup intentionally returns
errors for required data; add a brief inline comment above the inviter block
explaining that inviter information (data.InviterName and
data.InviterProfilePictureURL) is optional display data so failures from
s.User.Find and s.fileService.GetFile are intentionally ignored, and explicitly
state that required membership lookups still propagate errors—this documents the
design difference for future maintainers.
Comment on lines
+22
to
+25
| func StaticFS() fs.FS { | ||
| sub, _ := fs.Sub(staticFS, "static") | ||
| return sub | ||
| } |
Contributor
There was a problem hiding this comment.
Ignoring error from fs.Sub can mask initialization failures.
If the embedded "static" directory is missing or misnamed, fs.Sub will return an error and a nil filesystem. Subsequent fs.ReadFile calls in ServeStatic() will fail at runtime with potentially confusing errors. Handle the error at initialization or use Must-style panic.
Proposed fix
func StaticFS() fs.FS {
- sub, _ := fs.Sub(staticFS, "static")
+ sub, err := fs.Sub(staticFS, "static")
+ if err != nil {
+ panic(fmt.Sprintf("failed to get static subtree: %v", err))
+ }
return sub
}Note: You will need to import "fmt" if not already present.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/internal/templates/renderer.go` around lines 22 - 25, The StaticFS
function currently ignores the error returned by fs.Sub which can hide
initialization failures; update StaticFS to check the error from
fs.Sub(staticFS, "static") and either return a valid filesystem or fail fast
(panic with a clear message) so callers like ServeStatic() won't get nil FS
later—use the returned sub and err variables, and if err != nil then panic or
log/exit with the error (include the error text and the "static" context)
otherwise return sub.
|
|
||
| Put assets here for backend-rendered HTML (invite pages, etc.). They are embedded into the binary and served at `/static/*`. | ||
|
|
||
| - **toggo_logo.svg** – App logo (use in templates via `<img src="/static/logo.svg" alt="..." />` or inline) |
Contributor
There was a problem hiding this comment.
Example path does not match actual filename.
The example shows /static/logo.svg but the actual asset is toggo_logo.svg. Update the example to use /static/toggo_logo.svg for consistency with the actual file and template usage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/internal/templates/static/README.md` at line 5, Update the README
example path to match the actual asset filename: replace the example
`/static/logo.svg` with `/static/toggo_logo.svg` so the description for
**toggo_logo.svg** and the suggested template usage (`<img src="...">`)
accurately reference the real file name.
| @@ -1,4 +1,4 @@ | |||
| package utilities | |||
| package utilities //nolint:revive | |||
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add package documentation instead of suppressing linter warnings.
The blanket //nolint:revive directive suppresses all revive warnings for the entire package. This masks code quality issues and can hide future problems.
Add a package comment to describe the utilities package and document the exported functions. If specific rules must be suppressed, use targeted directives like //nolint:revive:exported.
📝 Proposed fix with documentation
-package utilities //nolint:revive
+// Package utilities provides helper functions for parsing and validating HTTP request parameters.
+package utilitiesThen add doc comments for exported functions:
// ParseEntityTypeParam extracts and validates an entity type from route parameters.
func ParseEntityTypeParam(c *fiber.Ctx, paramName string, errorField string, allowed ...models.EntityType) (models.EntityType, error) {// ParseAndValidateQueryParams parses query parameters into a struct and validates them.
func ParseAndValidateQueryParams(c *fiber.Ctx, validator *validator.Validate, params interface{}) error {// ExtractLimitAndCursor extracts pagination parameters from cursor-based query params.
func ExtractLimitAndCursor(params cursorQueryParams) (int, string) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/internal/utilities/params.go` at line 1, Replace the broad
package-level "//nolint:revive" in package utilities with a proper package
comment describing the purpose of the utilities package, and add doc comments
for the exported functions ParseEntityTypeParam, ParseAndValidateQueryParams,
and ExtractLimitAndCursor that explain what each does and their
parameters/returns; if any revive rules still need suppressing, use targeted
inline directives (e.g. "//nolint:revive:exported") next to the specific
declarations instead of a blanket package suppression.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
backend/internal/models/trip_invite_page.go (1)
3-10:⚠️ Potential issue | 🟠 MajorUse
stringforDeepLinkinstead oftemplate.URL.Line 9 marks deeplink data as trusted content. That weakens escaping guarantees for a value that is ultimately rendered into HTML/JS contexts. Keep
DeepLinkasstringand validate allowed schemes when composing it in the service layer.Proposed fix (model side)
-import "html/template" - // TripInvitePageData contains data rendered by the invite landing page. type TripInvitePageData struct { @@ - DeepLink template.URL + DeepLink string#!/bin/bash # Verify current deep link typing + construction/validation points. rg -n -C3 'type TripInvitePageData|DeepLink|DEEPLINK_BASE_URL|url\.Parse|Scheme' \ backend/internal/models/trip_invite_page.go \ backend/internal/services/invite_page.go \ backend/internal/templates/trip_invite.html🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/models/trip_invite_page.go` around lines 3 - 10, Change the TripInvitePageData.DeepLink field from template.URL to string (update the TripInvitePageData struct and remove the html/template import) so deeplinks are treated as untrusted text; then ensure the service that constructs the deep link (where DEEPLINK_BASE_URL / url.Parse / Scheme are used) validates allowed schemes and percent-encodes components before assigning the string to DeepLink.backend/internal/templates/trip_invite.html (1)
274-292:⚠️ Potential issue | 🟠 MajorMake fallback store URL platform-aware in
openApp().Line 276 hardcodes iOS App Store fallback. Android users may be redirected incorrectly when deep linking fails.
Proposed fix
function openApp() { var deepLink = "{{ .Data.DeepLink }}"; - var appStoreURL = "https://apps.apple.com"; + var iosStoreURL = "https://apps.apple.com"; + var androidStoreURL = "https://play.google.com/store/apps/details?id=com.toggo.app"; + var fallbackURL = /Android/i.test(navigator.userAgent) ? androidStoreURL : iosStoreURL; var didNavigate = false; @@ setTimeout(function() { if (!didNavigate) { - window.location.href = appStoreURL; + window.location.href = fallbackURL; } }, 1500); }#!/bin/bash # Verify current fallback behavior in the template. rg -n -C3 'function openApp|apps\.apple\.com|play\.google\.com|fallback' \ backend/internal/templates/trip_invite.html🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/templates/trip_invite.html` around lines 274 - 292, The openApp() function currently hardcodes appStoreURL to the iOS App Store; change it to detect platform (e.g., inspect navigator.userAgent / navigator.userAgentData / navigator.platform) and set appStoreURL conditionally (Android → Google Play URL like "https://play.google.com/..." , iOS → "https://apps.apple.com", otherwise a sensible default or web fallback). Update the logic in openApp() (refer to the deepLink variable and didNavigate flag) so the setTimeout fallback uses the platform-aware appStoreURL instead of the hardcoded iOS link.backend/internal/templates/join_enter_code.html (1)
15-24:⚠️ Potential issue | 🟠 MajorReplace runtime Tailwind CDN usage with a prebuilt static CSS asset.
Lines 15-24 add a runtime dependency on third-party JS for styling on a public entry page. This increases outage and supply-chain exposure for onboarding traffic.
Proposed template change
- <script src="https://cdn.tailwindcss.com"></script> - <script> - tailwind.config = { - theme: { - extend: { - colors: { toggo: '#FF7E00' } - } - } - } - </script> + <link rel="stylesheet" href="/static/invite.css" />#!/bin/bash # Verify CDN usage and whether prebuilt CSS exists in embedded static assets. rg -n 'cdn\.tailwindcss\.com|tailwind\.config|/static/.*\.css' backend/internal/templates fd -a '.*\.css' backend/internal/templates/static🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/templates/join_enter_code.html` around lines 15 - 24, Remove the runtime Tailwind CDN <script src="https://cdn.tailwindcss.com"></script> and the inline tailwind.config script block; instead reference a prebuilt static stylesheet from your embedded assets (e.g., add a <link rel="stylesheet" href="/static/prebuilt-tailwind.css">) and move the custom color override (toggo: '#FF7E00') into that CSS (via :root or utility class) so styling no longer depends on tailwind.config or the remote CDN.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@backend/internal/models/trip_invite_page.go`:
- Around line 3-10: Change the TripInvitePageData.DeepLink field from
template.URL to string (update the TripInvitePageData struct and remove the
html/template import) so deeplinks are treated as untrusted text; then ensure
the service that constructs the deep link (where DEEPLINK_BASE_URL / url.Parse /
Scheme are used) validates allowed schemes and percent-encodes components before
assigning the string to DeepLink.
In `@backend/internal/templates/join_enter_code.html`:
- Around line 15-24: Remove the runtime Tailwind CDN <script
src="https://cdn.tailwindcss.com"></script> and the inline tailwind.config
script block; instead reference a prebuilt static stylesheet from your embedded
assets (e.g., add a <link rel="stylesheet"
href="/static/prebuilt-tailwind.css">) and move the custom color override
(toggo: '#FF7E00') into that CSS (via :root or utility class) so styling no
longer depends on tailwind.config or the remote CDN.
In `@backend/internal/templates/trip_invite.html`:
- Around line 274-292: The openApp() function currently hardcodes appStoreURL to
the iOS App Store; change it to detect platform (e.g., inspect
navigator.userAgent / navigator.userAgentData / navigator.platform) and set
appStoreURL conditionally (Android → Google Play URL like
"https://play.google.com/..." , iOS → "https://apps.apple.com", otherwise a
sensible default or web fallback). Update the logic in openApp() (refer to the
deepLink variable and didNavigate flag) so the setTimeout fallback uses the
platform-aware appStoreURL instead of the hardcoded iOS link.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
backend/internal/templates/static/flight_path.svgis excluded by!**/*.svgbackend/internal/templates/static/logo.svgis excluded by!**/*.svgbackend/internal/templates/static/toggo_logo.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
backend/Makefilebackend/internal/controllers/invite_page.gobackend/internal/models/trip_invite_page.gobackend/internal/server/routers/invite_page.gobackend/internal/server/routers/routers.gobackend/internal/services/invite_page.gobackend/internal/services/trips.gobackend/internal/templates/join_enter_code.htmlbackend/internal/templates/renderer.gobackend/internal/templates/static/README.mdbackend/internal/templates/trip_invite.htmlbackend/internal/utilities/params.gofrontend/app/(app)/testing/index.tsxfrontend/app/(app)/testing/test-upload.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
gazzam
This is the join code page for a trip; if you think about it, technically counts as the initial step for onboarding.
There are changes that need to come; mainly including the dates and potential pitches for the trip dependent on the phase of the trip planning process. In order to not block onboarding development, I'm putting this through.
Another feature of this PR: testing with a local backend and prod database. Mainly because I think we're getting to the point where we want an appropriately seeded database. Mock data could be a coming ticket.
If reviewing this PR, sign up through the command
cd backend && make run-prod. This will connect your local to the prod db, and you'll be able to check functionality of this PR. If you were a really cool person, you'd add yourself to the trip via the API endpoints, and see if ur profile picture also gets populated.Here's a link to the url:
127.0.0.1:8000/join?code=c4c8242550f3
feel free to test in browser & ios. ios optimal tho.
Justifications
TailwindCSS has some issues with rendering server-side because of various CORS issues. Thus, I felt that the best approach would just be raw HTML and CSS, which flexes pretty well to web.
Deeplinking currently points to different apps depending on if Expo Go is installed. Once we're deployed to TestFlight or out of development, the variable can change in Doppler.
How has this been tested?
Checklist
User-Visible Changes
/join?code=...allowing users to enter invite codes to view trip detailsChanges by Category
Features
Improvements
/join?code={code}URL format instead of/invites/{code}relaxCOEPmiddleware for invite page routesInfra
run-prodto run local backend against production database.PHONYdeclaration for new Make targetsAuthor Contribution