Implement Splitwise import functionality #239
Conversation
✅ Deploy Preview for split-but-wiser ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughAdds Splitwise data import integration with OAuth authentication flow, including backend service layer and API routes, database schema for import jobs and OAuth tokens, frontend UI for web and mobile platforms, comprehensive architecture documentation, and settlement verification testing script. Changes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 30
🤖 Fix all issues with AI agents
In @backend/app/config.py:
- Around line 36-41: The Settings class exposes frontend_url which is used
directly in OAuth redirect URIs (router.py), allowing an attacker to configure
an untrusted domain; add a Pydantic validator on the Settings model (e.g.,
validate_frontend_url or a @validator for frontend_url) to ensure the value is a
well-formed URL and matches a whitelist of allowed origins (or at minimum only
allows specific schemes and hostnames), and update any code paths that consume
frontend_url (router functions that build redirect URIs) to rely on the
validated property so untrusted domains cannot be used for redirects.
In @backend/app/integrations/router.py:
- Around line 151-155: The HTTPException raised in the except block that handles
import startup failures should use exception chaining to preserve the original
traceback; update the raise of HTTPException (the one with
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR and detail=f"Failed to start
import: {str(e)}") to re-raise the HTTPException using "from e" so the original
exception e is chained to the HTTPException.
- Around line 98-101: In the except block that catches Exception as e in
backend/app/integrations/router.py (the block that raises HTTPException with
detail "Failed to exchange OAuth code"), re-raise the HTTPException using
exception chaining by adding "from e" to preserve the original traceback (i.e.,
raise HTTPException(...) from e) so the original exception context is not lost.
- Around line 84-90: ImportService is being instantiated without the required db
dependency (ImportService()) causing inconsistency with the later call
ImportService(db); update the endpoint to accept the database dependency and
pass it into the service: add the db parameter (injected via your existing DB
dependency provider) to the route handler signature and replace ImportService()
with ImportService(db) when calling start_import so both usages are consistent
and the service receives the DB connection.
- Around line 24-55: get_splitwise_oauth_url currently returns the OAuth state
secret to the client without server-side storage; store the generated `secret`
server-side (e.g., Redis or DB) keyed by the current user id (use the same
`current_user=Depends(get_current_user)` value) before returning the
`authorization_url` and return only a non-sensitive reference if needed. In the
Splitwise callback handler (the endpoint that accepts `state` and exchanges the
token, e.g., your callback function), fetch the stored state by the same user
key, compare it to the incoming `state`, reject the request with an HTTP 400/401
if they don’t match, and only then proceed to exchange the token; finally
delete/expire the stored state after validation. Ensure you reference
`get_splitwise_oauth_url`, the callback handler name, and the `state`/`secret`
symbols when making these changes.
In @backend/app/integrations/schemas.py:
- Around line 102-109: ImportStatusCheckpoint currently defines stage fields as
plain strings ("pending"); change each field (user, friends, groups, expenses,
settlements) to use the ImportStatus enum type and set their defaults to the
appropriate enum member (e.g., ImportStatus.PENDING) so the model enforces enum
values and improves type safety; update the class signature to annotate each
field as ImportStatus and ensure ImportStatus is imported into the module.
In @backend/app/integrations/service.py:
- Around line 155-166: The OAuth credentials are being stored in plaintext when
calling self.oauth_tokens.insert_one; encrypt apiKey, consumerKey, and
consumerSecret before insertion by invoking your encryption helper (e.g.,
encrypt_secret or a kms_encrypt function) and store the encrypted blobs instead
of raw strings, updating the document keys ("apiKey", "consumerKey",
"consumerSecret") to hold the encrypted values; ensure decrypt_secret is used
when reading and document the encryption scheme and required env/config (KMS
key, encryption key, or Fernet) so production uses a secure key management
system.
- Around line 369-374: The current call to client.get_expenses(limit=1000) can
miss records for users with >1000 expenses; replace the single call with a
paginated fetch that repeatedly calls client.get_expenses(limit=..., offset=...)
(or the client’s paging params) in a loop, accumulate results into all_expenses
until an empty/shorter-than-limit batch is returned, then call await
self._update_checkpoint(import_job_id, "expensesImported.total",
len(all_expenses)); update any variables that assume a single batch (e.g.,
references to all_expenses) to use the accumulated list.
- Line 132: The call to options.dict() uses the deprecated Pydantic v2 API;
update the serialization to use options.model_dump() instead (replace the
options.dict() invocation where the payload is built so the "options" object is
serialized with model_dump()). Ensure any downstream code expecting the prior
dict format still works with the returned dict from model_dump().
- Around line 254-262: The code currently does users.find_one({"email":
friend_data["email"]}) which will match documents with missing or null email
when friend_data["email"] is None; before querying or inserting validate
friend_data.get("email") is a non-empty string (or a valid email) and only then
call users.find_one and users.insert_one; alternatively, if you must allow empty
emails, change the query to users.find_one({"email": {"$exists": True, "$eq":
friend_data["email"]}}) to avoid matching documents missing the field, and skip
or assign a generated placeholder when friend_data["email"] is None.
- Around line 76-82: The warning currently includes raw PII via the f"Email
{splitwise_user['email']} already exists" string; update the warnings.append
payload to avoid exposing the full email by either using a generic message like
"Email already exists" or a masked version of the address (e.g., show only
username initial and domain) before inserting into the "message" field; locate
the dict added in warnings.append and replace the f-string that references
splitwise_user['email'] with the non-PII/generic or masked value (you can add a
small helper like mask_email(email) and call it where the message is built).
- Around line 168-173: The background task created with asyncio.create_task is
discarded; capture and retain the Task so exceptions aren't lost and the task
can be monitored/cancelled. Assign the result of asyncio.create_task(...) to a
variable (e.g., task), add a done callback on that Task to log or handle
exceptions coming from self._perform_import, and store the Task reference on the
service instance (for example in a self._background_tasks dict keyed by
import_job_id or a list) so callers can query or cancel it later; keep the
existing comment about using a real task queue for production.
- Around line 314-323: The admin-role check is comparing different ID systems
(member["userId"] is a Splitwise ID) so it never matches; update the condition
inside the mapped_members append to compare the application's user ID instead,
e.g. replace ("admin" if member["userId"] == user_id else "member") with
("admin" if ObjectId(mapping["splitwiserId"]) == user_id else "member") so you
compare the same Splitwiser IDs (or, alternatively, compare
mapping["splitwiserId"] as a string to str(user_id) if user_id is not an
ObjectId).
- Around line 452-456: The createdAt assignment using
datetime.fromisoformat(expense_data["createdAt"]) can raise ValueError for
non-ISO strings; update the code that builds "createdAt" (the expense_data ->
"createdAt" logic inside the expense processing block) to try fromisoformat
first, then fall back to a more lenient parser (e.g., dateutil.parser.parse)
inside a localized try/except, and if parsing still fails use
datetime.now(timezone.utc); alternatively extract this into a small helper like
parse_created_at(expense_data["createdAt"]) that returns a timezone-aware
datetime so parsing errors are handled locally and do not cause the whole
expense to be skipped.
In @backend/app/integrations/splitwise/client.py:
- Around line 16-29: The __init__ parameters use implicit Optional types by
assigning = None; update the signature of Splitwise client __init__ to use
explicit Optional[str] for api_key, consumer_key, and consumer_secret (and add
the corresponding typing import), e.g. import Optional from typing and change
the annotations for api_key, consumer_key, consumer_secret to Optional[str], and
optionally annotate the method return as -> None; keep the rest of the
initialization (self.sObj = Splitwise(...)) unchanged.
- Around line 176-222: The loops and try/except blocks around users, split
transforms, category extraction (expense.getCategory) and receipt extraction
(expense.getReceipt) currently swallow all exceptions; change each bare "except
Exception" to capture the exception as "e" and log it (e.g., logger.exception or
logger.warning(..., exc_info=True)) with contextual identifiers like
user.getId(), paid_by_user_id, or expense.getId()/other expense fields before
continuing, so you still skip bad records but retain stacktrace and context for
debugging.
- Around line 106-107: The bare "except Exception: pass" in the Splitwise
balance-processing block silently swallows errors; replace it with a logging
warning that includes the caught exception and any relevant balance/input data
so issues are visible. Catch Exception as e, call the module/class logger (e.g.,
logger.warning or self.logger.warning) with a message like "Failed to parse
Splitwise balance" including the exception (or use logger.exception) and the
problematic balance payload, then continue or handle fallback as before.
In @backend/verify_settlements.py:
- Around line 104-146: The create_group function currently declares it returns
str but can return None on failure; change its signature to async def
create_group(...) -> Optional[str] and add from typing import Optional, keep the
early return None on non-201 responses, and ensure callers of create_group (see
the invocation around the area previously shown near lines 487-493) check the
returned value and either abort/raise a clear RuntimeError or handle the None
case before using the group_id in URLs or further requests so failures don’t
cascade into invalid requests or misleading verification results.
- Around line 147-177: The equal-split logic in create_expense currently uses
split_amount = round(amount / len(members), 2) which can cause the sum of splits
to differ from amount; instead compute splits in integer cents to guarantee
totals match: convert amount to cents (int(round(amount * 100))), compute
base_cents = cents // n and remainder = cents - base_cents * n, then build
splits by assigning base_cents to every member and adding one extra cent to the
first `remainder` members; replace the current `split_amount`/`splits`
construction with this cents-based distribution while still using
self.user_ids[member] for "userId" and converting per-member cents back to
dollars for the "amount" field so the sum of `splits` equals `amount`.
- Around line 428-463: cleanup_test_data currently deletes every group the test
accounts can access; limit deletion to only test-created groups by either (a)
tracking created group IDs in this script (e.g., maintain a created_group_ids
set when creating groups and iterate that set in cleanup_test_data to delete
only those IDs), or (b) filter fetched groups by a safe name prefix (check
group.get("name") against the test prefix before deleting). Remove or use the
unused local group_name only for the prefix check, and keep the existing
token/header logic and HTTP delete calls but applied only to the tracked IDs or
filtered groups.
In @docs/splitwise-import-integration.md:
- Around line 24-36: Add explicit fenced code block languages (e.g., use
```text) for the ASCII diagrams (for example the block that begins with
"┌─────────────────┐" and the Splitwise import diagram), ensure there is exactly
one blank line before and after each fenced block, remove any trailing spaces on
lines inside and around those blocks, and fix spacing around headings so
markdownlint rules pass; apply the same changes to the other diagram/code blocks
noted (lines 110-137, 206-212, 221-255, 260-288).
In @mobile/screens/SplitwiseImportScreen.js:
- Around line 24-30: The loading spinner is never cleared after successfully
opening the OAuth URL; after calling Linking.openURL(authorization_url) you
should reset the component's loading state (e.g., call setLoading(false) or
equivalent) so the spinner stops; update the block around Linking.openURL and
Alert.alert to call the state reset immediately after openURL succeeds (or in
the Alert.alert onPress handler before navigation.goBack()) to ensure the
loading flag used by the spinner is cleared.
- Around line 143-167: Remove the unused style properties from the styles object
in SplitwiseImportScreen.js: delete input, link, progressContainer,
progressHeader, progressText, and progressBar from the StyleSheet so the styles
object only contains used keys; also search the file for any remaining
references to these symbol names (e.g., className/ style={styles.input} etc.)
and remove or replace them if found to avoid runtime errors.
In @web/pages/Profile.tsx:
- Around line 111-116: The Import menu entry currently reuses the Settings icon
which can confuse users; update the items array for the menu object with title
'Import' to use a more semantically appropriate icon (e.g., replace Settings
with Download or ArrowDownToLine from lucide-react) for the item whose onClick
navigates to '/import/splitwise' so the entry visually indicates
importing/download rather than settings.
In @web/pages/SplitwiseImport.tsx:
- Around line 41-61: The button element that invokes handleOAuthImport is
missing an explicit type (so it defaults to "submit") and the SVG lacks
accessible text; change the button to include type="button" to prevent
unintended form submissions and update the SVG used in the non-loading branch to
provide accessible alternative text (e.g., add a <title> like "Splitwise icon"
and ensure the SVG has role="img" or an aria-label) so screen readers announce
the icon; keep the loading spinner decorative (e.g., aria-hidden="true") while
the non-loading SVG provides a proper accessible name.
In @web/services/api.ts:
- Around line 52-58: The frontend function startSplitwiseImport is sending an
unused apiKey to /import/splitwise/start; remove the apiKey parameter from
startSplitwiseImport in web/services/api.ts and stop including { api_key: apiKey
} in the POST body, then update all call sites to call startSplitwiseImport()
with no args. Alternatively, if per-request credentials are desired instead,
modify the backend router handler referenced (router.py lines ~126–128) to
accept and validate an api_key from the request body and use it instead of the
environment config, and add server-side validation/tests accordingly so the
frontend payload becomes meaningful.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (20)
backend/app/config.pybackend/app/integrations/__init__.pybackend/app/integrations/router.pybackend/app/integrations/schemas.pybackend/app/integrations/service.pybackend/app/integrations/splitwise/__init__.pybackend/app/integrations/splitwise/client.pybackend/main.pybackend/requirements.txtbackend/verify_settlements.pydocs/splitwise-import-integration.mdmobile/api/client.jsmobile/navigation/AccountStackNavigator.jsmobile/screens/AccountScreen.jsmobile/screens/SplitwiseImportScreen.jsweb/App.tsxweb/pages/Profile.tsxweb/pages/SplitwiseCallback.tsxweb/pages/SplitwiseImport.tsxweb/services/api.ts
🧰 Additional context used
📓 Path-based instructions (4)
web/pages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Web pages should be created as components in
web/pages/and added to routing configuration
Files:
web/pages/Profile.tsxweb/pages/SplitwiseImport.tsxweb/pages/SplitwiseCallback.tsx
backend/app/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
backend/app/**/*.py: Backend API routes should be added to the appropriate service router file inbackend/app/based on the feature area (auth, user, groups, expenses)
Use RESTful API design patterns with FastAPI in the backend
Use MongoDB for persistent data storage in the backend with a nonrelational schema design
Files:
backend/app/integrations/__init__.pybackend/app/integrations/splitwise/client.pybackend/app/config.pybackend/app/integrations/splitwise/__init__.pybackend/app/integrations/router.pybackend/app/integrations/service.pybackend/app/integrations/schemas.py
{mobile/api/**/*.js,web/services/**/*.{ts,tsx,js}}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Axios for HTTP requests with Content-Type application/json header in API client calls
Files:
web/services/api.tsmobile/api/client.js
mobile/screens/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Mobile screens should be created as components in
mobile/screens/and registered in the navigation structure
Files:
mobile/screens/SplitwiseImportScreen.jsmobile/screens/AccountScreen.js
🧬 Code graph analysis (8)
web/pages/Profile.tsx (1)
backend/app/config.py (1)
Settings(12-52)
web/services/api.ts (1)
mobile/api/client.js (10)
getSplitwiseAuthUrl(120-122)getSplitwiseAuthUrl(120-122)handleSplitwiseCallback(124-126)handleSplitwiseCallback(124-126)startSplitwiseImport(128-130)startSplitwiseImport(128-130)getImportStatus(132-134)getImportStatus(132-134)rollbackImport(136-138)rollbackImport(136-138)
web/pages/SplitwiseImport.tsx (2)
mobile/screens/SplitwiseImportScreen.js (2)
loading(14-14)handleOAuthImport(16-43)web/contexts/ToastContext.tsx (1)
useToast(39-45)
mobile/api/client.js (1)
web/services/api.ts (5)
getSplitwiseAuthUrl(53-53)handleSplitwiseCallback(54-54)startSplitwiseImport(55-55)getImportStatus(56-56)rollbackImport(57-57)
web/pages/SplitwiseCallback.tsx (2)
web/contexts/ToastContext.tsx (1)
useToast(39-45)mobile/api/client.js (1)
status(84-84)
web/App.tsx (2)
web/pages/SplitwiseImport.tsx (1)
SplitwiseImport(5-98)web/pages/SplitwiseCallback.tsx (1)
SplitwiseCallback(6-114)
backend/verify_settlements.py (1)
backend/tests/user/test_user_routes.py (1)
client(15-17)
backend/app/integrations/service.py (3)
backend/app/integrations/schemas.py (5)
ImportError(46-52)ImportOptions(55-61)ImportProvider(12-15)ImportStatus(18-25)ImportSummary(124-132)backend/app/integrations/splitwise/client.py (9)
SplitwiseClient(13-276)get_current_user(31-33)get_friends(35-37)get_groups(39-41)get_expenses(43-56)transform_user(59-78)transform_friend(81-118)transform_group(121-155)transform_expense(171-276)backend/app/integrations/router.py (1)
get_import_status(159-243)
🪛 Biome (2.1.2)
web/pages/SplitwiseImport.tsx
[error] 55-55: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
[error] 41-47: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🪛 LanguageTool
docs/splitwise-import-integration.md
[grammar] ~264-~264: Use a hyphen to join words.
Context: ...esponse: Array of comments ``` ### Rate Limiting Strategy Splitwise API has rat...
(QB_NEW_EN_HYPHEN)
[uncategorized] ~989-~989: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ion API client, monitor for changes | | Rate limiting issues | Medium | High | Implement smar...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.18.1)
docs/splitwise-import-integration.md
24-24: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
52-52: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
110-110: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
118-118: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
126-126: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
206-206: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
269-269: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
436-436: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
441-441: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
446-446: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
451-451: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
453-453: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
458-458: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
687-687: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
688-688: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
689-689: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
690-690: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
692-692: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
692-692: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.10)
backend/app/integrations/splitwise/client.py
8-8: typing.Dict is deprecated, use dict instead
(UP035)
8-8: typing.List is deprecated, use list instead
(UP035)
16-16: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
17-17: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
17-17: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
17-17: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
106-107: try-except-pass detected, consider logging the exception
(S110)
106-106: Do not catch blind exception: Exception
(BLE001)
158-158: Missing return type annotation for staticmethod _safe_isoformat
(ANN205)
181-182: try-except-continue detected, consider logging the exception
(S112)
181-181: Do not catch blind exception: Exception
(BLE001)
201-202: try-except-continue detected, consider logging the exception
(S112)
201-201: Do not catch blind exception: Exception
(BLE001)
211-212: try-except-pass detected, consider logging the exception
(S110)
211-211: Do not catch blind exception: Exception
(BLE001)
221-222: try-except-pass detected, consider logging the exception
(S110)
221-221: Do not catch blind exception: Exception
(BLE001)
backend/verify_settlements.py
28-28: typing.Dict is deprecated, use dict instead
(UP035)
28-28: typing.List is deprecated, use list instead
(UP035)
47-47: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
77-77: f-string without any placeholders
Remove extraneous f prefix
(F541)
81-81: Do not catch blind exception: Exception
(BLE001)
181-181: f-string without any placeholders
Remove extraneous f prefix
(F541)
326-326: f-string without any placeholders
Remove extraneous f prefix
(F541)
447-447: Local variable group_name is assigned to but never used
Remove assignment to unused variable group_name
(F841)
462-462: f-string without any placeholders
Remove extraneous f prefix
(F541)
462-462: String contains ambiguous ℹ (INFORMATION SOURCE). Did you mean i (LATIN SMALL LETTER I)?
(RUF001)
575-575: Do not catch blind exception: Exception
(BLE001)
backend/app/integrations/router.py
25-25: Unused function argument: current_user
(ARG001)
25-25: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
60-60: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
98-98: Do not catch blind exception: Exception
(BLE001)
99-101: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
100-100: Use explicit conversion flag
Replace with conversion flag
(RUF010)
107-107: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
108-108: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
151-151: Do not catch blind exception: Exception
(BLE001)
152-155: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
154-154: Use explicit conversion flag
Replace with conversion flag
(RUF010)
161-161: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
162-162: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
249-249: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
250-250: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
backend/app/integrations/service.py
7-7: typing.Dict is deprecated, use dict instead
(UP035)
7-7: typing.List is deprecated, use list instead
(UP035)
25-25: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
169-173: Store a reference to the return value of asyncio.create_task
(RUF006)
177-177: Missing return type annotation for private function _perform_import
Add return type annotation: None
(ANN202)
211-211: Boolean positional value in function call
(FBT003)
217-217: Boolean positional value in function call
(FBT003)
241-241: Do not catch blind exception: Exception
(BLE001)
249-249: Missing return type annotation for private function _import_friends
Add return type annotation: None
(ANN202)
249-249: Unused method argument: user_id
(ARG002)
294-294: Do not catch blind exception: Exception
(BLE001)
297-297: Missing return type annotation for private function _import_groups
Add return type annotation: None
(ANN202)
358-358: Do not catch blind exception: Exception
(BLE001)
361-361: Missing return type annotation for private function _import_expenses
Add return type annotation: None
(ANN202)
478-478: Do not catch blind exception: Exception
(BLE001)
481-481: Missing return type annotation for private function _update_checkpoint
Add return type annotation: None
(ANN202)
482-482: Boolean-typed positional argument in function definition
(FBT001)
482-482: Boolean default positional argument in function definition
(FBT002)
496-496: Missing return type annotation for private function _increment_summary
Add return type annotation: None
(ANN202)
502-502: Missing return type annotation for private function _record_error
Add return type annotation: None
(ANN202)
554-558: Consider moving this statement to an else block
(TRY300)
560-560: Do not catch blind exception: Exception
(BLE001)
564-564: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backend/app/integrations/schemas.py
7-7: typing.Dict is deprecated, use dict instead
(UP035)
7-7: typing.List is deprecated, use list instead
(UP035)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: rn-bundle-analysis
- GitHub Check: preview
🔇 Additional comments (29)
mobile/screens/AccountScreen.js (1)
56-61: Looks good; just confirm the"import"icon name exists in your icon set. If not, the item may render with a missing icon glyph.backend/app/integrations/splitwise/__init__.py (1)
1-5: LGTM. Clear package docstring; no behavioral impact.mobile/navigation/AccountStackNavigator.js (1)
4-4: LGTM. Route registration matchesnavigation.navigate("SplitwiseImport").Also applies to: 13-13
backend/verify_settlements.py (1)
269-300: Verify expected balance values against actual backend calculation behavior. The script's expected values assume completed settlements don't affect balances, but the backend's aggregation pipelines include all settlements (pending and completed) in balance calculations. If actual API responses include completed settlements, these expected values are correct; if they exclude completed settlements despite the code including them, the assertions will fail.backend/requirements.txt (1)
14-14: Pinsplitwiseto an exact version to maintain reproducibility. All other dependencies in requirements.txt use==pinning (fastapi==0.116.1, pymongo==4.13.1, etc.), but splitwise is unpinned with>=3.0.0. Since the codebase heavily depends on splitwise for OAuth and data import flows, inconsistent versions across environments could cause breakage. Determine the appropriate version for this codebase and update tosplitwise==X.Y.Z.backend/app/integrations/__init__.py (1)
1-7: LGTM!Clean package initialization with appropriate documentation describing the module's purpose and future extensibility.
backend/main.py (2)
9-9: LGTM!Import follows the established pattern for other routers in the application.
132-132: Router is properly configured with prefix and tags.The
integrations_routerinbackend/app/integrations/router.pycorrectly defines both the prefix (/import) and tags (["import"]) for API documentation consistency. No changes needed.web/App.tsx (1)
15-16: LGTM!Imports for the new Splitwise integration pages follow the established pattern.
web/pages/SplitwiseCallback.tsx (1)
78-113: LGTM!The JSX structure is clean with good dark mode support, proper loading states, and helpful user guidance.
mobile/api/client.js (1)
118-138: LGTM!The new Splitwise import API functions are well-structured and consistent with the existing client patterns. They correctly use the configured
apiClientwhich already includes the requiredContent-Type: application/jsonheader. The API surface aligns with the web counterpart inweb/services/api.ts.web/pages/SplitwiseImport.tsx (1)
9-25: LGTM!The OAuth flow initiation is clean. The error handling properly displays backend error details and resets loading state on failure. Since
window.location.hrefnavigates away on success, not resettingloadingis acceptable.mobile/screens/SplitwiseImportScreen.js (1)
45-121: LGTM!The UI implementation is well-structured with proper use of react-native-paper components. The informational cards clearly communicate what will be imported and important notes for the user.
backend/app/integrations/router.py (2)
158-243: LGTM!The status endpoint correctly verifies ownership, calculates progress percentages, and provides detailed stage information. The authorization check properly compares user IDs.
246-275: LGTM!The rollback endpoint properly verifies ownership before allowing the operation. The authorization flow is consistent with the status endpoint.
backend/app/integrations/splitwise/client.py (5)
31-56: LGTM!The API wrapper methods are clean pass-throughs to the SDK with appropriate parameter handling.
58-78: LGTM!Defensive attribute access pattern is appropriate for handling SDK object variations.
120-155: LGTM!Group transformation logic is solid. Note that
joinedAtis set to import time rather than the original join date, which is acceptable if the original data isn't available from the SDK.
157-168: LGTM!Robust date handling with appropriate fallbacks for various input types.
173-182: Only the first payer is captured when multiple users contribute.The
breakon line 180 exits after finding the first user withpaidShare > 0. If Splitwise allows multiple payers on a single expense, only the first will be recorded. Verify this matches the expected behavior for multi-payer expenses.backend/app/integrations/schemas.py (6)
12-35: LGTM!Enums are well-defined with appropriate values. Using
str, Enumpattern allows direct JSON serialization.
38-62: LGTM!Well-structured models with sensible defaults for import configuration and error tracking.
64-85: LGTM!Preview schemas are appropriately flexible for the import preview flow.
87-100: LGTM!Request/response models for starting imports are clean and complete.
124-158: LGTM!Summary and job response models are well-structured with appropriate field types.
176-180: Verify OAuth state validation is enforced elsewhere.The
stateparameter inOAuthCallbackRequestis optional, but OAuth state is critical for CSRF protection. Ensure the router or service layer validates state before processing the callback.backend/app/integrations/service.py (3)
22-33: LGTM!Service initialization with MongoDB collections is clean and appropriate. Following coding guidelines, MongoDB is correctly used for persistent storage.
177-247: LGTM!Import orchestration has appropriate error handling with status updates on failure. The sequential approach is reasonable for an initial implementation.
481-519: LGTM!Helper methods are clean and appropriately handle MongoDB operations and ID conversions.
| # Splitwise Integration | ||
| splitwise_api_key: Optional[str] = None | ||
| splitwise_consumer_key: Optional[str] = None | ||
| splitwise_consumer_secret: Optional[str] = None | ||
| frontend_url: str = "http://localhost:5173" # Frontend URL for OAuth redirect | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's explore the backend directory structure
git ls-files backend/app/ | head -20Repository: Devasy/splitwiser
Length of output: 644
🏁 Script executed:
# Read the config.py file to see the actual implementation
cat -n backend/app/config.pyRepository: Devasy/splitwiser
Length of output: 3651
🏁 Script executed:
# Search for RequestResponseLoggingMiddleware to understand logging behavior
fd -t f "*.py" backend/ -x grep -l "RequestResponseLoggingMiddleware"Repository: Devasy/splitwiser
Length of output: 344
🏁 Script executed:
# Search for Splitwise usage throughout the codebase
rg -n "splitwise" --type py -iRepository: Devasy/splitwiser
Length of output: 12475
🏁 Script executed:
# Check if there's URL validation in the router file
cat -n backend/app/integrations/router.py | head -100Repository: Devasy/splitwiser
Length of output: 4161
🏁 Script executed:
# Search for any logging of credentials or sensitive data
rg -n "logger\.|splitwise_consumer|splitwise_api_key|frontend_url" backend/app/integrations/router.pyRepository: Devasy/splitwiser
Length of output: 854
🏁 Script executed:
# Check if there are any pydantic validators in config.py
rg -n "validator|field_validator" backend/app/config.pyRepository: Devasy/splitwiser
Length of output: 43
🏁 Script executed:
# Search for any logging statements that might expose secrets
rg -n "logger\." backend/app/config.py backend/app/integrations/service.py backend/app/integrations/router.pyRepository: Devasy/splitwiser
Length of output: 1184
Add URL validation for frontend_url to prevent OAuth redirect vulnerabilities.
The frontend_url configuration is used directly in OAuth redirect URIs (lines 47 and 81 in router.py) without validation. An attacker could set this to an untrusted domain, creating an open redirect vulnerability. Consider:
- Validating that
frontend_urlmatches an allowed list of origins - Using Pydantic validators in the Settings class to enforce valid URL formats and trusted domains
The RequestResponseLoggingMiddleware is safe—it only logs request/response metadata (method, URL, status, time) and does not expose request bodies or headers containing credentials.
🤖 Prompt for AI Agents
In @backend/app/config.py around lines 36 - 41, The Settings class exposes
frontend_url which is used directly in OAuth redirect URIs (router.py), allowing
an attacker to configure an untrusted domain; add a Pydantic validator on the
Settings model (e.g., validate_frontend_url or a @validator for frontend_url) to
ensure the value is a well-formed URL and matches a whitelist of allowed origins
(or at minimum only allows specific schemes and hostnames), and update any code
paths that consume frontend_url (router functions that build redirect URIs) to
rely on the validated property so untrusted domains cannot be used for
redirects.
| @router.get("/splitwise/authorize") | ||
| async def get_splitwise_oauth_url(current_user=Depends(get_current_user)): | ||
| """ | ||
| Get Splitwise OAuth 2.0 authorization URL. | ||
| Returns the URL where user should be redirected to authorize Splitwiser | ||
| to access their Splitwise data. | ||
| """ | ||
| if not all([settings.splitwise_consumer_key, settings.splitwise_consumer_secret]): | ||
| raise HTTPException( | ||
| status_code=500, | ||
| detail="Splitwise OAuth not configured. Please contact administrator.", | ||
| ) | ||
|
|
||
| # Initialize Splitwise SDK with OAuth credentials | ||
| sObj = Splitwise( | ||
| consumer_key=settings.splitwise_consumer_key, | ||
| consumer_secret=settings.splitwise_consumer_secret, | ||
| ) | ||
|
|
||
| # Get OAuth authorization URL | ||
| # User will be redirected back to: {FRONTEND_URL}/import/splitwise/callback | ||
| auth_url, secret = sObj.getOAuth2AuthorizeURL( | ||
| redirect_uri=f"{settings.frontend_url}/import/splitwise/callback" | ||
| ) | ||
|
|
||
| # Store the secret temporarily (you may want to use Redis/cache instead) | ||
| # For now, we'll include it in the response for the callback to use | ||
| return { | ||
| "authorization_url": auth_url, | ||
| "state": secret, # This will be needed in the callback | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if state validation exists in the callback or service layer
rg -n "state" backend/app/integrations/ -A 3 -B 1Repository: Devasy/splitwiser
Length of output: 767
🏁 Script executed:
#!/bin/bash
# Search for the callback endpoint
rg -n "splitwise_oauth_callback|callback" backend/app/integrations/ -A 5 -B 2Repository: Devasy/splitwiser
Length of output: 3455
🏁 Script executed:
#!/bin/bash
# Look for all routes in the integrations router
rg -n "@router\." backend/app/integrations/router.pyRepository: Devasy/splitwiser
Length of output: 376
🏁 Script executed:
#!/bin/bash
# Read the complete callback endpoint implementation
sed -n '58,103p' backend/app/integrations/router.pyRepository: Devasy/splitwiser
Length of output: 1618
🏁 Script executed:
#!/bin/bash
# Check if there's any state validation logic in the service layer
rg -n "validate.*state|state.*valid|csrf" backend/app/integrations/Repository: Devasy/splitwiser
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check for state storage in database, cache, or session
rg -n "state|redis|cache|session" backend/app/integrations/router.py | head -20Repository: Devasy/splitwiser
Length of output: 207
🏁 Script executed:
#!/bin/bash
# Check the full router.py file size and look for any state persistence
wc -l backend/app/integrations/router.pyRepository: Devasy/splitwiser
Length of output: 99
Implement OAuth state validation to prevent CSRF attacks.
The state parameter returned to the client is not validated in the callback endpoint. The callback accepts state in the request but never compares it against the original value. Implement state storage (Redis, session, or database) during the authorize request and validate the returned state matches the stored value before processing the token exchange. This is a required security measure for OAuth 2.0.
🧰 Tools
🪛 Ruff (0.14.10)
25-25: Unused function argument: current_user
(ARG001)
25-25: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🤖 Prompt for AI Agents
In @backend/app/integrations/router.py around lines 24 - 55,
get_splitwise_oauth_url currently returns the OAuth state secret to the client
without server-side storage; store the generated `secret` server-side (e.g.,
Redis or DB) keyed by the current user id (use the same
`current_user=Depends(get_current_user)` value) before returning the
`authorization_url` and return only a non-sensitive reference if needed. In the
Splitwise callback handler (the endpoint that accepts `state` and exchanges the
token, e.g., your callback function), fetch the stored state by the same user
key, compare it to the incoming `state`, reject the request with an HTTP 400/401
if they don’t match, and only then proceed to exchange the token; finally
delete/expire the stored state after validation. Ensure you reference
`get_splitwise_oauth_url`, the callback handler name, and the `state`/`secret`
symbols when making these changes.
| # Start import with the access token | ||
| service = ImportService() | ||
| import_job_id = await service.start_import( | ||
| user_id=current_user["_id"], | ||
| provider="splitwise", | ||
| api_key=access_token["access_token"], # Use access token | ||
| ) |
There was a problem hiding this comment.
Inconsistent ImportService instantiation: missing db parameter.
On line 85, ImportService() is instantiated without the db parameter, while on line 136 it's called with ImportService(db). This inconsistency will likely cause a runtime error or unexpected behavior if ImportService requires a database connection.
🐛 Proposed fix
You need to add the db dependency to the endpoint and pass it to ImportService:
@router.post("/splitwise/callback")
async def splitwise_oauth_callback(
- request: OAuthCallbackRequest, current_user=Depends(get_current_user)
+ request: OAuthCallbackRequest,
+ current_user=Depends(get_current_user),
+ db: AsyncIOMotorDatabase = Depends(get_database),
):
...
try:
access_token = sObj.getOAuth2AccessToken(
code=request.code,
redirect_uri=f"{settings.frontend_url}/import/splitwise/callback",
)
- service = ImportService()
+ service = ImportService(db)
import_job_id = await service.start_import(🤖 Prompt for AI Agents
In @backend/app/integrations/router.py around lines 84 - 90, ImportService is
being instantiated without the required db dependency (ImportService()) causing
inconsistency with the later call ImportService(db); update the endpoint to
accept the database dependency and pass it into the service: add the db
parameter (injected via your existing DB dependency provider) to the route
handler signature and replace ImportService() with ImportService(db) when
calling start_import so both usages are consistent and the service receives the
DB connection.
| except Exception as e: | ||
| raise HTTPException( | ||
| status_code=400, detail=f"Failed to exchange OAuth code: {str(e)}" | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use exception chaining for proper error context.
Re-raising without from e or from None obscures the original traceback. This makes debugging harder and is flagged by Ruff (B904).
♻️ Proposed fix
except Exception as e:
raise HTTPException(
- status_code=400, detail=f"Failed to exchange OAuth code: {str(e)}"
- )
+ status_code=400, detail=f"Failed to exchange OAuth code: {e!s}"
+ ) from e🧰 Tools
🪛 Ruff (0.14.10)
98-98: Do not catch blind exception: Exception
(BLE001)
99-101: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
100-100: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
In @backend/app/integrations/router.py around lines 98 - 101, In the except
block that catches Exception as e in backend/app/integrations/router.py (the
block that raises HTTPException with detail "Failed to exchange OAuth code"),
re-raise the HTTPException using exception chaining by adding "from e" to
preserve the original traceback (i.e., raise HTTPException(...) from e) so the
original exception context is not lost.
| except Exception as e: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, | ||
| detail=f"Failed to start import: {str(e)}", | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use exception chaining here as well.
Same issue as the callback endpoint—chain the exception for proper traceback context.
♻️ Proposed fix
except Exception as e:
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
- detail=f"Failed to start import: {str(e)}",
- )
+ detail=f"Failed to start import: {e!s}",
+ ) from e📝 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.
| except Exception as e: | |
| raise HTTPException( | |
| status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, | |
| detail=f"Failed to start import: {str(e)}", | |
| ) | |
| except Exception as e: | |
| raise HTTPException( | |
| status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, | |
| detail=f"Failed to start import: {e!s}", | |
| ) from e |
🧰 Tools
🪛 Ruff (0.14.10)
151-151: Do not catch blind exception: Exception
(BLE001)
152-155: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
154-154: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
In @backend/app/integrations/router.py around lines 151 - 155, The HTTPException
raised in the except block that handles import startup failures should use
exception chaining to preserve the original traceback; update the raise of
HTTPException (the one with status_code=status.HTTP_500_INTERNAL_SERVER_ERROR
and detail=f"Failed to start import: {str(e)}") to re-raise the HTTPException
using "from e" so the original exception e is chained to the HTTPException.
| <Route path="/import/splitwise" element={<ProtectedRoute><SplitwiseImport /></ProtectedRoute>} /> | ||
| <Route path="/import/splitwise/callback" element={<ProtectedRoute><SplitwiseCallback /></ProtectedRoute>} /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's explore the repository structure and locate the relevant files
git ls-files | grep -E "(App\.tsx|SplitwiseCallback\.tsx)" | head -20Repository: Devasy/splitwiser
Length of output: 104
🏁 Script executed:
# List files in the web directory to understand the structure
fd -t f "\.tsx$" | grep -E "(App|Splitwise)" | head -20Repository: Devasy/splitwiser
Length of output: 134
🏁 Script executed:
# Read App.tsx with line numbers, focusing on the route area
cat -n web/App.tsx | sed -n '40,50p'Repository: Devasy/splitwiser
Length of output: 871
🏁 Script executed:
# Read SplitwiseCallback.tsx to examine the useEffect structure
cat -n web/pages/SplitwiseCallback.tsxRepository: Devasy/splitwiser
Length of output: 5305
OAuth callback route should not be protected, and useEffect cleanup is incorrectly implemented.
The /import/splitwise/callback route is wrapped in ProtectedRoute, which redirects unauthenticated users to /login. If the user's session expires during the OAuth flow, they'll lose the authorization code from the URL parameters and the flow will break.
Additionally, in SplitwiseCallback.tsx, the cleanup function at line 63 is incorrectly placed inside the async handleCallback function rather than being returned from the useEffect itself. This means the cleanup function is never executed—the interval won't be cleared on component unmount, causing memory leaks and potential multiple intervals running simultaneously.
| { | ||
| title: 'Import', | ||
| items: [ | ||
| { label: 'Import from Splitwise', icon: Settings, onClick: () => navigate('/import/splitwise'), desc: 'Import all your Splitwise data' }, | ||
| ] | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using a more descriptive icon for the Import action.
The implementation is correct and follows the existing menu pattern. However, the Settings icon is reused here, which may cause visual confusion with the "Appearance" item in the App section. Consider using a more semantically appropriate icon like Download or ArrowDownToLine from lucide-react.
💡 Suggested icon change
-import { Camera, ChevronRight, CreditCard, LogOut, Mail, MessageSquare, Settings, Shield, User } from 'lucide-react';
+import { Camera, ChevronRight, CreditCard, Download, LogOut, Mail, MessageSquare, Settings, Shield, User } from 'lucide-react'; {
title: 'Import',
items: [
- { label: 'Import from Splitwise', icon: Settings, onClick: () => navigate('/import/splitwise'), desc: 'Import all your Splitwise data' },
+ { label: 'Import from Splitwise', icon: Download, onClick: () => navigate('/import/splitwise'), desc: 'Import all your Splitwise data' },
]
},🤖 Prompt for AI Agents
In @web/pages/Profile.tsx around lines 111 - 116, The Import menu entry
currently reuses the Settings icon which can confuse users; update the items
array for the menu object with title 'Import' to use a more semantically
appropriate icon (e.g., replace Settings with Download or ArrowDownToLine from
lucide-react) for the item whose onClick navigates to '/import/splitwise' so the
entry visually indicates importing/download rather than settings.
| useEffect(() => { | ||
| const handleCallback = async () => { | ||
| const code = searchParams.get('code'); | ||
| const state = searchParams.get('state'); | ||
|
|
||
| if (!code) { | ||
| showToast('Authorization failed - no code received', 'error'); | ||
| navigate('/import/splitwise'); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| // Send code to backend to exchange for access token and start import | ||
| const response = await handleSplitwiseCallback(code, state || ''); | ||
| const jobId = response.data.import_job_id || response.data.importJobId; | ||
|
|
||
| if (!jobId) { | ||
| throw new Error('No import job ID received'); | ||
| } | ||
|
|
||
| showToast('Authorization successful! Starting import...', 'success'); | ||
| setStatus('Import started...'); | ||
|
|
||
| // Poll for progress | ||
| const pollInterval = setInterval(async () => { | ||
| try { | ||
| const statusResponse = await getImportStatus(jobId); | ||
| const statusData = statusResponse.data; | ||
|
|
||
| setProgress(statusData.progress_percentage || 0); | ||
| setStatus(statusData.current_stage || 'Processing...'); | ||
|
|
||
| if (statusData.status === 'completed') { | ||
| clearInterval(pollInterval); | ||
| setImporting(false); | ||
| showToast('Import completed successfully!', 'success'); | ||
| setStatus('Completed! Redirecting to dashboard...'); | ||
| setTimeout(() => navigate('/dashboard'), 2000); | ||
| } else if (statusData.status === 'failed') { | ||
| clearInterval(pollInterval); | ||
| setImporting(false); | ||
| showToast('Import failed', 'error'); | ||
| setStatus(`Failed: ${statusData.error_details || 'Unknown error'}`); | ||
| } | ||
| } catch (error) { | ||
| console.error('Error polling import status:', error); | ||
| } | ||
| }, 2000); | ||
|
|
||
| return () => clearInterval(pollInterval); | ||
| } catch (error: any) { | ||
| console.error('Callback error:', error); | ||
| showToast( | ||
| error.response?.data?.detail || 'Failed to process authorization', | ||
| 'error' | ||
| ); | ||
| setImporting(false); | ||
| setTimeout(() => navigate('/import/splitwise'), 2000); | ||
| } | ||
| }; | ||
|
|
||
| handleCallback(); | ||
| }, [searchParams, navigate, showToast]); |
There was a problem hiding this comment.
Memory leak: interval cleanup function is unreachable.
The return () => clearInterval(pollInterval) on line 63 is inside the async handleCallback function. This return value is ignored—React's useEffect cleanup only works from the direct return of the effect callback, not from a nested async function. If the component unmounts while polling, the interval will continue running.
🔧 Proposed fix
export const SplitwiseCallback = () => {
const [searchParams] = useSearchParams();
const navigate = useNavigate();
const { showToast } = useToast();
const [status, setStatus] = useState('Processing authorization...');
const [progress, setProgress] = useState(0);
const [importing, setImporting] = useState(true);
+ const pollIntervalRef = useRef<ReturnType<typeof setInterval> | null>(null);
useEffect(() => {
const handleCallback = async () => {
const code = searchParams.get('code');
const state = searchParams.get('state');
if (!code) {
showToast('Authorization failed - no code received', 'error');
navigate('/import/splitwise');
return;
}
try {
const response = await handleSplitwiseCallback(code, state || '');
const jobId = response.data.import_job_id || response.data.importJobId;
if (!jobId) {
throw new Error('No import job ID received');
}
showToast('Authorization successful! Starting import...', 'success');
setStatus('Import started...');
- const pollInterval = setInterval(async () => {
+ pollIntervalRef.current = setInterval(async () => {
try {
const statusResponse = await getImportStatus(jobId);
const statusData = statusResponse.data;
setProgress(statusData.progress_percentage || 0);
setStatus(statusData.current_stage || 'Processing...');
if (statusData.status === 'completed') {
- clearInterval(pollInterval);
+ if (pollIntervalRef.current) clearInterval(pollIntervalRef.current);
setImporting(false);
showToast('Import completed successfully!', 'success');
setStatus('Completed! Redirecting to dashboard...');
setTimeout(() => navigate('/dashboard'), 2000);
} else if (statusData.status === 'failed') {
- clearInterval(pollInterval);
+ if (pollIntervalRef.current) clearInterval(pollIntervalRef.current);
setImporting(false);
showToast('Import failed', 'error');
setStatus(`Failed: ${statusData.error_details || 'Unknown error'}`);
}
} catch (error) {
console.error('Error polling import status:', error);
}
}, 2000);
-
- return () => clearInterval(pollInterval);
} catch (error: any) {
console.error('Callback error:', error);
showToast(
error.response?.data?.detail || 'Failed to process authorization',
'error'
);
setImporting(false);
setTimeout(() => navigate('/import/splitwise'), 2000);
}
};
handleCallback();
+
+ return () => {
+ if (pollIntervalRef.current) {
+ clearInterval(pollIntervalRef.current);
+ }
+ };
}, [searchParams, navigate, showToast]);Also add useRef to the import on line 1:
-import { useEffect, useState } from 'react';
+import { useEffect, useRef, useState } from 'react';📝 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.
| useEffect(() => { | |
| const handleCallback = async () => { | |
| const code = searchParams.get('code'); | |
| const state = searchParams.get('state'); | |
| if (!code) { | |
| showToast('Authorization failed - no code received', 'error'); | |
| navigate('/import/splitwise'); | |
| return; | |
| } | |
| try { | |
| // Send code to backend to exchange for access token and start import | |
| const response = await handleSplitwiseCallback(code, state || ''); | |
| const jobId = response.data.import_job_id || response.data.importJobId; | |
| if (!jobId) { | |
| throw new Error('No import job ID received'); | |
| } | |
| showToast('Authorization successful! Starting import...', 'success'); | |
| setStatus('Import started...'); | |
| // Poll for progress | |
| const pollInterval = setInterval(async () => { | |
| try { | |
| const statusResponse = await getImportStatus(jobId); | |
| const statusData = statusResponse.data; | |
| setProgress(statusData.progress_percentage || 0); | |
| setStatus(statusData.current_stage || 'Processing...'); | |
| if (statusData.status === 'completed') { | |
| clearInterval(pollInterval); | |
| setImporting(false); | |
| showToast('Import completed successfully!', 'success'); | |
| setStatus('Completed! Redirecting to dashboard...'); | |
| setTimeout(() => navigate('/dashboard'), 2000); | |
| } else if (statusData.status === 'failed') { | |
| clearInterval(pollInterval); | |
| setImporting(false); | |
| showToast('Import failed', 'error'); | |
| setStatus(`Failed: ${statusData.error_details || 'Unknown error'}`); | |
| } | |
| } catch (error) { | |
| console.error('Error polling import status:', error); | |
| } | |
| }, 2000); | |
| return () => clearInterval(pollInterval); | |
| } catch (error: any) { | |
| console.error('Callback error:', error); | |
| showToast( | |
| error.response?.data?.detail || 'Failed to process authorization', | |
| 'error' | |
| ); | |
| setImporting(false); | |
| setTimeout(() => navigate('/import/splitwise'), 2000); | |
| } | |
| }; | |
| handleCallback(); | |
| }, [searchParams, navigate, showToast]); | |
| import { useEffect, useRef, useState } from 'react'; | |
| export const SplitwiseCallback = () => { | |
| const [searchParams] = useSearchParams(); | |
| const navigate = useNavigate(); | |
| const { showToast } = useToast(); | |
| const [status, setStatus] = useState('Processing authorization...'); | |
| const [progress, setProgress] = useState(0); | |
| const [importing, setImporting] = useState(true); | |
| const pollIntervalRef = useRef<ReturnType<typeof setInterval> | null>(null); | |
| useEffect(() => { | |
| const handleCallback = async () => { | |
| const code = searchParams.get('code'); | |
| const state = searchParams.get('state'); | |
| if (!code) { | |
| showToast('Authorization failed - no code received', 'error'); | |
| navigate('/import/splitwise'); | |
| return; | |
| } | |
| try { | |
| // Send code to backend to exchange for access token and start import | |
| const response = await handleSplitwiseCallback(code, state || ''); | |
| const jobId = response.data.import_job_id || response.data.importJobId; | |
| if (!jobId) { | |
| throw new Error('No import job ID received'); | |
| } | |
| showToast('Authorization successful! Starting import...', 'success'); | |
| setStatus('Import started...'); | |
| // Poll for progress | |
| pollIntervalRef.current = setInterval(async () => { | |
| try { | |
| const statusResponse = await getImportStatus(jobId); | |
| const statusData = statusResponse.data; | |
| setProgress(statusData.progress_percentage || 0); | |
| setStatus(statusData.current_stage || 'Processing...'); | |
| if (statusData.status === 'completed') { | |
| if (pollIntervalRef.current) clearInterval(pollIntervalRef.current); | |
| setImporting(false); | |
| showToast('Import completed successfully!', 'success'); | |
| setStatus('Completed! Redirecting to dashboard...'); | |
| setTimeout(() => navigate('/dashboard'), 2000); | |
| } else if (statusData.status === 'failed') { | |
| if (pollIntervalRef.current) clearInterval(pollIntervalRef.current); | |
| setImporting(false); | |
| showToast('Import failed', 'error'); | |
| setStatus(`Failed: ${statusData.error_details || 'Unknown error'}`); | |
| } | |
| } catch (error) { | |
| console.error('Error polling import status:', error); | |
| } | |
| }, 2000); | |
| } catch (error: any) { | |
| console.error('Callback error:', error); | |
| showToast( | |
| error.response?.data?.detail || 'Failed to process authorization', | |
| 'error' | |
| ); | |
| setImporting(false); | |
| setTimeout(() => navigate('/import/splitwise'), 2000); | |
| } | |
| }; | |
| handleCallback(); | |
| return () => { | |
| if (pollIntervalRef.current) { | |
| clearInterval(pollIntervalRef.current); | |
| } | |
| }; | |
| }, [searchParams, navigate, showToast]); |
| <button | ||
| onClick={handleOAuthImport} | ||
| disabled={loading} | ||
| className="w-full py-4 px-6 bg-blue-500 hover:bg-blue-600 text-white font-semibold | ||
| rounded-lg transition-colors disabled:opacity-50 disabled:cursor-not-allowed | ||
| disabled:hover:bg-blue-500 shadow-lg hover:shadow-xl flex items-center justify-center gap-2" | ||
| > | ||
| {loading ? ( | ||
| <> | ||
| <div className="animate-spin rounded-full h-5 w-5 border-b-2 border-white"></div> | ||
| <span>Connecting to Splitwise...</span> | ||
| </> | ||
| ) : ( | ||
| <> | ||
| <svg className="w-5 h-5" fill="currentColor" viewBox="0 0 20 20"> | ||
| <path fillRule="evenodd" d="M10 18a8 8 0 100-16 8 8 0 000 16zm3.707-9.293a1 1 0 00-1.414-1.414L9 10.586 7.707 9.293a1 1 0 00-1.414 1.414l2 2a1 1 0 001.414 0l4-4z" clipRule="evenodd" /> | ||
| </svg> | ||
| <span>Connect with Splitwise & Import</span> | ||
| </> | ||
| )} | ||
| </button> |
There was a problem hiding this comment.
Fix accessibility issues: missing button type and SVG title.
Two accessibility issues flagged by static analysis:
- The button lacks an explicit
typeattribute. Default is"submit", which can cause unintended form submissions. - The SVG icon lacks alternative text for screen readers.
♿ Proposed fix
<button
+ type="button"
onClick={handleOAuthImport}
disabled={loading}
className="w-full py-4 px-6 bg-blue-500 hover:bg-blue-600 text-white font-semibold
rounded-lg transition-colors disabled:opacity-50 disabled:cursor-not-allowed
disabled:hover:bg-blue-500 shadow-lg hover:shadow-xl flex items-center justify-center gap-2"
>
{loading ? (
<>
<div className="animate-spin rounded-full h-5 w-5 border-b-2 border-white"></div>
<span>Connecting to Splitwise...</span>
</>
) : (
<>
- <svg className="w-5 h-5" fill="currentColor" viewBox="0 0 20 20">
+ <svg className="w-5 h-5" fill="currentColor" viewBox="0 0 20 20" aria-hidden="true">
<path fillRule="evenodd" d="M10 18a8 8 0 100-16 8 8 0 000 16zm3.707-9.293a1 1 0 00-1.414-1.414L9 10.586 7.707 9.293a1 1 0 00-1.414 1.414l2 2a1 1 0 001.414 0l4-4z" clipRule="evenodd" />
</svg>
<span>Connect with Splitwise & Import</span>
</>
)}
</button>📝 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.
| <button | |
| onClick={handleOAuthImport} | |
| disabled={loading} | |
| className="w-full py-4 px-6 bg-blue-500 hover:bg-blue-600 text-white font-semibold | |
| rounded-lg transition-colors disabled:opacity-50 disabled:cursor-not-allowed | |
| disabled:hover:bg-blue-500 shadow-lg hover:shadow-xl flex items-center justify-center gap-2" | |
| > | |
| {loading ? ( | |
| <> | |
| <div className="animate-spin rounded-full h-5 w-5 border-b-2 border-white"></div> | |
| <span>Connecting to Splitwise...</span> | |
| </> | |
| ) : ( | |
| <> | |
| <svg className="w-5 h-5" fill="currentColor" viewBox="0 0 20 20"> | |
| <path fillRule="evenodd" d="M10 18a8 8 0 100-16 8 8 0 000 16zm3.707-9.293a1 1 0 00-1.414-1.414L9 10.586 7.707 9.293a1 1 0 00-1.414 1.414l2 2a1 1 0 001.414 0l4-4z" clipRule="evenodd" /> | |
| </svg> | |
| <span>Connect with Splitwise & Import</span> | |
| </> | |
| )} | |
| </button> | |
| <button | |
| type="button" | |
| onClick={handleOAuthImport} | |
| disabled={loading} | |
| className="w-full py-4 px-6 bg-blue-500 hover:bg-blue-600 text-white font-semibold | |
| rounded-lg transition-colors disabled:opacity-50 disabled:cursor-not-allowed | |
| disabled:hover:bg-blue-500 shadow-lg hover:shadow-xl flex items-center justify-center gap-2" | |
| > | |
| {loading ? ( | |
| <> | |
| <div className="animate-spin rounded-full h-5 w-5 border-b-2 border-white"></div> | |
| <span>Connecting to Splitwise...</span> | |
| </> | |
| ) : ( | |
| <> | |
| <svg className="w-5 h-5" fill="currentColor" viewBox="0 0 20 20" aria-hidden="true"> | |
| <path fillRule="evenodd" d="M10 18a8 8 0 100-16 8 8 0 000 16zm3.707-9.293a1 1 0 00-1.414-1.414L9 10.586 7.707 9.293a1 1 0 00-1.414 1.414l2 2a1 1 0 001.414 0l4-4z" clipRule="evenodd" /> | |
| </svg> | |
| <span>Connect with Splitwise & Import</span> | |
| </> | |
| )} | |
| </button> |
🧰 Tools
🪛 Biome (2.1.2)
[error] 55-55: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
[error] 41-47: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🤖 Prompt for AI Agents
In @web/pages/SplitwiseImport.tsx around lines 41 - 61, The button element that
invokes handleOAuthImport is missing an explicit type (so it defaults to
"submit") and the SVG lacks accessible text; change the button to include
type="button" to prevent unintended form submissions and update the SVG used in
the non-loading branch to provide accessible alternative text (e.g., add a
<title> like "Splitwise icon" and ensure the SVG has role="img" or an
aria-label) so screen readers announce the icon; keep the loading spinner
decorative (e.g., aria-hidden="true") while the non-loading SVG provides a
proper accessible name.
| // Splitwise Import | ||
| export const getSplitwiseAuthUrl = async () => api.get('/import/splitwise/authorize'); | ||
| export const handleSplitwiseCallback = async (code: string, state: string) => api.post('/import/splitwise/callback', { code, state }); | ||
| export const startSplitwiseImport = async (apiKey: string) => api.post('/import/splitwise/start', { api_key: apiKey }); | ||
| export const getImportStatus = async (importJobId: string) => api.get(`/import/status/${importJobId}`); | ||
| export const rollbackImport = async (importJobId: string) => api.post(`/import/rollback/${importJobId}`); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for backend implementation of splitwise import endpoints
find . -type f -name "*.ts" -o -name "*.js" -o -name "*.py" -o -name "*.go" | head -20Repository: Devasy/splitwiser
Length of output: 670
🏁 Script executed:
# Look for backend app structure
git ls-files | grep -E '(backend|server|api)' | head -30Repository: Devasy/splitwiser
Length of output: 965
🏁 Script executed:
# Check api client configuration in web/services
cat -n web/services/api.ts | head -60Repository: Devasy/splitwiser
Length of output: 3946
🏁 Script executed:
# Search for splitwise import routes in backend
cat -n backend/app/integrations/router.pyRepository: Devasy/splitwiser
Length of output: 11555
🏁 Script executed:
# Check splitwise service implementation
cat -n backend/app/integrations/splitwise/client.pyRepository: Devasy/splitwiser
Length of output: 11918
🏁 Script executed:
# Search for import routes pattern
rg -n 'import.*start|import.*splitwise' backend/app/integrations/ -A 5 -B 2Repository: Devasy/splitwiser
Length of output: 7675
🏁 Script executed:
# Check StartImportRequest schema to confirm what fields it expects
rg -A 10 "class StartImportRequest" backend/app/integrations/schemas.pyRepository: Devasy/splitwiser
Length of output: 339
Remove apiKey parameter—backend ignores it entirely. The /import/splitwise/start endpoint doesn't use the apiKey from the request (backend schema has no api_key field). Instead, it retrieves credentials from environment config on lines 126–128 of router.py. Either remove the unused parameter from the frontend function, or update the backend to accept and use the per-request api_key. Note that the callback flow (lines 58–101) correctly uses OAuth tokens; verify whether the /import/splitwise/start endpoint should also use tokens instead of environment credentials.
🤖 Prompt for AI Agents
In @web/services/api.ts around lines 52 - 58, The frontend function
startSplitwiseImport is sending an unused apiKey to /import/splitwise/start;
remove the apiKey parameter from startSplitwiseImport in web/services/api.ts and
stop including { api_key: apiKey } in the POST body, then update all call sites
to call startSplitwiseImport() with no args. Alternatively, if per-request
credentials are desired instead, modify the backend router handler referenced
(router.py lines ~126–128) to accept and validate an api_key from the request
body and use it instead of the environment config, and add server-side
validation/tests accordingly so the frontend payload becomes meaningful.
Add integration for importing data from Splitwise, including OAuth authentication and API endpoints, along with UI components for user interaction.
@coderabbitai review
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.