Restructured and Unified Frontend Types/Interfaces#1568
Restructured and Unified Frontend Types/Interfaces#1568arkid15r merged 21 commits intoOWASP:mainfrom
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis change standardizes and unifies the frontend type and interface definitions, converting interfaces to type aliases, consolidating duplicate types, adopting camelCase naming conventions, and centralizing type declarations. It also updates all usages, imports, and related mock/test data to match the new type structure and naming conventions. Some backend adjustments support frontend data shape expectations. Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
backend/apps/core/utils/index.py (1)
165-181: Improve type annotations and add error handling.The core logic is correct, but the function could benefit from more precise type annotations and error handling.
-def deep_camelize(obj) -> dict | list: +def deep_camelize(obj) -> any: """Deep camelize. Args: - obj: The object to camelize. + obj: The object to camelize (dict, list, or any other type). Returns: - The camelize object. + The camelized object with the same structure but camelCase keys. """ + if obj is None: + return None + if isinstance(obj, dict): return { camelize(key.removeprefix("idx_")): deep_camelize(value) for key, value in obj.items() } if isinstance(obj, list): return [deep_camelize(item) for item in obj] return objfrontend/src/types/contributor.ts (1)
1-6: Remove commented-out code.The commented-out type definition should be removed rather than left in the codebase.
-// export type TopContributors = { -// avatarUrl: string -// contributionsCount: number -// login: string -// name: string -// }frontend/src/types/issue.ts (1)
1-5: Consider converting IssuesDataType to camelCase for consistency.The
IssuesDataTypeinterface still uses snake_case properties (open_issues_count,total_pages) whileIssueTypehas been converted to camelCase. Consider updating this interface for consistency.export interface IssuesDataType { issues: IssueType[] - open_issues_count: number - total_pages: number + openIssuesCount: number + totalPages: number }frontend/src/types/chapter.ts (1)
27-55: Consider removing commented code after transition.The commented-out interfaces may be useful during the transition period, but consider removing them once the refactoring is fully complete to keep the codebase clean.
frontend/src/types/project.ts (1)
90-136: Consider removing commented legacy code.The large blocks of commented code represent the old interface structure and may serve as temporary reference during migration, but they should be removed once the transition is complete to avoid code bloat.
Consider creating a follow-up task to remove these commented interfaces after confirming all consuming code has been successfully migrated to use the new unified types.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
backend/apps/core/api/algolia.py(2 hunks)backend/apps/core/utils/index.py(2 hunks)backend/pyproject.toml(1 hunks)frontend/src/app/contribute/page.tsx(2 hunks)frontend/src/app/projects/page.tsx(4 hunks)frontend/src/components/ChapterMap.tsx(5 hunks)frontend/src/components/ChapterMapWrapper.tsx(1 hunks)frontend/src/server/fetchAlgoliaData.ts(1 hunks)frontend/src/types/chapter.ts(1 hunks)frontend/src/types/contributor.ts(1 hunks)frontend/src/types/issue.ts(1 hunks)frontend/src/types/project.ts(3 hunks)frontend/src/utils/utility.ts(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
backend/apps/core/api/algolia.py (1)
backend/apps/core/utils/index.py (1)
deep_camelize(165-181)
frontend/src/components/ChapterMapWrapper.tsx (1)
frontend/src/types/chapter.ts (1)
ChapterType(9-25)
frontend/src/app/projects/page.tsx (3)
frontend/src/hooks/useSearchPage.ts (1)
useSearchPage(29-139)frontend/src/types/project.ts (1)
ProjectBase(59-79)frontend/src/utils/data.ts (1)
level(111-132)
frontend/src/components/ChapterMap.tsx (2)
frontend/src/types/chapter.ts (1)
ChapterType(9-25)backend/apps/owasp/graphql/queries/chapter.py (1)
chapter(14-19)
frontend/src/utils/utility.ts (5)
frontend/src/types/project.ts (2)
ProjectBase(59-79)ProjectTypeGraphql(81-88)frontend/src/types/issue.ts (1)
IssueType(7-19)frontend/src/types/committee.ts (1)
CommitteeTypeAlgolia(3-19)frontend/src/types/icon.ts (1)
IconType(1-3)frontend/src/utils/data.ts (2)
Icons(66-107)IconKeys(109-109)
frontend/src/types/project.ts (1)
frontend/src/types/contributor.ts (1)
TopContributorsType(8-15)
🪛 Biome (1.9.4)
frontend/src/app/projects/page.tsx
[error] 81-81: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Run backend tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (23)
backend/pyproject.toml (1)
54-54:Details
✅ Verification successful
Verify the pyhumps dependency version and security.
The
pyhumpsdependency is added to support camelCase transformation. Please ensure this version is current and has no known security vulnerabilities.
🌐 Web query:
What is the latest version of pyhumps Python package and are there any known security vulnerabilities in version 3.8.0?💡 Result:
The latest version of the
pyhumpsPython package is 3.8.0, released on October 21, 2022. (pypi.org) As of now, there are no known security vulnerabilities associated with this version. (data.safetycli.com)It's important to note that the
pyhumpspackage is distinct from the similarly namedhumpspackage. Thehumpspackage has a version 0.2.2, which is outdated and may lack recent updates or security patches. (github.com)Therefore, if you're looking for a package to convert strings between different cases in Python,
pyhumpsversion 3.8.0 is the recommended choice.Citations:
- 1: https://pypi.org/project/pyhumps/?utm_source=openai
- 2: https://data.safetycli.com/packages/pypi/pyhumps/?utm_source=openai
- 3: nficano/humps#268
pyhumps dependency version verified
We’ve confirmed that
pyhumps3.8.0 (released October 21, 2022) is the latest available version and has no known security vulnerabilities. This dependency is safe and appropriate to include.backend/apps/core/api/algolia.py (2)
17-17: LGTM: Clean import of the camelCase utility.The import of
deep_camelizealigns with the PR objective to provide camelCase data formatting.
61-65: LGTM: Proper application of camelCase transformation.The
deep_camelizefunction is correctly applied to the search hits before returning them. This ensures consistent camelCase formatting for frontend consumption while maintaining the existing API structure.frontend/src/server/fetchAlgoliaData.ts (1)
47-47:Details
✅ Verification successful
LGTM: Simplified data handling with backend transformation.
Good simplification! The frontend now receives camelCase data directly from the backend, eliminating the need for client-side transformation.
Let me verify the removed imports are no longer used anywhere:
🏁 Script executed:
#!/bin/bash # Description: Verify that removeIdxPrefix and IndexedObject are no longer used in the codebase # Expected: No references to these should be found echo "Checking for remaining usage of removeIdxPrefix..." rg "removeIdxPrefix" --type ts --type tsx echo "Checking for remaining usage of IndexedObject..." rg "IndexedObject" --type ts --type tsxLength of output: 355
🏁 Script executed:
#!/bin/bash # Re-run searches without unsupported type flags to verify removal of imports echo "Searching for removeIdxPrefix usage..." rg -g '*.ts' -g '*.tsx' 'removeIdxPrefix' echo "Searching for IndexedObject usage..." rg -g '*.ts' -g '*.tsx' 'IndexedObject'Length of output: 644
LGTM: Removed unused imports
Verified that
removeIdxPrefixandIndexedObjectare no longer referenced infetchAlgoliaData.ts. Returninghitsdirectly is safe now that the backend provides camelCase data, and no local calls or type usages remain.frontend/src/components/ChapterMapWrapper.tsx (1)
3-3:Details
✅ Verification successful
LGTM: Good type unification with ChapterType.
The switch to a unified
ChapterType[]simplifies the component interface and aligns with the PR's goal of restructuring frontend types. TheChapterTypeinterface includes both_geolocandgeoLocationproperties, ensuring compatibility with different data sources.Let me verify that ChapterMap can handle the unified ChapterType:
Also applies to: 8-8
🏁 Script executed:
#!/bin/bash # Description: Check how ChapterMap component handles the geoLocData prop # Expected: Should be compatible with ChapterType interface echo "Checking ChapterMap component implementation..." rg -A 10 -B 5 "geoLocData.*ChapterType" --type ts --type tsx echo "Checking for any geolocation property usage..." rg "_geoloc|geoLocation" frontend/src/components/ChapterMap.tsxLength of output: 752
I’d like to confirm the exact props in ChapterMapWrapper and the full shape of ChapterType before wrapping up:
This will verify that
ChapterTypeincludes both_geolocandgeoLocationand that the wrapper passes props correctly.
🏁 Script executed:
#!/bin/bash # Inspect ChapterMapWrapper usage echo ">> ChapterMapWrapper geoLocData prop and ChapterMap invocation:" rg -n "geoLocData" -n frontend/src/components/ChapterMapWrapper.tsx rg -A5 -B5 "ChapterMap" -n frontend/src/components/ChapterMapWrapper.tsx # Locate and display ChapterType definition echo ">> Locating ChapterType definition..." FILE=$(rg -l "export (interface|type) ChapterType" -n . | head -n1) echo "Found in: $FILE" echo "--- Definition snippet ---" sed -n '1,50p' "$FILE"Length of output: 2160
Approve unified ChapterType usage in ChapterMapWrapper
The
geoLocData: ChapterType[]prop is passed directly toChapterMap, and theChapterTypedefinition infrontend/src/types/chapter.tsincludes both_geolocandgeoLocationfields. InChapterMap, all location accesses correctly fall back between_geolocandgeoLocation. No breaking changes detected.• ChapterMapWrapper (lines 7–13) simply forwards props to ChapterMap
• ChapterType intypes/chapter.tsdefines optional_geolocandgeoLocation
• ChapterMap uses both properties for latitude/longitude without errorsEverything is compatible—no further changes required.
frontend/src/app/contribute/page.tsx (2)
31-31: LGTM! Correct camelCase conversion for filter parameters.The conversion from snake_case to camelCase is consistent with the updated
IssueTypeinterface.
52-53:Details
✅ Verification successful
Verify prop name consistency between
projectUrlandprojectLink.There's a potential inconsistency: the issue property is
projectUrlbut the Card component prop isprojectLink. Ensure this mapping is correct.
🏁 Script executed:
#!/bin/bash # Description: Verify the Card component prop names to ensure correct mapping # Search for Card component definition and its props interface ast-grep --pattern $'interface $_ { $$$ projectLink$$$ $$$ }' # Also search for any usage of projectUrl vs projectLink rg -A 3 -B 3 "projectLink|projectUrl" frontend/src/components/CardLength of output: 206
I couldn’t locate the Card component or its props via the previous search. Let’s pinpoint where
Cardis defined and confirm its prop names:
🏁 Script executed:
#!/bin/bash # 1. Find the Card component file rg -l "export (function|const) Card" -g "*.tsx" # 2. Search for any occurrences of projectLink rg -n "projectLink" -g "*.tsx" # 3. Search for any occurrences of projectUrl rg -n "projectUrl" -g "*.tsx"Length of output: 394
Prop mapping is correct:
issue.projectUrl→projectLink.
TheCardcomponent’s props includeprojectLink(used for itshref), and the page passesissue.projectUrlinto that prop as intended. No changes required.backend/apps/core/utils/index.py (1)
8-8: LGTM! Import correctly added for camelCase functionality.The
camelizeimport fromhumpslibrary is properly added to support the new camelCase transformation feature.frontend/src/types/contributor.ts (1)
8-15: LGTM! Type unification with consistent camelCase naming.The renaming from
TopContributorsTypeGraphqltoTopContributorsTypeand the addition of optional project fields support the type unification effort. The camelCase naming is consistent throughout.frontend/src/types/issue.ts (1)
8-19: LGTM! Correct camelCase conversion for IssueType interface.All property name conversions from snake_case to camelCase are correct and consistent. The type definitions remain unchanged, ensuring compatibility.
frontend/src/app/projects/page.tsx (4)
5-5: LGTM: Type import updated correctly.The import change from
ProjectTypeAlgoliatoProjectBasealigns with the type unification effort described in the PR objectives.
25-25: LGTM: Type annotations updated consistently.The generic type parameter and function parameter both correctly use the new unified
ProjectBasetype.Also applies to: 33-33
34-34: LGTM: Property names converted to camelCase.The property names have been correctly updated from snake_case to camelCase (
forks_count→forksCount,stars_count→starsCount,contributors_count→contributorsCount,top_contributors→topContributors).Also applies to: 54-54
50-50:Details
❓ Verification inconclusive
Verify the key property change.
The React list key was changed from
project.objectIDtoproject.key. Ensure that thekeyproperty is stable and unique for each project to maintain proper React list rendering behavior.
🏁 Script executed:
#!/bin/bash # Description: Verify that the key property exists and is unique in the ProjectBase type ast-grep --pattern 'interface ProjectBase { $$$ key: $_ $$$ }'Length of output: 1196
I’d like to confirm where and how
project.keyis populated to ensure it truly is unique and stable at render time. Let’s inspect the top offrontend/src/app/projects/page.tsxto see the data fetch and mapping logic:
🏁 Script executed:
#!/bin/bash # Show the fetch/mapping and usage of `key` in page.tsx sed -n '1,200p' frontend/src/app/projects/page.tsxLength of output: 2512
Confirm
project.keyUniqueness and StabilityWe’ve verified that
ProjectBaseincludes akey: stringfield (used for URLs and list keys) and thatpage.tsxnow consistently usesproject.key. Before merge, please ensure:
- The
keyvalue is derived from a stable, unique identifier (e.g. a slug or backend‐generated ID).- No two projects can share the same
keyat indexing or ingest time.This will guarantee React’s list rendering remains correct.
frontend/src/components/ChapterMap.tsx (3)
3-4: LGTM: Type unification and simplified imports.The changes correctly adopt the unified
ChapterType[]type and remove the unuseduseMemoimport, simplifying the component.Also applies to: 16-16
51-51: LGTM: Robust coordinate access pattern.The coordinate access pattern using
chapter._geoloc?.lat || chapter.geoLocation?.latprovides good fallback handling for both Algolia (_geoloc) and GraphQL (geoLocation) data sources. This ensures compatibility during the transition period.Also applies to: 64-66, 83-103
103-103: LGTM: Dependency array correctly updated.The useEffect dependency array now correctly depends on
geoLocDatadirectly instead of the removed intermediatechaptersvariable.frontend/src/types/chapter.ts (1)
5-5: LGTM: Type unification with robust geolocation handling.The new unified
ChapterTypeinterface correctly consolidates the previous separate types and uses camelCase naming. The optional_geolocandgeoLocationproperties provide good backward compatibility for both Algolia and GraphQL data sources.Also applies to: 9-25
frontend/src/utils/utility.ts (2)
10-10: LGTM: Type imports and aliases updated consistently.The imports and type aliases have been correctly updated to use the unified
ProjectBaseandProjectTypeGraphqltypes, removing references to the deprecated types.Also applies to: 19-19, 37-37
24-28: LGTM: Improved property names and type casting.The property names have been correctly updated to camelCase (
'created_at'→'createdAt'), and the type casting has been improved to useas unknown as numberfor safer type assertions.Also applies to: 43-46
frontend/src/types/project.ts (3)
1-1: LGTM: Import consolidation looks good.The import has been properly updated to use the unified
TopContributorsTypeinstead of separate Algolia and GraphQL variants, which aligns with the type unification goals.
6-6: LGTM: ProjectDataType updated correctly.The
projectsfield has been properly updated to useProjectBase[]instead of the removedProjectTypeAlgolia[], maintaining type safety while using the new unified interface.
59-79: LGTM: Well-structured base interface with consistent camelCase naming.The
ProjectBaseinterface successfully consolidates common project fields and maintains consistent camelCase naming throughout. The field types are appropriate and the structure is logical.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
frontend/src/types/user.ts (1)
3-18:⚠️ Potential issueRemove the deprecated
usertype to avoid confusion.This old snake_case
usertype conflicts with the new camelCaseUserBaseandUsertypes, potentially causing confusion and bugs. Since the PR aims to unify naming conventions to camelCase, this legacy type should be removed.-export type user = { - avatar_url: string - bio: string - company: string - contributions_count: number - created_at: number - email: string - followers_count: number - following_count: number - key: string - location: string - login: string - name: string - objectID: string - public_repositories_count: number -}
🧹 Nitpick comments (1)
frontend/src/app/chapters/page.tsx (1)
103-103: Apply optional chaining for cleaner code.The static analysis suggestion is valid. The
chapters &&check can be simplified using optional chaining.- {chapters && chapters.filter((chapter) => chapter.isActive).map(renderChapterCard)} + {chapters?.filter((chapter) => chapter.isActive).map(renderChapterCard)}🧰 Tools
🪛 Biome (1.9.4)
[error] 103-103: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
frontend/src/app/chapters/page.tsx(6 hunks)frontend/src/app/committees/page.tsx(3 hunks)frontend/src/app/members/page.tsx(1 hunks)frontend/src/app/organizations/page.tsx(3 hunks)frontend/src/types/chapter.ts(1 hunks)frontend/src/types/committee.ts(1 hunks)frontend/src/types/organization.ts(1 hunks)frontend/src/types/user.ts(1 hunks)frontend/src/utils/utility.ts(3 hunks)
✅ Files skipped from review due to trivial changes (4)
- frontend/src/app/members/page.tsx
- frontend/src/types/organization.ts
- frontend/src/app/organizations/page.tsx
- frontend/src/types/committee.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/utils/utility.ts
- frontend/src/types/chapter.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
frontend/src/app/chapters/page.tsx (5)
frontend/src/types/chapter.ts (1)
ChapterType(9-25)frontend/src/hooks/useSearchPage.ts (1)
useSearchPage(29-139)frontend/src/types/algolia.ts (1)
AlgoliaResponseType(1-4)frontend/src/server/fetchAlgoliaData.ts (1)
fetchAlgoliaData(6-59)frontend/src/utils/utility.ts (2)
getFilteredIcons(22-35)handleSocialUrls(55-63)
frontend/src/types/user.ts (1)
backend/apps/github/models/user.py (1)
User(14-129)
🪛 Biome (1.9.4)
frontend/src/app/chapters/page.tsx
[error] 103-103: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (10)
frontend/src/app/chapters/page.tsx (3)
8-8: LGTM: Clean type import update.The import has been correctly updated to use the unified
ChapterTypeinterface instead of the previous type, aligning with the restructuring objectives.
15-15: LGTM: Consistent type usage across state and hooks.All type annotations have been properly updated to use the unified
ChapterType, ensuring type consistency across state management, hook generics, and data fetching.Also applies to: 24-24, 37-37
49-52: LGTM: Property names correctly updated to camelCase.All property accesses have been properly converted from snake_case to camelCase format (
updatedAt,relatedUrls,topContributors), matching the newChapterTypeinterface definition.Also applies to: 71-71
frontend/src/types/user.ts (1)
42-61: Excellent design with the generic UserBase interface!The introduction of
UserBase<T>with a generic type parameter forcreatedAtis a clean approach that allows flexibility for different timestamp formats (number vs string). The camelCase naming convention aligns well with the PR objectives.frontend/src/app/committees/page.tsx (6)
5-5: LGTM: Type import updated correctly.The import change from
CommitteeTypeAlgoliatoCommitteeBasealigns with the type unification objectives.
19-19: LGTM: Type parameter updated consistently.The generic type parameter correctly reflects the new
CommitteeBasetype.
24-24: LGTM: Function parameter type updated consistently.The parameter type correctly uses the new
CommitteeBasetype for proper type safety.
27-27:Details
✅ Verification successful
Verify utility function compatibility with camelCase URLs.
The property access change to
committee.relatedUrlsis consistent with the camelCase naming convention. Ensure that thehandleSocialUrlsutility function has been updated to handle the new camelCase property structure.
🏁 Script executed:
#!/bin/bash # Description: Verify that handleSocialUrls utility function handles camelCase property names # Expected: Function should work with camelCase properties # Search for handleSocialUrls function implementation ast-grep --pattern 'function handleSocialUrls($$$) { $$$ }' # Also search for const/export declarations ast-grep --pattern 'const handleSocialUrls = $$$' ast-grep --pattern 'export $_ handleSocialUrls = $$$'Length of output: 787
Utility function signature is compatible with camelCase property access
The
handleSocialUrlsutility accepts the URLs array directly (its internal parameter name doesn’t affect how you passcommittee.relatedUrls), so callinghandleSocialUrls(committee.relatedUrls)works correctly without any further changes.
25-25:Details
✅ Verification successful
Verify utility function compatibility with camelCase.
The params array now uses camelCase property names, which is consistent with the refactoring goals. Ensure that the
getFilteredIconsutility function has been updated to handle camelCase property names.
🏁 Script executed:
#!/bin/bash # Description: Verify that getFilteredIcons utility function handles camelCase property names # Expected: Function should work with camelCase properties like 'updatedAt' # Search for getFilteredIcons function implementation ast-grep --pattern 'function getFilteredIcons($$$) { $$$ }' # Also search for const/export declarations ast-grep --pattern 'const getFilteredIcons = $$$' ast-grep --pattern 'export $_ getFilteredIcons = $$$'Length of output: 1158
getFilteredIcons supports camelCase properties
The implementation in frontend/src/utils/utility.ts (lines 22–35) uses dynamic bracket notation (
project[key as keyof projectType]) and checks against theIconsmap, so passing'updatedAt'in camelCase works correctly. No changes required.
45-45:Details
✅ Verification successful
Verify Card component compatibility with camelCase props.
The property access change to
committee.topContributorsis consistent with the camelCase naming convention. Ensure that theCardcomponent'stopContributorsprop is properly typed and handled.
🏁 Script executed:
#!/bin/bash # Description: Verify that Card component handles topContributors prop correctly # Expected: Component should accept and properly use topContributors prop # Search for Card component definition and topContributors prop usage ast-grep --pattern 'interface $_ { $$$ topContributors: $$$ $$$ }' # Search for Card component implementation ast-grep --pattern 'const Card = $$$' ast-grep --pattern 'function Card($$$) { $$$ }'Length of output: 11969
🏁 Script executed:
#!/bin/bash # Verify that CardProps includes topContributors rg "CardProps" -n frontend/src/components/Card.tsxLength of output: 113
🏁 Script executed:
#!/bin/bash # Locate CardProps definition to verify topContributors inclusion rg "export interface CardProps" -n frontend/src/types/card.tsLength of output: 95
🏁 Script executed:
#!/bin/bash # Check CardProps definition for topContributors rg -n "topContributors" frontend/src/types/card.tsLength of output: 185
Card component
topContributorsprop is correctly handled
- The
CardPropsinterface infrontend/src/types/card.tsdefinestopContributors?: TopContributorsTypeGraphql[] | TopContributorsTypeAlgolia[].- The
Cardcomponent destructures and renderstopContributorsas expected.No further changes required.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
frontend/src/components/ContributorAvatar.tsx (1)
12-21:⚠️ Potential issueFix type guard logic inconsistency.
The type guard function checks for snake_case properties (
'avatar_url'and'contributions_count') but the actualTopContributorsTypeuses camelCase properties (avatarUrlandcontributionsCount). This will cause the function to always returnfalse, breaking the component's conditional logic.const isAlgoliaContributor = ( contributor: TopContributorsType ): contributor is TopContributorsType => { return ( typeof contributor === 'object' && contributor !== null && - 'avatar_url' in contributor && - 'contributions_count' in contributor + 'avatarUrl' in contributor && + 'contributionsCount' in contributor ) }
♻️ Duplicate comments (1)
frontend/src/types/project.ts (1)
77-77:⚠️ Potential issueRemove redundant field definition.
The
topContributorsfield is redundant sinceProjectTypeextendsProjectBase, which already defines this field on line 66.export interface ProjectType extends ProjectBase { recentIssues: ProjectIssuesType[] recentPullRequests: ProjectPullRequestsType[] recentReleases: ProjectReleaseType[] repositories: RepositoryCardProps[] - topContributors: TopContributorsType[] recentMilestones: ProjectMilestonesType[] }
🧹 Nitpick comments (1)
frontend/src/components/ContributorAvatar.tsx (1)
26-40: Consider simplifying the logic since types are now unified.Since all contributors now use the unified
TopContributorsType, the type guard checks and explicit casting seem unnecessary. The component could be simplified to directly access properties without conditional logic.Consider this simplified approach:
- const isAlgolia = isAlgoliaContributor(contributor) - - const avatarUrl = isAlgolia - ? contributor.avatarUrl - : (contributor as TopContributorsType).avatarUrl - - const contributionsCount = isAlgolia - ? contributor.contributionsCount - : (contributor as TopContributorsType).contributionsCount + const { avatarUrl, contributionsCount, login, name, projectName } = contributor const { login, name } = contributor const displayName = name || login - const repositoryInfo = - !isAlgolia && (contributor as TopContributorsType).projectName - ? ` in ${(contributor as TopContributorsType).projectName}` - : '' + const repositoryInfo = projectName ? ` in ${projectName}` : ''However, if the distinction between Algolia and GraphQL contributors is still meaningful for the avatar URL formatting (
&s=60parameter), please clarify the intended logic.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
frontend/src/app/chapters/[chapterKey]/page.tsx(2 hunks)frontend/src/app/members/page.tsx(1 hunks)frontend/src/app/organizations/page.tsx(3 hunks)frontend/src/app/projects/[projectKey]/page.tsx(2 hunks)frontend/src/app/snapshots/[id]/page.tsx(3 hunks)frontend/src/components/CardDetailsPage.tsx(3 hunks)frontend/src/components/ContributorAvatar.tsx(2 hunks)frontend/src/components/MultiSearch.tsx(5 hunks)frontend/src/components/TopContributors.tsx(2 hunks)frontend/src/components/UserCard.tsx(2 hunks)frontend/src/server/utility.ts(0 hunks)frontend/src/types/card.ts(5 hunks)frontend/src/types/chapter.ts(1 hunks)frontend/src/types/committee.ts(1 hunks)frontend/src/types/contributor.ts(1 hunks)frontend/src/types/home.ts(1 hunks)frontend/src/types/project.ts(6 hunks)frontend/src/types/search.ts(2 hunks)frontend/src/types/snapshot.ts(2 hunks)frontend/src/utils/logger.ts(0 hunks)frontend/src/utils/utility.ts(2 hunks)
💤 Files with no reviewable changes (2)
- frontend/src/utils/logger.ts
- frontend/src/server/utility.ts
✅ Files skipped from review due to trivial changes (7)
- frontend/src/types/home.ts
- frontend/src/components/UserCard.tsx
- frontend/src/types/snapshot.ts
- frontend/src/components/MultiSearch.tsx
- frontend/src/types/search.ts
- frontend/src/types/card.ts
- frontend/src/components/TopContributors.tsx
🚧 Files skipped from review as they are similar to previous changes (9)
- frontend/src/types/contributor.ts
- frontend/src/app/members/page.tsx
- frontend/src/app/chapters/[chapterKey]/page.tsx
- frontend/src/app/organizations/page.tsx
- frontend/src/app/projects/[projectKey]/page.tsx
- frontend/src/app/snapshots/[id]/page.tsx
- frontend/src/types/committee.ts
- frontend/src/utils/utility.ts
- frontend/src/types/chapter.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
frontend/src/components/ContributorAvatar.tsx (1)
frontend/src/types/contributor.ts (1)
TopContributorsType(1-8)
frontend/src/types/project.ts (2)
frontend/src/types/contributor.ts (1)
TopContributorsType(1-8)backend/apps/owasp/models/project.py (1)
ProjectType(47-63)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (5)
frontend/src/types/project.ts (2)
3-8: Good unification with the new Author type.The introduction of the reusable
Authortype effectively consolidates author structures across multiple interfaces, improving consistency and maintainability.
50-70: Well-structured base interface for project types.The
ProjectBaseinterface provides a clean foundation with standardized camelCase naming and comprehensive project properties. The inheritance pattern withProjectTypeextendingProjectBasepromotes good code organization.frontend/src/components/CardDetailsPage.tsx (3)
30-30: LGTM: Consistent camelCase conversion.The parameter rename from
is_activetoisActivealigns with the PR objective of standardizing frontend naming conventions to camelCase.
55-55: LGTM: Correct usage of renamed parameter.The conditional check correctly uses the renamed
isActiveparameter, maintaining the same logic for displaying the "Inactive" badge.
126-126:Details
✅ Verification successful
Verify type consistency for geolocation data.
The change from wrapping
geolocationDatain an array to passing it directly suggests the type changed from a single item to an array. Please ensure that:
- The
geolocationDataparameter is now typed asChapterType[]- The
ChapterMapWrappercomponent properly handles this array type- All callers of this component pass the correct array type
🏁 Script executed:
#!/bin/bash # Verify the type definitions and usage are consistent echo "Checking DetailsCardProps interface definition:" rg -A 10 "interface DetailsCardProps" --type ts --type tsx echo -e "\nChecking ChapterMapWrapper component props:" rg -A 5 "geoLocData.*:" --type ts --type tsx echo -e "\nChecking how DetailsCard is called with geolocationData:" rg -A 3 -B 3 "geolocationData.*=" --type ts --type tsxLength of output: 585
I’ll broaden the search to eliminate the file-type filters and locate the relevant definitions and usages across all
.ts/.tsxfiles.
🏁 Script executed:
#!/bin/bash # Search for DetailsCardProps interface definition echo "=== DetailsCardProps ===" rg -A10 "interface DetailsCardProps" -g "*.ts" -g "*.tsx" # Search for ChapterMapWrapper component and its prop types echo "=== ChapterMapWrapper ===" rg -A10 "ChapterMapWrapper" -g "*.ts" -g "*.tsx" # Search for geoLocData usages in JSX/TSX echo "=== geoLocData ===" rg -A5 "geoLocData" -g "*.ts" -g "*.tsx" # Search for geolocationData usages (calls and definitions) echo "=== geolocationData ===" rg -A5 "geolocationData" -g "*.ts" -g "*.tsx"Length of output: 16264
Type consistency for
geolocationDataconfirmed.The
DetailsCardPropsinterface defines
•geolocationData?: ChapterType[]
andChapterMapWrapperexpectsgeoLocData: ChapterType[]. InCardDetailsPage.tsxwe only render the wrapper whengeolocationDatais non-null (and thus an array), and all callers supplyChapterType[]([chapter],geoLocData,snapshot.newChapters). No mismatches detected.
aramattamara
left a comment
aramattamara
left a comment
There was a problem hiding this comment.
Thanks for working on this. I left some comments, please take a look
arkid15r
left a comment
arkid15r
left a comment
There was a problem hiding this comment.
It doesn't look like you addressed the main concerns described in the issue:
- inconsistent usage of type vs interface
- duplicate or near-identical types
I partially updated naming by removing Type suffix from names. Please do the same for the rest of types/interfaces.
Please also handle the bot's comment (either implement or mark as resolved).
backend/pyproject.toml
Outdated
| strawberry-graphql = {extras = ["django"], version = "^0.270.1"} | ||
| thefuzz = "^0.22.1" | ||
| strawberry-graphql-django = "^0.59.1" | ||
| pyhumps = "^3.8.0" |
There was a problem hiding this comment.
This is not the best option here. While I strongly believe in code reuse it's just not worth it to install a new package for string conversion. Please add a new method with tests similar to what we have here.
Moreover, the package hasn't been updated for a while.
backend/apps/core/api/algolia.py
Outdated
| response = client.search(search_method_params={"requests": [search_params]}) | ||
| search_result = response.results[0].to_dict() | ||
|
|
||
| cleaned_search_result = deep_camelize(search_result["hits"]) |
There was a problem hiding this comment.
Please don't introduce variables used just once.
| RepositoryCardProps, | ||
| } from 'types/project' | ||
| import type { ItemCardPullRequests, UserDetailsProps } from 'types/user' | ||
| import type { ItemCardPullRequests, UserDetails } from 'types/user' |
There was a problem hiding this comment.
Should we update all type import w/ import type?
There was a problem hiding this comment.
Do we want to keep GitHub related types in user types file?
There was a problem hiding this comment.
yeah, i mean it's the page i think we can change it to member or should i creatre it like githubUser , but from my pov User is fine
There was a problem hiding this comment.
Same here -- a lot of GitHub related entity types tied to Project... prefix.
How is it different from just Issue vs ProjectIssuesType (why plural name btw?)
frontend/src/types/project.ts
Outdated
| login: string | ||
| name: string | ||
| } | ||
| author: Author |
There was a problem hiding this comment.
How is this type different from types::user::Release?
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
frontend/src/app/chapters/page.tsx (2)
29-46: 🛠️ Refactor suggestionAdd error handling & abort-on-unmount to avoid unhandled promise rejections
fetchAlgoliaDatais awaited without atry / catch, so any network or parsing error will bubble up to the browser console and potentially leave the component in an inconsistent state.
Additionally, ifcurrentPagechanges quickly (or the component unmounts) thesetGeoLocDatacall from an outdated request can land after a newer one, causing visual flicker or React “state update on unmounted component” warnings.useEffect(() => { - const fetchData = async () => { - const searchParams = { - indexName: 'chapters', - query: '', - currentPage, - hitsPerPage: currentPage === 1 ? 1000 : 25, - } - const data: AlgoliaResponse<Chapter> = await fetchAlgoliaData( - searchParams.indexName, - searchParams.query, - searchParams.currentPage, - searchParams.hitsPerPage - ) - setGeoLocData(data.hits) - } - fetchData() + const abortController = new AbortController() + const fetchData = async () => { + try { + const searchParams = { + indexName: 'chapters', + query: '', + currentPage, + hitsPerPage: currentPage === 1 ? 1000 : 25, + } + const data: AlgoliaResponse<Chapter> = await fetchAlgoliaData( + searchParams.indexName, + searchParams.query, + searchParams.currentPage, + searchParams.hitsPerPage, + abortController.signal // pass signal if fetchAlgoliaData supports it + ) + setGeoLocData(data.hits) + } catch (err) { + if (err.name !== 'AbortError') { + /* eslint-disable-next-line no-console */ + console.error('Failed to fetch chapter geo data', err) + } + } + } + fetchData() + return () => abortController.abort() }, [currentPage])This guards against race conditions and makes the component resilient to failed requests.
58-62:⚠️ Potential issueBug:
onclickkey & trailing space will break the button icon
- The
Cardcomponent (and common React conventions) expect the handler to be namedonClick, notonclick. Using the wrong case will silently drop the callback.- The icon string has a trailing space, which prevents Font Awesome from matching the class name.
- const submitButton = { - label: 'View Details', - icon: <FontAwesomeIconWrapper icon="fa-solid fa-right-to-bracket " />, - onclick: handleButtonClick, - } + const submitButton = { + label: 'View Details', + icon: <FontAwesomeIconWrapper icon="fa-solid fa-right-to-bracket" />, + onClick: handleButtonClick, + }Please ensure the
Cardprops also use the correctedonClickkey.
🧹 Nitpick comments (1)
frontend/src/app/chapters/page.tsx (1)
103-103: Prefer optional chaining for cleaner null-safetyStatic analysis is right:
chapterscan beundefineduntil the first search returns. Optional chaining removes the need for the&&short-circuit and is more idiomatic.- {chapters && chapters.filter((chapter) => chapter.isActive).map(renderChapterCard)} + {chapters?.filter((chapter) => chapter.isActive).map(renderChapterCard)}🧰 Tools
🪛 Biome (1.9.4)
[error] 103-103: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
backend/apps/core/api/algolia.py(2 hunks)frontend/src/app/about/page.tsx(3 hunks)frontend/src/app/chapters/[chapterKey]/page.tsx(2 hunks)frontend/src/app/chapters/page.tsx(6 hunks)frontend/src/app/committees/[committeeKey]/page.tsx(1 hunks)frontend/src/app/committees/page.tsx(3 hunks)frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx(1 hunks)frontend/src/app/page.tsx(5 hunks)frontend/src/app/projects/[projectKey]/page.tsx(2 hunks)frontend/src/app/snapshots/[id]/page.tsx(3 hunks)frontend/src/components/CardDetailsPage.tsx(4 hunks)frontend/src/components/ChapterMap.tsx(5 hunks)frontend/src/components/ChapterMapWrapper.tsx(1 hunks)frontend/src/components/ContributorAvatar.tsx(2 hunks)frontend/src/components/ItemCardList.tsx(2 hunks)frontend/src/components/MultiSearch.tsx(5 hunks)frontend/src/components/TopContributorsList.tsx(2 hunks)frontend/src/server/fetchAlgoliaData.ts(3 hunks)frontend/src/types/algolia.ts(1 hunks)frontend/src/types/card.ts(5 hunks)frontend/src/types/chapter.ts(1 hunks)frontend/src/types/committee.ts(1 hunks)frontend/src/types/contributor.ts(1 hunks)frontend/src/types/home.ts(1 hunks)frontend/src/types/project.ts(4 hunks)frontend/src/types/search.ts(2 hunks)frontend/src/types/snapshot.ts(2 hunks)frontend/src/types/user.ts(1 hunks)frontend/src/utils/utility.ts(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- frontend/src/components/TopContributorsList.tsx
- frontend/src/components/ItemCardList.tsx
🚧 Files skipped from review as they are similar to previous changes (26)
- frontend/src/app/about/page.tsx
- frontend/src/types/home.ts
- frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx
- frontend/src/types/contributor.ts
- backend/apps/core/api/algolia.py
- frontend/src/server/fetchAlgoliaData.ts
- frontend/src/types/snapshot.ts
- frontend/src/app/page.tsx
- frontend/src/app/committees/[committeeKey]/page.tsx
- frontend/src/app/chapters/[chapterKey]/page.tsx
- frontend/src/app/committees/page.tsx
- frontend/src/app/snapshots/[id]/page.tsx
- frontend/src/components/ChapterMapWrapper.tsx
- frontend/src/types/committee.ts
- frontend/src/app/projects/[projectKey]/page.tsx
- frontend/src/components/CardDetailsPage.tsx
- frontend/src/types/user.ts
- frontend/src/types/search.ts
- frontend/src/components/MultiSearch.tsx
- frontend/src/components/ContributorAvatar.tsx
- frontend/src/components/ChapterMap.tsx
- frontend/src/types/card.ts
- frontend/src/utils/utility.ts
- frontend/src/types/algolia.ts
- frontend/src/types/chapter.ts
- frontend/src/types/project.ts
🧰 Additional context used
🪛 Biome (1.9.4)
frontend/src/app/chapters/page.tsx
[error] 103-103: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
- GitHub Check: CodeQL (javascript-typescript)
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (6)
frontend/src/components/RepositoriesCard.tsx (2)
17-33:⚠️ Potential issue
repositoriescan now beundefined– component will crash at runtime
RepositoriesCardProps.repositorieswas made optional in the new type definition, but the component still dereferences it immediately (slice,length,map).
If a caller omits the prop the page will throw before React can show an error boundary.Suggested minimal fix:
-const RepositoriesCard: React.FC<RepositoriesCardProps> = ({ repositories }) => { +const RepositoriesCard: React.FC<RepositoriesCardProps> = ({ repositories = [] }) => {Optionally short-circuit the render for an empty list:
if (!repositories.length) return nullProtecting against
undefinedkeeps the component aligned with the updated type contract.Also applies to: 29-33
25-27: 🛠️ Refactor suggestionUse a stable key instead of the array index
Using the array index (
key={index}) prevents React from accurately tracking list items after re-ordering or filtering and can lead to subtle UI bugs. The repository object already exposes a unique identifier (repository.key), so prefer that:- {displayedRepositories.map((repository, index) => { - return <RepositoryItem key={index} details={repository} /> - })} + {displayedRepositories.map((repository) => ( + <RepositoryItem key={repository.key} details={repository} /> + ))}This is a low-effort improvement that increases render stability.
frontend/src/components/RecentReleases.tsx (3)
40-42: 🛠️ Refactor suggestionUse a stable, domain-specific key instead of the loop index
indexis re-created on every re-render, defeating React’s key-based diffing and risking subtle UI bugs. Prefer a property that is guaranteed unique and stable, e.g.item.tagNameoritem.url.- {data.map((item, index) => ( - <div key={index} className="mb-4 w-full …"> + {data.map((item) => ( + <div key={item.tagName ?? item.url} className="mb-4 w-full …">
57-63:⚠️ Potential issueEmpty string passed to
Image.srccan crash Next 13+
next/imagethrows whensrcis an empty string. Provide a fallback placeholder or render the<Image>conditionally.- <Image … src={item?.author?.avatarUrl || ''} … /> + {item?.author?.avatarUrl && ( + <Image … src={item.author.avatarUrl} … /> + )}
85-94: 🛠️ Refactor suggestionGuard against undefined path segments
If
organizationNameis undefined the URL becomes/organizations/undefined/repositories/which breaks navigation.-onClick={() => - router.push( - `/organizations/${item?.organizationName}/repositories/${item.repositoryName || ''}` - ) -} +onClick={() => { + if (item.organizationName && item.repositoryName) { + router.push( + `/organizations/${item.organizationName}/repositories/${item.repositoryName}` + ) + } +}}backend/apps/core/utils/index.py (1)
183-194:⚠️ Potential issue
camelizedoes not actually produce camelCase
"idx_created_at"→"Createdat"instead of"createdAt". Consider splitting on'_', lower-casing the first token, capitalising the rest, and then joining.-def camelize(key: str) -> str: - """Camelize a string.""" - return key.replace("_", "").title() +import re + +def camelize(key: str) -> str: + """Convert snake_case to camelCase (e.g., created_at → createdAt).""" + parts = re.sub(r"[_\s]+", "_", key).split("_") + return parts[0].lower() + "".join(word.title() for word in parts[1:])
♻️ Duplicate comments (1)
frontend/src/app/projects/page.tsx (1)
81-81: Duplicate: optional chaining still missing
Same issue previously raised – please apply the change.- {projects && projects.filter((project) => project.isActive).map(renderProjectCard)} + {projects?.filter((project) => project.isActive).map(renderProjectCard)}🧰 Tools
🪛 Biome (1.9.4)
[error] 81-81: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🧹 Nitpick comments (5)
frontend/src/components/SortBy.tsx (1)
5-5: Consistent use ofimport typeNice catch converting this to a type-only import; keeps emitted JS lean and matches the rest of the refactor.
You might consider grouping allimport typestatements together (e.g., after runtime imports) for readability, but that’s optional.frontend/src/app/snapshots/page.tsx (1)
8-14: Consider renamingSnapshotsto singularSnapshotfor clarityThroughout the file
Snapshotsis used as the type of a single element (Snapshots[], function arg, etc.). A plural name for a singular entity is counter-intuitive and can trip up new contributors.-import type { Snapshots } from 'types/snapshot' +import type { Snapshot } from 'types/snapshot' -const [snapshots, setSnapshots] = useState<Snapshots[] | null>(null) +const [snapshots, setSnapshots] = useState<Snapshot[] | null>(null) -const handleButtonClick = (snapshot: Snapshots) => { +const handleButtonClick = (snapshot: Snapshot) => { -const renderSnapshotCard = (snapshot: Snapshots) => { +const renderSnapshotCard = (snapshot: Snapshot) => { - snapshots.map((snapshot: Snapshots) => ( + snapshots.map((snapshot: Snapshot) => (Refactor now or add a TODO before this naming pattern spreads further.
Also applies to: 38-43, 71-73
backend/apps/core/utils/index.py (1)
164-180:deep_camelizereturn annotation is inaccurateThe function may return primitives (e.g.
str,int) when the input is not adict/list. Update the annotation or wrap intyping.Any.-def deep_camelize(obj) -> dict | list: +from typing import Any + +def deep_camelize(obj: Any) -> Any:frontend/src/app/chapters/page.tsx (1)
103-103: Apply optional chaining to silence undefined checksStatic analysis flagged this; optional chaining is safer and reads better.
- {chapters && chapters.filter((chapter) => chapter.isActive).map(renderChapterCard)} + {chapters?.filter((chapter) => chapter.isActive).map(renderChapterCard)}🧰 Tools
🪛 Biome (1.9.4)
[error] 103-103: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
frontend/src/app/projects/page.tsx (1)
40-45: Inconsistent local variable naming
SubmitButtonhere starts with an upper-case letter whereas the same pattern inChaptersPageusessubmitButton. Consistent camelCase for variables prevents confusion with React components.- const SubmitButton = { + const submitButton = { … - <Card - button={SubmitButton} + <Card + button={submitButton}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (61)
backend/apps/core/utils/index.py(1 hunks)frontend/src/app/about/page.tsx(3 hunks)frontend/src/app/chapters/[chapterKey]/page.tsx(2 hunks)frontend/src/app/chapters/page.tsx(6 hunks)frontend/src/app/committees/[committeeKey]/page.tsx(1 hunks)frontend/src/app/contribute/page.tsx(3 hunks)frontend/src/app/members/[memberKey]/page.tsx(1 hunks)frontend/src/app/members/page.tsx(2 hunks)frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx(1 hunks)frontend/src/app/organizations/page.tsx(3 hunks)frontend/src/app/projects/[projectKey]/page.tsx(2 hunks)frontend/src/app/projects/page.tsx(4 hunks)frontend/src/app/snapshots/[id]/page.tsx(3 hunks)frontend/src/app/snapshots/page.tsx(1 hunks)frontend/src/components/Card.tsx(1 hunks)frontend/src/components/CardDetailsPage.tsx(5 hunks)frontend/src/components/ChapterMap.tsx(5 hunks)frontend/src/components/ChapterMapWrapper.tsx(1 hunks)frontend/src/components/ContributorAvatar.tsx(2 hunks)frontend/src/components/DisplayIcon.tsx(1 hunks)frontend/src/components/Footer.tsx(1 hunks)frontend/src/components/InfoBlock.tsx(1 hunks)frontend/src/components/ItemCardList.tsx(2 hunks)frontend/src/components/LeadersList.tsx(1 hunks)frontend/src/components/LogoCarousel.tsx(1 hunks)frontend/src/components/Milestones.tsx(1 hunks)frontend/src/components/Modal.tsx(1 hunks)frontend/src/components/MultiSearch.tsx(5 hunks)frontend/src/components/NavButton.tsx(1 hunks)frontend/src/components/NavDropDown.tsx(1 hunks)frontend/src/components/RecentIssues.tsx(1 hunks)frontend/src/components/RecentPullRequests.tsx(1 hunks)frontend/src/components/RecentReleases.tsx(1 hunks)frontend/src/components/RepositoriesCard.tsx(1 hunks)frontend/src/components/SnapshotCard.tsx(1 hunks)frontend/src/components/SortBy.tsx(1 hunks)frontend/src/components/TopContributorsList.tsx(2 hunks)frontend/src/components/UserCard.tsx(3 hunks)frontend/src/components/skeletons/Card.tsx(1 hunks)frontend/src/components/skeletons/UserCard.tsx(1 hunks)frontend/src/server/fetchAlgoliaData.ts(3 hunks)frontend/src/types/algolia.ts(1 hunks)frontend/src/types/button.ts(2 hunks)frontend/src/types/card.ts(4 hunks)frontend/src/types/chapter.ts(1 hunks)frontend/src/types/committee.ts(1 hunks)frontend/src/types/home.ts(1 hunks)frontend/src/types/issue.ts(1 hunks)frontend/src/types/milestone.ts(1 hunks)frontend/src/types/modal.ts(1 hunks)frontend/src/types/organization.ts(1 hunks)frontend/src/types/project.ts(2 hunks)frontend/src/types/pullRequest.ts(1 hunks)frontend/src/types/release.ts(1 hunks)frontend/src/types/search.ts(1 hunks)frontend/src/types/skeleton.ts(2 hunks)frontend/src/types/snapshot.ts(1 hunks)frontend/src/types/sortBy.ts(1 hunks)frontend/src/types/user.ts(1 hunks)frontend/src/utils/constants.ts(1 hunks)frontend/src/utils/utility.ts(2 hunks)
✅ Files skipped from review due to trivial changes (21)
- frontend/src/utils/constants.ts
- frontend/src/components/LogoCarousel.tsx
- frontend/src/components/skeletons/UserCard.tsx
- frontend/src/components/Card.tsx
- frontend/src/components/LeadersList.tsx
- frontend/src/components/skeletons/Card.tsx
- frontend/src/components/SnapshotCard.tsx
- frontend/src/components/NavDropDown.tsx
- frontend/src/components/Modal.tsx
- frontend/src/components/InfoBlock.tsx
- frontend/src/types/sortBy.ts
- frontend/src/components/Footer.tsx
- frontend/src/types/skeleton.ts
- frontend/src/types/release.ts
- frontend/src/components/NavButton.tsx
- frontend/src/components/RecentPullRequests.tsx
- frontend/src/types/button.ts
- frontend/src/types/pullRequest.ts
- frontend/src/components/RecentIssues.tsx
- frontend/src/types/milestone.ts
- frontend/src/components/Milestones.tsx
🚧 Files skipped from review as they are similar to previous changes (30)
- frontend/src/app/members/page.tsx
- frontend/src/app/projects/[projectKey]/page.tsx
- frontend/src/components/TopContributorsList.tsx
- frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx
- frontend/src/components/ChapterMapWrapper.tsx
- frontend/src/app/chapters/[chapterKey]/page.tsx
- frontend/src/app/members/[memberKey]/page.tsx
- frontend/src/types/algolia.ts
- frontend/src/app/committees/[committeeKey]/page.tsx
- frontend/src/app/about/page.tsx
- frontend/src/components/UserCard.tsx
- frontend/src/app/snapshots/[id]/page.tsx
- frontend/src/app/contribute/page.tsx
- frontend/src/components/ContributorAvatar.tsx
- frontend/src/components/ItemCardList.tsx
- frontend/src/app/organizations/page.tsx
- frontend/src/components/CardDetailsPage.tsx
- frontend/src/types/organization.ts
- frontend/src/types/committee.ts
- frontend/src/components/ChapterMap.tsx
- frontend/src/components/MultiSearch.tsx
- frontend/src/types/issue.ts
- frontend/src/server/fetchAlgoliaData.ts
- frontend/src/types/search.ts
- frontend/src/types/chapter.ts
- frontend/src/utils/utility.ts
- frontend/src/types/snapshot.ts
- frontend/src/types/home.ts
- frontend/src/types/project.ts
- frontend/src/types/card.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
frontend/src/components/RecentReleases.tsx (1)
frontend/src/types/release.ts (1)
ReleaseType(3-13)
frontend/src/types/user.ts (4)
frontend/src/types/issue.ts (1)
IssueType(7-19)frontend/src/types/release.ts (1)
ReleaseType(3-13)frontend/src/types/project.ts (1)
RepositoryCardProps(49-59)backend/apps/github/models/user.py (1)
User(14-129)
frontend/src/app/projects/page.tsx (3)
frontend/src/hooks/useSearchPage.ts (1)
useSearchPage(29-139)frontend/src/types/project.ts (1)
ProjectType(16-43)frontend/src/utils/data.ts (1)
level(111-132)
🪛 Biome (1.9.4)
frontend/src/app/chapters/page.tsx
[error] 103-103: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
frontend/src/app/projects/page.tsx
[error] 81-81: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (4)
frontend/src/components/DisplayIcon.tsx (1)
4-4: Type-only import is a good moveSwitching to
import typemakes the intent clear and helps TS/webpack eliminate dead code. Looks good.frontend/src/types/modal.ts (2)
2-2: 👍 Good move to a type-only importUsing
import typekeeps the compiled JS lean by stripping this import at runtime—exactly what we want for purely-type dependencies.
4-13:Details
✅ Verification successful
Check downstream code that previously used
extends ModalPropsSwitching from an
interfaceto a type alias prevents declaration merging and disallowsinterface Foo extends ModalProps {}. If any component still relies on that syntax, the build will break.Run this quick scan to ensure no remaining
extends ModalPropsusages survive:If the command returns nothing, you’re clear. Otherwise refactor those interfaces to
type Foo = ModalProps & { … }.
🏁 Script executed:
#!/bin/bash # Locate interface extensions that would now fail rg -n $'interface\\s+\\w+\\s+extends\\s+ModalProps' --type ts --type tsxLength of output: 97
Let’s broaden the file filter so
rgpicks up both.tsand.tsxfiles:
🏁 Script executed:
#!/bin/bash # Re-scan all TS/TSX files for any interface extending ModalProps rg -n "interface\s+\w+\s+extends\s+ModalProps" -g '*.ts' -g '*.tsx'Length of output: 69
No remaining
interface … extends ModalPropsusages detected
I ran:rg -n "interface\s+\w+\s+extends\s+ModalProps" -g '*.ts' -g '*.tsx'and found no matches. Switching from an
interfaceto a type alias is safe—no downstream code relies on declaration merging forModalProps.frontend/src/app/snapshots/page.tsx (1)
8-8: Good move switching to a type-only importChanging the import to
import typemakes the intent explicit and helps avoid tree-shaking / bundler confusion. 👍
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
frontend/src/types/issue.ts (2)
10-15: Consider strong typing fornumber(numeric) and clarify units for timestamps
- GitHub issue numbers are integers; modelling them as
stringloses type safety and forces casts at call-sites.createdAt/updatedAtare typed asnumber, but it is unclear if this is a Unix epoch (seconds? ms?) or an ISO string already parsed toDate.getTime(). Ambiguity fuels bugs.Suggested tweak:
- createdAt: number + /** Unix epoch milliseconds. Use Date.toISOString() when serialising */ + createdAt: number - number?: string + number?: numberApplying this early prevents a cascade of
parseInt/Number()fixes later.
18-21: Nit: renamerepositoryLanguagestolanguagesfor cohesionEvery other field avoids data-source prefixes (
projectName,organizationName), butrepositoryLanguageskeeps the origin baked in.languages(orrepoLanguages) would keep the shape tight and future-proof if the same type is reused for non-repository contexts.Not blocking, but worth considering before the API freezes.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/types/issue.ts(1 hunks)frontend/src/types/release.ts(1 hunks)frontend/src/types/user.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/types/user.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
frontend/src/types/release.ts (1)
frontend/src/types/user.ts (2)
User(32-32)RepositoryDetails(5-8)
frontend/src/types/issue.ts (1)
frontend/src/types/user.ts (2)
User(32-32)RepositoryDetails(5-8)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (2)
frontend/src/types/release.ts (1)
10-12: RequiredrepositoryNamewhenrepositoryis optional may lead to inconsistent state
repositoryis optional butrepositoryNameis mandatory. Whenrepositoryisundefined, it’s unclear whatrepositoryNameshould represent (empty string? orphaned data?). Either:
- Make both fields optional and document fallback behaviour, or
- Always require
repositoryand derive itskey/ownerKeyinstead of duplicating a name string.Consider tightening the model to avoid partial data bugs.
frontend/src/types/issue.ts (1)
1-1: Verify path aliastypes/userexists intsconfigThe import relies on a path alias (
types/user). If the alias is missing or mis-configured,tsc/Vite will fail outside the IDE.Ask yourself:
- Is
"paths": { "types/*": ["src/types/*"] }present intsconfig.base.json?- Are Jest/ESLint aliases aligned?
If not, update the config or use a relative path.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
frontend/src/app/projects/page.tsx (1)
81-81:⚠️ Potential issueOptional-chain the projects collection (repeat of previous review)
The unsafe
projects && …pattern survived the last review. Swap it for optional chaining so the component renders gracefully when the query hasn’t returned yet:- {projects && projects.filter((project) => project.isActive).map(renderProjectCard)} + {projects?.filter((project) => project.isActive).map(renderProjectCard)}🧰 Tools
🪛 Biome (1.9.4)
[error] 81-81: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
frontend/src/types/issue.ts (1)
1-19: 💡 Verification agent🧩 Analysis chain
Left-over snake_case keys could still exist in the codebase
Previous reviews flagged
open_issues_count/total_pagesunder
IssuesDataType. This file deletes that type, but we need to be sure all
references were removed or renamed.Run the script to confirm no lingering snake_case usages:
🏁 Script executed:
#!/bin/bash # Fail if any snake_case pagination keys remain set -e patterns=("open_issues_count" "total_pages") for p in "${patterns[@]}"; do if rg -n "${p}" frontend | grep -q .; then echo "❌ Found residual usage of ${p}" exit 1 fi done echo "✅ No residual snake_case pagination keys"Length of output: 293
🏁 Script executed:
#!/bin/bash # Locate all occurrences of total_pages in the frontend directory rg -n "total_pages" frontendLength of output: 104
Update remaining snake_case pagination key
A residual
total_pagesreference was found in a test and needs to be renamed to camelCase:• frontend/tests/unit/pages/Contribute.test.tsx:138
- total_pages: 1, + totalPages: 1,No occurrences of
open_issues_countwere detected. Please update this test to match the renamed pagination key.
🧹 Nitpick comments (1)
frontend/src/app/projects/page.tsx (1)
33-36: Type-safe param list – usekeyof Projectinstead of plain stringsHard-coding property names as
string[]bypasses the compiler’s help; a later rename inProjectwould silently break the icon rendering. Declare the list with akeyof Projectunion (and ideallyas const) so TypeScript flags typos automatically.- const params: string[] = ['forksCount', 'starsCount', 'contributorsCount'] + const params: Array<keyof Project> = [ + 'forksCount', + 'starsCount', + 'contributorsCount', + ] as const
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (34)
backend/apps/core/api/algolia.py(2 hunks)frontend/src/app/about/page.tsx(3 hunks)frontend/src/app/committees/page.tsx(3 hunks)frontend/src/app/contribute/page.tsx(3 hunks)frontend/src/app/members/[memberKey]/page.tsx(1 hunks)frontend/src/app/organizations/page.tsx(3 hunks)frontend/src/app/projects/[projectKey]/page.tsx(2 hunks)frontend/src/app/projects/page.tsx(4 hunks)frontend/src/app/snapshots/[id]/page.tsx(3 hunks)frontend/src/components/DisplayIcon.tsx(1 hunks)frontend/src/components/ItemCardList.tsx(2 hunks)frontend/src/components/LogoCarousel.tsx(1 hunks)frontend/src/components/Milestones.tsx(1 hunks)frontend/src/components/MultiSearch.tsx(6 hunks)frontend/src/components/RecentIssues.tsx(1 hunks)frontend/src/components/RecentPullRequests.tsx(1 hunks)frontend/src/components/RecentReleases.tsx(1 hunks)frontend/src/types/button.ts(1 hunks)frontend/src/types/card.ts(3 hunks)frontend/src/types/event.ts(1 hunks)frontend/src/types/home.ts(2 hunks)frontend/src/types/icon.ts(1 hunks)frontend/src/types/issue.ts(1 hunks)frontend/src/types/milestone.ts(1 hunks)frontend/src/types/modal.ts(1 hunks)frontend/src/types/organization.ts(1 hunks)frontend/src/types/project.ts(2 hunks)frontend/src/types/pullRequest.ts(1 hunks)frontend/src/types/release.ts(1 hunks)frontend/src/types/search.ts(1 hunks)frontend/src/types/section.ts(1 hunks)frontend/src/types/snapshot.ts(1 hunks)frontend/src/types/user.ts(1 hunks)frontend/src/utils/utility.ts(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- frontend/src/types/event.ts
- frontend/src/types/icon.ts
🚧 Files skipped from review as they are similar to previous changes (30)
- frontend/src/components/RecentIssues.tsx
- frontend/src/app/projects/[projectKey]/page.tsx
- frontend/src/app/members/[memberKey]/page.tsx
- frontend/src/types/section.ts
- backend/apps/core/api/algolia.py
- frontend/src/app/about/page.tsx
- frontend/src/components/RecentReleases.tsx
- frontend/src/app/committees/page.tsx
- frontend/src/types/button.ts
- frontend/src/app/contribute/page.tsx
- frontend/src/components/LogoCarousel.tsx
- frontend/src/types/pullRequest.ts
- frontend/src/components/DisplayIcon.tsx
- frontend/src/types/modal.ts
- frontend/src/components/Milestones.tsx
- frontend/src/types/milestone.ts
- frontend/src/types/organization.ts
- frontend/src/components/RecentPullRequests.tsx
- frontend/src/types/snapshot.ts
- frontend/src/types/release.ts
- frontend/src/components/MultiSearch.tsx
- frontend/src/types/user.ts
- frontend/src/utils/utility.ts
- frontend/src/app/snapshots/[id]/page.tsx
- frontend/src/app/organizations/page.tsx
- frontend/src/components/ItemCardList.tsx
- frontend/src/types/home.ts
- frontend/src/types/search.ts
- frontend/src/types/project.ts
- frontend/src/types/card.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
frontend/src/types/issue.ts (1)
frontend/src/types/user.ts (2)
User(10-30)RepositoryDetails(5-8)
frontend/src/app/projects/page.tsx (3)
frontend/src/hooks/useSearchPage.ts (1)
useSearchPage(29-139)frontend/src/types/project.ts (1)
Project(16-43)frontend/src/utils/data.ts (1)
level(111-132)
🪛 Biome (1.9.4)
frontend/src/app/projects/page.tsx
[error] 81-81: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/apps/common/utils.py(1 hunks)backend/apps/core/utils/index.py(2 hunks)backend/tests/apps/common/utils_test.py(2 hunks)frontend/src/app/page.tsx(6 hunks)frontend/src/components/Milestones.tsx(1 hunks)frontend/src/types/chapter.ts(1 hunks)frontend/src/types/issue.ts(1 hunks)frontend/src/types/milestone.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/src/types/milestone.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- frontend/src/components/Milestones.tsx
- backend/apps/core/utils/index.py
- frontend/src/app/page.tsx
- frontend/src/types/chapter.ts
- frontend/src/types/issue.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/tests/apps/common/utils_test.py (1)
backend/apps/common/utils.py (1)
convert_to_camel_case(15-28)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run frontend e2e tests
🔇 Additional comments (1)
backend/tests/apps/common/utils_test.py (1)
20-34: Add edge-case cases for empty and double-underscore stringsThe parametrised suite is solid, but the implementation bug above isn’t surfaced because inputs like
"","_"or"__double"are missing. Including them would prevent regressions and document the intended behaviour:[ + ("", ""), ("simple", "simple"), ("my_variable", "myVariable"), @@ ("multiple__underscores", "multipleUnderscores"), ("alreadyCamelCase", "alreadyCamelCase"), + ("_", "_"), # single underscore + ("__double_lead", "__doubleLead"), ],After adding, assert that they pass with the refactored util.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/src/app/projects/page.tsx (1)
81-81: Optional chaining still missing (repeat of earlier feedback)
projects && …risks a runtime error whenprojectsisundefined. Switch toprojects?.filter(...).map(...)as previously suggested.🧰 Tools
🪛 Biome (1.9.4)
[error] 81-81: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🧹 Nitpick comments (1)
frontend/src/app/projects/page.tsx (1)
33-35: Leveragekeyof Projectfor stronger type-safetyUsing raw strings forfeits compile-time checks; a typo will silently slip through. Declare the list as
Array<keyof Project>(and move it outside the render function to avoid re-allocating on every render).- const params: string[] = ['forksCount', 'starsCount', 'contributorsCount'] + const params: Array<keyof Project> = ['forksCount', 'starsCount', 'contributorsCount']
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
frontend/src/app/about/page.tsx(3 hunks)frontend/src/app/chapters/[chapterKey]/page.tsx(2 hunks)frontend/src/app/committees/[committeeKey]/page.tsx(1 hunks)frontend/src/app/members/page.tsx(2 hunks)frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx(1 hunks)frontend/src/app/organizations/page.tsx(2 hunks)frontend/src/app/projects/[projectKey]/page.tsx(2 hunks)frontend/src/app/projects/page.tsx(3 hunks)frontend/src/app/snapshots/[id]/page.tsx(3 hunks)frontend/src/app/snapshots/page.tsx(3 hunks)frontend/src/components/ContributorAvatar.tsx(2 hunks)frontend/src/components/ItemCardList.tsx(2 hunks)frontend/src/components/TopContributorsList.tsx(2 hunks)frontend/src/types/button.ts(1 hunks)frontend/src/types/card.ts(3 hunks)frontend/src/types/chapter.ts(1 hunks)frontend/src/types/committee.ts(1 hunks)frontend/src/types/contributor.ts(1 hunks)frontend/src/types/home.ts(2 hunks)frontend/src/types/modal.ts(1 hunks)frontend/src/types/project.ts(2 hunks)frontend/src/types/section.ts(1 hunks)frontend/src/types/skeleton.ts(1 hunks)frontend/src/types/snapshot.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (22)
- frontend/src/types/section.ts
- frontend/src/app/snapshots/page.tsx
- frontend/src/app/projects/[projectKey]/page.tsx
- frontend/src/app/committees/[committeeKey]/page.tsx
- frontend/src/app/chapters/[chapterKey]/page.tsx
- frontend/src/app/members/page.tsx
- frontend/src/app/about/page.tsx
- frontend/src/types/contributor.ts
- frontend/src/types/modal.ts
- frontend/src/components/TopContributorsList.tsx
- frontend/src/components/ItemCardList.tsx
- frontend/src/types/button.ts
- frontend/src/app/organizations/page.tsx
- frontend/src/components/ContributorAvatar.tsx
- frontend/src/types/committee.ts
- frontend/src/app/snapshots/[id]/page.tsx
- frontend/src/types/chapter.ts
- frontend/src/types/snapshot.ts
- frontend/src/types/project.ts
- frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx
- frontend/src/types/home.ts
- frontend/src/types/card.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/app/projects/page.tsx (4)
frontend/src/hooks/useSearchPage.ts (1)
useSearchPage(29-139)frontend/src/types/project.ts (1)
Project(16-43)frontend/src/utils/utility.ts (1)
getFilteredIcons(22-35)frontend/src/utils/data.ts (1)
level(111-132)
🪛 Biome (1.9.4)
frontend/src/app/projects/page.tsx
[error] 81-81: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (3)
frontend/src/types/skeleton.ts (2)
1-4: Interface-to-type switch may break declaration merging /extendschainsChanging
CardSkeletonPropsfrominterfaceto atyperemoves two TS capabilities:
- Declaration merging – consumers can no longer augment the shape in a different file.
extendsclauses – existing code like
interface Foo extends CardSkeletonProps { … }will now error.Before merging, grep the codebase for
extends CardSkeletonPropsor any duplicate declarations to ensure nothing breaks, and convert them to intersection types (Foo = CardSkeletonProps & { … }) if necessary.
13-17: Same merging concern forUserCardSkeletonPropsThe interface-to-type conversion brings the same risk of breaking
extendsor merged interface patterns. Run the same verification forUserCardSkeletonProps.frontend/src/app/projects/page.tsx (1)
40-44: Double-check prop naming consistency (onClickvsonclick)The button object uses the lowercase
onclick. Most components – and React conventions in general – expectonClick.
Please confirmCardis indeed looking foronclick; otherwise the handler will never fire.
| numIcons?: number | ||
| showActionButton?: boolean | ||
| showContributors?: boolean |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential ambiguity between numIcons and showIcons
numIcons?: number was added, but the existing boolean flag showIcons? remains.
If showIcons is false yet numIcons is a positive value, which prop wins? Consider either:
- numIcons?: number
- showIcons?: boolean
+ /**
+ * When > 0, component renders that many icons.
+ * If provided, `showIcons` is ignored.
+ */
+ numIcons?: numberor deprecate one of the two to avoid conflicting states.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/types/skeleton.ts around lines 2 to 4, there is ambiguity
between the boolean `showIcons` and the numeric `numIcons` properties. To fix
this, clarify their relationship by either removing or deprecating one of the
properties to avoid conflicting states, or implement logic that clearly defines
which property takes precedence when both are set. Update the type definitions
and any related usage accordingly to ensure consistent behavior.
|
arkid15r
left a comment
arkid15r
left a comment
There was a problem hiding this comment.
It's a great clean up work even though we still have a few cases w/ interfaces. Let's take care of that after merging this PR.
* fixed backend to return camelCase * fixed types for chapters, committees, members and organizations * fixed types for all pages * fixed types of all the pages, component, utility, types * fixed unit tests * fixed types and interfaces * updated code * Update code * fixed types and import with types * fixed more bugs * updated code * fixed code * fixed camelcase fn * fixed testcases * fixed unit test * Update code * Update code --------- Co-authored-by: Tamara Lazerka <98753789+aramattamara@users.noreply.github.com> Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org>
3 participants
Resolves #1477