Skip to content

Fix#9

Open
Mainak-2006 wants to merge 9 commits into
adrianhajdin:mainfrom
Mainak-2006:fix
Open

Fix#9
Mainak-2006 wants to merge 9 commits into
adrianhajdin:mainfrom
Mainak-2006:fix

Conversation

@Mainak-2006

@Mainak-2006 Mainak-2006 commented May 23, 2026

Copy link
Copy Markdown

Summary by CodeRabbit

  • New Features

    • Added username to sign-up and identifier-based sign-in flows
    • Full subscription CRUD with per-user initialization and upcoming-renewal days
  • Improvements

    • Store-backed subscription management with pause/activate/delete actions and create modal
    • Enhanced subscription cards (icon support, action buttons) and loading/error states
    • App branding updated (app name/slug/scheme)
    • Removed analytics/PostHog and related example env values for improved privacy

Review Change Stack

Remove PostHog integration and related configuration from the project
…entication

Add username feature for authentication in sign-in and sign-up forms
Implement Appwrite integration for subscription management
…functionality

- Implement user-based subscription initialization and management in the subscription store.
- Add subscription creation, update, and deletion functionalities with error handling.
- Update SubscriptionCard component to support status toggling and deletion actions.
- Refactor subscription filtering and loading states in the Subscriptions component.
- Introduce a new subscription service for managing subscription data.
- Update type definitions to accommodate new subscription properties and methods.
- Add react-native-url-polyfill dependency for improved URL handling.
@coderabbitai

coderabbitai Bot commented May 23, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Removes PostHog analytics across the app, adds an in-memory subscription service, expands the Zustand subscription store with async CRUD and user-scoped initialization, updates subscription UIs and components to use the new store and types, and adjusts auth flows and app/config accordingly.

Changes

PostHog Removal and Subscription CRUD System

Layer / File(s) Summary
Type contracts and data shape updates
type.d.ts, constants/data.ts
Global exports added for subscription types; CreateSubscriptionInput utility type introduced; Subscription expanded with userId, createdAt, and optional icon string URIs; SubscriptionCardProps gains onUpdatePress and onDeletePress callbacks. Hardcoded subscription data constants removed in favor of dynamic date computation.
Subscription service CRUD layer
services/subscriptionService.ts
New in-memory service module provides fetchSubscriptions, addSubscription, updateSubscription, and deleteSubscription async functions keyed by userId, with automatic ID generation and reverse-chronological sorting.
Zustand store async CRUD and initialization
lib/subscriptionStore.ts
Store interface expanded with loading, error, initialized, and currentUserId state; new async initialize(userId?) method fetches subscriptions once per user; CRUD operations converted from sync setters to async flows with full loading/error state management and service integration.
PostHog removal from auth flows and settings
app/(auth)/sign-in.tsx, app/(auth)/sign-up.tsx, app/(tabs)/settings.tsx
Eliminates all posthog-react-native imports and analytics calls from sign-in (refactored to identifier-based create flow), sign-up (now collects username with validation), and settings (simplified sign-out).
Root layout and app config PostHog removal
app/_layout.tsx, app.config.js, src/config/posthog.ts, package.json, README.md
Removes PostHogProvider wrapper from root layout, eliminates PostHog environment variable resolution from app config, deletes the posthog config module entirely, removes posthog-react-native dependency, and updates documentation and .env.example.
Subscription screens and components CRUD integration
app/(tabs)/index.tsx, app/(tabs)/subscriptions.tsx, app/subscriptions/[id].tsx, components/SubscriptionCard.tsx, components/CreateSubscriptionModal.tsx, app.json
Home screen now initializes store, computes daysLeft for upcoming renewals, and provides update/delete handlers to cards; subscriptions list adds loading/error/empty state rendering and search header; subscription card normalizes icon URIs and renders conditional action buttons; modal validates form before submit; details screen simplified; app name/slug/scheme updated to "recurrly".
Utility and formatting updates
lib/utils.ts, package.json, README.md
formatSubscriptionDateTime reformatted with multi-line ternary; react-native-url-polyfill dependency added; README updated to remove PostHog and adjust quick-start/env examples.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

  • adrianhajdin/react-native-recurrly#3: Both PRs refactor Clerk auth flows in sign-in and sign-up screens; main PR builds on earlier Clerk implementation by removing PostHog tracking and updating validation patterns.
  • adrianhajdin/react-native-recurrly#4: Main PR removes PostHog integration (config, events, dependencies, analytics calls) which is the inverse of the retrieved PR's PostHog setup and feature addition.
  • adrianhajdin/react-native-recurrly#2: Both PRs modify subscription UI stack including SubscriptionCard and home index; main PR extends earlier card/list behavior with store-driven CRUD and new update/delete action support.

"🐰 The PostHog has left the building,
Subscriptions now dance with CRUD and cheer,
The store hums async, types exported bold,
Auth flows cleaned and configs made clear,
Hop on — the app's new rhythm is here."

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Fix' is extremely vague and does not convey any meaningful information about the changeset; it fails to describe the actual scope of changes which include removing PostHog analytics, adding subscription CRUD operations, and updating app metadata. Consider retitling to something more specific, such as 'Remove PostHog analytics and add subscription management' or 'Refactor auth flows and migrate away from PostHog analytics.'
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
README.md (1)

