feat: implement phases 1D-1G (iCal import, notifications, settings, website/API) + deploy checklist#2
Open
nathanialhenniges wants to merge 5 commits intomainfrom
Open
Conversation
…ebsite/API) + deploy checklist - Phase 1D: DB schema (Drizzle/SQLite), iCal parser, Sched extractor, import hook, convention detail screen, import UI, Vitest tests - Phase 1E: Notification service, event reminders hook, action sheet, bell icon, permission banner - Phase 1F: Data export/import services, settings/profile/about screen rewrites, empty states, haptics - Phase 1G: Hono API (Cloudflare Workers), Next.js static site (Cloudflare Pages), Fumadocs scaffold - TODO.md: step-by-step go-live checklist for deploying API + website Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
WalkthroughAdds a monthly license-year updater and a large set of native, web, and server features: Expo native app (UI kit, onboarding, tabs), local Drizzle/SQLite DB + repositories, ICS parsing and sched.com fetcher, reminders/notifications, import/export, i18n (8 locales), React Query hooks, new web pages and lead-capture, server subscribe endpoint, docs, and various tooling/config updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as Native App
participant Storage as AsyncStorage
participant Onboard as Onboarding
participant Tabs as Main Tabs
User->>App: Launch
App->>Storage: read hasCompletedOnboarding
alt not completed
App->>Onboard: show onboarding flow
Onboard->>Storage: write hasCompletedOnboarding = "true"
Onboard->>Tabs: navigate to main tabs
else completed
App->>Tabs: navigate to main tabs
end
sequenceDiagram
autonumber
actor User
participant Home as HomeScreen
participant Query as ReactQuery
participant Repo as EventsRepo
participant Detail as ConventionDetail
participant Notif as NotificationService
User->>Home: open app
Home->>Query: fetch conventions
Query->>Repo: getAll()
Repo-->>Query: conventions
User->>Home: select convention
Home->>Detail: navigate
Detail->>Query: fetch events
Query->>Repo: getByConventionId()
Repo-->>Query: events
User->>Detail: set reminder
Detail->>Notif: scheduleEventReminder(event, minutes)
Notif->>Repo: update reminderMinutes
Repo->>Query: invalidate events
Query->>Detail: refreshed events
sequenceDiagram
autonumber
actor User
participant Import as ImportScreen
participant Fetch as Network/File
participant Parser as ICSParser
participant Preview as PreviewUI
participant Mutate as ImportMutation
participant Repo as EventsRepo
participant Query as ReactQuery
User->>Import: pick .ics file or enter sched URL
Import->>Fetch: read file / fetch URL
Fetch-->>Import: ICS text
Import->>Parser: parseIcs(raw)
Parser-->>Preview: events + categories
User->>Preview: confirm import
Preview->>Mutate: mutate(parsedEvents)
Mutate->>Repo: upsertBySourceUid(parsedEvents)
Repo-->>Mutate: upsert result
Mutate->>Query: invalidate events
Query->>User: success UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 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🧪 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 |
Runs on Jan 1 at 06:00 UTC (midnight CST) and opens a PR to bump the copyright year in LICENSE. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (10)
apps/native/src/locales/pl.json-37-37 (1)
37-37:⚠️ Potential issue | 🟡 MinorFix two Polish strings that currently change the intended meaning.
- Line 37:
"w tym samym konie"reads as “in the same horse”; likely intended “in the same convention”.- Line 124:
"Zresetuj Wdrożenie"means “Reset deployment”, but key is onboarding reset.Suggested wording update
- "description": "Utwórz swój osobisty harmonogram i udostępnij go przyjaciołom uczestniczącym w tym samym konie." + "description": "Utwórz swój osobisty harmonogram i udostępnij go przyjaciołom uczestniczącym w tym samym konwencie." ... - "resetOnboarding": "Zresetuj Wdrożenie" + "resetOnboarding": "Zresetuj onboarding"Also applies to: 124-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/src/locales/pl.json` at line 37, Replace the incorrect Polish phrasing in the locales file: update the "description" value that currently ends with "w tym samym konie" to use the correct word for convention, e.g. change it to "Utwórz swój osobisty harmonogram i udostępnij go przyjaciołom uczestniczącym w tym samym konwencie."; also update the onboarding-related string that currently reads "Zresetuj Wdrożenie" to a proper onboarding phrase such as "Zresetuj proces powitalny" (or another approved translation of "Reset onboarding") so it no longer implies deployment.apps/native/src/locales/sv.json-39-42 (1)
39-42:⚠️ Potential issue | 🟡 MinorFix the Swedish offline copy.
Line 41 reads ungrammatically as
Ingen internet krävs. UseIngen internetanslutning krävsor equivalent before shipping.✏️ Suggested copy tweak
- "description": "All din data lagras lokalt. Ingen internet krävs när du har importerat ditt schema." + "description": "All din data lagras lokalt. Ingen internetanslutning krävs när du har importerat ditt schema."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/src/locales/sv.json` around lines 39 - 42, Update the Swedish copy for the offline screen by fixing the ungrammatical phrase in the JSON key offline.description; replace "Ingen internet krävs" with "Ingen internetanslutning krävs" (or an equivalent correct phrase) so the value for "offline.description" reads correctly alongside "offline.title".apps/native/src/components/ConventionCard.tsx-5-14 (1)
5-14:⚠️ Potential issue | 🟡 Minor
idprop is declared but unused.The
idproperty is defined inConventionCardPropsbut never referenced in the component body. Either remove it or use it (e.g., for accessibilitytestIDor analytics).🧹 Proposed fix to remove unused prop
interface ConventionCardProps { - id: string; name: string; startDate: string; endDate: string;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/src/components/ConventionCard.tsx` around lines 5 - 14, The id prop in ConventionCardProps is declared but unused; keep the prop and wire it into the ConventionCard component by applying it as a stable identifier (e.g., set testID or accessibilityLabel on the root touchable/View) and include it in any analytics payloads inside onPress (e.g., pass id to the analytics event), so id is consumed; update the ConventionCard component to read props.id and use it for testID/accessibilityLabel and in the onPress analytics call.apps/server/src/index.ts-51-63 (1)
51-63:⚠️ Potential issue | 🟡 MinorMissing timeout on the Brevo API call.
The fetch to Brevo has no timeout. If Brevo is slow or unresponsive, the Worker could hang until Cloudflare's execution limit kicks in. Consider adding an
AbortControllerwith a reasonable timeout (e.g., 10 seconds).⏱️ Proposed fix to add timeout
// Add to Brevo try { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 10_000); const res = await fetch('https://api.brevo.com/v3/contacts', { method: 'POST', headers: { 'Content-Type': 'application/json', 'api-key': c.env.BREVO_API_KEY, }, body: JSON.stringify({ email: email.trim().toLowerCase(), attributes: { FIRSTNAME: name.trim() }, listIds: [parseInt(c.env.BREVO_LIST_ID, 10)], updateEnabled: true, }), + signal: controller.signal, }); + clearTimeout(timeout);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/index.ts` around lines 51 - 63, The Brevo fetch call lacks a timeout and can hang; wrap the request in an AbortController (create controller and a setTimeout for ~10000 ms that calls controller.abort()), pass controller.signal to the fetch options used when creating `res`, and clear the timeout after fetch completes; update the block that constructs the POST to 'https://api.brevo.com/v3/contacts' (the code using `fetch(...)`, `res`, `c.env.BREVO_API_KEY`, and `c.env.BREVO_LIST_ID`) so the request is aborted on timeout and does not leak the timer.apps/native/package.json-47-47 (1)
47-47:⚠️ Potential issue | 🟡 MinorWildcard version
"*"forreact-native-workletsis inconsistent with project standards.All other dependencies use specific versions or semver ranges. Pin to a specific version such as
0.7.4(the current latest) to ensure reproducible builds and avoid unexpected breaking changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/package.json` at line 47, Replace the wildcard dependency for "react-native-worklets" in package.json with a pinned version (e.g., "0.7.4") to match the project's dependency versioning policy; update the "react-native-worklets" entry in package.json from "*" to the chosen specific version and run install to verify reproducible builds.apps/native/app/(tabs)/profile.tsx-26-27 (1)
26-27:⚠️ Potential issue | 🟡 MinorDisable non-functional auth buttons until implemented.
Both handlers are empty callbacks, but the controls still behave as tappable actions. Mark them disabled to avoid misleading interactions.
♻️ Proposed fix
- <Button variant="default" size="lg" onPress={() => {}} className="w-full opacity-50"> + <Button variant="default" size="lg" disabled onPress={() => {}} className="w-full opacity-50"> ... - <Button variant="outline" size="lg" onPress={() => {}} className="w-full opacity-50"> + <Button variant="outline" size="lg" disabled onPress={() => {}} className="w-full opacity-50">Also applies to: 38-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/app/`(tabs)/profile.tsx around lines 26 - 27, The sign-in Buttons use empty onPress callbacks and remain tappable; mark them non-interactive by adding the disabled prop to the Button instances (the one rendering {t('profile.signInWithApple')} and the other sign-in Button), remove or replace the empty onPress handlers with undefined/no-op only if the component still requires it, and add a visual and accessibility hint (e.g. keep opacity-50 and add a disabled class like cursor-not-allowed or aria-disabled when supported) so they are clearly non-functional until implemented.apps/native/src/locales/de.json-143-143 (1)
143-143:⚠️ Potential issue | 🟡 MinorFix German label wording/capitalization for import preview.
Line 143 currently says
Events vorschau; in German this should be capitalized/compounded (for example,Event-Vorschau).✍️ Suggested correction
- "preview": "Events vorschau", + "preview": "Event-Vorschau",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/src/locales/de.json` at line 143, The German localization value for the "preview" key is incorrectly cased/worded ("Events vorschau"); update the string value for "preview" to the correct German compound and capitalization, e.g., "Event-Vorschau" (or "Event‑Vorschau" with a non-breaking hyphen) so the label reads properly in German.apps/native/app/convention/[id].tsx-315-320 (1)
315-320:⚠️ Potential issue | 🟡 MinorDon't ship a dead-end CTA.
+ Add event manuallyrenders as a tappable action, but its handler is empty. Hide it or disable it until manual entry exists.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/app/convention/`[id].tsx around lines 315 - 320, The "+ Add event manually" CTA is rendered with an empty onPress handler on the Pressable; either remove/hide the Pressable or make it non-interactive until manual entry is implemented. Update the JSX around the Pressable (the Pressable component with onPress={() => {}} and its child Text "+ Add event manually") to conditionally render only when a feature flag or state (e.g., manualEntryEnabled / hasManualEntry) is true, or replace the empty onPress with a disabled state (no-op + aria-disabled/disabled styling) so it is visually and functionally inactive until the manual entry path exists.apps/native/app/convention/[id]/import.tsx-234-242 (1)
234-242:⚠️ Potential issue | 🟡 MinorMake Retry reopen the picker on file errors.
On the file tab, this button only clears
error; it never callshandleFilePick(). Users still need a second tap onChoose .ics File, so this is not a real retry action.♻️ Suggested fix
<Button variant="outline" onPress={() => { setError(null); - if (activeTab === 'url') handleUrlFetch(); + if (activeTab === 'file') { + void handleFilePick(); + } else { + void handleUrlFetch(); + } }} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/app/convention/`[id]/import.tsx around lines 234 - 242, The Retry button currently only clears the error and calls handleUrlFetch when activeTab === 'url', but it does not reopen the file picker for the 'file' tab; update the Button onPress handler to, after setError(null), call handleFilePick() when activeTab === 'file' (parallel to the existing handleUrlFetch call) so that clicking Retry on file errors immediately re-triggers the file selection flow; locate this behavior in the component where the Button's onPress references setError, activeTab, handleUrlFetch and add the conditional call to handleFilePick.apps/native/app/convention/[id]/import.tsx-53-55 (1)
53-55:⚠️ Potential issue | 🟡 MinorLocalize all user-facing strings using the
t()function that's already imported.The following untranslated English literals will render as-is in non-English locales, since the component imports and has access to
useTranslation():
- Error messages (lines 54, 78, 86, 102–108)
- Dialog/alert titles and text (lines 144–149)
- UI labels and CTAs: "Import Schedule", "From File", "From URL", "Choose .ics File", "Fetch Schedule", "Loading...", "Retry", event counts, import button text (lines 166, 183, 193, 208, 217, 241, 249, 282–284)
Wrap these with
t()calls to enable proper translation coverage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/app/convention/`[id]/import.tsx around lines 53 - 55, Several user-facing literals in the import component must be wrapped with the translation function t(); replace naked strings passed to setError (e.g., setError({ type: 'no-events', message: 'No events found in this calendar file.' })), dialog/alert titles and bodies, and all button/label text ("Import Schedule", "From File", "From URL", "Choose .ics File", "Fetch Schedule", "Loading...", "Retry", event counts, import button text, etc.) with t('...') calls so the component uses useTranslation(). Update any event count or dynamic strings to use t() with interpolation/variables where needed, and ensure the existing import/useTranslation() usage provides the t function for these replacements.
🧹 Nitpick comments (24)
apps/web/src/app/page.tsx (1)
167-167: Make copyright year dynamic.Hardcoding
2026will age quickly; use runtime year to avoid annual edits.Suggested fix
- <p>© 2026 ConPaws. All rights reserved.</p> + <p>© {new Date().getFullYear()} ConPaws. All rights reserved.</p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/page.tsx` at line 167, Replace the hardcoded "2026" in the footer paragraph with a runtime year expression: locate the JSX paragraph containing "© 2026 ConPaws. All rights reserved." (in the page component render) and change the text to use {new Date().getFullYear()} so it renders the current year dynamically (e.g., "© {new Date().getFullYear()} ConPaws. All rights reserved.").apps/native/.env.example (1)
1-2: Consider documenting validAPP_VARIANTvalues.Adding a comment with the valid options (e.g.,
development,preview,production) would help developers understand the expected values, similar to how other env files often include inline documentation.📝 Suggested improvement
+# App variant: development | preview | production APP_VARIANT=🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/.env.example` around lines 1 - 2, Add an inline comment above or beside the APP_VARIANT entry in apps/native/.env.example that documents the accepted values (for example: development, preview, production) and an optional recommended default; update the APP_VARIANT line to show an example value or placeholder (e.g., APP_VARIANT=development) so developers immediately see valid options and intended usage.apps/web/src/app/terms/page.tsx (1)
9-10: Centralize the legal-doc revision date.
/termsand/privacynow hard-code the same “Last updated” string. Pull this from a shared constant so the two pages don’t drift the next time one of them changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/terms/page.tsx` around lines 9 - 10, Extract the hard-coded revision date into a shared constant (e.g., export const LEGAL_LAST_UPDATED = "March 17, 2026") in a common module (like a new/legal constants file) and replace the inline string in the Terms page's JSX (the h1/p block rendering "Last updated: ...") with an import of LEGAL_LAST_UPDATED; do the same replacement in the Privacy page so both pages read the date from the same exported symbol (LEGAL_LAST_UPDATED) to prevent drift.apps/web/src/app/layout.tsx (1)
8-18: Avoid baking the production domain into metadata.Preview/staging deploys will emit production Open Graph URLs from every environment. Pull the site URL from shared config/env so metadata matches the actual deployment.
♻️ Suggested refactor
+const siteUrl = process.env.SITE_URL ?? 'https://conpaws.com'; + export const metadata: Metadata = { title: 'ConPaws — Your Furry Convention Companion', description: 'Navigate conventions, import schedules, build your personal agenda, and never miss a panel. Coming to iOS & Android.', openGraph: { title: 'ConPaws — Your Furry Convention Companion', description: 'Navigate conventions, import schedules, build your personal agenda.', - url: 'https://conpaws.com', + url: siteUrl, siteName: 'ConPaws', type: 'website', },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/layout.tsx` around lines 8 - 18, The Open Graph and Twitter metadata in layout.tsx currently hardcode the production URL ('https://conpaws.com') causing preview/staging to emit production URLs; update the openGraph.url and corresponding twitter fields to read the site base URL from the shared configuration/environment (e.g., import and use the central siteUrl value from your env/config module) instead of the literal string, and ensure the openGraph and twitter objects (openGraph, twitter) use that runtime config so metadata reflects the current deployment.apps/server/wrangler.toml (1)
6-7: Consider environment-specific list IDs.
BREVO_LIST_ID = "1"is hardcoded. If you need different lists for staging vs production, consider making this a secret as well, or using Wrangler environments ([env.production],[env.staging]).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/wrangler.toml` around lines 6 - 7, The BREVO_LIST_ID is hardcoded in wrangler.toml under [vars] which prevents environment-specific values; move BREVO_LIST_ID into per-environment sections (e.g., [env.production], [env.staging]) or mark it as a secret and configure it via environment-specific secrets so each deploy uses the correct list ID; update any code that reads BREVO_LIST_ID to expect it from environment variables (process.env or the platform's runtime env) rather than a single top-level [vars] entry.apps/server/package.json (1)
5-9: Consider adding a test script.The package has
type-checkbut notestscript. Given the PR objectives mention runningbun testacross the monorepo, you may want to add a placeholder or actual test configuration for the server package.📝 Proposed addition
"scripts": { "dev": "wrangler dev", "deploy": "wrangler deploy", - "type-check": "tsc --noEmit" + "type-check": "tsc --noEmit", + "test": "echo 'No tests yet' && exit 0" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/package.json` around lines 5 - 9, Add a "test" script to the package.json scripts block so the server package can participate in monorepo test runs; update the "scripts" object (alongside "dev", "deploy", "type-check") to include a "test" entry that invokes the chosen test runner (for example "bun test" if using Bun across the monorepo, or "npm test" / "vitest" / "jest" as appropriate), and ensure the script name is exactly "test" so CI/monorepo tooling picks it up.apps/server/src/index.ts (1)
44-47: Email regex may reject valid addresses.The regex
/^[^\s@]+@[^\s@]+\.[^\s@]+$/rejects valid edge cases likeuser@localhostor addresses with IP literals. For a subscription form this is likely acceptable, but consider that Brevo also validates emails and returns400for invalid formats (handled at Line 77-78).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/index.ts` around lines 44 - 47, The current strict emailRegex (/^[^\s@]+@[^\s@]+\.[^\s@]+$/) in apps/server/src/index.ts can reject valid addresses; update the validation in the block that uses emailRegex so it only checks that email exists and is a non-empty string (e.g., remove the regex test) or replace it with a more permissive pattern (e.g., /^[^\s@]+@[^\s@]+$/) and keep returning the same c.json({ error: 'Valid email is required' }, 400) when the basic check fails; this change involves the emailRegex constant and the conditional that returns the 400 response so Brevo can perform final format validation.apps/native/src/lib/sched-extractor.ts (2)
22-22: SCHED_PATTERN allows HTTP (non-secure) URLs.The regex
^https?://accepts both HTTP and HTTPS. Consider enforcing HTTPS-only for security, especially since this fetches potentially sensitive calendar data.🔒 Proposed fix to enforce HTTPS
-const SCHED_PATTERN = /^https?:\/\/([a-zA-Z0-9-]+)\.sched\.com(\/.*)?$/; +const SCHED_PATTERN = /^https:\/\/([a-zA-Z0-9-]+)\.sched\.com(\/.*)?$/;Update the error message in
InvalidSchedUrlErroraccordingly:- super(`Invalid Sched URL: "${url}". Expected format: https://yourcon.sched.com`); + super(`Invalid Sched URL: "${url}". Expected format: https://<subdomain>.sched.com`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/src/lib/sched-extractor.ts` at line 22, SCHED_PATTERN currently allows both HTTP and HTTPS because it uses ^https?://; change the regex to require HTTPS only (e.g., ^https://...) so non-secure URLs are rejected, and update the InvalidSchedUrlError message to state that only HTTPS Sched URLs are accepted; ensure any validation logic that tests SCHED_PATTERN (wherever SCHED_PATTERN is referenced) still behaves the same after tightening the pattern.
53-60: Consider distinguishing HTTP errors from network failures.Both non-2xx HTTP responses (Line 42) and actual network failures (Line 60) throw
NetworkError. This may make it harder for callers to differentiate between "server returned 404" vs "network unreachable". Consider introducing anHttpErrorclass for non-2xx responses.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/src/lib/sched-extractor.ts` around lines 53 - 60, The catch block currently conflates HTTP non-2xx responses and real network failures under NetworkError; introduce a new HttpError class (e.g., class HttpError extends Error { status: number; statusText?: string; body?: any }) and update the code path that handles non-2xx responses to throw HttpError with status/statusText/body; then adjust this catch to first rethrow HttpError (alongside InvalidSchedUrlError and InvalidResponseError), keep the AbortError => NetworkError mapping, and only wrap other errors as NetworkError so callers can distinguish HTTP errors (HttpError) from network failures (NetworkError).apps/native/package.json (1)
63-65: Document the reason for thelightningcssoverride.The override pins
lightningcssto1.30.1. Consider adding a comment in the package.json or a note explaining why this override is necessary (e.g., compatibility issue with another dependency).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/package.json` around lines 63 - 65, Add explicit documentation for the "overrides" entry that pins "lightningcss" to "1.30.1": explain the compatibility or security reason for the pin (e.g., conflicting dependency version, regression or vulnerability) and where it was discovered. Since package.json does not support comments, add the rationale as either a new top-level key like "overridesReason" or "resolutionsNote" in package.json (alongside the existing "overrides": { "lightningcss": "1.30.1" }) or record the explanation in the repository README/DEVELOPER_NOTES referencing the exact override line so future maintainers know why "lightningcss": "1.30.1" was pinned.apps/native/src/components/ConventionCard.tsx (2)
16-20: Status labels are not internationalized.Given the app has i18n support (per learnings), consider using translation keys instead of hardcoded English strings for consistency.
🌐 Proposed i18n approach
+import { useTranslation } from 'react-i18next'; + -const statusLabels: Record<ConventionCardProps['status'], string> = { - upcoming: 'Upcoming', - active: 'Active', - ended: 'Ended', -}; +const statusLabelKeys: Record<ConventionCardProps['status'], string> = { + upcoming: 'convention.status.upcoming', + active: 'convention.status.active', + ended: 'convention.status.ended', +}; export function ConventionCard({ ... }: ConventionCardProps) { + const { t } = useTranslation(); return ( ... - <Badge variant={status} label={statusLabels[status]} /> + <Badge variant={status} label={t(statusLabelKeys[status])} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/src/components/ConventionCard.tsx` around lines 16 - 20, Replace the hardcoded English strings in the statusLabels map with translation keys and call the app's i18n translator (e.g., the component's existing translation hook/function) when rendering; specifically update the statusLabels definition (used by ConventionCard and keyed by ConventionCardProps['status']) to return translation keys like 'convention.status.upcoming'/'active'/'ended' and invoke the translator (t(...)) in the ConventionCard render path so labels are localized rather than literal English.
44-48: Event count pluralization could use i18n plural forms.The inline pluralization
{eventCount !== 1 ? 's' : ''}won't work correctly for all languages. Consider using i18next's pluralization feature.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/src/components/ConventionCard.tsx` around lines 44 - 48, The current inline pluralization using {eventCount !== 1 ? 's' : ''} in the ConventionCard component should be replaced with i18next pluralization: import and call useTranslation (e.g., const { t } = useTranslation()) in the ConventionCard component and replace the Text content that uses eventCount with a t(...) call that passes count: eventCount (for example t('events', { count: eventCount }) or t('convention.events', { count: eventCount })), then add the matching pluralization keys to your i18n resource files so translations use correct plural forms across languages.apps/native/src/services/data-import.ts (2)
139-141: Alert messages are hardcoded in English.The app uses i18next for internationalization, but these Alert messages bypass the translation system. Consider using
t()from i18next or moving these strings to locale files for consistency with the rest of the app.Based on learnings: "Use i18next + react-i18next with expo-localization for internationalization, with translation files in src/locales/".
Also applies to: 162-164, 168-168
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/src/services/data-import.ts` around lines 139 - 141, Replace hardcoded Alert.alert strings in the data-import flow with translated strings using i18next: import and use the t() function to look up messages (e.g., for the title "Import Data" and the confirmation body that interpolates convCount and eventCount), and move those translation keys into the locale files under src/locales/ (add plural/interpolation entries for the convention/event counts). Update each Alert.alert invocation (the one at the shown diff and the other occurrences around lines 162–168) to call t('yourKey', { convCount, eventCount }) or appropriate pluralized keys so the alerts are localized via i18next/react-i18next. Ensure the component has access to t() (via useTranslation or passing t) before replacing the strings.
67-73: Redundant database fetch can be eliminated.
conventionsRepo.getAll()is called twice: once at line 42 and again at line 68. After importing conventions, you can reuse and extend theexistingIdsset instead of re-fetching.♻️ Proposed optimization
- // Import events (skip existing IDs) - const existingAllConventions = await conventionsRepo.getAll(); const allEventIds = new Set<string>(); - for (const conv of existingAllConventions) { + for (const conv of existingConventions) { const events = await eventsRepo.getByConventionId(conv.id); events.forEach((e) => allEventIds.add(e.id)); } + // Also fetch events for newly added conventions (they should have none, but for completeness)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/src/services/data-import.ts` around lines 67 - 73, The code redundantly calls conventionsRepo.getAll() again to build allEventIds; instead reuse the previously obtained existingIds set (from the earlier conventions import) and extend it with event IDs to avoid the second DB fetch: iterate the existing conventions collection you already have (or the existingIds source) and for each convention call eventsRepo.getByConventionId(conv.id) and add event.id into the same set (e.g., existingIds or allEventIds) so you remove the extra conventionsRepo.getAll() call and reuse existingIds across the import logic.apps/native/src/components/ui/Avatar.tsx (1)
30-31: Add error handling for failed image loads and consider accessibility.The
Imagecomponent lacks error handling for invalid URIs and aresizeModeprop. If the image fails to load, users see a broken state. Additionally, accessibility props would improve screen reader support.♻️ Proposed improvements
+import { useState } from 'react'; + export function Avatar({ uri, initials, size = 'md', className }: AvatarProps) { const styles = sizeStyles[size]; + const [imageError, setImageError] = useState(false); + return ( <View className={cn( styles.container, 'bg-primary items-center justify-center overflow-hidden', className, )} + accessibilityRole="image" + accessibilityLabel={initials ? `Avatar for ${initials}` : 'Avatar'} > - {uri ? ( - <Image source={{ uri }} style={{ width: styles.px, height: styles.px }} /> + {uri && !imageError ? ( + <Image + source={{ uri }} + style={{ width: styles.px, height: styles.px }} + resizeMode="cover" + onError={() => setImageError(true)} + /> ) : (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/src/components/ui/Avatar.tsx` around lines 30 - 31, The Avatar component's Image rendering (when uri is present) needs error handling and accessibility props: add an onError handler in the Image to flip a local fallback state (e.g., setImageError) and render a placeholder/avatar fallback when loading fails, add resizeMode (e.g., "cover") to the Image to prevent layout/stretch issues, and include accessibility props like accessible and accessibilityLabel (or a prop-based label) so screen readers can announce the avatar; update the conditional that uses uri and styles.px in the Image to respect the fallback state and show the alternate UI when onError fires.apps/native/app/(tabs)/index.tsx (2)
13-16: Consider handling query error state.The query destructures
dataandisLoadingbut ignoresisError/error. If the database query fails, users will see the empty state instead of an error indication.♻️ Suggested improvement
- const { data: conventions = [], isLoading } = useQuery({ + const { data: conventions = [], isLoading, isError } = useQuery({ queryKey: ['conventions'], queryFn: conventionsRepo.getAll, });Then add error handling in the render logic before the empty state check.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/app/`(tabs)/index.tsx around lines 13 - 16, The component currently ignores query errors from useQuery (call site: useQuery with queryKey ['conventions'] and queryFn conventionsRepo.getAll) and will render the empty state when a fetch fails; update the destructure to include isError and error (e.g., const { data: conventions = [], isLoading, isError, error } = useQuery(...)) and add a render branch before the empty-state check that displays an error UI/message (or retry control) when isError is true, using error to show details; ensure this logic executes before returning the empty conventions view.
51-51: Minor: Inline style object creates new reference each render.The
contentContainerStyleobject is recreated on every render. For optimal performance, consider extracting it to a constant or usinguseMemo.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/app/`(tabs)/index.tsx at line 51, The inline object passed to contentContainerStyle in the component recreates a new reference each render; move that style object out of the render path by declaring a stable constant (e.g., CONTENT_CONTAINER_STYLE) outside the component or memoize it with useMemo inside the component, then replace the inline object passed to contentContainerStyle with that constant or memoized value so the reference remains stable across renders.apps/native/src/services/data-export.ts (1)
22-26: Sequential event fetching could be parallelized.The for loop fetches events for each convention sequentially. For users with many conventions, this could be slow. Consider using
Promise.allfor parallel fetching.♻️ Suggested improvement
- const allEvents: ConventionEvent[] = []; - for (const conv of conventions) { - const events = await eventsRepo.getByConventionId(conv.id); - allEvents.push(...events); - } + const eventArrays = await Promise.all( + conventions.map((conv) => eventsRepo.getByConventionId(conv.id)) + ); + const allEvents = eventArrays.flat();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/src/services/data-export.ts` around lines 22 - 26, The current loop that builds allEvents by awaiting eventsRepo.getByConventionId(conv.id) sequentially is slow; change to parallelize by mapping conventions to promises (using conventions.map(c => eventsRepo.getByConventionId(c.id))), await Promise.all on that array, then flatten the resulting array of arrays into allEvents (e.g., Array.prototype.flat or reduce) so allEvents remains a ConventionEvent[]; ensure the new code still preserves the type and handles the aggregated results from Promise.all.apps/native/src/components/ui/Button.tsx (1)
67-71: Consider accessibility for loading state.When loading, the button shows only a spinner with no accessible label. Screen reader users won't know the button is processing.
♻️ Suggested improvement
{loading ? ( <ActivityIndicator size="small" color={variant === 'default' || variant === 'destructive' ? '#FFFFFF' : '#0FACED'} + accessibilityLabel="Loading" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/src/components/ui/Button.tsx` around lines 67 - 71, The loading state in Button.tsx renders only an ActivityIndicator, which provides no accessible label for screen readers; update the loading branch in the Button component (check the loading prop and the ActivityIndicator usage) to add proper accessibility attributes—set accessibilityRole="button" (if not already), accessibilityState={{ busy: true }} and an accessibilityLabel that includes context (e.g., `${label} loading` or `"Loading"`) or render a visually-hidden text node for screen readers when loading—to ensure screen readers announce the button is processing while preserving the visual spinner and existing variant-based color logic.apps/native/src/lib/i18n.ts (1)
33-38: Unsafe type cast for saved language value.The
AsyncStorage.getItemresult is cast directly toSupportedLanguage | null, but the stored value could be an outdated or invalid language code. Consider validating againstSUPPORTED_LANGUAGES.♻️ Suggested improvement
- const saved = (await AsyncStorage.getItem('appLanguage')) as SupportedLanguage | null; + const savedRaw = await AsyncStorage.getItem('appLanguage'); + const saved = SUPPORTED_LANGUAGES.includes(savedRaw as SupportedLanguage) + ? (savedRaw as SupportedLanguage) + : null; const deviceCode = Localization.getLocales()[0]?.languageCode ?? 'en';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/src/lib/i18n.ts` around lines 33 - 38, The code unsafely casts AsyncStorage.getItem('appLanguage') to SupportedLanguage | null (saved) without validating the stored string; instead retrieve the raw string, verify it exists in SUPPORTED_LANGUAGES (e.g., SUPPORTED_LANGUAGES.includes(savedValue)) before using it as a SupportedLanguage, and only assign to lng from the validated saved value—otherwise fall back to deviceLang or 'en'; update references to AsyncStorage.getItem, the saved variable, SupportedLanguage, and SUPPORTED_LANGUAGES accordingly so invalid/outdated codes are rejected.apps/native/app/_layout.tsx (1)
19-24: Consider handling i18n initialization errors explicitly.Using
.finally()will hide the splash screen even ifinitI18n()rejects, silently swallowing any initialization errors. Consider adding error handling to at least log failures for debugging.♻️ Suggested improvement
useEffect(() => { - initI18n().finally(async () => { + initI18n() + .catch((err) => console.error('i18n init failed:', err)) + .finally(async () => { setReady(true); await SplashScreen.hideAsync(); }); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/app/_layout.tsx` around lines 19 - 24, Replace the .finally() pattern in the useEffect with an async IIFE that awaits initI18n() inside a try/catch so initialization errors are logged (e.g., console.error or your logger) and not swallowed; then in a finally block call setReady(true) and await SplashScreen.hideAsync() so the splash screen is still hidden but failures are visible. Reference: useEffect, initI18n, setReady, SplashScreen.hideAsync.apps/native/app/settings/about.tsx (2)
81-83: Hardcoded copyright year will become stale.The copyright year "2026" is hardcoded. Consider deriving it dynamically to avoid annual maintenance.
♻️ Suggested fix
<Text variant="caption" className="text-center text-muted-foreground mt-1"> - © 2026 ConPaws. All rights reserved. + © {new Date().getFullYear()} ConPaws. All rights reserved. </Text>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/app/settings/about.tsx` around lines 81 - 83, Replace the hardcoded "© 2026 ConPaws. All rights reserved." string in the JSX Text element with a dynamically computed year; compute const year = new Date().getFullYear() (or inline {new Date().getFullYear()}) and use it inside the Text component (the JSX element using variant="caption" and className="text-center text-muted-foreground mt-1") so the displayed copyright updates automatically each year.
39-41: Hardcoded strings should use i18n translations.The tagline and other labels (e.g., "GitHub (Open Source)", "Join our Discord") are hardcoded in English. For consistency with the i18n setup used elsewhere in the app, consider extracting these to translation files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/app/settings/about.tsx` around lines 39 - 41, The hardcoded UI strings in the About component (apps/native/app/settings/about.tsx)—notably the tagline "Your furry convention companion" and labels like "GitHub (Open Source)" and "Join our Discord"—should be moved into your i18n translation files and referenced via the app's translation hook; add keys (e.g., about.tagline, about.githubLabel, about.discordLabel) to your locale JSON, import the project's translation function (e.g., useTranslation/useTranslations or t) at the top of the About component, and replace the literal Text contents with t('about.tagname') calls so the component uses localized strings. Ensure the new keys are present for all supported locales and keep the same casing/formatting in the UI components (Text variants) when swapping to translated values.apps/native/src/db/schema.ts (1)
10-10: Add DB-level CHECK constraints for enum and numeric field validation.The
statuscolumn (line 10) uses{ enum: [...] }which provides only TypeScript typing—no database-level enforcement. Similarly,reminderMinutes(line 29) lacks a non-negative constraint. Addcheck()constraints to both tables to prevent invalid rows from direct database writes or data imports.Suggested schema hardening diff
-import { sqliteTable, text, integer, real } from 'drizzle-orm/sqlite-core'; +import { sqliteTable, text, integer, real, check } from 'drizzle-orm/sqlite-core'; -export const conventions = sqliteTable('conventions', { - id: text('id').primaryKey(), - name: text('name').notNull(), - startDate: text('start_date').notNull(), - endDate: text('end_date').notNull(), - icalUrl: text('ical_url'), - status: text('status', { enum: ['upcoming', 'active', 'ended'] }).notNull().default('upcoming'), - createdAt: text('created_at').notNull().default(sql`(datetime('now'))`), - updatedAt: text('updated_at').notNull().default(sql`(datetime('now'))`), -}); +export const conventions = sqliteTable( + 'conventions', + { + id: text('id').primaryKey(), + name: text('name').notNull(), + startDate: text('start_date').notNull(), + endDate: text('end_date').notNull(), + icalUrl: text('ical_url'), + status: text('status', { enum: ['upcoming', 'active', 'ended'] }).notNull().default('upcoming'), + createdAt: text('created_at').notNull().default(sql`(datetime('now'))`), + updatedAt: text('updated_at').notNull().default(sql`(datetime('now'))`), + }, + (table) => ({ + statusCheck: check( + 'conventions_status_check', + sql`${table.status} in ('upcoming', 'active', 'ended')` + ), + }), +); -export const conventionEvents = sqliteTable('convention_events', { - id: text('id').primaryKey(), - conventionId: text('convention_id') - .notNull() - .references(() => conventions.id, { onDelete: 'cascade' }), - title: text('title').notNull(), - description: text('description'), - startTime: text('start_time').notNull(), - endTime: text('end_time'), - location: text('location'), - room: text('room'), - category: text('category'), - type: text('type'), - isInSchedule: integer('is_in_schedule', { mode: 'boolean' }).notNull().default(false), - reminderMinutes: integer('reminder_minutes'), - sourceUid: text('source_uid'), - sourceUrl: text('source_url'), - isAgeRestricted: integer('is_age_restricted', { mode: 'boolean' }).notNull().default(false), - contentWarning: integer('content_warning', { mode: 'boolean' }).notNull().default(false), - createdAt: text('created_at').notNull().default(sql`(datetime('now'))`), - updatedAt: text('updated_at').notNull().default(sql`(datetime('now'))`), -}); +export const conventionEvents = sqliteTable( + 'convention_events', + { + id: text('id').primaryKey(), + conventionId: text('convention_id') + .notNull() + .references(() => conventions.id, { onDelete: 'cascade' }), + title: text('title').notNull(), + description: text('description'), + startTime: text('start_time').notNull(), + endTime: text('end_time'), + location: text('location'), + room: text('room'), + category: text('category'), + type: text('type'), + isInSchedule: integer('is_in_schedule', { mode: 'boolean' }).notNull().default(false), + reminderMinutes: integer('reminder_minutes'), + sourceUid: text('source_uid'), + sourceUrl: text('source_url'), + isAgeRestricted: integer('is_age_restricted', { mode: 'boolean' }).notNull().default(false), + contentWarning: integer('content_warning', { mode: 'boolean' }).notNull().default(false), + createdAt: text('created_at').notNull().default(sql`(datetime('now'))`), + updatedAt: text('updated_at').notNull().default(sql`(datetime('now'))`), + }, + (table) => ({ + reminderMinutesCheck: check( + 'convention_events_reminder_minutes_check', + sql`${table.reminderMinutes} is null or ${table.reminderMinutes} >= 0` + ), + }), +);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/src/db/schema.ts` at line 10, Add DB-level CHECK constraints to the table definition so the "status" column enforces only 'upcoming'|'active'|'ended' at the database and "reminderMinutes" enforces non-negative values; update the schema in apps/native/src/db/schema.ts by augmenting the column declarations for status (currently text('status', { enum: [...] }).notNull().default('upcoming')) to include a .check(...) that restricts values to the allowed set, and for reminderMinutes (the reminderMinutes column) add a .check(...) ensuring value >= 0 and keep .notNull() as appropriate so direct DB writes/imports cannot insert invalid enums or negative minutes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 20982ff1-2649-4f2e-84ec-114aa6099a31
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (82)
.github/workflows/update-license-year.ymlTODO.mdapps/docs/content/docs/api/overview.mdxapps/docs/content/docs/dev/setup.mdxapps/docs/content/docs/guides/getting-started.mdxapps/native/.env.exampleapps/native/app.config.tsapps/native/app/(onboarding)/_layout.tsxapps/native/app/(onboarding)/complete.tsxapps/native/app/(onboarding)/features.tsxapps/native/app/(onboarding)/get-started.tsxapps/native/app/(onboarding)/welcome.tsxapps/native/app/(tabs)/_layout.tsxapps/native/app/(tabs)/index.tsxapps/native/app/(tabs)/profile.tsxapps/native/app/(tabs)/settings.tsxapps/native/app/_layout.tsxapps/native/app/convention/[id].tsxapps/native/app/convention/[id]/_layout.tsxapps/native/app/convention/[id]/import.tsxapps/native/app/index.tsxapps/native/app/settings/_layout.tsxapps/native/app/settings/about.tsxapps/native/app/settings/language.tsxapps/native/drizzle.config.tsapps/native/eas.jsonapps/native/metro.config.jsapps/native/nativewind-env.d.tsapps/native/package.jsonapps/native/postcss.config.mjsapps/native/src/components/CategoryPill.tsxapps/native/src/components/ConventionCard.tsxapps/native/src/components/EventItem.tsxapps/native/src/components/OnboardingSlide.tsxapps/native/src/components/SectionHeader.tsxapps/native/src/components/ui/Avatar.tsxapps/native/src/components/ui/Badge.tsxapps/native/src/components/ui/Button.tsxapps/native/src/components/ui/Card.tsxapps/native/src/components/ui/EmptyState.tsxapps/native/src/components/ui/Input.tsxapps/native/src/components/ui/LoadingSpinner.tsxapps/native/src/components/ui/SafeView.tsxapps/native/src/components/ui/Separator.tsxapps/native/src/components/ui/Switch.tsxapps/native/src/components/ui/Text.tsxapps/native/src/components/ui/index.tsapps/native/src/db/index.tsapps/native/src/db/repositories/conventions.tsapps/native/src/db/repositories/events.tsapps/native/src/db/schema.tsapps/native/src/global.cssapps/native/src/hooks/useEventReminder.tsapps/native/src/hooks/useImportSchedule.tsapps/native/src/lib/__tests__/ical-parser.test.tsapps/native/src/lib/i18n.tsapps/native/src/lib/ical-parser.tsapps/native/src/lib/sched-extractor.tsapps/native/src/locales/de.jsonapps/native/src/locales/en.jsonapps/native/src/locales/es.jsonapps/native/src/locales/fr.jsonapps/native/src/locales/nl.jsonapps/native/src/locales/pl.jsonapps/native/src/locales/pt-BR.jsonapps/native/src/locales/sv.jsonapps/native/src/services/data-export.tsapps/native/src/services/data-import.tsapps/native/src/services/notifications.tsapps/native/tsconfig.jsonapps/native/vitest.config.tsapps/server/package.jsonapps/server/src/index.tsapps/server/tsconfig.jsonapps/server/wrangler.tomlapps/web/.env.exampleapps/web/next.config.tsapps/web/src/app/globals.cssapps/web/src/app/layout.tsxapps/web/src/app/page.tsxapps/web/src/app/privacy/page.tsxapps/web/src/app/terms/page.tsx
Comment on lines
+72
to
+100
| /** Parse iCal datetime string to Date. | ||
| * Handles: YYYYMMDDTHHMMSSZ (UTC), YYYYMMDDTHHMMSS (local), YYYYMMDD (all-day) | ||
| */ | ||
| function parseDateTime(value: string): Date | null { | ||
| // Strip any TZID parameter prefix if present in the value (rare but possible) | ||
| const val = value.split(':').pop() ?? value; | ||
|
|
||
| // UTC: 20260612T160000Z | ||
| const utcMatch = val.match(/^(\d{4})(\d{2})(\d{2})T(\d{2})(\d{2})(\d{2})Z$/); | ||
| if (utcMatch) { | ||
| const [, y, mo, d, h, mi, s] = utcMatch; | ||
| return new Date(Date.UTC(+y, +mo - 1, +d, +h, +mi, +s)); | ||
| } | ||
|
|
||
| // Local datetime: 20260612T160000 | ||
| const localMatch = val.match(/^(\d{4})(\d{2})(\d{2})T(\d{2})(\d{2})(\d{2})$/); | ||
| if (localMatch) { | ||
| const [, y, mo, d, h, mi, s] = localMatch; | ||
| return new Date(+y, +mo - 1, +d, +h, +mi, +s); | ||
| } | ||
|
|
||
| // All-day: 20260612 | ||
| const dateMatch = val.match(/^(\d{4})(\d{2})(\d{2})$/); | ||
| if (dateMatch) { | ||
| const [, y, mo, d] = dateMatch; | ||
| return new Date(+y, +mo - 1, +d, 0, 0, 0); | ||
| } | ||
|
|
||
| return null; |
There was a problem hiding this comment.
Honor TZID / calendar timezone when building event dates.
parseIcs() extracts X-WR-TIMEZONE, but parseDateTime() still turns floating DTSTART/DTEND values into device-local Dates. If a user imports a convention while their phone is in another timezone, every affected event is stored at the wrong absolute time.
Also applies to: 152-159, 207-210
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/native/src/lib/ical-parser.ts` around lines 72 - 100, parseDateTime
currently ignores TZID and the calendar-level X-WR-TIMEZONE extracted in
parseIcs, which causes floating DTSTART/DTEND to be interpreted in the device
local zone; update parseDateTime to accept an optional timezone parameter (e.g.,
tzid or calendarTz passed from parseIcs) and use that tz when constructing dates
for non-Z UTC-less values (and when a TZID param is present on the value)
instead of new Date(...)-local behavior—resolve by passing the calendar timezone
from parseIcs into parseDateTime (and into the calls at the other locations
noted) and compute the absolute UTC timestamp using the specified IANA zone (via
Intl.DateTimeFormat/timeZone or a timezone lib) before creating the Date object
so stored events keep the correct absolute time.
Pin GitHub Actions to commit SHAs for supply-chain safety, rewrite README to reflect the current project scope, add error handling to onboarding completion, update Badge component usage, and refine web landing/privacy pages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
apps/web/src/app/page.tsx (1)
157-190:⚠️ Potential issue | 🟠 MajorA11y issue still present: labels + live-region feedback are missing.
Line 173 and Line 181 still use placeholder-only inputs, and Line 157-160 / Line 189-190 feedback isn’t announced to assistive tech.
Proposed fix
- {form.status === 'success' ? ( - <div className="text-center py-4"> + {form.status === 'success' ? ( + <div className="text-center py-4" role="status" aria-live="polite"> <p className="text-[`#0FACED`] font-medium">{form.message}</p> </div> ) : ( <form onSubmit={handleSubmit} className="flex flex-col gap-3"> {/* Honeypot — hidden from humans */} <input type="text" name="website" value={honeypot} onChange={(e) => setHoneypot(e.target.value)} tabIndex={-1} autoComplete="off" + aria-hidden="true" style={{ display: 'none' }} /> + <label htmlFor="signup-name" className="sr-only">Your name</label> <input + id="signup-name" type="text" placeholder="Your name" value={name} onChange={(e) => setName(e.target.value)} required className="bg-[`#091533`] border border-[`#1e3a5f`] rounded-xl px-4 py-3 text-white placeholder-slate-500 focus:outline-none focus:border-[`#0FACED`] transition-colors" /> + <label htmlFor="signup-email" className="sr-only">Email address</label> <input + id="signup-email" type="email" placeholder="your@email.com" value={email} onChange={(e) => setEmail(e.target.value)} required className="bg-[`#091533`] border border-[`#1e3a5f`] rounded-xl px-4 py-3 text-white placeholder-slate-500 focus:outline-none focus:border-[`#0FACED`] transition-colors" /> {form.status === 'error' && ( - <p className="text-red-400 text-sm">{form.message}</p> + <p className="text-red-400 text-sm" role="alert" aria-live="assertive">{form.message}</p> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/page.tsx` around lines 157 - 190, Inputs currently rely on placeholders only (name/email at the inputs bound to name and email) and form feedback (form.status/form.message) isn’t announced; update the form to add proper <label> associations for the name and email inputs (give inputs ids and use htmlFor on labels), ensure the honeypot input (honeypot) remains hidden from AT by using aria-hidden and removing it from tab order, and wrap success/error messages (where form.status and form.message are rendered) in an aria-live polite/assertive region (and ensure the error paragraph uses role="alert" or the live region) so assistive tech announces updates; touch the form JSX around handleSubmit, the name/email input elements, the honeypot input, and the blocks that render form.status/form.message.
🧹 Nitpick comments (1)
README.md (1)
48-48: Clarify that Supabase is optional in current phases.Line 48 reads like Supabase is a required self-hosting step, but current positioning is local-first/offline core. Suggest wording that marks Supabase as optional/future cloud sync to avoid confusion.
Suggested wording update
-Fork it. The app is built to be self-hostable — swap in your own Supabase instance, configure your RevenueCat keys, and point it at your conventions. The iCal import works with any standard `.ics` file, so it should work with most convention schedule tools out of the box. +Fork it. The app is built to be self-hostable — core features work offline, and you can optionally wire in your own Supabase instance for cloud sync when needed, configure your RevenueCat keys, and point it at your conventions. The iCal import works with any standard `.ics` file, so it should work with most convention schedule tools out of the box.Based on learnings: "The app is local-first with all core features working offline, with cloud sync (Supabase) deferred to Phase 2+"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 48, Update the README sentence that implies Supabase is required so it clearly states Supabase is optional and cloud sync is planned for a later phase; edit the line beginning "The app is built to be self-hostable — swap in your own Supabase instance..." to something like "The app is local-first with all core features working offline; Supabase (cloud sync) is optional and planned for Phase 2+, so you can self-host with Supabase if you want cloud sync." Ensure the wording preserves mentions of RevenueCat and iCal compatibility while emphasizing offline-first/local-first behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/native/src/services/data-import.ts`:
- Around line 16-38: validateImportFile currently only validates the top-level
envelope (version/app/data arrays) while importData writes conventions and
events incrementally, which can leave a half-committed DB if a later row fails;
update the import flow so either (A) validate every convention and event row
shape/required fields before any mutations (validate each element in
data.conventions and data.events using the same schema checks you use when
creating records), or (B) perform the entire import inside a single DB
transaction so all inserts are committed or rolled back together (wrap the logic
in importData with a transactional boundary and move per-row validation there),
referencing validateImportFile and importData and the
data.conventions/data.events arrays to locate where to add full-row validation
or the transaction wrapper.
- Around line 143-149: Change the Cancel handlers in the confirmation Alert(s)
to resolve null instead of resolving a zeroed ImportResult so the existing
caller guard (if (!result) return) short-circuits the success path; update the
Promise return type from Promise<ImportResult> to Promise<ImportResult | null>
in the enclosing function (the one that shows the Alert and returns the Promise)
and update both Cancel onPress handlers (the two Alert blocks shown) to call
resolve(null).
- Around line 55-68: The import loop is creating new local IDs via
conventionsRepo.create and eventsRepo.batchInsert while duplicate checks use the
exported conv.id/event.id, causing non-idempotent restores; modify the import to
either (A) pass the exported ID through to the repository create/insert call
(e.g., add an optional id param when calling conventionsRepo.create and
eventsRepo.batchInsert) or (B) persist the exported ID as a stable
sourceId/externalId column on the created records and update the duplicate
detection to check that sourceId; update idMap to map exported IDs to the
created local id or stored sourceId accordingly so future imports dedupe
correctly (adjust repositories create/batchInsert signatures to accept and store
the external id if needed).
In `@apps/web/.env.example`:
- Around line 5-6: The dotenv key order in apps/web/.env.example is out of
sequence; swap the two keys so NEXT_PUBLIC_GOOGLE_PLAY_BETA_URL appears before
NEXT_PUBLIC_TESTFLIGHT_URL to satisfy dotenv-linter ordering. Locate the two
environment variable entries NEXT_PUBLIC_TESTFLIGHT_URL and
NEXT_PUBLIC_GOOGLE_PLAY_BETA_URL and reorder them accordingly so CI/lint passes.
In `@apps/web/src/app/page.tsx`:
- Around line 49-50: The code currently falls back to the production URL by
using const apiUrl = process.env.NEXT_PUBLIC_API_URL ??
'https://api.conpaws.com', which can accidentally send local/preview
subscription data to prod; change this to fail fast by requiring
NEXT_PUBLIC_API_URL to be defined (remove the hardcoded fallback) and throw or
return an explicit error if it's missing before calling
fetch(`${apiUrl}/subscribe`, ...)—update the logic around the apiUrl variable in
page.tsx (where apiUrl is defined and used for the subscribe fetch) so the app
does not silently default to production.
In `@README.md`:
- Line 5: Change the phrase "open source" to the hyphenated compound
"open-source" in the sentence that begins "ConPaws is an open source furry
convention companion app..." so it reads "ConPaws is an open-source furry
convention companion app..."; update that single occurrence in the README
content to maintain grammar/style consistency.
- Around line 16-22: The fenced code block showing the project tree in README.md
is missing a language tag (MD040); update the opening fence for the snippet
containing the conpaws/ tree (the triple-backtick block that begins before
"conpaws/") to include a language identifier such as text (i.e., replace the
opening "```" with "```text") so markdownlint recognizes it as a code block.
---
Duplicate comments:
In `@apps/web/src/app/page.tsx`:
- Around line 157-190: Inputs currently rely on placeholders only (name/email at
the inputs bound to name and email) and form feedback (form.status/form.message)
isn’t announced; update the form to add proper <label> associations for the name
and email inputs (give inputs ids and use htmlFor on labels), ensure the
honeypot input (honeypot) remains hidden from AT by using aria-hidden and
removing it from tab order, and wrap success/error messages (where form.status
and form.message are rendered) in an aria-live polite/assertive region (and
ensure the error paragraph uses role="alert" or the live region) so assistive
tech announces updates; touch the form JSX around handleSubmit, the name/email
input elements, the honeypot input, and the blocks that render
form.status/form.message.
---
Nitpick comments:
In `@README.md`:
- Line 48: Update the README sentence that implies Supabase is required so it
clearly states Supabase is optional and cloud sync is planned for a later phase;
edit the line beginning "The app is built to be self-hostable — swap in your own
Supabase instance..." to something like "The app is local-first with all core
features working offline; Supabase (cloud sync) is optional and planned for
Phase 2+, so you can self-host with Supabase if you want cloud sync." Ensure the
wording preserves mentions of RevenueCat and iCal compatibility while
emphasizing offline-first/local-first behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 97b79e20-e153-4e98-8879-bc48e58a0949
📒 Files selected for processing (8)
.github/workflows/update-license-year.ymlREADME.mdapps/native/app/(onboarding)/complete.tsxapps/native/app/(tabs)/profile.tsxapps/native/src/services/data-import.tsapps/web/.env.exampleapps/web/src/app/page.tsxapps/web/src/app/privacy/page.tsx
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/update-license-year.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/native/app/(onboarding)/complete.tsx
- apps/native/app/(tabs)/profile.tsx
Comment on lines
+55
to
+68
| for (const conv of payload.data.conventions) { | ||
| if (existingIds.has(conv.id)) { | ||
| idMap.set(conv.id, conv.id); | ||
| skipped++; | ||
| continue; | ||
| } | ||
| const created = await conventionsRepo.create({ | ||
| name: conv.name, | ||
| startDate: conv.startDate, | ||
| endDate: conv.endDate, | ||
| icalUrl: conv.icalUrl, | ||
| status: conv.status, | ||
| }); | ||
| idMap.set(conv.id, created.id); |
There was a problem hiding this comment.
Preserve imported IDs or this restore path stays non-idempotent.
apps/native/src/db/repositories/conventions.ts:create() and apps/native/src/db/repositories/events.ts:batchInsert() both mint fresh local IDs, but the duplicate checks here still compare against conv.id / event.id from the export. A backup imported into a fresh install will therefore import again on the next run, because those exported IDs were never stored locally. Either insert with the exported IDs or persist a stable external/source ID for future dedupe.
Also applies to: 80-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/native/src/services/data-import.ts` around lines 55 - 68, The import
loop is creating new local IDs via conventionsRepo.create and
eventsRepo.batchInsert while duplicate checks use the exported conv.id/event.id,
causing non-idempotent restores; modify the import to either (A) pass the
exported ID through to the repository create/insert call (e.g., add an optional
id param when calling conventionsRepo.create and eventsRepo.batchInsert) or (B)
persist the exported ID as a stable sourceId/externalId column on the created
records and update the duplicate detection to check that sourceId; update idMap
to map exported IDs to the created local id or stored sourceId accordingly so
future imports dedupe correctly (adjust repositories create/batchInsert
signatures to accept and store the external id if needed).
- Connect setReminderMutation to scheduleEventReminder/cancelEventReminder so OS notifications are actually scheduled when users set reminders - Fix AsyncStorage onboarding check to compare against 'true' explicitly and add error handling so the UI doesn't get stuck - Include RECURRENCE-ID in iCal dedupe key to preserve recurring instances - Wrap data import in a DB transaction for atomicity on partial failures - Resolve null on import cancel instead of a zeroed result object - Remove hardcoded api.conpaws.com fallback; fail fast if env var missing - Add accessible labels, aria-live feedback region, and aria-hidden honeypot - Alphabetize .env.example keys and fix README grammar/markdown lint Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The lockfile was out of sync causing --frozen-lockfile to fail in CI. Also fixed working-directory references from apps/mobile to apps/native. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
apps/native/src/services/data-import.ts (2)
121-129: Consider cleaning up cached files after import.Files copied to the cache directory (
copyToCacheDirectory: true) are never deleted, which could accumulate over time. Consider deleting the file after reading its contents.♻️ Proposed cleanup
const file = result.assets[0]; const content = await FileSystem.readAsStringAsync(file.uri); + // Clean up cached file + await FileSystem.deleteAsync(file.uri, { idempotent: true });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/src/services/data-import.ts` around lines 121 - 129, The selected file is copied to cache (copyToCacheDirectory: true) but never removed; after calling DocumentPicker.getDocumentAsync and reading the contents with FileSystem.readAsStringAsync (using result.assets[0].uri), delete the cached file URI (use FileSystem.deleteAsync on file.uri) once reading finishes — ideally in a finally block or after successful read and error-handle deletion failures; reference the DocumentPicker.getDocumentAsync / result.assets[0] and FileSystem.readAsStringAsync calls when adding the cleanup.
90-109: CallingbatchInsertwith a single event per iteration defeats batching.Each loop iteration calls
batchInsert([...])with exactly one event, executing N separate insert statements instead of one batched insert. Collect eligible events first, then batch insert once after the loop.♻️ Proposed refactor
+ // Collect events for batch insert + const eventsToInsert: Array<{ + conventionId: string; + title: string; + description: string | null; + startTime: string; + endTime: string; + location: string | null; + room: string | null; + category: string | null; + type: string | null; + isInSchedule: boolean; + reminderMinutes: number | null; + sourceUid: string | null; + sourceUrl: string | null; + isAgeRestricted: boolean; + contentWarning: string | null; + }> = []; + // Import events (skip existing IDs) for (const event of payload.data.events) { if (allEventIds.has(event.id)) { skipped++; continue; } // Resolve the convention ID via the mapping const mappedConventionId = idMap.get(event.conventionId); if (!mappedConventionId) { skipped++; continue; } - await eventsRepo.batchInsert([ - { - conventionId: mappedConventionId, - title: event.title, - description: event.description, - startTime: event.startTime, - endTime: event.endTime, - location: event.location, - room: event.room, - category: event.category, - type: event.type, - isInSchedule: event.isInSchedule, - reminderMinutes: event.reminderMinutes, - sourceUid: event.sourceUid, - sourceUrl: event.sourceUrl, - isAgeRestricted: event.isAgeRestricted, - contentWarning: event.contentWarning, - }, - ]); + eventsToInsert.push({ + conventionId: mappedConventionId, + title: event.title, + description: event.description, + startTime: event.startTime, + endTime: event.endTime, + location: event.location, + room: event.room, + category: event.category, + type: event.type, + isInSchedule: event.isInSchedule, + reminderMinutes: event.reminderMinutes, + sourceUid: event.sourceUid, + sourceUrl: event.sourceUrl, + isAgeRestricted: event.isAgeRestricted, + contentWarning: event.contentWarning, + }); eventsAdded++; } + + if (eventsToInsert.length > 0) { + await eventsRepo.batchInsert(eventsToInsert); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/native/src/services/data-import.ts` around lines 90 - 109, The current loop calls eventsRepo.batchInsert with a single-item array each iteration, defeating batching; instead, inside the loop accumulate each transformed event object into a local array (e.g., eventsToInsert) rather than calling batchInsert immediately, then after the loop call eventsRepo.batchInsert(eventsToInsert) once and increment eventsAdded by eventsToInsert.length; update any references to the single-item insert to use the collected array and ensure you preserve the same event fields (title, description, startTime, endTime, location, room, category, type, isInSchedule, reminderMinutes, sourceUid, sourceUrl, isAgeRestricted, contentWarning, conventionId).apps/web/src/app/page.tsx (1)
167-169: Consider removing redundantrole="alert"from error message.The error message is already inside an
aria-live="polite"region. Addingrole="alert"(which impliesaria-live="assertive") may cause double announcements in some screen readers since the same content update triggers both the parent live region and the alert role.♻️ Proposed simplification
{form.status === 'error' && ( - <p className="text-red-400 text-sm" role="alert">{form.message}</p> + <p className="text-red-400 text-sm">{form.message}</p> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/page.tsx` around lines 167 - 169, The <p> rendering that displays the form error (conditional on form.status === 'error' and showing form.message) should drop the redundant role="alert" because the parent container uses aria-live="polite"; remove the role attribute from that <p> so the message is only announced by the live region and avoid double/assertive announcements by screen readers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/native/app/convention/`[id].tsx:
- Around line 210-220: The mutationFn currently updates
eventsRepo.update(event.id, { reminderMinutes: minutes }) before attempting
scheduleEventReminder, which can return null for past trigger times; change the
flow so you call scheduleEventReminder(...) first and only persist
reminderMinutes via eventsRepo.update(...) if scheduling returned a
non-null/confirmed result, otherwise call eventsRepo.update(event.id, {
reminderMinutes: null }) (or cancelEventReminder(event.id) as appropriate) and
surface feedback to the user that the reminder could not be scheduled; keep the
symbols mutationFn, eventsRepo.update, scheduleEventReminder,
cancelEventReminder and the reminderMinutes field as the touchpoints to modify.
- Around line 325-329: The "+ Add event manually" Pressable currently has a
no-op onPress handler; either remove the Pressable entirely or disable it until
the manual-create flow exists. Locate the Pressable containing Text "+ Add event
manually" and: a) if hiding, delete the Pressable/Text nodes or wrap them with a
feature flag/condition (e.g., isManualCreateEnabled) so they don't render; or b)
if disabling, replace the no-op onPress with a disabled state (remove onPress or
set it to undefined), add the disabled prop/styling (e.g., reduced opacity and
cursor-not-allowed) and ensure accessibilityLabel/aria-disabled reflect the
disabled state so users aren't shown a non-functional CTA.
- Around line 187-197: The current useQuery calls for convention and events
swallow failures by falling back to falsy/empty values; update the queries (the
useQuery calls that set convention, conventionLoading and events, eventsLoading)
to check for query error states (isError and error) and surface an explicit
error UI before you render the "not found" or "no events" branches;
specifically, inspect the returned isError/error from both convention and events
queries and render an error component or message when either isError is true
(instead of treating undefined convention or [] events as not-found/empty), and
apply the same change to the other similar query blocks (the other useQuery
instances that return convention/events around the file).
- Around line 264-276: The header's icon-only Pressable controls (the back
Pressable that calls Haptics.impactAsync and router.back, and the import
Pressable that calls router.push and renders the Upload icon) lack accessibility
labels; add accessibilityLabel (e.g., "Back" or "Go back to conventions" for the
ChevronLeft Pressable and "Import convention" for the Upload Pressable) and set
accessibilityRole="button" (optionally accessibilityHint) on each Pressable so
screen readers can announce their purpose; update the two Pressable components
in this file to include these props while keeping their existing onPress
handlers.
---
Nitpick comments:
In `@apps/native/src/services/data-import.ts`:
- Around line 121-129: The selected file is copied to cache
(copyToCacheDirectory: true) but never removed; after calling
DocumentPicker.getDocumentAsync and reading the contents with
FileSystem.readAsStringAsync (using result.assets[0].uri), delete the cached
file URI (use FileSystem.deleteAsync on file.uri) once reading finishes —
ideally in a finally block or after successful read and error-handle deletion
failures; reference the DocumentPicker.getDocumentAsync / result.assets[0] and
FileSystem.readAsStringAsync calls when adding the cleanup.
- Around line 90-109: The current loop calls eventsRepo.batchInsert with a
single-item array each iteration, defeating batching; instead, inside the loop
accumulate each transformed event object into a local array (e.g.,
eventsToInsert) rather than calling batchInsert immediately, then after the loop
call eventsRepo.batchInsert(eventsToInsert) once and increment eventsAdded by
eventsToInsert.length; update any references to the single-item insert to use
the collected array and ensure you preserve the same event fields (title,
description, startTime, endTime, location, room, category, type, isInSchedule,
reminderMinutes, sourceUid, sourceUrl, isAgeRestricted, contentWarning,
conventionId).
In `@apps/web/src/app/page.tsx`:
- Around line 167-169: The <p> rendering that displays the form error
(conditional on form.status === 'error' and showing form.message) should drop
the redundant role="alert" because the parent container uses aria-live="polite";
remove the role attribute from that <p> so the message is only announced by the
live region and avoid double/assertive announcements by screen readers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7448e26b-9ce6-4522-9fca-d8d3e45589c9
📒 Files selected for processing (7)
README.mdapps/native/app/convention/[id].tsxapps/native/app/index.tsxapps/native/src/lib/ical-parser.tsapps/native/src/services/data-import.tsapps/web/.env.exampleapps/web/src/app/page.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/.env.example
- apps/native/app/index.tsx
- apps/native/src/lib/ical-parser.ts
Comment on lines
+187
to
+197
| const { data: convention, isLoading: conventionLoading } = useQuery({ | ||
| queryKey: ['convention', id], | ||
| queryFn: () => conventionsRepo.getById(id!), | ||
| enabled: !!id, | ||
| }); | ||
|
|
||
| const { data: events = [], isLoading: eventsLoading } = useQuery({ | ||
| queryKey: ['events', id], | ||
| queryFn: () => eventsRepo.getByConventionId(id!), | ||
| enabled: !!id, | ||
| }); |
There was a problem hiding this comment.
Failed reads currently fall through to “not found” or “no events.”
These branches never distinguish a rejected repo call from valid empty data. If either query fails, convention stays falsy and events falls back to [], so the screen renders a misleading fallback instead of surfacing the failure. Please add an explicit error state before the not-found and empty-state branches.
Also applies to: 247-258, 316-324
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/native/app/convention/`[id].tsx around lines 187 - 197, The current
useQuery calls for convention and events swallow failures by falling back to
falsy/empty values; update the queries (the useQuery calls that set convention,
conventionLoading and events, eventsLoading) to check for query error states
(isError and error) and surface an explicit error UI before you render the "not
found" or "no events" branches; specifically, inspect the returned isError/error
from both convention and events queries and render an error component or message
when either isError is true (instead of treating undefined convention or []
events as not-found/empty), and apply the same change to the other similar query
blocks (the other useQuery instances that return convention/events around the
file).
Comment on lines
+210
to
+220
| mutationFn: async ({ event, minutes }: { event: ConventionEvent; minutes: number | null }) => { | ||
| await eventsRepo.update(event.id, { reminderMinutes: minutes }); | ||
|
|
||
| if (minutes !== null && event.startTime) { | ||
| await scheduleEventReminder( | ||
| { id: event.id, title: event.title, startTime: event.startTime, room: event.room }, | ||
| minutes, | ||
| ); | ||
| } else { | ||
| await cancelEventReminder(event.id); | ||
| } |
There was a problem hiding this comment.
Don't persist a reminder that couldn't actually be scheduled.
scheduleEventReminder(...) can return null for already-past trigger times, but this mutation stores reminderMinutes first. For events starting in less than the selected lead time, the bell will still appear even though no OS reminder exists. Persist the value only after a notification was actually created, otherwise clear it and surface feedback.
💡 Keep SQLite and notification state aligned
const setReminderMutation = useMutation({
mutationFn: async ({ event, minutes }: { event: ConventionEvent; minutes: number | null }) => {
- await eventsRepo.update(event.id, { reminderMinutes: minutes });
-
if (minutes !== null && event.startTime) {
- await scheduleEventReminder(
+ const scheduledId = await scheduleEventReminder(
{ id: event.id, title: event.title, startTime: event.startTime, room: event.room },
minutes,
);
+ await eventsRepo.update(event.id, {
+ reminderMinutes: scheduledId ? minutes : null,
+ });
} else {
await cancelEventReminder(event.id);
+ await eventsRepo.update(event.id, { reminderMinutes: null });
}
},Also applies to: 348-348
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/native/app/convention/`[id].tsx around lines 210 - 220, The mutationFn
currently updates eventsRepo.update(event.id, { reminderMinutes: minutes })
before attempting scheduleEventReminder, which can return null for past trigger
times; change the flow so you call scheduleEventReminder(...) first and only
persist reminderMinutes via eventsRepo.update(...) if scheduling returned a
non-null/confirmed result, otherwise call eventsRepo.update(event.id, {
reminderMinutes: null }) (or cancelEventReminder(event.id) as appropriate) and
surface feedback to the user that the reminder could not be scheduled; keep the
symbols mutationFn, eventsRepo.update, scheduleEventReminder,
cancelEventReminder and the reminderMinutes field as the touchpoints to modify.
Comment on lines
+264
to
+276
| <Pressable | ||
| onPress={() => { Haptics.impactAsync(Haptics.ImpactFeedbackStyle.Light); router.back(); }} | ||
| className="active:opacity-70" | ||
| > | ||
| <ChevronLeft size={24} color="#94A3B8" /> | ||
| </Pressable> | ||
| <Text variant="h3" className="flex-1 mx-3" numberOfLines={1}>{convention.name}</Text> | ||
| <Pressable | ||
| onPress={() => router.push(`/convention/${id}/import`)} | ||
| className="active:opacity-70" | ||
| > | ||
| <Upload size={20} color="#94A3B8" /> | ||
| </Pressable> |
There was a problem hiding this comment.
Label the icon-only header actions for assistive tech.
The back and import controls are visually clear, but screen readers will only get generic buttons here. Add accessible names so VoiceOver/TalkBack users can tell which action is which.
💡 Minimal accessibility fix
<Pressable
+ accessibilityRole="button"
+ accessibilityLabel={t('common.goBack')}
onPress={() => { Haptics.impactAsync(Haptics.ImpactFeedbackStyle.Light); router.back(); }}
className="active:opacity-70"
>
@@
<Pressable
+ accessibilityRole="button"
+ accessibilityLabel={t('convention.importSchedule')}
onPress={() => router.push(`/convention/${id}/import`)}
className="active:opacity-70"
>📝 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
| <Pressable | |
| onPress={() => { Haptics.impactAsync(Haptics.ImpactFeedbackStyle.Light); router.back(); }} | |
| className="active:opacity-70" | |
| > | |
| <ChevronLeft size={24} color="#94A3B8" /> | |
| </Pressable> | |
| <Text variant="h3" className="flex-1 mx-3" numberOfLines={1}>{convention.name}</Text> | |
| <Pressable | |
| onPress={() => router.push(`/convention/${id}/import`)} | |
| className="active:opacity-70" | |
| > | |
| <Upload size={20} color="#94A3B8" /> | |
| </Pressable> | |
| <Pressable | |
| accessibilityRole="button" | |
| accessibilityLabel={t('common.goBack')} | |
| onPress={() => { Haptics.impactAsync(Haptics.ImpactFeedbackStyle.Light); router.back(); }} | |
| className="active:opacity-70" | |
| > | |
| <ChevronLeft size={24} color="#94A3B8" /> | |
| </Pressable> | |
| <Text variant="h3" className="flex-1 mx-3" numberOfLines={1}>{convention.name}</Text> | |
| <Pressable | |
| accessibilityRole="button" | |
| accessibilityLabel={t('convention.importSchedule')} | |
| onPress={() => router.push(`/convention/${id}/import`)} | |
| className="active:opacity-70" | |
| > | |
| <Upload size={20} color="#94A3B8" /> | |
| </Pressable> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/native/app/convention/`[id].tsx around lines 264 - 276, The header's
icon-only Pressable controls (the back Pressable that calls Haptics.impactAsync
and router.back, and the import Pressable that calls router.push and renders the
Upload icon) lack accessibility labels; add accessibilityLabel (e.g., "Back" or
"Go back to conventions" for the ChevronLeft Pressable and "Import convention"
for the Upload Pressable) and set accessibilityRole="button" (optionally
accessibilityHint) on each Pressable so screen readers can announce their
purpose; update the two Pressable components in this file to include these props
while keeping their existing onPress handlers.
Comment on lines
+325
to
+329
| <Pressable | ||
| onPress={() => {}} | ||
| className="active:opacity-70" | ||
| > | ||
| <Text className="text-primary text-center">+ Add event manually</Text> |
There was a problem hiding this comment.
Remove or disable the manual-add CTA until it actually does something.
This button is presented as a real action, but onPress={() => {}} sends users nowhere. Either wire it to the manual-create flow or hide/disable it until that feature exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/native/app/convention/`[id].tsx around lines 325 - 329, The "+ Add event
manually" Pressable currently has a no-op onPress handler; either remove the
Pressable entirely or disable it until the manual-create flow exists. Locate the
Pressable containing Text "+ Add event manually" and: a) if hiding, delete the
Pressable/Text nodes or wrap them with a feature flag/condition (e.g.,
isManualCreateEnabled) so they don't render; or b) if disabling, replace the
no-op onPress with a disabled state (remove onPress or set it to undefined), add
the disabled prop/styling (e.g., reduced opacity and cursor-not-allowed) and
ensure accessibilityLabel/aria-disabled reflect the disabled state so users
aren't shown a non-functional CTA.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 32-34: The CI step named "Test" currently runs "bun test" which
invokes Bun's builtin runner instead of the project's configured Vitest script;
update the step so the run command uses "bun run test" (matching the
package.json "test": "vitest run" and consistent with the other steps using "bun
run lint" and "bun run type-check") so the Vitest suite defined in
apps/native/package.json is executed by CI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c5d86061-9f33-4a42-83b5-72bccb9870e9
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
.github/workflows/ci.yml
Comment on lines
32
to
+34
| - name: Test | ||
| run: bun test | ||
| working-directory: apps/mobile | ||
| working-directory: apps/native |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect CI test step:"
rg -n -C2 'name:\s*Test|run:\s*bun test|run:\s*bun run test|working-directory:\s*apps/native' .github/workflows/ci.yml
echo
echo "Inspect native package test script:"
rg -n -C2 '"test"\s*:\s*".*"' apps/native/package.jsonRepository: MrDemonWolf/conpaws
Length of output: 576
Use bun run test so CI executes the configured Vitest suite.
Line 33 currently runs bun test, which invokes Bun's built-in test runner. However, apps/native/package.json defines test as vitest run. This mismatch means CI runs a different test path than intended. Additionally, this is inconsistent with the other steps (lines 25, 29) which use bun run lint and bun run type-check.
Proposed fix
- name: Test
- run: bun test
+ run: bun run test
working-directory: apps/native📝 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
| - name: Test | |
| run: bun test | |
| working-directory: apps/mobile | |
| working-directory: apps/native | |
| - name: Test | |
| run: bun run test | |
| working-directory: apps/native |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 32 - 34, The CI step named "Test"
currently runs "bun test" which invokes Bun's builtin runner instead of the
project's configured Vitest script; update the step so the run command uses "bun
run test" (matching the package.json "test": "vitest run" and consistent with
the other steps using "bun run lint" and "bun run type-check") so the Vitest
suite defined in apps/native/package.json is executed by CI.
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.
Summary
apps/server/), Next.js static site for Cloudflare Pages (apps/web/), Fumadocs scaffold (apps/docs/)Test plan
bun testpasses (16/16 including 12+ iCal parser tests)test-data/indyfurcon2025.ics→ 16 events grouped by dayisInSchedulestateGET /health→{ "status": "ok" }POST /subscribewith valid data → 200; with honeypot → silent 200conpaws.comcoming soon page renders;/privacyand/termsload🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores