Feat: Implement Sponsors Program Support#4525
Open
mrkeshav-05 wants to merge 7 commits intoOWASP:mainfrom
Open
Feat: Implement Sponsors Program Support#4525mrkeshav-05 wants to merge 7 commits intoOWASP:mainfrom
mrkeshav-05 wants to merge 7 commits intoOWASP:mainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughAdds sponsor management: model fields (status, contact_email, chapter/project FKs) and migration; exposes Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
mrkeshav-05
dismissed stale reviews from cubic-dev-ai[bot] and coderabbitai[bot]
via
1040971
mrkeshav-05
force-pushed
the
feat/add-sponsors-program
branch
from
1040971 to
684ce40
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
backend/apps/owasp/migrations/0073_sponsor_status_email_entity_fks.py (1)
12-61: Consider indexingstatusfor sponsor listing paths.
Given repeated filtering by sponsor status in public listing endpoints, adding a DB index here will help query performance as data grows.Proposed migration addition
operations = [ migrations.AddField( model_name="sponsor", name="status", @@ verbose_name="Status", ), ), + migrations.AddIndex( + model_name="sponsor", + index=models.Index(fields=["status"], name="owasp_sponsor_status_idx"), + ), migrations.AddField( model_name="sponsor", name="contact_email",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/owasp/migrations/0073_sponsor_status_email_entity_fks.py` around lines 12 - 61, The Sponsor status field lacks a DB index; update the AddField operation for model_name="sponsor" name="status" to add db_index=True (or create a new migration that alters the "status" CharField to include db_index=True) so queries filtering on Sponsor.status are indexed and perform well as the table grows.backend/tests/unit/apps/api/rest/v0/sponsor_test.py (2)
106-121: Strengthen ordering test to assert default ACTIVE filtering.
This test currently can pass even if Line-level default status filtering is removed, because it only checks the final returned mock.Proposed test assertion updates
result = list_sponsors(mock_request, mock_filters, ordering="-name") mock_sponsor_model.objects.order_by.assert_called_with("-name") + mock_queryset.filter.assert_called_once_with( + status=mock_sponsor_model.SponsorStatus.ACTIVE + ) + mock_filters.filter.assert_called_once_with(mock_filtered_queryset) assert result == mock_filtered_queryset🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/unit/apps/api/rest/v0/sponsor_test.py` around lines 106 - 121, The test should additionally assert that after ordering the queryset the default active status filter is applied: after calling list_sponsors(mock_request, mock_filters, ordering="-name"), add an assertion that mock_queryset.filter was called with the model's ACTIVE status (e.g. mock_queryset.filter.assert_called_with(status=mock_sponsor_model.Status.ACTIVE)) to ensure default status filtering is enforced, then keep the existing assert that the returned result equals mock_filtered_queryset; locate this in the test_list_sponsors_with_ordering function referencing mock_sponsor_model, mock_queryset and list_sponsors.
191-210: Assert draft-status payload in sponsor application success test.
The success test should verify constructor arguments (especiallystatus=draft) to lock in the endpoint contract.Proposed test assertion updates
result = apply_sponsor(mock_request, payload) assert result.status_code == HTTPStatus.CREATED + mock_sponsor_model.assert_called_once() + created_kwargs = mock_sponsor_model.call_args.kwargs + assert created_kwargs["status"] == mock_sponsor_model.SponsorStatus.DRAFT + assert created_kwargs["name"] == payload.organization_name + assert created_kwargs["contact_email"] == payload.contact_email mock_sponsor_instance.save.assert_called_once()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/unit/apps/api/rest/v0/sponsor_test.py` around lines 191 - 210, The test test_apply_sponsor_success currently omits verifying that the created sponsor is initialized with draft status; update the test to assert the status is set to SponsorStatus.DRAFT by either asserting the model was constructed/called with status="draft" (use mock_sponsor_model.assert_called_with(..., status=mock_sponsor_model.SponsorStatus.DRAFT) or equivalent) or by checking the mock_sponsor_instance.status == mock_sponsor_model.SponsorStatus.DRAFT after apply_sponsor returns; reference apply_sponsor, mock_sponsor_model, mock_sponsor_instance and SponsorStatus.DRAFT when adding the assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/apps/api/rest/v0/sponsor.py`:
- Around line 126-129: The current queryset in
SponsorModel.objects.filter(...).order_by("sponsor_type", "name") uses lexical
sponsor_type ordering and must be replaced with the established sponsorship-tier
precedence used elsewhere; change the ordering to reuse the existing
sponsor-tier ordering strategy (the same sort used in
backend/apps/owasp/video.py and
backend/apps/owasp/api/internal/queries/sponsor.py) by either applying the same
in-memory sort/comparator or by annotating the queryset with a tier-precedence
Case/When expression before ordering, ensuring SponsorModel.SponsorStatus.ACTIVE
is still the filter and that final ordering falls back to name within the
correct tier precedence.
- Around line 148-166: Validate that slugify(payload.organization_name) yields a
non-empty key and return a BAD_REQUEST when it doesn't; then wrap sponsor
creation in an atomic transaction and use SponsorModel.objects.create(...)
instead of separate save to avoid the exists()-then-save race, and catch
django.db.utils.IntegrityError to detect duplicate-key insertions and return the
existing duplicate response. Reference slugify, payload.organization_name,
SponsorModel (and SponsorModel.SponsorStatus.DRAFT), transaction.atomic, and
IntegrityError in your changes.
In `@frontend/src/app/sponsors/apply/page.tsx`:
- Around line 73-80: The fetch call concatenates API_URL and the path which can
produce malformed URLs; update the fetch invocation in page.tsx to build the
endpoint safely (e.g., use the URL constructor or new
URL('/api/v0/sponsors/apply', API_URL)) instead of string concatenation so the
final URL is correct regardless of whether API_URL has a trailing slash; keep
the same method, headers, credentials and body (payload) but replace the URL
expression passed to fetch with the safely constructed URL.
- Around line 93-95: The code calls response.json() unguarded which can throw
for non-JSON error responses; wrap parsing in a try/catch or check
response.headers.get('content-type') for 'application/json' before calling
response.json(), and if not JSON fall back to await response.text() (or a
default message) and pass that text into addToast's description so addToast
receives actionable error details; modify the logic around response.json() and
addToast(...) (references: response.json(), addToast) to safely parse or
fallback without throwing.
In `@frontend/src/app/sponsors/page.tsx`:
- Around line 107-171: When graphQLRequestError is truthy the component still
falls through to render the "No Sponsors Yet" empty state; change the render
flow so that when graphQLRequestError is set you return an explicit error UI
instead of the empty-state. Locate the top-level render logic around isLoading,
sponsors, groupedSponsors and the empty-state SecondaryCard and add an early
return that displays an error card/message (use the existing SecondaryCard or
create an ErrorCard) with a clear "Unable to load sponsors" message and optional
retry/link; keep the existing useEffect toast behavior (addToast) but do not let
graphQLRequestError reach the groupedSponsors empty check.
---
Nitpick comments:
In `@backend/apps/owasp/migrations/0073_sponsor_status_email_entity_fks.py`:
- Around line 12-61: The Sponsor status field lacks a DB index; update the
AddField operation for model_name="sponsor" name="status" to add db_index=True
(or create a new migration that alters the "status" CharField to include
db_index=True) so queries filtering on Sponsor.status are indexed and perform
well as the table grows.
In `@backend/tests/unit/apps/api/rest/v0/sponsor_test.py`:
- Around line 106-121: The test should additionally assert that after ordering
the queryset the default active status filter is applied: after calling
list_sponsors(mock_request, mock_filters, ordering="-name"), add an assertion
that mock_queryset.filter was called with the model's ACTIVE status (e.g.
mock_queryset.filter.assert_called_with(status=mock_sponsor_model.Status.ACTIVE))
to ensure default status filtering is enforced, then keep the existing assert
that the returned result equals mock_filtered_queryset; locate this in the
test_list_sponsors_with_ordering function referencing mock_sponsor_model,
mock_queryset and list_sponsors.
- Around line 191-210: The test test_apply_sponsor_success currently omits
verifying that the created sponsor is initialized with draft status; update the
test to assert the status is set to SponsorStatus.DRAFT by either asserting the
model was constructed/called with status="draft" (use
mock_sponsor_model.assert_called_with(...,
status=mock_sponsor_model.SponsorStatus.DRAFT) or equivalent) or by checking the
mock_sponsor_instance.status == mock_sponsor_model.SponsorStatus.DRAFT after
apply_sponsor returns; reference apply_sponsor, mock_sponsor_model,
mock_sponsor_instance and SponsorStatus.DRAFT when adding the assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 08366a89-7b34-4da4-a855-a391fe8e3c84
⛔ Files ignored due to path filters (3)
frontend/src/types/__generated__/graphql.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/homeQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/sponsorQueries.generated.tsis excluded by!**/__generated__/**
📒 Files selected for processing (16)
backend/apps/api/rest/v0/sponsor.pybackend/apps/owasp/admin/sponsor.pybackend/apps/owasp/api/internal/nodes/sponsor.pybackend/apps/owasp/api/internal/queries/sponsor.pybackend/apps/owasp/migrations/0073_sponsor_status_email_entity_fks.pybackend/apps/owasp/models/sponsor.pybackend/tests/unit/apps/api/rest/v0/sponsor_test.pyfrontend/src/app/sponsors/apply/page.tsxfrontend/src/app/sponsors/layout.tsxfrontend/src/app/sponsors/page.tsxfrontend/src/components/LogoCarousel.tsxfrontend/src/server/queries/homeQueries.tsfrontend/src/server/queries/sponsorQueries.tsfrontend/src/types/home.tsfrontend/src/utils/constants.tsfrontend/src/utils/metadata.ts
✅ Files skipped from review due to trivial changes (6)
- frontend/src/types/home.ts
- backend/apps/owasp/api/internal/nodes/sponsor.py
- frontend/src/utils/metadata.ts
- frontend/src/server/queries/homeQueries.ts
- frontend/src/utils/constants.ts
- frontend/src/app/sponsors/layout.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/apps/owasp/api/internal/queries/sponsor.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
backend/tests/unit/apps/api/rest/v0/sponsor_test.py (2)
107-121: Assert ACTIVE default filtering in the ordered-path test too.Right now, Line 120 only verifies ordering. Add an assertion for the default ACTIVE filter so this test catches regressions in
list_sponsorswhenfilters.status is None.Suggested test-strengthening diff
def test_list_sponsors_with_ordering(self, mock_sponsor_model): @@ result = list_sponsors(mock_request, mock_filters, ordering="-name") mock_sponsor_model.objects.order_by.assert_called_with("-name") + mock_queryset.filter.assert_called_once_with( + status=mock_sponsor_model.SponsorStatus.ACTIVE + ) + mock_filters.filter.assert_called_once_with(mock_filtered_queryset) assert result == mock_filtered_queryset🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/unit/apps/api/rest/v0/sponsor_test.py` around lines 107 - 121, The test test_list_sponsors_with_ordering currently only asserts ordering; also assert that the default ACTIVE status filter is applied when mock_filters.status is None by adding an assertion that the queryset filter was called with status=Status.ACTIVE (i.e. mock_queryset.filter.assert_called_with(status=Status.ACTIVE)); ensure the Status symbol used matches the codebase import used by list_sponsors and add the import in the test if missing.
143-160: Add an explicitannotate(tier_order=...)assertion.Line 160 validates ordering, but the test should also verify that
tier_orderannotation is actually produced before ordering.Suggested assertion addition
list_nest_sponsors(mock_request) @@ mock_sponsor_model.objects.filter.assert_called_once_with( status=mock_sponsor_model.SponsorStatus.ACTIVE ) + annotate_mock = mock_sponsor_model.objects.filter.return_value.annotate + annotate_mock.assert_called_once() + assert "tier_order" in annotate_mock.call_args.kwargs ( mock_sponsor_model.objects.filter.return_value.annotate.return_value.order_by ).assert_called_once_with("tier_order", "name")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/unit/apps/api/rest/v0/sponsor_test.py` around lines 143 - 160, The test currently asserts ordering but not that the queryset was annotated with tier_order first; update test_list_nest_sponsors to also assert that mock_sponsor_model.objects.filter.return_value.annotate was called with a tier_order keyword (e.g. assert_called_once_with(tier_order=...)) before the order_by call — reference the test function test_list_nest_sponsors, the helper list_nest_sponsors, and the mock path mock_sponsor_model.objects.filter.return_value.annotate to add this assertion (use mock.ANY or the specific expression used in list_nest_sponsors if you want to validate the actual annotation).frontend/__tests__/unit/pages/Sponsors.test.tsx (1)
70-77: Consider a local helper foruseQuerymock setup.Repeated
(useQuery as unknown as jest.Mock).mockReturnValue(...)makes updates noisy. A tinysetUseQueryResult(...)helper would simplify maintenance.Also applies to: 80-84, 90-94, 103-107, 115-119, 132-136, 180-195, 236-240, 248-263
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/__tests__/unit/pages/Sponsors.test.tsx` around lines 70 - 77, Create a small local helper (e.g., setUseQueryResult) in the test file that wraps the repeated cast and mockReturnValue call for useQuery; implement it to accept the result object and call (useQuery as unknown as jest.Mock).mockReturnValue(result), then replace every repeated inline mockReturnValue usage (the several occurrences in the test file) with calls to setUseQueryResult({ data: ..., error: ..., loading: ... }) to simplify updates and remove the noisy repeated cast.frontend/__tests__/unit/pages/SponsorApply.test.tsx (1)
38-41: Match the test assertion to the test name.The test says it validates a
/sponsorsdestination, but it currently checks only link text. Asserthrefexplicitly to catch route regressions.Diff suggestion
test('renders cancel link pointing to /sponsors', () => { render(<SponsorApplyPage />) - expect(screen.getByText('Cancel')).toBeInTheDocument() + const cancelLink = screen.getByRole('link', { name: 'Cancel' }) + expect(cancelLink).toBeInTheDocument() + expect(cancelLink).toHaveAttribute('href', '/sponsors') })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/__tests__/unit/pages/SponsorApply.test.tsx` around lines 38 - 41, The test in SponsorApply.test.tsx currently only checks the presence of the "Cancel" text but the test name promises a link to "/sponsors"; update the assertion to verify the link target. After rendering SponsorApplyPage (render(<SponsorApplyPage />)), locate the cancel link (e.g., const link = screen.getByRole('link', { name: /Cancel/i }) or screen.getByText('Cancel').closest('a')) and assert its href explicitly (e.g., expect(link).toHaveAttribute('href', '/sponsors')) so route regressions are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/tests/unit/apps/owasp/api/internal/queries/sponsor_test.py`:
- Around line 19-21: The test currently patches Sponsor.objects.filter and only
verifies the output order from SponsorQuery.sponsors, but doesn't assert that
the resolver applied the active-status filter; update the test to capture the
mock used for Sponsor.objects.filter (or replace the patch with a mock) and add
an assertion that it was called with status=Sponsor.SponsorStatus.ACTIVE (e.g.,
using assert_called_with or inspecting call_args), referencing
Sponsor.objects.filter and SponsorQuery.sponsors to ensure the resolver filters
by Sponsor.SponsorStatus.ACTIVE.
---
Nitpick comments:
In `@backend/tests/unit/apps/api/rest/v0/sponsor_test.py`:
- Around line 107-121: The test test_list_sponsors_with_ordering currently only
asserts ordering; also assert that the default ACTIVE status filter is applied
when mock_filters.status is None by adding an assertion that the queryset filter
was called with status=Status.ACTIVE (i.e.
mock_queryset.filter.assert_called_with(status=Status.ACTIVE)); ensure the
Status symbol used matches the codebase import used by list_sponsors and add the
import in the test if missing.
- Around line 143-160: The test currently asserts ordering but not that the
queryset was annotated with tier_order first; update test_list_nest_sponsors to
also assert that mock_sponsor_model.objects.filter.return_value.annotate was
called with a tier_order keyword (e.g. assert_called_once_with(tier_order=...))
before the order_by call — reference the test function test_list_nest_sponsors,
the helper list_nest_sponsors, and the mock path
mock_sponsor_model.objects.filter.return_value.annotate to add this assertion
(use mock.ANY or the specific expression used in list_nest_sponsors if you want
to validate the actual annotation).
In `@frontend/__tests__/unit/pages/SponsorApply.test.tsx`:
- Around line 38-41: The test in SponsorApply.test.tsx currently only checks the
presence of the "Cancel" text but the test name promises a link to "/sponsors";
update the assertion to verify the link target. After rendering SponsorApplyPage
(render(<SponsorApplyPage />)), locate the cancel link (e.g., const link =
screen.getByRole('link', { name: /Cancel/i }) or
screen.getByText('Cancel').closest('a')) and assert its href explicitly (e.g.,
expect(link).toHaveAttribute('href', '/sponsors')) so route regressions are
caught.
In `@frontend/__tests__/unit/pages/Sponsors.test.tsx`:
- Around line 70-77: Create a small local helper (e.g., setUseQueryResult) in
the test file that wraps the repeated cast and mockReturnValue call for
useQuery; implement it to accept the result object and call (useQuery as unknown
as jest.Mock).mockReturnValue(result), then replace every repeated inline
mockReturnValue usage (the several occurrences in the test file) with calls to
setUseQueryResult({ data: ..., error: ..., loading: ... }) to simplify updates
and remove the noisy repeated cast.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: da0058db-55bc-4898-bde2-cf01ed3cfee8
📒 Files selected for processing (8)
backend/apps/api/rest/v0/sponsor.pybackend/tests/unit/apps/api/rest/v0/sponsor_test.pybackend/tests/unit/apps/owasp/api/internal/queries/sponsor_test.pyfrontend/__tests__/unit/pages/SponsorApply.test.tsxfrontend/__tests__/unit/pages/Sponsors.test.tsxfrontend/__tests__/unit/utils/deadlineUtils.test.tsfrontend/src/app/sponsors/apply/page.tsxfrontend/src/app/sponsors/page.tsx
✅ Files skipped from review due to trivial changes (1)
- frontend/tests/unit/utils/deadlineUtils.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/app/sponsors/apply/page.tsx
- backend/apps/api/rest/v0/sponsor.py
backend/tests/unit/apps/owasp/api/internal/queries/sponsor_test.py
Outdated
Show resolved
Hide resolved
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/app/sponsors/apply/page.tsx">
<violation number="1" location="frontend/src/app/sponsors/apply/page.tsx:73">
P1: Using `new URL('/api/v0/sponsors/apply', API_URL)` can throw at runtime for relative `API_URL` values (e.g., `/`) and can drop configured base path prefixes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
mrkeshav-05
force-pushed
the
feat/add-sponsors-program
branch
from
bc17477 to
a71c02d
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/__tests__/unit/pages/SponsorApply.test.tsx (1)
16-20: MakesubmitFormfail fast when form lookup breaks.The hard cast of
closest('form')hides nullability; if markup changes, failure becomes less clear. Add an explicit guard for maintainability.Optional helper hardening
const submitForm = () => { - fireEvent.submit( - screen.getByRole('button', { name: /submit application/i }).closest('form') as HTMLFormElement - ) + const form = screen + .getByRole('button', { name: /submit application/i }) + .closest('form') + if (!form) { + throw new Error('Submit button is not inside a form.') + } + fireEvent.submit(form) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/__tests__/unit/pages/SponsorApply.test.tsx` around lines 16 - 20, The current submitForm helper casts closest('form') to HTMLFormElement which hides nullability; update submitForm to explicitly retrieve the button via screen.getByRole('button', { name: /submit application/i }), call .closest('form') and if it returns null throw an informative error (e.g., "submit button has no enclosing form") so tests fail fast and make the failure obvious; keep references to submitForm, getByRole and .closest('form') to locate where to add the null check and error throw.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/__tests__/unit/pages/SponsorApply.test.tsx`:
- Around line 39-47: The tests for SponsorApplyPage currently only assert link
text presence but not destinations; update the two tests (the 'renders back to
sponsors link' and 'renders cancel link pointing to /sponsors' cases) to assert
the anchor hrefs too by locating the link (e.g., getByRole('link', { name: /Back
to Sponsors/i }) or getByText('Back to Sponsors').closest('a')) and adding an
expectation that the link's href equals '/sponsors' (or the full expected path)
so the route is verified, similarly for the 'Cancel' link.
---
Nitpick comments:
In `@frontend/__tests__/unit/pages/SponsorApply.test.tsx`:
- Around line 16-20: The current submitForm helper casts closest('form') to
HTMLFormElement which hides nullability; update submitForm to explicitly
retrieve the button via screen.getByRole('button', { name: /submit application/i
}), call .closest('form') and if it returns null throw an informative error
(e.g., "submit button has no enclosing form") so tests fail fast and make the
failure obvious; keep references to submitForm, getByRole and .closest('form')
to locate where to add the null check and error throw.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a78dc7d4-08b9-47e6-b241-db69e4692c99
📒 Files selected for processing (3)
backend/tests/unit/apps/owasp/api/internal/queries/sponsor_test.pyfrontend/__tests__/unit/pages/SponsorApply.test.tsxfrontend/src/app/sponsors/apply/page.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/app/sponsors/apply/page.tsx
- backend/tests/unit/apps/owasp/api/internal/queries/sponsor_test.py
mrkeshav-05
force-pushed
the
feat/add-sponsors-program
branch
from
2bbb775 to
b145f66
Compare
|
There was a problem hiding this comment.
Pull request overview
Implements initial Sponsors Program support across backend and frontend, including a public /sponsors page, an application flow, and API/query updates to support sponsor status and metadata.
Changes:
- Added new Sponsors and Sponsor Application pages in the frontend, plus footer/SEO updates.
- Extended the backend Sponsor model with
status,contact_email, and optionalchapter/projectassociations (with migration + admin updates). - Added/updated GraphQL + REST API surfaces and tests to expose sponsor
description, filter active sponsors, and accept sponsor applications.
Reviewed changes
Copilot reviewed 19 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/utils/metadata.ts | Adds SEO metadata config for the new sponsors route. |
| frontend/src/utils/constants.ts | Updates footer link to point to /sponsors. |
| frontend/src/types/home.ts | Extends Sponsor type with description. |
| frontend/src/types/generated/sponsorQueries.generated.ts | New generated types/document for sponsors page query. |
| frontend/src/types/generated/homeQueries.generated.ts | Updates main page query types to include sponsor description. |
| frontend/src/types/generated/graphql.ts | Updates SponsorNode type to include description. |
| frontend/src/server/queries/sponsorQueries.ts | Adds GraphQL query used for sponsors page/codegen. |
| frontend/src/server/queries/homeQueries.ts | Adds description to sponsors selection set. |
| frontend/src/app/sponsors/page.tsx | New sponsors listing UI with tier grouping and error/empty states. |
| frontend/src/app/sponsors/layout.tsx | Adds static metadata for /sponsors. |
| frontend/src/app/sponsors/apply/page.tsx | New sponsor application form posting to backend endpoint. |
| frontend/tests/unit/utils/deadlineUtils.test.ts | Refactors date-only comparison test setup. |
| frontend/tests/unit/pages/Sponsors.test.tsx | Adds unit tests for sponsors page behavior. |
| frontend/tests/unit/pages/SponsorApply.test.tsx | Adds unit tests for sponsor application form submission/validation. |
| backend/tests/unit/apps/owasp/api/internal/queries/sponsor_test.py | Updates GraphQL sponsor query test to assert ACTIVE filtering. |
| backend/tests/unit/apps/api/rest/v0/sponsor_test.py | Adds/updates REST tests for status filtering, nest sponsors, and apply endpoint. |
| backend/apps/owasp/models/sponsor.py | Adds SponsorStatus, status, contact_email, and chapter/project FKs. |
| backend/apps/owasp/migrations/0073_sponsor_status_email_entity_fks.py | Migration introducing new Sponsor fields and associations. |
| backend/apps/owasp/api/internal/queries/sponsor.py | Filters GraphQL sponsors to status=ACTIVE. |
| backend/apps/owasp/api/internal/nodes/sponsor.py | Exposes description on SponsorNode. |
| backend/apps/owasp/admin/sponsor.py | Adds admin support for status/contact/entity association fields. |
| backend/apps/api/rest/v0/sponsor.py | Adds status filtering default, /nest endpoint, and /apply endpoint. |
Files not reviewed (3)
- frontend/src/types/generated/graphql.ts: Language not supported
- frontend/src/types/generated/homeQueries.generated.ts: Language not supported
- frontend/src/types/generated/sponsorQueries.generated.ts: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+144
to
+158
| @router.post( | ||
| "/apply", | ||
| description="Submit a sponsor application. Creates a new sponsor record with draft status.", | ||
| operation_id="apply_sponsor", | ||
| response={ | ||
| HTTPStatus.BAD_REQUEST: ValidationErrorSchema, | ||
| HTTPStatus.CREATED: SponsorApplicationResponse, | ||
| }, | ||
| summary="Apply to become a sponsor", | ||
| ) | ||
| def apply_sponsor( | ||
| request: HttpRequest, | ||
| payload: SponsorApplication, | ||
| ) -> Response: | ||
| """Submit a sponsor application.""" |
There was a problem hiding this comment.
The /api/v0 Ninja API is configured with auth=ApiKey() (see backend/apps/api/rest/v0/__init__.py:41-48), so this POST endpoint will return 401 in production unless you explicitly override auth for this operation/router. Since the frontend sponsor application form calls this endpoint without an API key, consider setting auth=None for this route (and adding appropriate anti-abuse controls like throttling/CAPTCHA) or proxying the request through a server-side endpoint that can attach credentials safely.
Comment on lines
+116
to
+122
| @router.get( | ||
| "/nest", | ||
| description="Retrieve active OWASP Nest sponsors for external integrations.", | ||
| operation_id="list_nest_sponsors", | ||
| response=list[Sponsor], | ||
| summary="List Nest sponsors", | ||
| ) |
There was a problem hiding this comment.
The issue/requirements specify a REST endpoint like /api/v0/projects/nest/sponsors, but this PR introduces /api/v0/sponsors/nest. If external integrations are expected to follow the documented project-scoped route, consider adding the endpoint under the Projects router (or adding an alias/redirect) to avoid breaking the intended API contract.
| output_field=IntegerField(), | ||
| ) | ||
| return list( | ||
| SponsorModel.objects.filter(status=SponsorModel.SponsorStatus.ACTIVE) |
There was a problem hiding this comment.
list_nest_sponsors currently filters only by status=ACTIVE and will include rows where sponsor_type is NOT_SPONSOR (a defined non-sponsor value). For an endpoint explicitly named "Nest sponsors", it should likely exclude SponsorType.NOT_SPONSOR (and potentially use the new project/chapter associations) so consumers don’t get non-sponsor records.
Suggested change
| SponsorModel.objects.filter(status=SponsorModel.SponsorStatus.ACTIVE) | |
| SponsorModel.objects.filter(status=SponsorModel.SponsorStatus.ACTIVE) | |
| .exclude(sponsor_type=SponsorModel.SponsorType.NOT_SPONSOR) |
Comment on lines
+67
to
+81
| try { | ||
| const payload: Record<string, string> = {} | ||
| payload['contact_email'] = formData.contactEmail | ||
| payload['message'] = formData.message | ||
| payload['organization_name'] = formData.organizationName | ||
| payload['website'] = formData.website | ||
| const baseUrl = (API_URL ?? '').replace(/\/$/, '') | ||
| const response = await fetch(`${baseUrl}/api/v0/sponsors/apply`, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| credentials: 'include', | ||
| body: JSON.stringify(payload), | ||
| }) |
There was a problem hiding this comment.
This client-side form posts to /api/v0/sponsors/apply without any authentication header. In production the v0 REST API is API-key protected, so this submission will likely fail (401). Consider routing this through a Next.js server endpoint (so a server-side API key can be attached) or switching to an internal/public backend endpoint intended for browser submissions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
This PR implement the feature of showing Sponsors Program on
/sponsorspage in OWASP Nest.Resolves #4259
Screen.Recording.2026-04-12.at.11.59.49.PM.mov
Checklist
make check-testlocally: all warnings addressed, tests passed