171-173: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix the start command in docs (npm run dev is undefined).

package.json doesn’t define a dev script, so this setup step fails for readers.

Suggested fix
-```bash
-npm run dev
-```
+```bash
+npm start
+```
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` around lines 171 - 173, The README's start command uses `npm run
dev` which is undefined in package.json; update the documentation to use the
correct start command by replacing `npm run dev` with `npm start` (or the actual
script name from package.json if different) in the README example so readers run
the correct command; verify the referenced script exists in package.json (e.g.,
"start") and adjust the README text accordingly.
components/CreateSubscriptionModal.tsx (1)

69-96: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Await async submit before closing CreateSubscriptionModal (so failures can be handled/retried)

CreateSubscriptionModal calls onSubmit(newSubscription) and then immediately resetForm() + onClose(). The callsite (app/(tabs)/index.tsx) passes handleCreateSubscription, which fire-and-forgets addSubscription(...).catch(...), so any async failure happens after the modal is already dismissed and the user can’t retry or see an error.

💡 Proposed fix
 interface CreateSubscriptionModalProps {
   visible: boolean;
   onClose: () => void;
-  onSubmit: (subscription: Subscription) => void;
+  onSubmit: (subscription: Subscription) => Promise<void> | void;
 }
@@
-  const handleSubmit = () => {
+  const handleSubmit = async () => {
     if (!isValidForm) return;
@@
-    onSubmit(newSubscription);
-
-    resetForm();
-    onClose();
+    try {
+      await onSubmit(newSubscription);
+      resetForm();
+      onClose();
+    } catch {
+      // Keep the modal open so the parent can surface error state and allow retry.
+    }
   };
@@
-                onPress={handleSubmit}
+                onPress={() => void handleSubmit()}
                 disabled={!isValidForm}
               >

Update handleCreateSubscription in app/(tabs)/index.tsx to return the addSubscription(...) promise (and avoid swallowing errors with .catch(...)), so the modal’s catch path can keep the modal open.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/CreateSubscriptionModal.tsx` around lines 69 - 96, The modal
currently calls onSubmit(newSubscription) and immediately calls resetForm() and
onClose(), which hides the UI before async failures can be reported; change
CreateSubscriptionModal.handleSubmit so it awaits the promise returned by
onSubmit (i.e., make handleSubmit async and await onSubmit(newSubscription)) and
only call resetForm() and onClose() after the awaited promise resolves
successfully; also ensure the caller handleCreateSubscription (in
app/(tabs)/index.tsx) returns the addSubscription(...) promise and does not
swallow errors with a .catch so errors propagate back to the modal for
handling/retry.
🧹 Nitpick comments (1)
type.d.ts (1)

22-22: ⚡ Quick win

Don't widen status back to plain string.

"active" | "paused" | "cancelled" | string collapses to string, so typos still type-check and the status logic in app/(tabs)/index.tsx loses the safety this change is trying to add.

