feat: add sponsors program support#4342
Open
Utkarsh-0304 wants to merge 11 commits intoOWASP:mainfrom
Open
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:
WalkthroughAdds sponsor support across backend and frontend: model and migration additions, GraphQL nodes/queries/mutation and schema wiring, a REST endpoint exposing active sponsors, admin fieldset update, frontend sponsor list and application pages/components, GraphQL documents, and local Docker Compose volume renames. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 19 files
Confidence score: 2/5
- There are multiple high-confidence, user-impacting issues (sev 7–8) across infrastructure and API behavior, so merge risk is currently high.
- Most severe:
docker-compose/local/compose.yamlhas suffixed service volume names (_4259) without matching top-level volume declarations, which can trigger undefined volume reference errors and break local Docker Compose startup. - In
backend/apps/owasp/api/internal/mutations/sponsor.py, slug-basedkeygeneration can collide against aunique=Truefield and lead to unhandled integrity failures during sponsor creation;backend/apps/api/rest/v0/project.pyalso returns globally active sponsors instead of Nest-scoped results, causing incorrect API output. - Pay close attention to
docker-compose/local/compose.yaml,backend/apps/owasp/api/internal/mutations/sponsor.py,backend/apps/api/rest/v0/project.py- fix volume declaration mismatches, enforce unique key handling, and scope/nest/sponsorsto the Nest project.
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="backend/apps/owasp/api/internal/mutations/sponsor.py">
<violation number="1" location="backend/apps/owasp/api/internal/mutations/sponsor.py:22">
P1: Potential key collision from slugified sponsor name can cause unhandled DB integrity failures. The `key` field in `Sponsor` model is defined as `unique=True`. When creating a sponsor via `create_sponsor` mutation, if two sponsors share names that slugify to the same value (e.g., "ACME Corp" and "acme corp" both become "acme-corp"), an unhandled `IntegrityError` will be raised, causing a server error instead of a controlled GraphQL validation response. Consider adding collision handling logic similar to the existing `update_data` pattern in the Sponsor model.</violation>
</file>
<file name="backend/apps/api/rest/v0/project.py">
<violation number="1" location="backend/apps/api/rest/v0/project.py:164">
P1: The `/nest/sponsors` endpoint filters only by active status without scoping to the Nest project. The description claims to return "sponsors associated with project nest" but returns all active sponsors globally.</violation>
</file>
<file name="docker-compose/local/compose.yaml">
<violation number="1" location="docker-compose/local/compose.yaml:34">
P1: Service volume names were suffixed with `_4259` but top-level volume declarations were not updated, causing undefined volume reference errors in Docker Compose</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (9)
backend/apps/owasp/api/internal/queries/sponsor.py (1)
14-42: Extract duplicated sorting logic to reduce maintenance burden.The sponsor type priority dictionary is duplicated between
sponsorsandactive_sponsorsresolvers. Consider extracting it to a module-level constant or helper function.♻️ Proposed refactor
+SPONSOR_TYPE_PRIORITY = { + Sponsor.SponsorType.DIAMOND: 1, + Sponsor.SponsorType.PLATINUM: 2, + Sponsor.SponsorType.GOLD: 3, + Sponsor.SponsorType.SILVER: 4, + Sponsor.SponsorType.SUPPORTER: 5, + Sponsor.SponsorType.NOT_SPONSOR: 6, +} + + `@strawberry.type` class SponsorQuery: """Sponsor queries.""" `@strawberry_django.field` def sponsors(self) -> list[SponsorNode]: """Resolve sponsors.""" return sorted( Sponsor.objects.all(), - key=lambda x: { - Sponsor.SponsorType.DIAMOND: 1, - Sponsor.SponsorType.PLATINUM: 2, - Sponsor.SponsorType.GOLD: 3, - Sponsor.SponsorType.SILVER: 4, - Sponsor.SponsorType.SUPPORTER: 5, - Sponsor.SponsorType.NOT_SPONSOR: 6, - }[x.sponsor_type], + key=lambda x: SPONSOR_TYPE_PRIORITY[x.sponsor_type], ) `@strawberry_django.field` def active_sponsors(self) -> list[SponsorNode]: """Resolve active sponsors ordered by Sponsor level.""" return sorted( Sponsor.objects.filter(status=Sponsor.StatusType.ACTIVE), - key=lambda x: { - Sponsor.SponsorType.DIAMOND: 1, - Sponsor.SponsorType.PLATINUM: 2, - Sponsor.SponsorType.GOLD: 3, - Sponsor.SponsorType.SILVER: 4, - Sponsor.SponsorType.SUPPORTER: 5, - Sponsor.SponsorType.NOT_SPONSOR: 6, - }[x.sponsor_type], + key=lambda x: SPONSOR_TYPE_PRIORITY[x.sponsor_type], )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/owasp/api/internal/queries/sponsor.py` around lines 14 - 42, The duplicated sponsor-type priority mapping used in the sponsors and active_sponsors resolvers should be extracted to a single reusable symbol: create a module-level constant (e.g., SPONSOR_PRIORITY) or a small helper function (e.g., get_sponsor_priority) that maps Sponsor.SponsorType values to priority integers, then update sponsors() and active_sponsors() to use that constant/helper in their key functions (replace the inline dict and lambda logic with key=lambda x: SPONSOR_PRIORITY[x.sponsor_type] or key=get_sponsor_priority), ensuring both resolvers share the same source of truth.backend/apps/owasp/admin/sponsor.py (1)
13-29: Consider addingstatustolist_displayandlist_filter.Since
status(Draft/Active/Archived) is a key field for sponsor workflow management, adding it to bothlist_displayandlist_filterwould improve the admin experience for filtering and quickly identifying sponsors by their approval state.♻️ Proposed enhancement
list_display = ( "name", "sort_name", "sponsor_type", "is_member", "member_type", + "status", ) search_fields = ( "name", "sort_name", "description", ) list_filter = ( "sponsor_type", "is_member", "member_type", + "status", )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/owasp/admin/sponsor.py` around lines 13 - 29, Add the "status" field to the admin configuration so admins can view and filter by Draft/Active/Archived; update the ModelAdmin's list_display tuple to include "status" (alongside "name", "sort_name", etc.) and add "status" to the list_filter tuple so it appears in filters — locate and edit the admin class that defines list_display and list_filter (e.g., SponsorAdmin) and insert "status" into both tuples.frontend/src/app/sponsors/apply/page.tsx (1)
1-12: Sponsor application is publicly accessible without authentication—verify if intentional.The sponsor mutation and frontend page lack authentication checks (no
useSession(), no permission validation), unlike program creation which requires authentication and project leader privileges. If allowing external organizations to apply is intentional, implement rate limiting or CAPTCHA to prevent spam. If authentication is required, add a session check consistent with other mutation pages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/sponsors/apply/page.tsx` around lines 1 - 12, The sponsor application page CreateProgramPage exposes the CreateSponsor mutation via createSponsor and SponsorForm without any authentication or anti-abuse controls; either enforce the same session/permission checks used elsewhere (e.g., call useSession()/getServerSession and verify project leader role before rendering or invoking createSponsor) or, if public submissions are intended, add anti-abuse measures (rate limiting or CAPTCHA integration around SponsorForm submissions and server-side throttling) so anonymous requests cannot spam the mutation; update the rendering logic and mutation handler to gate calls accordingly and reuse existing auth helpers used by other pages to keep behavior consistent.backend/apps/owasp/api/internal/mutations/sponsor.py (2)
19-19: Remove blank line after docstring.Per static analysis (D202), remove the blank line after the function docstring.
✨ Proposed fix
`@strawberry.mutation` def create_sponsor(self, input_data: CreateSponsorInput) -> SponsorNode: """Create a new sponsor.""" - sponsor = Sponsor.objects.create(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/owasp/api/internal/mutations/sponsor.py` at line 19, The function docstring """Create a new sponsor.""" in the sponsor module has a blank line immediately after it; remove that empty line so the docstring is followed directly by the function body (i.e., edit the create_sponsor function in sponsor.py to eliminate the blank line after the docstring to satisfy D202).
31-31: Consider including the sponsor key or name in the log message.The current log message lacks context to identify which sponsor was created. Including the name would aid debugging and auditing.
✨ Proposed fix
- logger.info("Sponsor created successfully") + logger.info("Sponsor '%s' created successfully", sponsor.name)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/owasp/api/internal/mutations/sponsor.py` at line 31, The log call logger.info("Sponsor created successfully") lacks context—update the logging in the sponsor creation flow (the logger.info call in sponsor.py inside the sponsor creation function) to include an identifier from the created sponsor (e.g., sponsor.id, sponsor.key or sponsor.name/created_sponsor.name) so the message becomes descriptive (e.g., "Sponsor created successfully: <identifier>"). Ensure you reference the same variable name used in the function that holds the created sponsor object.frontend/src/app/sponsors/page.tsx (2)
79-85: Consider usingLinkinstead ofbuttonwithrouter.pushfor navigation.Using a
Linkcomponent for navigation to/sponsors/applyprovides better semantics, accessibility (proper anchor behavior), and enables prefetching.♻️ Proposed fix
+import Link from 'next/link' ... - <button - type="button" - className="inline-flex items-center justify-center rounded-xl bg-blue-600 px-6 py-3 text-sm font-semibold text-white transition-colors hover:bg-blue-700 focus:ring-2 focus:ring-blue-500 focus:ring-offset-2 focus:outline-none dark:focus:ring-offset-gray-800" - onClick={() => router.push('/sponsors/apply')} - > + <Link + href="/sponsors/apply" + className="inline-flex items-center justify-center rounded-xl bg-blue-600 px-6 py-3 text-sm font-semibold text-white transition-colors hover:bg-blue-700 focus:ring-2 focus:ring-blue-500 focus:ring-offset-2 focus:outline-none dark:focus:ring-offset-gray-800" + > Apply here for Sponsorship - </button> + </Link>This also allows removing
useRouterif not used elsewhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/sponsors/page.tsx` around lines 79 - 85, Replace the navigation button that calls router.push('/sponsors/apply') with a Next.js Link to '/sponsors/apply' to improve semantics, accessibility, and enable prefetching; update the JSX by swapping the <button> using onClick={...router.push...} for a <Link> that preserves the same className and inner text ("Apply here for Sponsorship"), and remove the useRouter import/usage from this page component if it is no longer referenced.
11-17: Consider using generated types instead of a localSponsortype.The local
Sponsortype may drift from the GraphQL schema. Using the generated types fromGetSponsorsDataDocumentensures type safety and consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/sponsors/page.tsx` around lines 11 - 17, Replace the local Sponsor type with the generated GraphQL type to avoid schema drift: remove the local "type Sponsor" and import the appropriate generated type that corresponds to GetSponsorsDataDocument (e.g., GetSponsorsData or GetSponsorsDataQuery) from your GraphQL codegen output, then use that imported type wherever "Sponsor" was referenced (for example in props, state, or fetch result handling) so the component relies on the generated schema types instead of a manually defined one.frontend/src/components/SponsorForm.tsx (1)
173-186: Minor inconsistency:FormTextareausesonChangewhileFormTextInputusesonValueChange.This is a minor API inconsistency between the two form components. Consider aligning them if the underlying components support it, or document this difference.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/SponsorForm.tsx` around lines 173 - 186, The FormTextarea usage is inconsistent with FormTextInput's API (Textarea uses onChange while TextInput uses onValueChange); update the SponsorForm.tsx to use the same prop name across both components — either switch the FormTextarea call to onValueChange (calling handleInputChange('message', value) and setTouched(...) with the new value parameter) or refactor FormTextInput to also accept onChange so both components share the same handler name; locate FormTextarea and FormTextInput usages and adjust the prop name and corresponding handler (handleInputChange, setTouched, errors, touched) so the API is consistent project-wide.backend/apps/owasp/migrations/0074_sponsor_chapter_sponsor_project_sponsor_status.py (1)
14-23: Consider addingblank=Trueto ForeignKey fields for admin form compatibility.The
chapterandprojectForeignKey fields arenull=Truebut notblank=True. In Django Admin, this means the fields will appear required even though they can be null in the database. If these fields are intended to be optional in forms, addblank=True.✨ Proposed model adjustment
-field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='owasp.chapter'), +field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='owasp.chapter'),Same for the
projectfield.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/owasp/migrations/0074_sponsor_chapter_sponsor_project_sponsor_status.py` around lines 14 - 23, The ForeignKey additions for model_name='sponsor' (fields name='chapter' and name='project' added via migrations.AddField) set null=True but not blank=True, causing admin forms to treat them as required; update those migration field definitions to include blank=True on both the chapter and project ForeignKey declarations so forms treat them as optional (i.e., add blank=True alongside null=True and on_delete=... in the migrations.AddField entries for 'chapter' and 'project').
🤖 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/project.py`:
- Around line 160-164: The REST endpoint list_nest_sponsors currently returns
active sponsors unsorted; update it to apply the same sponsor-type ordering used
by the GraphQL active_sponsors resolver (Diamond → Platinum → Gold → Silver →
Supporter → Not Sponsor) so results are consistent. Modify the SponsorModel
query in list_nest_sponsors to order by sponsor type using the same priority
mapping (either replicate the Case/When ordering here or reference the shared
priority constant if one exists), e.g., convert the filter-only return to a
queryset with .order_by(...) that uses the priority mapping for
SponsorModel.StatusType values. Ensure the ordering logic references
SponsorModel and SponsorModel.StatusType so it stays tied to the model enum.
In `@backend/apps/owasp/api/internal/mutations/sponsor.py`:
- Around line 21-29: Sponsor creation can raise IntegrityError because
key=slugify(input_data.name) is unique; change the creation logic around
Sponsor.objects.create to ensure a unique key before saving (or catch
IntegrityError and retry). Build a candidate_key from slugify(input_data.name)
and if Sponsor.objects.filter(key=candidate_key).exists() (or on
IntegrityError), append a deterministic suffix (e.g., incrementing counter or
short uuid) and retry until unique, then call Sponsor.objects.create(...,
key=candidate_key, ...); reference Sponsor.objects.create and
slugify(input_data.name) when making the change.
In `@backend/apps/owasp/models/sponsor.py`:
- Around line 76-81: The ForeignKey fields chapter and project on the Sponsor
model (status = ..., then chapter = models.ForeignKey("owasp.Chapter", ...) and
project = models.ForeignKey("owasp.Project", ...)) are nullable in the DB but
missing blank=True, causing ModelForm/Admin validation failures; update both
field definitions to include blank=True (e.g., chapter = models.ForeignKey(...,
null=True, blank=True) and project = models.ForeignKey(..., null=True,
blank=True)) and create/update the corresponding migration (0074) to reflect
this change so forms properly accept empty values.
In `@docker-compose/local/compose.yaml`:
- Line 34: Update the cleanup targets that remove Docker volumes to match the
renamed volumes in compose.yaml (e.g., backend-venv_4259 and the other volumes
suffixed with _4259 referenced in the compose file) so the Makefile targets
actually remove the new names; edit the cleanup targets in the project Makefiles
that currently reference the old volume names and replace them with the
corresponding _4259 names (or change the targets to use docker-compose down
--volumes / docker compose ... to drive cleanup from the compose file).
- Line 34: Service definitions reference suffixed named volumes like
backend-venv_4259, but the top-level volumes section still lists unsuffixed
names causing Compose to fail; update the top-level volumes declarations to
include the matching suffixed names (e.g., add entries for backend-venv_4259,
postgres_data_4259, redis_data_4259, etc.) or change the service mount names to
the unsuffixed identifiers so they match; ensure every volume referenced in
service mounts (lines showing backend-venv_4259 and the other indicated volumes)
has a corresponding declaration in the top-level volumes block (or vice‑versa)
so no referenced volume is undefined.
In `@frontend/src/app/sponsors/apply/page.tsx`:
- Line 10: The component is misnamed CreateProgramPage; rename the React
component function to SponsorApplicationPage (or SponsorApplyPage consistently)
and update the default export at the end of the file to export that new name so
the file reflects the sponsor application page; update any internal references
within this file (e.g., the component declaration const CreateProgramPage = ()
=> { and the export default CreateProgramPage) to use SponsorApplicationPage
instead.
In `@frontend/src/app/sponsors/page.tsx`:
- Around line 67-71: The component currently destructures only data and loading
from useQuery(GetSponsorsDataDocument) and returns an empty <div> while loading;
update the query usage to also destructure error (e.g., const { data:
sponsorsData, loading, error } = useQuery(GetSponsorsDataDocument)), render a
meaningful loading state (spinner/skeleton or accessible "Loading..." element)
when loading is true, and render an error UI when error is present (friendly
message and optional retry action) instead of silently showing nothing; keep
references to useQuery, GetSponsorsDataDocument, sponsorsData, loading, and
error so reviewers can find the changes.
---
Nitpick comments:
In `@backend/apps/owasp/admin/sponsor.py`:
- Around line 13-29: Add the "status" field to the admin configuration so admins
can view and filter by Draft/Active/Archived; update the ModelAdmin's
list_display tuple to include "status" (alongside "name", "sort_name", etc.) and
add "status" to the list_filter tuple so it appears in filters — locate and edit
the admin class that defines list_display and list_filter (e.g., SponsorAdmin)
and insert "status" into both tuples.
In `@backend/apps/owasp/api/internal/mutations/sponsor.py`:
- Line 19: The function docstring """Create a new sponsor.""" in the sponsor
module has a blank line immediately after it; remove that empty line so the
docstring is followed directly by the function body (i.e., edit the
create_sponsor function in sponsor.py to eliminate the blank line after the
docstring to satisfy D202).
- Line 31: The log call logger.info("Sponsor created successfully") lacks
context—update the logging in the sponsor creation flow (the logger.info call in
sponsor.py inside the sponsor creation function) to include an identifier from
the created sponsor (e.g., sponsor.id, sponsor.key or
sponsor.name/created_sponsor.name) so the message becomes descriptive (e.g.,
"Sponsor created successfully: <identifier>"). Ensure you reference the same
variable name used in the function that holds the created sponsor object.
In `@backend/apps/owasp/api/internal/queries/sponsor.py`:
- Around line 14-42: The duplicated sponsor-type priority mapping used in the
sponsors and active_sponsors resolvers should be extracted to a single reusable
symbol: create a module-level constant (e.g., SPONSOR_PRIORITY) or a small
helper function (e.g., get_sponsor_priority) that maps Sponsor.SponsorType
values to priority integers, then update sponsors() and active_sponsors() to use
that constant/helper in their key functions (replace the inline dict and lambda
logic with key=lambda x: SPONSOR_PRIORITY[x.sponsor_type] or
key=get_sponsor_priority), ensuring both resolvers share the same source of
truth.
In
`@backend/apps/owasp/migrations/0074_sponsor_chapter_sponsor_project_sponsor_status.py`:
- Around line 14-23: The ForeignKey additions for model_name='sponsor' (fields
name='chapter' and name='project' added via migrations.AddField) set null=True
but not blank=True, causing admin forms to treat them as required; update those
migration field definitions to include blank=True on both the chapter and
project ForeignKey declarations so forms treat them as optional (i.e., add
blank=True alongside null=True and on_delete=... in the migrations.AddField
entries for 'chapter' and 'project').
In `@frontend/src/app/sponsors/apply/page.tsx`:
- Around line 1-12: The sponsor application page CreateProgramPage exposes the
CreateSponsor mutation via createSponsor and SponsorForm without any
authentication or anti-abuse controls; either enforce the same
session/permission checks used elsewhere (e.g., call
useSession()/getServerSession and verify project leader role before rendering or
invoking createSponsor) or, if public submissions are intended, add anti-abuse
measures (rate limiting or CAPTCHA integration around SponsorForm submissions
and server-side throttling) so anonymous requests cannot spam the mutation;
update the rendering logic and mutation handler to gate calls accordingly and
reuse existing auth helpers used by other pages to keep behavior consistent.
In `@frontend/src/app/sponsors/page.tsx`:
- Around line 79-85: Replace the navigation button that calls
router.push('/sponsors/apply') with a Next.js Link to '/sponsors/apply' to
improve semantics, accessibility, and enable prefetching; update the JSX by
swapping the <button> using onClick={...router.push...} for a <Link> that
preserves the same className and inner text ("Apply here for Sponsorship"), and
remove the useRouter import/usage from this page component if it is no longer
referenced.
- Around line 11-17: Replace the local Sponsor type with the generated GraphQL
type to avoid schema drift: remove the local "type Sponsor" and import the
appropriate generated type that corresponds to GetSponsorsDataDocument (e.g.,
GetSponsorsData or GetSponsorsDataQuery) from your GraphQL codegen output, then
use that imported type wherever "Sponsor" was referenced (for example in props,
state, or fetch result handling) so the component relies on the generated schema
types instead of a manually defined one.
In `@frontend/src/components/SponsorForm.tsx`:
- Around line 173-186: The FormTextarea usage is inconsistent with
FormTextInput's API (Textarea uses onChange while TextInput uses onValueChange);
update the SponsorForm.tsx to use the same prop name across both components —
either switch the FormTextarea call to onValueChange (calling
handleInputChange('message', value) and setTouched(...) with the new value
parameter) or refactor FormTextInput to also accept onChange so both components
share the same handler name; locate FormTextarea and FormTextInput usages and
adjust the prop name and corresponding handler (handleInputChange, setTouched,
errors, touched) so the API is consistent project-wide.
🪄 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: a2817fd1-b688-494f-b95f-9e7c9d6a3be5
⛔ Files ignored due to path filters (3)
frontend/src/types/__generated__/graphql.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/sponsorMutations.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/project.pybackend/apps/owasp/admin/sponsor.pybackend/apps/owasp/api/internal/mutations/sponsor.pybackend/apps/owasp/api/internal/nodes/sponsor.pybackend/apps/owasp/api/internal/queries/sponsor.pybackend/apps/owasp/migrations/0073_sponsor_contact_email_sponsor_created_at.pybackend/apps/owasp/migrations/0074_sponsor_chapter_sponsor_project_sponsor_status.pybackend/apps/owasp/migrations/0075_sponsor_message.pybackend/apps/owasp/models/sponsor.pybackend/settings/graphql.pydocker-compose/local/compose.yamlfrontend/src/app/sponsors/apply/page.tsxfrontend/src/app/sponsors/page.tsxfrontend/src/components/SponsorForm.tsxfrontend/src/server/mutations/sponsorMutations.tsfrontend/src/server/queries/sponsorQueries.ts
Contributor
There was a problem hiding this comment.
1 issue found across 10 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="docker-compose/local/compose.yaml">
<violation number="1" location="docker-compose/local/compose.yaml:150">
P2: Renaming all compose named volumes to ticket-suffixed names (`*_4259`) breaks existing cleanup automation. The `backend/Makefile` cleanup targets reference the old volume names (`nest-local_backend-venv`, `nest-local_cache-data`) which won't match the new suffixed volumes, causing cleanup failures and leaving orphaned volumes on developer/CI machines. Additionally, this change appears unrelated to the PR's stated purpose of adding sponsors program support.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
frontend/src/app/sponsors/page.tsx (3)
93-99: Consider usingLinkinstead ofbuttonwithrouter.pushfor navigation.Using a
Linkcomponent for navigation to/sponsors/applyprovides better accessibility (keyboard navigation, right-click context menu) and allows browsers to prefetch the destination. The button approach works but is less semantically correct for navigation.♻️ Proposed fix using Link
- <button - type="button" - className="inline-flex items-center justify-center rounded-xl bg-blue-600 px-6 py-3 text-sm font-semibold text-white transition-colors hover:bg-blue-700 focus:ring-2 focus:ring-blue-500 focus:ring-offset-2 focus:outline-none dark:focus:ring-offset-gray-800" - onClick={() => router.push('/sponsors/apply')} - > - Apply here for Sponsorship - </button> + <Link + href="/sponsors/apply" + className="inline-flex items-center justify-center rounded-xl bg-blue-600 px-6 py-3 text-sm font-semibold text-white transition-colors hover:bg-blue-700 focus:ring-2 focus:ring-blue-500 focus:ring-offset-2 focus:outline-none dark:focus:ring-offset-gray-800" + > + Apply here for Sponsorship + </Link>This also allows removing
useRouterimport and hook if no longer needed elsewhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/sponsors/page.tsx` around lines 93 - 99, The current navigation uses a button with onClick calling router.push('/sponsors/apply'), which is less semantic and harms accessibility; replace the button with Next.js's Link component pointing to '/sponsors/apply' (preserve the same className and children text "Apply here for Sponsorship") so the element behaves as a proper navigational link and allows prefetch/context menu, and remove the useRouter import and router hook if they are no longer used elsewhere in the component.
101-111: Add spacing between the CTA card and sponsors grid.There's no margin or gap between the
SecondaryCardand the sponsors grid/empty message below it. Consider adding vertical spacing for better visual separation.♻️ Proposed fix to add spacing
</SecondaryCard> - {sponsorsData?.activeSponsors && sponsorsData?.activeSponsors.length > 0 ? ( + <div className="mt-8"> + {sponsorsData?.activeSponsors && sponsorsData.activeSponsors.length > 0 ? ( <div className="grid grid-cols-1 justify-items-center gap-6 sm:grid-cols-2 xl:grid-cols-4"> - {sponsorsData.activeSponsors.map((sponsor: Sponsor) => ( + {sponsorsData.activeSponsors.map((sponsor) => ( <SponsorEntryCard key={sponsor.id} sponsor={sponsor} /> ))} </div> ) : ( <p className="text-center text-lg font-medium">No Sponsors Found</p> )} + </div>Note: Also removed the redundant optional chain on the second
sponsorsData?.activeSponsorssince it's already checked in the condition.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/sponsors/page.tsx` around lines 101 - 111, Add vertical spacing between the SecondaryCard and the sponsors block by applying a top margin or gap to the container that follows it (e.g., wrap the conditional block or the grid div with a class like "mt-6" or similar) so the CTA and the sponsors grid/empty message are visually separated; also simplify the condition by removing the redundant optional chaining on sponsorsData?.activeSponsors in the map (use sponsorsData.activeSponsors.map and keep the initial null/length check as-is). Ensure the change targets the JSX around SecondaryCard, the conditional rendering of sponsorsData.activeSponsors, and the SponsorEntryCard list.
11-17: Consider using generated GraphQL types or marking fields as optional.The local
Sponsortype declares all fields as required strings, but GraphQL responses typically include nullable fields (e.g.,imageUrl,description,url). Using this type for the type assertion at line 105 could mask runtime nulls.Either import and use the generated type from
GetSponsorsDataDocument, or mark potentially nullable fields as optional:♻️ Proposed fix using optional fields
type Sponsor = { id: string name: string - url: string - description: string - imageUrl: string + url?: string | null + description?: string | null + imageUrl?: string | null }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/sponsors/page.tsx` around lines 11 - 17, The local Sponsor type currently marks all fields as non-null strings which can mask nullable GraphQL responses; update the code to either import and use the generated GraphQL type (from the GetSponsorsDataDocument/its generated types) in place of the local Sponsor type where the data is asserted (around the usage at the type assertion near line 105), or change the local Sponsor definition so fields that can be null in GraphQL (e.g., imageUrl, description, url) are optional/nullable (string | null) and then update any downstream code to handle possible null/undefined values accordingly.backend/apps/owasp/models/sponsor.py (1)
76-76: Consider addingverbose_namefor consistency.Most other fields in this model include an explicit
verbose_name. Adding one here would maintain consistency with the model's style.♻️ Suggested change
- status = models.CharField(max_length=20, choices=StatusType.choices, default=StatusType.DRAFT) + status = models.CharField( + verbose_name="Status", + max_length=20, + choices=StatusType.choices, + default=StatusType.DRAFT, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/owasp/models/sponsor.py` at line 76, The Sponsor model's status field (status = models.CharField(..., choices=StatusType.choices, default=StatusType.DRAFT)) lacks an explicit verbose_name which breaks consistency with other fields; update this field to include a concise verbose_name (e.g., verbose_name="status" or "Status") by adding the verbose_name argument to the models.CharField declaration so it matches the model's styling conventions.
🤖 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/owasp/api/internal/mutations/__init__.py`:
- Line 3: The file exporting SponsorMutation is missing a final newline (Ruff
W292); open the file containing the import of SponsorMutation (the line "from
.sponsor import SponsorMutation") and add a single trailing newline character at
EOF so the file ends with a newline to satisfy the linter.
In `@frontend/src/app/sponsors/page.tsx`:
- Around line 52-60: The Link currently uses sponsor.url directly which can be
null/undefined/empty; wrap the Link (and its FaArrowUpRightFromSquare icon) in a
conditional that only renders when sponsor.url is a non-empty string (e.g.,
sponsor.url && sponsor.url.trim() !== '') or after a simple URL validation;
otherwise render a fallback (plain text like "Visit website" without a link or
nothing) so you never produce an invalid anchor by rendering Link with a
missing/empty URL.
---
Nitpick comments:
In `@backend/apps/owasp/models/sponsor.py`:
- Line 76: The Sponsor model's status field (status = models.CharField(...,
choices=StatusType.choices, default=StatusType.DRAFT)) lacks an explicit
verbose_name which breaks consistency with other fields; update this field to
include a concise verbose_name (e.g., verbose_name="status" or "Status") by
adding the verbose_name argument to the models.CharField declaration so it
matches the model's styling conventions.
In `@frontend/src/app/sponsors/page.tsx`:
- Around line 93-99: The current navigation uses a button with onClick calling
router.push('/sponsors/apply'), which is less semantic and harms accessibility;
replace the button with Next.js's Link component pointing to '/sponsors/apply'
(preserve the same className and children text "Apply here for Sponsorship") so
the element behaves as a proper navigational link and allows prefetch/context
menu, and remove the useRouter import and router hook if they are no longer used
elsewhere in the component.
- Around line 101-111: Add vertical spacing between the SecondaryCard and the
sponsors block by applying a top margin or gap to the container that follows it
(e.g., wrap the conditional block or the grid div with a class like "mt-6" or
similar) so the CTA and the sponsors grid/empty message are visually separated;
also simplify the condition by removing the redundant optional chaining on
sponsorsData?.activeSponsors in the map (use sponsorsData.activeSponsors.map and
keep the initial null/length check as-is). Ensure the change targets the JSX
around SecondaryCard, the conditional rendering of sponsorsData.activeSponsors,
and the SponsorEntryCard list.
- Around line 11-17: The local Sponsor type currently marks all fields as
non-null strings which can mask nullable GraphQL responses; update the code to
either import and use the generated GraphQL type (from the
GetSponsorsDataDocument/its generated types) in place of the local Sponsor type
where the data is asserted (around the usage at the type assertion near line
105), or change the local Sponsor definition so fields that can be null in
GraphQL (e.g., imageUrl, description, url) are optional/nullable (string | null)
and then update any downstream code to handle possible null/undefined values
accordingly.
🪄 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: 22a849c4-7e6f-4c32-8f73-4d7bad1c44cc
📒 Files selected for processing (10)
backend/apps/owasp/api/internal/mutations/__init__.pybackend/apps/owasp/api/internal/mutations/sponsor.pybackend/apps/owasp/migrations/0073_sponsor_contact_email_sponsor_created_at.pybackend/apps/owasp/migrations/0074_sponsor_chapter_sponsor_project_sponsor_status.pybackend/apps/owasp/migrations/0075_sponsor_message.pybackend/apps/owasp/models/sponsor.pybackend/settings/graphql.pydocker-compose/local/compose.yamlfrontend/src/app/sponsors/apply/page.tsxfrontend/src/app/sponsors/page.tsx
✅ Files skipped from review due to trivial changes (4)
- backend/apps/owasp/migrations/0075_sponsor_message.py
- backend/apps/owasp/migrations/0073_sponsor_contact_email_sponsor_created_at.py
- backend/apps/owasp/migrations/0074_sponsor_chapter_sponsor_project_sponsor_status.py
- docker-compose/local/compose.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/settings/graphql.py
- frontend/src/app/sponsors/apply/page.tsx
- backend/apps/owasp/api/internal/mutations/sponsor.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
frontend/src/app/sponsors/page.tsx (2)
11-17: Consider using the generated GraphQL type instead of a local definition.The
Sponsortype duplicates the shape from the generatedGetSponsorsDataQuery['activeSponsors'][number]. Using the generated type ensures type safety stays in sync with the GraphQL schema.♻️ Proposed refactor
-type Sponsor = { - id: string - name: string - url: string - description: string - imageUrl: string -} +import type { GetSponsorsDataQuery } from 'types/__generated__/sponsorQueries.generated' + +type Sponsor = GetSponsorsDataQuery['activeSponsors'][number]Note: Adjust the existing import on line 8 to include both the document and the type.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/sponsors/page.tsx` around lines 11 - 17, The local Sponsor type duplicates the GraphQL-generated type and risks drifting from the schema; replace the custom type definition in page.tsx with the generated type GetSponsorsDataQuery['activeSponsors'][number] by importing the generated query/types (add to the existing import on line 8) and update any references to Sponsor to use that generated type instead (e.g., type annotations and props) so the component uses the canonical GraphQL type.
94-100: Consider usingLinkinstead of button withrouter.pushfor navigation.Using
Linkprovides better semantics (anchor element), accessibility (keyboard navigation, right-click "open in new tab"), and automatic prefetching.♻️ Proposed refactor
- <button - type="button" - className="inline-flex items-center justify-center rounded-xl bg-blue-600 px-6 py-3 text-sm font-semibold text-white transition-colors hover:bg-blue-700 focus:ring-2 focus:ring-blue-500 focus:ring-offset-2 focus:outline-none dark:focus:ring-offset-gray-800" - onClick={() => router.push('/sponsors/apply')} - > + <Link + href="/sponsors/apply" + className="inline-flex items-center justify-center rounded-xl bg-blue-600 px-6 py-3 text-sm font-semibold text-white transition-colors hover:bg-blue-700 focus:ring-2 focus:ring-blue-500 focus:ring-offset-2 focus:outline-none dark:focus:ring-offset-gray-800" + > Apply here for Sponsorship - </button> + </Link>This also allows removing the
useRouterimport and hook call if not used elsewhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/sponsors/page.tsx` around lines 94 - 100, Replace the navigation button that calls router.push with a Next.js Link anchor: swap the <button> that uses onClick={() => router.push('/sponsors/apply')} for a <Link href="/sponsors/apply"> element and apply the same className and inner text to that Link, and then remove the useRouter import and const router = useRouter() call if they’re no longer used; ensure you import Link from 'next/link' at the top of the file.
🤖 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/src/app/sponsors/page.tsx`:
- Around line 41-63: SponsorEntryCard currently never renders
Sponsor.description; update the component to conditionally render
sponsor.description (e.g., check sponsor.description && ...) under the
organization name (inside SponsorEntryCard, after the div that shows
{sponsor.name} and before the link) as a short paragraph with appropriate
styling (small/gray or muted text, optional line-clamp/truncate) so the optional
short description from the Sponsor type is visible for each entry.
- Around line 19-39: The SponsorLogo component may throw when next/image loads
from remote domains not allowed by next.config.ts; update SponsorLogo to handle
image load failures by adding an onError handler on the Image component that
switches rendering to the placeholder fallback (same UI as the FaBuilding block)
and ensure any cleanup state (e.g., a local boolean like hasImageError) is used
to conditionally render the placeholder; alternatively, if you prefer whitelist
changes, add the missing sponsor domains to next.config.ts remotePatterns so
Image can load them without runtime errors.
---
Nitpick comments:
In `@frontend/src/app/sponsors/page.tsx`:
- Around line 11-17: The local Sponsor type duplicates the GraphQL-generated
type and risks drifting from the schema; replace the custom type definition in
page.tsx with the generated type GetSponsorsDataQuery['activeSponsors'][number]
by importing the generated query/types (add to the existing import on line 8)
and update any references to Sponsor to use that generated type instead (e.g.,
type annotations and props) so the component uses the canonical GraphQL type.
- Around line 94-100: Replace the navigation button that calls router.push with
a Next.js Link anchor: swap the <button> that uses onClick={() =>
router.push('/sponsors/apply')} for a <Link href="/sponsors/apply"> element and
apply the same className and inner text to that Link, and then remove the
useRouter import and const router = useRouter() call if they’re no longer used;
ensure you import Link from 'next/link' at the top of the file.
🪄 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: a4c12538-1ed1-4ee7-ba54-111eb6adc60a
📒 Files selected for processing (2)
backend/apps/owasp/api/internal/mutations/__init__.pyfrontend/src/app/sponsors/page.tsx
✅ Files skipped from review due to trivial changes (1)
- backend/apps/owasp/api/internal/mutations/init.py
Utkarsh-0304
dismissed stale reviews from coderabbitai[bot] and cubic-dev-ai[bot]
via
2365f35
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/components/SponsorForm.tsx (1)
44-86: Consolidate duplicated validation definitions to prevent drift.Lines 44-68 and Lines 81-86 repeat the same field-validator mapping. If a new field is added, one path can be missed and submit behavior can diverge from inline errors.
Refactor sketch
+ const fieldValidators = { + name: () => validateName(formData.name), + website: () => validateWebsite(formData.website), + contactEmail: () => validateContactEmail(formData.contactEmail), + message: () => validateMessage(formData.message), + } as const + const errors = useFormValidation( - [ - { - field: 'name', - shouldValidate: touched.name ?? false, - validator: () => validateName(formData.name), - }, - { - field: 'website', - shouldValidate: touched.website ?? false, - validator: () => validateWebsite(formData.website), - }, - { - field: 'contactEmail', - shouldValidate: touched.contactEmail ?? false, - validator: () => validateContactEmail(formData.contactEmail), - }, - { - field: 'message', - shouldValidate: touched.message ?? false, - validator: () => validateMessage(formData.message), - }, - ], + Object.entries(fieldValidators).map(([field, validator]) => ({ + field, + shouldValidate: touched[field] ?? false, + validator, + })), [formData, touched] ) @@ - if ( - validateName(formData.name) || - validateWebsite(formData.website) || - validateContactEmail(formData.contactEmail) || - validateMessage(formData.message) - ) { + const hasErrors = Object.values(fieldValidators).some((validate) => !!validate()) + if (hasErrors) { return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/SponsorForm.tsx` around lines 44 - 86, The validation rules are duplicated between the useFormValidation call and the handleSubmit checks which can drift; consolidate by defining a single validators structure (e.g., an array or object named validators or validationRules) that maps fields to their shouldValidate flag and validator functions (using validateName, validateWebsite, validateContactEmail, validateMessage), then pass that structure into useFormValidation and reuse it in handleSubmit to set nextTouched and to run the submit-time validation check (instead of calling validateXxx individually); update setTouched to derive keys from this validators structure so adding a field requires one change only.
🤖 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/src/components/forms/shared/formValidationUtils.ts`:
- Around line 41-42: The URL parsing uses the raw input variable value directly
(const parsedUrl = new URL(value)), which rejects valid inputs with
leading/trailing spaces; trim the input first (e.g., const trimmed =
value.trim()) and use that trimmed string when constructing new URL and when
checking parsedUrl.protocol so behavior matches the email trimming on line 55;
update any subsequent uses in the same validation function to use the trimmed
variable.
---
Nitpick comments:
In `@frontend/src/components/SponsorForm.tsx`:
- Around line 44-86: The validation rules are duplicated between the
useFormValidation call and the handleSubmit checks which can drift; consolidate
by defining a single validators structure (e.g., an array or object named
validators or validationRules) that maps fields to their shouldValidate flag and
validator functions (using validateName, validateWebsite, validateContactEmail,
validateMessage), then pass that structure into useFormValidation and reuse it
in handleSubmit to set nextTouched and to run the submit-time validation check
(instead of calling validateXxx individually); update setTouched to derive keys
from this validators structure so adding a field requires one change only.
🪄 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: a9a36b88-f127-4031-a3a2-a906a336303c
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
frontend/package.jsonfrontend/src/components/SponsorForm.tsxfrontend/src/components/forms/shared/formValidationUtils.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/package.json
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (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/components/forms/shared/formValidationUtils.ts">
<violation number="1" location="frontend/src/components/forms/shared/formValidationUtils.ts:42">
P2: Website validation became case-sensitive for scheme prefix, causing valid URLs like `HTTPS://...` to be rejected.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Utkarsh-0304
dismissed stale reviews from cubic-dev-ai[bot] and coderabbitai[bot]
via
af65812
Utkarsh-0304
force-pushed
the
feat/sponsors
branch
from
af65812 to
b3c22a2
Compare
|
Contributor
Author
|
Hi @arkid15r, please review this and let me know if any changes are required. |
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
1 participant
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
Resolves #4259
This PR adds support for the sponsors program within the Nest Platform.
It extends the existing Sponsor model with
status,message,contact_email,created_atand optional entity fields. Also,statuscan now be handled through the Django Admin Panel.It also adds a
/sponsorsroute to display active sponsors, ordered by sponsor level, in a clear and professional manner.Another route,
/sponsors/apply, allows organisations to apply for sponsorship by filling out a form with their organisation name, contact email, website URL, and a message indicating their interest in sponsoring.Additionally, it adds a REST Endpoint for exposing active sponsor information for external integrations.
demo.webm
fill_sponsorship_form.webm
Checklist
make check-testlocally: all warnings addressed, tests passed