Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| }} | ||
| textZoom={125} | ||
| onShouldStartLoadWithRequest={(event) => { | ||
| if (!/^[data:text, about:blank]/.test(event.url)) { |
There was a problem hiding this comment.
Bug: The regex /^[data:text, about:blank]/ uses a character class instead of alternation, incorrectly trapping URLs like tel: or tg: in the WebView.
Severity: MEDIUM
Suggested Fix
The regular expression should be changed to use alternation | to match the entire intended prefixes. Replace the current regex with /^(data:|about:blank)/ to correctly test if the URL starts with either the string data: or about:blank.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/screens/NotificationDetail.tsx#L61
Potential issue: The regular expression `/^[data:text, about:blank]/` is intended to
prevent `data:` URIs and `about:blank` from opening in an external browser. However, it
incorrectly uses a character class `[]`, which matches any single character in the set
(`d`, `a`, `t`, `e`, `x`, etc.) at the start of the URL. While this correctly handles
`http://` links and accidentally handles `data:` links, it will incorrectly trap other
external links like `tel:` or `tg://` within the WebView instead of opening them
externally. This happens because their starting characters (`t`) are present in the
character class, causing the `if` condition to be false and preventing
`Linking.openURL()` from being called.
Did we get this right? 👍 / 👎 to inform future reviews.
| }} | ||
| textZoom={125} | ||
| onShouldStartLoadWithRequest={(event) => { | ||
| if (!/^[data:text, about:blank]/.test(event.url)) { |
There was a problem hiding this comment.
Bug: The regex for handling internal links uses a character class [] instead of a group (), causing it to incorrectly match URLs and prevent them from opening externally.
Severity: HIGH
Suggested Fix
Update the regular expression to use alternation with a non-capturing group to correctly match the intended URI schemes. Change /^[data:text, about:blank]/ to /^(?:data:|about:blank)/.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/screens/NotificationDetail.tsx#L61
Potential issue: The regular expression at `src/screens/NotificationDetail.tsx` is
intended to identify internal URIs like `data:` and `about:blank` to keep them within
the WebView. However, it incorrectly uses a character class (`[...]`) instead of a group
with alternation (`(...)`). The pattern `/^[data:text, about:blank]/` matches any URL
starting with a single character from the set `d, a, t, a, :, t, e, x, t, ,, , a, b, o,
u, t, :, b, l, a, n, k`. As a result, URLs with schemes like `bitcoin://` or
`android-app://` will be incorrectly handled by the WebView instead of being opened
externally, breaking the intended behavior for external links.
| const { data: response } = await http.get( | ||
| "notification/list" + | ||
| (pageParam | ||
| ? `?${new URLSearchParams({ |
There was a problem hiding this comment.
Bug: The frontend now expects a new paginated object structure from the notifications API, but the backend may still return a simple array, which would cause a runtime error.
Severity: CRITICAL
Suggested Fix
Ensure the backend API at the /notification/list endpoint has been updated to return the new paginated response object ({ data: [], hasMore: boolean, nextCursor: number }) and is deployed simultaneously with the frontend. Otherwise, the frontend code must be reverted to handle the old array-based response.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/hooks/notifications-hooks.ts#L18-L21
Potential issue: The code in `notifications-hooks.ts` was updated to expect a new
paginated API response structure from the `/notification/list` endpoint. It now expects
an object like `{ data: [...], hasMore: boolean, nextCursor: number }`. The previous
implementation expected a simple array of notifications. If the backend API has not been
updated to return this new object structure, the frontend will encounter a runtime error
when trying to access `response.data` and `response.hasMore` on what would be an array.
This would cause the notification list to fail to load for all users.
1 participant
No description provided.