Suggested change
-  status?: "active" | "paused" | "cancelled" | string;
+  status?: "active" | "paused" | "cancelled";
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@type.d.ts` at line 22, The current type definition widens status to plain
string because of `| string`; change the declaration in type.d.ts from `status?:
"active" | "paused" | "cancelled" | string;` to a strict literal union (e.g.
`status?: "active" | "paused" | "cancelled";` or `type Status = "active" |
"paused" | "cancelled"; status?: Status;`) and then update any handling in
app/(tabs)/index.tsx to explicitly handle unknown/missing values (add a default
branch or guard) so the code remains type-safe and exhaustive.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/`(auth)/sign-in.tsx:
- Around line 46-48: The current sign-in code logs full Clerk objects (variables
named error and signIn) using JSON.stringify which can expose sensitive auth
details; replace those detailed logs in sign-in.tsx (the branches that check
error and the places referencing signIn) with generic, non-identifying log
messages (e.g., "Sign-in error" or "Sign-in result received") and remove or
redact any serialized Clerk object output; update the handlers that reference
error and signIn at the three spots (the if (error) block and the two other
locations where signIn/error are logged) to only log minimal context and, if
needed for debugging, record safe metadata (status codes or boolean flags)
rather than the full objects.
- Line 305: Replace the unescaped apostrophe inside the Text element (className
"auth-link-copy") to satisfy JSX linting by turning the literal JSX text into a
JS string expression or using an HTML entity; e.g., change the content of the
Text component "Don't have an account?" to a JSX expression like {"Don't have an
account?"} (or use "Don&apos;t have an account?") so that
react/no-unescaped-entities is resolved.

In `@app/`(auth)/sign-up.tsx:
- Around line 51-53: The code currently logs full Clerk error/state objects
(JSON.stringify(error, ...)) and the signUp object, which can leak PII; update
the handlers that reference the error and signUp variables to stop serializing
or printing those objects and instead log a minimal, sanitized message (e.g.,
"Sign-up failed" or include only a non-sensitive error code/message field if
Clerk exposes one), remove any JSON.stringify calls, and avoid logging the
entire signUp object; make this change for both places where error and signUp
are logged.
- Around line 95-97: The component currently returns null when signUp.status ===
"complete" or isSignedIn, causing a blank screen for authenticated users;
replace the return null with a redirect to "/(tabs)" (e.g., call
next/navigation's redirect('/(tabs)')) so that when signUp.status === "complete"
or isSignedIn the user is routed to the main tabs UI instead of rendering an
empty screen; update the conditional in the sign-up component where signUp and
isSignedIn are checked to perform the redirect.

In `@app/`(tabs)/index.tsx:
- Around line 55-58: The icon mapping in upcomingSubscriptions currently
replaces any string sub.icon with icons.plus; change the mapping so that if
typeof sub.icon === "string" you return an object { uri: sub.icon }, if sub.icon
=== undefined fall back to icons.plus, otherwise pass through sub.icon as-is;
update the mapping used when building upcomingSubscriptions (referencing
sub.icon, icons.plus, and the component UpcomingSubscriptionCard) so remote URI
icons render correctly.
- Around line 48-63: The filter logic treats undefined renewalDate as "now" via
dayjs and can wrongly include subscriptions; update the .filter to first guard
that sub.renewalDate exists and is a valid date (e.g., check sub.renewalDate &&
dayjs(sub.renewalDate).isValid()) before computing daysLeft, compute daysLeft
only after that guard, and exclude entries without a valid renewalDate; also
ensure the subsequent .map (which sets daysLeft and uses sub.icon, sub.name,
etc.) assumes renewalDate is present so it can reuse the same daysLeft value
rather than recomputing with a possibly undefined date.

In `@app/`(tabs)/settings.tsx:
- Line 68: The UI always appends "..." after the account ID; change the
rendering logic around user?.id?.substring(0, 20) to only append an ellipsis
when the ID is longer than the truncation length and handle missing IDs
gracefully — e.g., check user?.id and use user.id.length > 20 ?
user.id.substring(0, 20) + '...' : user.id (or a fallback like '-') instead of
always adding "..." so the ellipsis is only shown when truncation occurs.

In `@components/SubscriptionCard.tsx`:
- Line 68: The subscription card can render empty strings because it uses
nullish coalescing after trim; update the display fallbacks to treat empty
strings as missing by replacing uses like paymentMethod?.trim() ?? "Not
provided" with an empty-string-aware fallback (e.g., use || instead of ??), and
do the same for the category/plan expression in SubscriptionCard (ensure
expressions like planCategory?.trim() and planName?.trim() are joined with ||
"Not provided" so trimmed empty strings fall back to "Not provided").

In `@lib/subscriptionStore.ts`:
- Around line 49-50: The initialize function in the subscription store currently
returns early when userId is falsy, leaving previous user's subscriptions in
Zustand; change initialize (in lib/subscriptionStore.ts) to reset the store
state when userId is not provided—e.g., call the store updater to clear
subscriptions (set subscriptions to an empty array or restore initial state)
instead of returning, so the subscriptions field is emptied on logout; locate
the initialize function and replace the early return with a store reset (clear
subscriptions) path.

In `@README.md`:
- Line 167: Update the README line that currently points users to Expo by
replacing the Expo link with Clerk and instructing users to obtain the
EXPO_PUBLIC_CLERK_PUBLISHABLE_KEY from Clerk; specifically, change the
placeholder guidance that references Expo to instead point to Clerk's
dashboard/signup and mention the exact env var name
EXPO_PUBLIC_CLERK_PUBLISHABLE_KEY so users know where to retrieve their Clerk
publishable key.

In `@services/subscriptionService.ts`:
- Line 1: The module-level Map subscriptionDatabase is non-persistent and will
lose data on restarts; replace it with a persistent store and remove reliance on
the in-memory Map. Refactor code that reads/writes subscriptionDatabase (look
for uses of subscriptionDatabase and any CRUD functions in subscriptionService)
to use an injected persistence client (e.g., a database repository, ORM model,
or a file-backed store) and ensure create/read/update/delete operations call
that client asynchronously; add initialization/loading logic to hydrate any
in-memory caches from the persistent store and update tests/consumers to use the
new async API.

---

Outside diff comments:
In `@components/CreateSubscriptionModal.tsx`:
- Around line 69-96: The modal currently calls onSubmit(newSubscription) and
immediately calls resetForm() and onClose(), which hides the UI before async
failures can be reported; change CreateSubscriptionModal.handleSubmit so it
awaits the promise returned by onSubmit (i.e., make handleSubmit async and await
onSubmit(newSubscription)) and only call resetForm() and onClose() after the
awaited promise resolves successfully; also ensure the caller
handleCreateSubscription (in app/(tabs)/index.tsx) returns the
addSubscription(...) promise and does not swallow errors with a .catch so errors
propagate back to the modal for handling/retry.

In `@README.md`:
- Around line 171-173: The README's start command uses `npm run dev` which is
undefined in package.json; update the documentation to use the correct start
command by replacing `npm run dev` with `npm start` (or the actual script name
from package.json if different) in the README example so readers run the correct
command; verify the referenced script exists in package.json (e.g., "start") and
adjust the README text accordingly.

---

Nitpick comments:
In `@type.d.ts`:
- Line 22: The current type definition widens status to plain string because of
`| string`; change the declaration in type.d.ts from `status?: "active" |
"paused" | "cancelled" | string;` to a strict literal union (e.g. `status?:
"active" | "paused" | "cancelled";` or `type Status = "active" | "paused" |
"cancelled"; status?: Status;`) and then update any handling in
app/(tabs)/index.tsx to explicitly handle unknown/missing values (add a default
branch or guard) so the code remains type-safe and exhaustive.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 19f8e656-ec87-4495-90d6-0bf7a5e20e98

📥 Commits

Reviewing files that changed from the base of the PR and between 8f2eb15 and 55f42f7.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (21)
  • .env.example
  • .posthog-events.json
  • README.md
  • app.config.js
  • app.json
  • app/(auth)/sign-in.tsx
  • app/(auth)/sign-up.tsx
  • app/(tabs)/index.tsx
  • app/(tabs)/settings.tsx
  • app/(tabs)/subscriptions.tsx
  • app/_layout.tsx
  • app/subscriptions/[id].tsx
  • components/CreateSubscriptionModal.tsx
  • components/SubscriptionCard.tsx
  • constants/data.ts
  • lib/subscriptionStore.ts
  • lib/utils.ts
  • package.json
  • services/subscriptionService.ts
  • src/config/posthog.ts
  • type.d.ts
💤 Files with no reviewable changes (3)
  • .env.example
  • .posthog-events.json
  • src/config/posthog.ts

Comment thread app/(auth)/sign-in.tsx
Comment on lines +46 to +48
if (error) {
console.error(JSON.stringify(error, null, 2));
return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Sanitize auth error logging in sign-in path.

Logging full Clerk objects (error / signIn) may expose sensitive auth details in client logs. Keep logs generic and non-identifying.

Suggested fix
-    if (error) {
-      console.error(JSON.stringify(error, null, 2));
-      return;
-    }
+    if (error) {
+      console.error("Sign-in request failed");
+      return;
+    }
...
-    } else {
-      console.error("Sign-in attempt not complete:", signIn);
-    }
+    } else {
+      console.error("Sign-in attempt not complete");
+    }
...
-    } else {
-      console.error("Sign-in attempt not complete:", signIn);
-    }
+    } else {
+      console.error("Sign-in attempt not complete");
+    }

