fix: getting tangled stats from our backend#981
fix: getting tangled stats from our backend#981fatfingers23 wants to merge 2 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThe client-side composable 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 3
🧹 Nitpick comments (1)
server/api/atproto/tangled-stats/[owner]/[...repo].ts (1)
49-51: Consider logging failed Constellation lookups for debugging.Whilst silently failing is appropriate for user-facing behaviour, having no visibility into Constellation failures may make debugging production issues difficult.
💡 Optional: Add debug logging
- } catch { - //failing silently since this is just an enhancement to the information already showing + } catch (e) { + // Failing silently since this is just an enhancement to the information already showing + console.debug('Constellation lookup failed for', atUri, e) }
| @@ -0,0 +1,64 @@ | |||
| import type { CachedFetchFunction } from '#shared/utils/fetch-cache-config' | |||
There was a problem hiding this comment.
Missing imports will cause compilation and runtime errors.
The file uses CachedFetchResult<T> (line 17) and Constellation (line 44) but neither is imported. This will cause TypeScript compilation errors and runtime failures.
🐛 Proposed fix to add missing imports
-import type { CachedFetchFunction } from '#shared/utils/fetch-cache-config'
+import type { CachedFetchFunction, CachedFetchResult } from '#shared/utils/fetch-cache-config'
+import { Constellation } from '#shared/utils/constellation'📝 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.
| import type { CachedFetchFunction } from '#shared/utils/fetch-cache-config' | |
| import type { CachedFetchFunction, CachedFetchResult } from '#shared/utils/fetch-cache-config' | |
| import { Constellation } from '#shared/utils/constellation' |
| let owner = getRouterParam(event, 'owner') | ||
| let repo = getRouterParam(event, 'repo') |
There was a problem hiding this comment.
Validate owner and repo parameters before use.
getRouterParam can return undefined. Using undefined values directly in the URL (line 27) would result in https://tangled.org/undefined/undefined. Return an error response early if parameters are missing.
🛡️ Proposed fix to validate parameters
- let owner = getRouterParam(event, 'owner')
- let repo = getRouterParam(event, 'repo')
+ const owner = getRouterParam(event, 'owner')
+ const repo = getRouterParam(event, 'repo')
+
+ if (!owner || !repo) {
+ return { stars: 0, forks: 0 }
+ }As per coding guidelines, ensure you write strictly type-safe code, for example by ensuring you always check when accessing values that may be undefined.
📝 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.
| let owner = getRouterParam(event, 'owner') | |
| let repo = getRouterParam(event, 'repo') | |
| const owner = getRouterParam(event, 'owner') | |
| const repo = getRouterParam(event, 'repo') | |
| if (!owner || !repo) { | |
| return { stars: 0, forks: 0 } | |
| } |
| //Get counts of records that reference this repo in the atmosphere using constellation | ||
| const { data: allLinks } = await constellation.getAllLinks(atUri) | ||
| stars = allLinks.links['sh.tangled.feed.star']?.['.subject']?.distinct_dids ?? stars | ||
| forks = allLinks.links['sh.tangled.repo']?.['.source']?.distinct_dids ?? stars |
There was a problem hiding this comment.
Bug: forks fallback incorrectly uses stars instead of forks.
The nullish coalescing fallback for forks uses stars, which means if the Constellation lookup fails to find fork data, forks will incorrectly equal the star count.
🐛 Proposed fix for forks fallback
- forks = allLinks.links['sh.tangled.repo']?.['.source']?.distinct_dids ?? stars
+ forks = allLinks.links['sh.tangled.repo']?.['.source']?.distinct_dids ?? forks📝 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.
| forks = allLinks.links['sh.tangled.repo']?.['.source']?.distinct_dids ?? stars | |
| forks = allLinks.links['sh.tangled.repo']?.['.source']?.distinct_dids ?? forks |
1 participant
Resolves: #844
The easiest way to get a at-uri for a tangled repo is from calling their webpage and parsing it out of the HTML. Moved this to the backend to resolve the current CORS errors.
I debated adding tangled.org to the cached fetch so that the HTML response can be cached, but that seems like a pretty big cached entry and I think in prod API endpoints are cached for a while.
Test repo:
@weaver.sh/editor-collab