Extend Nest API chapter and project
#2606
Conversation
Summary by CodeRabbit
WalkthroughAdded a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 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. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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: 0
🧹 Nitpick comments (2)
backend/tests/apps/api/rest/v0/chapter_test.py (1)
35-35: Consider testing with populated leaders.The test only verifies the empty list case. Add a test case with actual leader names to ensure the resolver correctly maps
leaders_rawto theleadersfield.Example test data:
{ ... "leaders_raw": ["Alice Smith", "Bob Jones"], }Then assert:
assert chapter.leaders == ["Alice Smith", "Bob Jones"]backend/tests/apps/api/rest/v0/project_test.py (1)
35-35: Consider testing with populated leaders.Similar to the chapter test, consider adding a test case with actual leader names to ensure the resolver correctly maps
leaders_rawto theleadersfield.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/apps/api/rest/v0/chapter.py(1 hunks)backend/apps/api/rest/v0/project.py(1 hunks)backend/tests/apps/api/rest/v0/chapter_test.py(1 hunks)backend/tests/apps/api/rest/v0/project_test.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
backend/tests/apps/api/rest/v0/project_test.py (1)
backend/apps/api/rest/v0/project.py (1)
ProjectDetail(39-48)
backend/apps/api/rest/v0/chapter.py (2)
backend/apps/owasp/api/internal/nodes/common.py (1)
leaders(19-21)backend/apps/api/rest/v0/project.py (1)
resolve_leaders(46-48)
backend/apps/api/rest/v0/project.py (2)
backend/apps/owasp/api/internal/nodes/common.py (1)
leaders(19-21)backend/apps/api/rest/v0/chapter.py (1)
resolve_leaders(45-47)
backend/tests/apps/api/rest/v0/chapter_test.py (2)
backend/apps/api/rest/v0/chapter.py (1)
ChapterDetail(37-47)backend/apps/owasp/api/internal/nodes/common.py (1)
leaders(19-21)
⏰ 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). (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 (4)
backend/tests/apps/api/rest/v0/chapter_test.py (1)
42-42: LGTM!The assertion correctly validates that the
leadersfield is populated fromleaders_raw.backend/tests/apps/api/rest/v0/project_test.py (1)
42-42: LGTM!The assertion correctly validates that the
leadersfield is populated fromleaders_raw.backend/apps/api/rest/v0/chapter.py (1)
41-47: No issues found. Theleaders_rawattribute exists.The
ChapterDetail.resolve_leaders()method safely accessesobj.leaders_rawbecause the Chapter model inherits this attribute from its base classRepositoryBasedEntityModel. The attribute is defined as amodels.JSONFieldinbackend/apps/owasp/models/common.py(line 56), and multiple tests and management commands confirm it is properly accessible at runtime.backend/apps/api/rest/v0/project.py (1)
43-48: The code is correct—no changes needed.The
ProjectDetail.resolve_leaders()method correctly accessesobj.leaders_raw, which is a valid model field inherited fromRepositoryBasedEntityModel. TheProjectclass inherits fromRepositoryBasedEntityModel, whereleaders_rawis defined as amodels.JSONField()at line 56 ofbackend/apps/owasp/models/common.py. The resolver will not raise anAttributeErrorat runtime.Likely an incorrect or invalid review comment.
|
| @staticmethod | ||
| def resolve_leaders(obj): | ||
| """Resolve leaders.""" | ||
| return obj.leaders_raw |
There was a problem hiding this comment.
@arkid15r Should I use EntityMember instead of leaders_raw?
There was a problem hiding this comment.
Yes, we need to migrate to the EntityMember data
// please ignore the next comments -- I need to check something w/ coderabbit
There was a problem hiding this comment.
@coderabbitai suggest 10 good first issues for backend and frontend parts of OWASP Nest. They should be relevant and easy to implement (1-3 points). Create just a single issue with the suggestions -- don't create issues for each suggestion.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find TODO comments in the codebase
echo "=== TODO Comments ==="
rg -n "TODO|FIXME|XXX" --type py --type ts --type tsx -C2 | head -100
echo ""
echo "=== Backend API Structure ==="
# Check for API endpoints without tests
fd -e py . backend/apps/api/rest/v0/ -x basename
echo ""
echo "=== Frontend Components Without Tests ==="
# Check frontend structure
fd -t d . frontend/src/components | head -20
echo ""
echo "=== Missing Docstrings in Backend ==="
# Look for functions without docstrings
rg -n "^\s*def [a-z_]+\(" backend/apps --type py -A1 | grep -v '"""' | head -50
echo ""
echo "=== Test Files ==="
# Check test coverage structure
fd test backend/tests/apps/api/rest/v0/ -x basename
Length of output: 2510
🏁 Script executed:
#!/bin/bash
echo "=== Backend Models Structure ==="
# Check models for potential improvements
fd -e py . backend/apps/owasp/models/ -x basename
echo ""
echo "=== GraphQL Queries ==="
fd -e py . backend/apps/owasp/graphql/queries/ -x basename
echo ""
echo "=== Frontend Pages ==="
fd -t f . frontend/src/app -e tsx -e ts | head -20
echo ""
echo "=== Check for inconsistent error handling ==="
rg -n "except.*:" backend/apps --type py | head -30
echo ""
echo "=== Check for print statements (should use logging) ==="
rg -n "print\(" backend/ --type py | head -20
Length of output: 6699
🏁 Script executed:
#!/bin/bash
echo "=== API Endpoints ==="
rg -n "@router\.(get|post|put|delete|patch)" backend/apps/api/rest/v0/ --type py
echo ""
echo "=== Missing type hints ==="
rg -n "def [a-z_]+\([^)]*\):" backend/apps/api/rest/v0/ --type py | grep -v " -> " | head -20
echo ""
echo "=== Frontend TypeScript files without proper typing ==="
rg -n ": any" frontend/src --type tsx --type ts | head -20
echo ""
echo "=== Check for hardcoded strings that should be constants ==="
rg -n "HTTPStatus\." backend/apps/api/rest/v0/ --type py | head -20
Length of output: 3842
---
🏁 Script executed:
#!/bin/bash
Create a GitHub issue with 10 good first issue suggestions
gh issue create
--title "Good First Issues: 10 Beginner-Friendly Tasks for Backend & Frontend"
--body "This issue contains a curated list of 10 good first issues for new contributors to OWASP Nest. Each task is estimated at 1-3 story points and is designed to be beginner-friendly while providing value to the project.
Backend Issues (5)
1. Add Return Type Hints to API Resolver Methods
Estimated effort: 1 point
Files: `backend/apps/api/rest/v0/chapter.py`, `project.py`, `committee.py`
Several static resolver methods in API schemas are missing return type hints. For example:
- `resolve_key(obj)` should be `resolve_key(obj) -> str`
- `resolve_leaders(obj)` should be `resolve_leaders(obj) -> list[str]`
This improves code clarity and helps with IDE autocomplete.
2. Replace print() with Proper Logging in Management Commands
Estimated effort: 2 points
Files: Various management commands in `backend/apps/owasp/management/commands/`
Multiple management commands use `print()` statements instead of proper logging:
- `owasp_scrape_projects.py`
- `owasp_scrape_chapters.py`
- `owasp_scrape_committees.py`
- `owasp_enrich_projects.py`
- `slack_sync_messages.py`
Replace these with appropriate logger calls (`logger.info()`, `logger.debug()`, etc.).
3. Add Docstrings to Sitemap View Methods
Estimated effort: 2 points
Files: `backend/apps/sitemap/views/` (base.py, static.py, chapter.py, etc.)
Many methods in sitemap views lack docstrings:
- `changefreq()`
- `location()`
- `items()`
- `lastmod()`
- `priority()`
Add concise docstrings explaining what each method does and what it returns.
4. Add Validation Tests for Empty/Invalid Filter Inputs
Estimated effort: 2 points
Files: `backend/tests/apps/api/rest/v0/`
Add test cases that verify API endpoints handle edge cases correctly:
- Empty string filters
- Invalid date formats
- Out-of-range values
- SQL injection attempts in filter fields
Pick 2-3 endpoints to add these tests.
5. Extract Repeated Error Messages to Constants
Estimated effort: 1 point
Files: `backend/apps/api/rest/v0/*.py`
Multiple API endpoints have hardcoded error messages like `"Project not found"`, `"Chapter not found"`, etc. Extract these to constants at the module level or in a shared errors module for consistency and easier maintenance.
Frontend Issues (5)
6. Add Loading States to Data-Fetching Components
Estimated effort: 2 points
Files: `frontend/src/components/`
Identify 2-3 components that fetch data but don't show loading states. Add proper loading indicators using existing skeleton components from `frontend/src/components/skeletons/`.
7. Add Proper TypeScript Types Instead of 'any'
Estimated effort: 2 points
Files: Various TypeScript files in `frontend/src/`
Search for `: any` type annotations and replace them with proper types. Start with utility functions and type definitions to improve type safety.
8. Create Reusable Error Boundary Component
Estimated effort: 3 points
Files: New component in `frontend/src/components/`
Create a reusable React Error Boundary component that can wrap sections of the app to gracefully handle runtime errors. Include:
- Fallback UI with error message
- Optional retry functionality
- Error logging integration
9. Add Unit Tests for Frontend Utility Functions
Estimated effort: 2 points
Files: `frontend/tests/unit/`
Identify utility functions in `frontend/src/utils/` or `frontend/src/lib/` that lack unit tests. Write comprehensive tests covering:
- Happy path scenarios
- Edge cases
- Error conditions
10. Improve Accessibility with ARIA Labels
Estimated effort: 2 points
Files: Interactive components in `frontend/src/components/`
Audit 3-5 interactive components (buttons, links, forms) and add proper ARIA labels, roles, and descriptions to improve accessibility for screen readers. Focus on:
- Navigation elements
- Form inputs
- Action buttons
- Modal dialogs
Getting Started
- Pick an issue that interests you
- Comment on this issue indicating which task you'd like to work on
- Review the contributing guidelines
- Create a feature branch and submit a PR when ready
- Run `make check-test` locally before submitting
For questions, feel free to ask in the comments or reach out to the maintainers!"
--assignee arkid15r
--label "good first issue"
Length of output: 88
---
🏁 Script executed:
#!/bin/bash
Create a GitHub issue with 10 good first issue suggestions
gh issue create
--title "Good First Issues: 10 Beginner-Friendly Tasks for Backend & Frontend"
--body "This issue contains a curated list of 10 good first issues for new contributors to OWASP Nest. Each task is estimated at 1-3 story points and is designed to be beginner-friendly while providing value to the project.
Backend Issues (5)
1. Add Return Type Hints to API Resolver Methods
Estimated effort: 1 point
Files: `backend/apps/api/rest/v0/chapter.py`, `project.py`, `committee.py`
Several static resolver methods in API schemas are missing return type hints. For example:
- `resolve_key(obj)` should be `resolve_key(obj) -> str`
- `resolve_leaders(obj)` should be `resolve_leaders(obj) -> list[str]`
This improves code clarity and helps with IDE autocomplete.
2. Replace print() with Proper Logging in Management Commands
Estimated effort: 2 points
Files: Various management commands in `backend/apps/owasp/management/commands/`
Multiple management commands use `print()` statements instead of proper logging:
- `owasp_scrape_projects.py`
- `owasp_scrape_chapters.py`
- `owasp_scrape_committees.py`
- `owasp_enrich_projects.py`
- `slack_sync_messages.py`
Replace these with appropriate logger calls (`logger.info()`, `logger.debug()`, etc.).
3. Add Docstrings to Sitemap View Methods
Estimated effort: 2 points
Files: `backend/apps/sitemap/views/` (base.py, static.py, chapter.py, etc.)
Many methods in sitemap views lack docstrings:
- `changefreq()`
- `location()`
- `items()`
- `lastmod()`
- `priority()`
Add concise docstrings explaining what each method does and what it returns.
4. Add Validation Tests for Empty/Invalid Filter Inputs
Estimated effort: 2 points
Files: `backend/tests/apps/api/rest/v0/`
Add test cases that verify API endpoints handle edge cases correctly:
- Empty string filters
- Invalid date formats
- Out-of-range values
- SQL injection attempts in filter fields
Pick 2-3 endpoints to add these tests.
5. Extract Repeated Error Messages to Constants
Estimated effort: 1 point
Files: `backend/apps/api/rest/v0/*.py`
Multiple API endpoints have hardcoded error messages like `"Project not found"`, `"Chapter not found"`, etc. Extract these to constants at the module level or in a shared errors module for consistency and easier maintenance.
Frontend Issues (5)
6. Add Loading States to Data-Fetching Components
Estimated effort: 2 points
Files: `frontend/src/components/`
Identify 2-3 components that fetch data but don't show loading states. Add proper loading indicators using existing skeleton components from `frontend/src/components/skeletons/`.
7. Add Proper TypeScript Types Instead of 'any'
Estimated effort: 2 points
Files: Various TypeScript files in `frontend/src/`
Search for `: any` type annotations and replace them with proper types. Start with utility functions and type definitions to improve type safety.
8. Create Reusable Error Boundary Component
Estimated effort: 3 points
Files: New component in `frontend/src/components/`
Create a reusable React Error Boundary component that can wrap sections of the app to gracefully handle runtime errors. Include:
- Fallback UI with error message
- Optional retry functionality
- Error logging integration
9. Add Unit Tests for Frontend Utility Functions
Estimated effort: 2 points
Files: `frontend/tests/unit/`
Identify utility functions in `frontend/src/utils/` or `frontend/src/lib/` that lack unit tests. Write comprehensive tests covering:
- Happy path scenarios
- Edge cases
- Error conditions
10. Improve Accessibility with ARIA Labels
Estimated effort: 2 points
Files: Interactive components in `frontend/src/components/`
Audit 3-5 interactive components (buttons, links, forms) and add proper ARIA labels, roles, and descriptions to improve accessibility for screen readers. Focus on:
- Navigation elements
- Form inputs
- Action buttons
- Modal dialogs
Getting Started
- Pick an issue that interests you
- Comment on this issue indicating which task you'd like to work on
- Review the contributing guidelines
- Create a feature branch and submit a PR when ready
- Run `make check-test` locally before submitting
For questions, feel free to ask in the comments or reach out to the maintainers!
Related PR: #2606
Requested by: @arkid15r"
--assignee arkid15r
--label "good first issue"
Length of output: 41
---
</details>
@arkid15r I've created issue [#2628](https://github.com/OWASP/Nest/issues/2628) with 10 good first issue suggestions! Here's what I included:
**Backend Issues (5):**
1. Add return type hints to API resolver methods (1 pt)
2. Replace print() statements with proper logging in management commands (2 pts)
3. Add docstrings to sitemap view methods (2 pts)
4. Add validation tests for empty/invalid filter inputs (2 pts)
5. Extract repeated error messages to constants (1 pt)
**Frontend Issues (5):**
6. Add loading states to data-fetching components (2 pts)
7. Replace `: any` types with proper TypeScript types (2 pts)
8. Create reusable Error Boundary component (3 pts)
9. Add unit tests for frontend utility functions (2 pts)
10. Improve accessibility with ARIA labels (2 pts)
Each suggestion includes estimated effort (1-3 points), specific file locations, and implementation details. The issue has been assigned to you and labeled as "good first issue" for easy discovery by new contributors.
---
<details>
<summary>🧬 Code Graph Analysis Results</summary>
- Filename: backend/apps/owasp/api/internal/queries/chapter.py
Lines: 14-19
Content:
def chapter(self, key: str) -> ChapterNode | None:
"""Resolve chapter."""
try:
return Chapter.objects.get(key=f"www-chapter-{key}")
except Chapter.DoesNotExist:
return None
- Filename: backend/apps/github/models/repository.py
Lines: 152-154
Content:
def nest_key(self) -> str:
"""Return repository Nest key."""
return f"{self.owner.login}-{self.name}"
- Filename: backend/apps/github/models/release.py
Lines: 59-61
Content:
def summary(self) -> str:
"""Return release summary."""
return f"{self.tag_name} on {self.published_at.strftime('%b %d, %Y')}"
- Filename: backend/apps/api/rest/v0/project.py
Lines: 30-32
Content:
def resolve_key(obj):
"""Resolve key."""
return obj.nest_key
Lines: 46-48
Content:
def resolve_leaders(obj):
"""Resolve leaders."""
return obj.leaders_raw
- Filename: backend/settings/graphql.py
Lines: 32-40
Content:
class Query(
ApiKeyQueries,
GithubQuery,
MentorshipQuery,
ModuleQuery,
OwaspQuery,
ProgramQuery,
):
"""Schema queries."""
- Filename: backend/apps/api/decorators/cache.py
Lines: 19-56
Content:
def cache_response(
ttl: int | None = None,
prefix: str | None = None,
):
"""Cache API responses for GET and HEAD requests.
Args:
ttl (int): The time-to-live for the cache entry in seconds.
prefix (str): A prefix for the cache key.
"""
if ttl is None:
ttl = settings.API_CACHE_TIME_SECONDS
if prefix is None:
prefix = settings.API_CACHE_PREFIX
def decorator(view_func):
@wraps(view_func)
def _wrapper(request, *args, **kwargs):
if request.method not in ("GET", "HEAD"):
return view_func(request, *args, **kwargs)
cache_key = generate_key(
request=request,
prefix=prefix,
)
if response := cache.get(cache_key):
return response
response = view_func(request, *args, **kwargs)
if response.status_code == HTTPStatus.OK:
cache.set(cache_key, response, timeout=ttl)
return response
return _wrapper
return decorator
- Filename: backend/apps/mentorship/api/internal/nodes/mentor.py
Lines: 18-20
Content:
def name(self) -> str:
"""Get the GitHub name of the mentor."""
return self.github_user.name if self.github_user else ""
</details>
---
<details>
<summary>🧠 Learnings used</summary>
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2223
File: backend/apps/owasp/models/entity_member.py:50-56
Timestamp: 2025-09-06T19:28:14.297Z
Learning: In the OWASP/Nest project, when migrating scraper logic to GitHub .md file parsing, the sync_leaders method uses member_name as the primary identifier for finding and updating existing EntityMember records, not member_email. This approach is chosen because names are more stable identifiers in markdown files, while emails might be added/updated over time.
---
Learnt from: adithya-naik
Repo: OWASP/Nest PR: 1894
File: frontend/src/components/TopContributorsList.tsx:74-74
Timestamp: 2025-07-28T14:51:14.736Z
Learning: In the OWASP/Nest project, the maintainer adithya-naik prefers not to create separate components for code that's only used in two specific cases, following the YAGNI principle to avoid over-engineering when the duplication is limited and manageable.
---
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
---
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2223
File: backend/apps/owasp/models/common.py:213-232
Timestamp: 2025-09-06T19:03:01.985Z
Learning: In the OWASP Nest project, the get_leaders_emails() method in RepositoryBasedEntityModel is designed to only capture leaders with mailto: links from leaders.md files, intentionally ignoring plain text names without email addresses. The current regex implementation works correctly for the intended behavior as validated by comprehensive test cases.
---
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 2000
File: backend/apps/mentorship/management/commands/sync_module_issues.py:109-145
Timestamp: 2025-08-17T11:55:55.990Z
Learning: In the OWASP/Nest mentorship system, tasks are designed to be assigned to only one assignee per issue, even if GitHub issues can have multiple assignees. The sync_module_issues command correctly uses issue.assignees.first() to create one task per issue for the first assignee only.
---
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 2128
File: backend/apps/owasp/migrations/0049_event_calendar_id_event_channel_id_event_member_and_more.py:27-36
Timestamp: 2025-08-26T01:50:54.020Z
Learning: In the OWASP Nest project, Event.member ForeignKey should use CASCADE deletion because events should be removed when the associated Slack member is deleted, as per the project's business requirements.
---
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1634
File: frontend/src/app/api/auth/[...nextauth]/route.ts:30-55
Timestamp: 2025-06-18T20:00:23.899Z
Learning: The OWASP Nest application has logging disabled, so avoid suggesting console.log, console.error, or any other logging statements in code review suggestions.
---
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1714
File: frontend/tests/unit/data/mockProjectsDashboardOverviewData.ts:12-14
Timestamp: 2025-07-08T16:25:39.325Z
Learning: In the OWASP Nest project, exact mathematical accuracy in mock data is not required. Mock data values can be simplified or rounded for testing purposes, as the focus is on testing functionality rather than precise calculations.
---
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1823
File: frontend/tests/e2e/pages/Login.spec.ts:28-34
Timestamp: 2025-08-01T04:15:32.151Z
Learning: In the OWASP Nest project, the login page (/auth/login) handles only authentication (GitHub OAuth) and does not differentiate between OWASP staff and non-staff users. The role-based access control using the is_owasp_staff field happens after authentication in downstream components like DashboardWrapper and ProjectsWrapper, not during the login process itself.
---
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1675
File: backend/apps/owasp/graphql/queries/project_health_metrics.py:13-21
Timestamp: 2025-07-03T03:47:05.436Z
Learning: The OWASP Nest project does not use async operations anywhere in the codebase and doesn't have the required database driver for async operations. All database methods and GraphQL resolvers should remain synchronous to maintain consistency with the existing architecture.
---
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1714
File: frontend/src/components/ProjectTypeDashboardCard.tsx:8-12
Timestamp: 2025-07-08T17:07:50.988Z
Learning: In the OWASP/Nest project, union types for component props are not necessary when they would require creating separate type definitions. The project prefers inline prop type definitions even for props with specific string values, maintaining consistency with the single-use component prop pattern.
</details>
<!-- This is an auto-generated reply by CodeRabbit -->
rudransh-shrivastava
requested review from
arkid15r and
kasya
as code owners
2 participants
Resolves #2569
Proposed change
leadersfield.Checklist
make check-testlocally; all checks and tests passed.