Also applies to: 86-87, 116-117

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/`(auth)/sign-in.tsx around lines 46 - 48, The current sign-in code logs
full Clerk objects (variables named error and signIn) using JSON.stringify which
can expose sensitive auth details; replace those detailed logs in sign-in.tsx
(the branches that check error and the places referencing signIn) with generic,
non-identifying log messages (e.g., "Sign-in error" or "Sign-in result
received") and remove or redact any serialized Clerk object output; update the
handlers that reference error and signIn at the three spots (the if (error)
block and the two other locations where signIn/error are logged) to only log
minimal context and, if needed for debugging, record safe metadata (status codes
or boolean flags) rather than the full objects.

Comment thread app/(auth)/sign-in.tsx
);
{/* Sign-Up Link */}
<View className="auth-link-row">
<Text className="auth-link-copy">Don't have an account?</Text>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix unescaped apostrophe to satisfy JSX linting.

This currently triggers react/no-unescaped-entities.

Suggested fix
-              <Text className="auth-link-copy">Don't have an account?</Text>
+              <Text className="auth-link-copy">Don&apos;t have an account?</Text>
📝 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
<Text className="auth-link-copy">Don't have an account?</Text>
<Text className="auth-link-copy">Don&apos;t have an account?</Text>
🧰 Tools
🪛 ESLint

[error] 305-305: ' can be escaped with &apos;, &lsquo;, &#39;, &rsquo;.

(react/no-unescaped-entities)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/`(auth)/sign-in.tsx at line 305, Replace the unescaped apostrophe inside
the Text element (className "auth-link-copy") to satisfy JSX linting by turning
the literal JSX text into a JS string expression or using an HTML entity; e.g.,
change the content of the Text component "Don't have an account?" to a JSX
expression like {"Don't have an account?"} (or use "Don&apos;t have an
account?") so that react/no-unescaped-entities is resolved.

Comment thread app/(auth)/sign-up.tsx
Comment on lines +51 to +53
if (error) {
console.error(JSON.stringify(error, null, 2));
return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don’t log raw Clerk error/state objects in auth flows.

JSON.stringify(error, null, 2) and logging signUp can leak user identifiers and internal auth metadata into device logs. Log only minimal, sanitized messages.

Suggested fix
-    if (error) {
-      console.error(JSON.stringify(error, null, 2));
-      return;
-    }
+    if (error) {
+      console.error("Sign-up request failed");
+      return;
+    }
...
-    } else {
-      console.error("Sign-up attempt not complete:", signUp);
-    }
+    } else {
+      console.error("Sign-up attempt not complete");
+    }

Also applies to: 90-91

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/`(auth)/sign-up.tsx around lines 51 - 53, The code currently logs full
Clerk error/state objects (JSON.stringify(error, ...)) and the signUp object,
which can leak PII; update the handlers that reference the error and signUp
variables to stop serializing or printing those objects and instead log a
minimal, sanitized message (e.g., "Sign-up failed" or include only a
non-sensitive error code/message field if Clerk exposes one), remove any
JSON.stringify calls, and avoid logging the entire signUp object; make this
change for both places where error and signUp are logged.

Comment thread app/(auth)/sign-up.tsx
Comment on lines +95 to +97
if (signUp.status === "complete" || isSignedIn) {
return null;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid blank-screen auth state by redirecting instead of returning null.

When an already-authenticated user opens this route, the screen renders nothing. Redirect to /(tabs) to prevent a dead-end UI.

Suggested fix
-  if (signUp.status === "complete" || isSignedIn) {
-    return null;
-  }
+  if (signUp.status === "complete" || isSignedIn) {
+    router.replace("/(tabs)" as Href);
+    return null;
+  }
📝 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
if (signUp.status === "complete" || isSignedIn) {
return null;
}
if (signUp.status === "complete" || isSignedIn) {
router.replace("/(tabs)" as Href);
return null;
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/`(auth)/sign-up.tsx around lines 95 - 97, The component currently returns
null when signUp.status === "complete" or isSignedIn, causing a blank screen for
authenticated users; replace the return null with a redirect to "/(tabs)" (e.g.,
call next/navigation's redirect('/(tabs)')) so that when signUp.status ===
"complete" or isSignedIn the user is routed to the main tabs UI instead of
rendering an empty screen; update the conditional in the sign-up component where
signUp and isSignedIn are checked to perform the redirect.

Comment thread app/(tabs)/index.tsx
Comment on lines +48 to +63
.filter((sub) => {
if (sub.status !== "active") return false;
const daysLeft = dayjs(sub.renewalDate).startOf("day").diff(now, "day");
return daysLeft >= 0 && daysLeft <= 7;
})
.map((sub) => ({
id: sub.id,
icon:
typeof sub.icon === "string" || sub.icon === undefined
? icons.plus
: sub.icon,
name: sub.name,
price: sub.price,
currency: sub.currency,
daysLeft: dayjs(sub.renewalDate).startOf("day").diff(now, "day"),
}))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate relevant files
fd -a "index\.tsx" app -t f | head
fd -a "type\.d\.ts" -t f | head -n 20
fd -a "UpcomingSubscriptionCard\.tsx" -t f | head -n 20
fd -a "upcomingSubscriptions" -t f | head -n 50

# Show the reviewed snippet area
FILE="app/(tabs)/index.tsx"
if [ -f "$FILE" ]; then
  echo "---- $FILE (lines 1-120) ----"
  sed -n '1,140p' "$FILE" | nl -ba | sed -n '1,140p' >/tmp/index_tsx_head.txt
  sed -n '35,90p' "$FILE" | nl -ba | sed -n '35,90p' 
fi

# Find and print any mapping/filter logic referencing renewalDate near the snippet
if [ -f "$FILE" ]; then
  echo "---- renewalDate occurrences in $FILE ----"
  rg -n "renewalDate" "$FILE" || true
fi

# Show definition of Subscription type (type.d.ts)
TYPEFILE="$(fd -a "type\.d\.ts" -t f | head -n 1 || true)"
if [ -n "${TYPEFILE:-}" ] && [ -f "$TYPEFILE" ]; then
  echo "---- $TYPEFILE (Subscription-related lines) ----"
  rg -n "type\s+Subscription|interface\s+Subscription|Subscription" "$TYPEFILE" || true
  # Print a small surrounding window around the found lines
  # (fallback: print first 200 lines if rg matches not found)
  if rg -n "renewalDate" "$TYPEFILE" >/dev/null; then
    sed -n '1,220p' "$TYPEFILE" | nl -ba | sed -n '1,220p'
  else
    sed -n '1,220p' "$TYPEFILE" | nl -ba | sed -n '1,220p'
  fi
fi

# Show UpcomingSubscriptionCard usage of icon + renewalDate if present
CARD="$(fd -a "UpcomingSubscriptionCard\.tsx" -t f | head -n 1 || true)"
if [ -n "${CARD:-}" ] && [ -f "$CARD" ]; then
  echo "---- $CARD (lines 1-200) ----"
  sed -n '1,220p' "$CARD" | nl -ba | sed -n '1,220p'
  echo "---- icon-related lines in $CARD ----"
  rg -n "icon|Image" "$CARD" || true
fi

# Search for other upcoming mapping paths that handle renewalDate/icon
fd -a "Upcoming" app -t f | head -n 50
rg -n "UpcomingSubscriptionCard|upcomingSubscriptions|daysLeft|icons\.plus|sub\.icon" app || true

Repository: adrianhajdin/react-native-recurrly

Length of output: 313


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="app/(tabs)/index.tsx"
TYPEFILE="$(fd -a "type\.d\.ts" -t f | head -n 1 || true)"
CARD="$(fd -a "UpcomingSubscriptionCard\.tsx" -t f | head -n 1 || true)"

echo "---- $FILE (context around lines 35-90) ----"
sed -n '1,120p' "$FILE" | cat -n | sed -n '35,90p'

echo "---- $FILE: renewalDate occurrences ----"
rg -n "renewalDate" "$FILE" || true

if [ -n "${TYPEFILE:-}" ] && [ -f "$TYPEFILE" ]; then
  echo "---- $TYPEFILE (search for Subscription + renewalDate) ----"
  rg -n "interface\s+Subscription|type\s+Subscription|renewalDate|icon" "$TYPEFILE" || true
  echo "---- $TYPEFILE: showing first 220 lines ----"
  sed -n '1,220p' "$TYPEFILE" | cat -n
fi

if [ -n "${CARD:-}" ] && [ -f "$CARD" ]; then
  echo "---- $CARD (first 220 lines) ----"
  sed -n '1,220p' "$CARD" | cat -n
  echo "---- $CARD: icon/Image usage ----"
  rg -n "Image|icon|uri" "$CARD" || true
fi

echo "---- app: upcomingSubscriptions occurrences ----"
rg -n "upcomingSubscriptions|UpcomingSubscriptionCard|daysLeft|icons\.plus" app || true

Repository: adrianhajdin/react-native-recurrly

Length of output: 7152


🌐 Web query:

dayjs(undefined) behavior returns current date or invalid date

💡 Result:

dayjs(undefined) returns a valid Day.js object representing the current date/time (i.e., behaves like dayjs), not an “Invalid Date”. Evidence: - The Day.js docs for “Now” state that calling dayjs without parameters returns a fresh Day.js object with the current date/time, and also that “Day.js treats dayjs(undefined) as dayjs due to that function parameters default to undefined when not passed in.” [1] - The Day.js issue tracker shows the same behavior: dayjs(undefined).isValid is true (current date/time), while dayjs(null) is invalid. [2] - The docs/behavior difference is also discussed: dayjs(undefined) is essentially equivalent to dayjs (current time), whereas passing null is treated as an invalid input. [3] So if you specifically want an invalid date when the argument is undefined, you must guard it yourself (e.g., only call dayjs(value) when value!== undefined).

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="app/(tabs)/index.tsx"

echo "---- $FILE (imports + icons usage) ----"
sed -n '1,90p' "$FILE" | cat -n

echo "---- $FILE: icons.* occurrences ----"
rg -n "icons\." "$FILE" || true

# Locate icons definition file(s) if any
echo "---- searching for definition of icons.plus ----"
fd -a "icons" . -t f | head -n 50
# Grep for "plus" in icons file candidates
for f in $(fd -a "icons" . -t f | head -n 50); do
  if rg -n "plus" "$f" >/dev/null 2>&1; then
    echo ">>> candidate: $f"
    rg -n "plus" "$f" | head -n 20
  fi
done

Repository: adrianhajdin/react-native-recurrly

Length of output: 4319


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find store + any places constructing/updating subscription objects
rg -n "useSubscriptionStore|addSubscription|updateSubscription|initialize\(" -S . || true
rg -n "renewalDate" -S lib components app | head -n 200 || true
rg -n "icon\s*:" -S lib components app | head -n 200 || true
rg -n "sub\.icon|subscription\.icon|icon\)" -S lib components app | head -n 200 || true
rg -n "ImageSourcePropType \| string|string\)" -S . || true

# Show subscriptionStore implementation
STOREFILE="$(fd -a "subscriptionStore" . -t f | head -n 20 | head -n 1)"
if [ -n "${STOREFILE:-}" ] && [ -f "$STOREFILE" ]; then
  echo "---- $STOREFILE (first 260 lines) ----"
  sed -n '1,260p' "$STOREFILE" | cat -n
fi

# Show CreateSubscriptionModal if it touches icon/renewalDate
MODFILE="$(fd -a "CreateSubscriptionModal\.tsx" . -t f | head -n 1 || true)"
if [ -n "${MODFILE:-}" ] && [ -f "$MODFILE" ]; then
  echo "---- $MODFILE (first 260 lines) ----"
  sed -n '1,260p' "$MODFILE" | cat -n
fi

# Show SubscriptionCard for icon and daysLeft behavior
CARD2="$(fd -a "SubscriptionCard\.tsx" . -t f | head -n 1 || true)"
if [ -n "${CARD2:-}" ] && [ -f "$CARD2" ]; then
  echo "---- $CARD2 (first 260 lines) ----"
  sed -n '1,260p' "$CARD2" | cat -n
fi

Repository: adrianhajdin/react-native-recurrly

Length of output: 23608


🏁 Script executed:

#!/bin/bash
set -euo pipefail
SERVICEFILE="services/subscriptionService.ts"
if [ -f "$SERVICEFILE" ]; then
  echo "---- $SERVICEFILE (first 260 lines) ----"
  sed -n '1,260p' "$SERVICEFILE" | cat -n
  echo "---- $SERVICEFILE: icon/renewalDate occurrences ----"
  rg -n "icon|renewalDate" "$SERVICEFILE" || true
fi

Repository: adrianhajdin/react-native-recurrly

Length of output: 3845


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "updateSubscription\(" -S . || true

echo "---- payloads mentioning renewalDate ----"
rg -n "updateSubscription\([\s\S]*renewalDate" -S . || true
rg -n "renewalDate\s*:" -S . || true

echo "---- subscriptions constructed elsewhere (newSubscription/icon/renewalDate) ----"
rg -n "newSubscription|CreateSubscriptionInput|addSubscription\(" -S . || true
rg -n "icon\s*:" -S . || true

echo "---- any usage that sets renewalDate to undefined/null ----"
rg -n "renewalDate\s*:\s*(undefined|null)" -S . || true

Repository: adrianhajdin/react-native-recurrly

Length of output: 2692


Guard renewalDate before classifying a subscription as upcoming.

type.d.ts makes renewalDate optional, but dayjs(sub.renewalDate) treats undefined as “now”, so missing renewal dates can yield daysLeft = 0 and pass the 0..7 filter (resulting in incorrect items in Upcoming). This is a low-effort robustness fix.

Suggested change
     return subscriptions
       .filter((sub) => {
         if (sub.status !== "active") return false;
-        const daysLeft = dayjs(sub.renewalDate).startOf("day").diff(now, "day");
+        if (!sub.renewalDate) return false;
+        const renewalDate = dayjs(sub.renewalDate);
+        if (!renewalDate.isValid()) return false;
+        const daysLeft = renewalDate.startOf("day").diff(now, "day");
         return daysLeft >= 0 && daysLeft <= 7;
       })
       .map((sub) => ({
         id: sub.id,
         icon:
           typeof sub.icon === "string" || sub.icon === undefined
             ? icons.plus
             : sub.icon,
         name: sub.name,
         price: sub.price,
         currency: sub.currency,
-        daysLeft: dayjs(sub.renewalDate).startOf("day").diff(now, "day"),
+        daysLeft: dayjs(sub.renewalDate!).startOf("day").diff(now, "day"),
       }))
📝 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
.filter((sub) => {
if (sub.status !== "active") return false;
const daysLeft = dayjs(sub.renewalDate).startOf("day").diff(now, "day");
return daysLeft >= 0 && daysLeft <= 7;
})
.map((sub) => ({
id: sub.id,
icon:
typeof sub.icon === "string" || sub.icon === undefined
? icons.plus
: sub.icon,
name: sub.name,
price: sub.price,
currency: sub.currency,
daysLeft: dayjs(sub.renewalDate).startOf("day").diff(now, "day"),
}))
.filter((sub) => {
if (sub.status !== "active") return false;
if (!sub.renewalDate) return false;
const renewalDate = dayjs(sub.renewalDate);
if (!renewalDate.isValid()) return false;
const daysLeft = renewalDate.startOf("day").diff(now, "day");
return daysLeft >= 0 && daysLeft <= 7;
})
.map((sub) => ({
id: sub.id,
icon:
typeof sub.icon === "string" || sub.icon === undefined
? icons.plus
: sub.icon,
name: sub.name,
price: sub.price,
currency: sub.currency,
daysLeft: dayjs(sub.renewalDate!).startOf("day").diff(now, "day"),
}))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/`(tabs)/index.tsx around lines 48 - 63, The filter logic treats undefined
renewalDate as "now" via dayjs and can wrongly include subscriptions; update the
.filter to first guard that sub.renewalDate exists and is a valid date (e.g.,
check sub.renewalDate && dayjs(sub.renewalDate).isValid()) before computing
daysLeft, compute daysLeft only after that guard, and exclude entries without a
valid renewalDate; also ensure the subsequent .map (which sets daysLeft and uses
sub.icon, sub.name, etc.) assumes renewalDate is present so it can reuse the
same daysLeft value rather than recomputing with a possibly undefined date.

Comment thread app/(tabs)/settings.tsx
</SafeAreaView>
)
}
{user?.id?.substring(0, 20)}...

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Only append ellipsis when the account ID is actually truncated.

Current rendering always appends ..., including short or missing IDs.

Suggested fix
-              {user?.id?.substring(0, 20)}...
+              {user?.id
+                ? `${user.id.substring(0, 20)}${user.id.length > 20 ? "..." : ""}`
+                : "N/A"}
📝 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
{user?.id?.substring(0, 20)}...
{user?.id
? `${user.id.substring(0, 20)}${user.id.length > 20 ? "..." : ""}`
: "N/A"}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/`(tabs)/settings.tsx at line 68, The UI always appends "..." after the
account ID; change the rendering logic around user?.id?.substring(0, 20) to only
append an ellipsis when the ID is longer than the truncation length and handle
missing IDs gracefully — e.g., check user?.id and use user.id.length > 20 ?
user.id.substring(0, 20) + '...' : user.id (or a fallback like '-') instead of
always adding "..." so the ellipsis is only shown when truncation occurs.

numberOfLines={1}
ellipsizeMode="tail"
>
{paymentMethod?.trim() ?? "Not provided"}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify nullish-coalescing is used on trimmed strings in this component.
rg -nP --type=tsx 'trim\(\)\s*\?\?' components/SubscriptionCard.tsx -C2

Repository: adrianhajdin/react-native-recurrly

Length of output: 106


Fix empty-string fallbacks for display fields in components/SubscriptionCard.tsx.

The fallback logic in components/SubscriptionCard.tsx uses ?? "Not provided" on trimmed values (e.g., paymentMethod?.trim() ?? "Not provided"), which will not replace empty strings after .trim()—so it can render "". Prefer an empty-string-aware fallback such as paymentMethod?.trim() || "Not provided", and similarly for the category/plan expression (add "Not provided" to the || chain).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/SubscriptionCard.tsx` at line 68, The subscription card can render
empty strings because it uses nullish coalescing after trim; update the display
fallbacks to treat empty strings as missing by replacing uses like
paymentMethod?.trim() ?? "Not provided" with an empty-string-aware fallback
(e.g., use || instead of ??), and do the same for the category/plan expression
in SubscriptionCard (ensure expressions like planCategory?.trim() and
planName?.trim() are joined with || "Not provided" so trimmed empty strings fall
back to "Not provided").

Comment thread lib/subscriptionStore.ts
Comment on lines +49 to +50
initialize: async (userId?: string) => {
if (!userId) return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reset the store when there is no authenticated user.

Returning early here leaves the previous user's subscriptions in Zustand after logout, so both tab screens can keep rendering stale data until another user initializes the store.

Suggested change
   initialize: async (userId?: string) => {
-    if (!userId) return;
+    if (!userId) {
+      set({
+        subscriptions: [],
+        loading: false,
+        error: null,
+        initialized: false,
+        currentUserId: null,
+      });
+      return;
+    }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/subscriptionStore.ts` around lines 49 - 50, The initialize function in
the subscription store currently returns early when userId is falsy, leaving
previous user's subscriptions in Zustand; change initialize (in
lib/subscriptionStore.ts) to reset the store state when userId is not
provided—e.g., call the store updater to clear subscriptions (set subscriptions
to an empty array or restore initial state) instead of returning, so the
subscriptions field is emptied on logout; locate the initialize function and
replace the early return with a store reset (clear subscriptions) path.

Comment thread README.md
```

Replace the placeholder values with your real credentials. You can get these by signing up at: [**Expo**](https://jsm.dev/nativesub-exp), [**PostHog**](https://jsm.dev/nativesub-posthog) .
Replace the placeholder values with your real credentials. You can get these by signing up at: [**Expo**](https://jsm.dev/nativesub-exp).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Point users to Clerk for the Clerk publishable key.

This line currently directs to Expo, but EXPO_PUBLIC_CLERK_PUBLISHABLE_KEY should come from Clerk.

Suggested fix
-Replace the placeholder values with your real credentials. You can get these by signing up at: [**Expo**](https://jsm.dev/nativesub-exp).
+Replace the placeholder value with your real credential. You can get it from your [**Clerk Dashboard**](https://dashboard.clerk.com/).
📝 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
Replace the placeholder values with your real credentials. You can get these by signing up at: [**Expo**](https://jsm.dev/nativesub-exp).
Replace the placeholder value with your real credential. You can get it from your [**Clerk Dashboard**](https://dashboard.clerk.com/).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` at line 167, Update the README line that currently points users to
Expo by replacing the Expo link with Clerk and instructing users to obtain the
EXPO_PUBLIC_CLERK_PUBLISHABLE_KEY from Clerk; specifically, change the
placeholder guidance that references Expo to instead point to Clerk's
dashboard/signup and mention the exact env var name
EXPO_PUBLIC_CLERK_PUBLISHABLE_KEY so users know where to retrieve their Clerk
publishable key.

@@ -0,0 +1,103 @@
const subscriptionDatabase = new Map<string, Subscription[]>();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

This CRUD layer is non-persistent.

A module-level Map is wiped on every reload / cold start, so users can add or edit subscriptions and lose everything the next time the app process restarts.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@services/subscriptionService.ts` at line 1, The module-level Map
subscriptionDatabase is non-persistent and will lose data on restarts; replace
it with a persistent store and remove reliance on the in-memory Map. Refactor
code that reads/writes subscriptionDatabase (look for uses of
subscriptionDatabase and any CRUD functions in subscriptionService) to use an
injected persistence client (e.g., a database repository, ORM model, or a
file-backed store) and ensure create/read/update/delete operations call that
client asynchronously; add initialization/loading logic to hydrate any in-memory
caches from the persistent store and update tests/consumers to use the new async
API.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
constants/images.ts (1)

5-11: ⚡ Quick win

Export the typed images map to avoid contract drift.

You define a typed images object, but the default export creates a separate object literal. Export images directly so there’s one source of truth.

Suggested diff
-const images: { [key: string]: ImageSourcePropType } = {
+const images: Record<string, ImageSourcePropType> = {
   splashPattern,
   avatar,
 };
 
-export default { splashPattern, avatar }
+export default images
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@constants/images.ts` around lines 5 - 11, Replace the current default export
object literal with the already-typed images map: export the const images (which
references splashPattern and avatar) as the module default so there’s a single
source of truth and the declared type { [key: string]: ImageSourcePropType } is
preserved instead of returning a new object literal.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@constants/images.ts`:
- Around line 5-11: Replace the current default export object literal with the
already-typed images map: export the const images (which references
splashPattern and avatar) as the module default so there’s a single source of
truth and the declared type { [key: string]: ImageSourcePropType } is preserved
instead of returning a new object literal.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8373bfd3-3cb3-4fbe-a932-e94e6e42d066

📥 Commits

Reviewing files that changed from the base of the PR and between 55f42f7 and 7c777a4.

📒 Files selected for processing (1)
  • constants/images.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant