Implement Sponsors Program Support #4520
Draft
Mr-Rahul-Paul wants to merge 15 commits intoOWASP:mainfrom
Draft
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)
🚧 Files skipped from review as they are similar to previous changes (1)
Summary by CodeRabbit
WalkthroughAdds sponsor support: model fields and migrations; admin actions; REST apply endpoint and active-only filtering; dump-data backfills; GraphQL filter change; new frontend sponsors pages, layouts, components, types, and tests; form component adjustments and Jest coverage exclusions. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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.
2 issues found across 38 files
Confidence score: 2/5
- There is a concrete privacy risk in
backend/apps/common/management/commands/dump_data.py: masked dumps only clearemail, so populatedcontact_emailvalues can be exported and leak sponsor PII. backend/apps/api/rest/v0/sponsor.pyhas a data-quality/regression risk whereapply_sponsorcan save an empty sponsor key whenslugifyreturns an empty string, which can cause downstream lookup or validation issues.- The score is lowered because the top issue is user-impacting and high-confidence (PII exposure), with an additional medium-confidence persistence bug increasing merge risk.
- Pay close attention to
backend/apps/common/management/commands/dump_data.pyandbackend/apps/api/rest/v0/sponsor.py- preventcontact_emailleakage in dumps and block empty sponsor keys from being persisted.
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/common/management/commands/dump_data.py">
<violation number="1" location="backend/apps/common/management/commands/dump_data.py:123">
P2: Masked dump only clears columns named `email`; the new `contact_email` field is only normalized when NULL, so real sponsor contact emails will remain in the dump and leak PII.</violation>
</file>
<file name="backend/apps/api/rest/v0/sponsor.py">
<violation number="1" location="backend/apps/api/rest/v0/sponsor.py:146">
P2: apply_sponsor can persist an empty sponsor key when slugify yields an empty string (e.g., whitespace/punctuation-only organization_name).</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: 4
🧹 Nitpick comments (17)
frontend/__tests__/unit/components/forms/shared/FormTextarea.test.tsx (1)
7-7: Add an explicit assertion for the new requirednameprop.The fixture now provides
name, but no test verifies it reaches the rendered<textarea>.Suggested test addition
it('renders with default props (required=false, rows=4)', () => { render(<FormTextarea {...defaultProps} />) const textarea = screen.getByRole('textbox') expect(textarea).toBeInTheDocument() + expect(textarea).toHaveAttribute('name', 'test-textarea') expect(textarea).toHaveAttribute('rows', '4') expect(textarea).not.toBeRequired() expect(screen.queryByText('*')).not.toBeInTheDocument() })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/__tests__/unit/components/forms/shared/FormTextarea.test.tsx` at line 7, Add an assertion in the FormTextarea unit test to verify the required name prop is passed through to the rendered <textarea>: after rendering the FormTextarea in frontend/__tests__/unit/components/forms/shared/FormTextarea.test.tsx (the test that supplies name: 'test-textarea'), query the rendered textarea (e.g., getByRole('textbox') or container.querySelector('textarea')) and assert its name attribute equals 'test-textarea' so the fixture's name prop is explicitly verified.frontend/jest.config.ts (1)
11-13: Avoid blanket coverage exclusion for new sponsors code.Excluding
src/app/sponsors/**andsrc/components/sponsors/**removes the new feature surface from coverage enforcement. Please keep these included (or add narrower, justified exclusions) so regressions are still caught.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/jest.config.ts` around lines 11 - 13, The coverage config currently excludes the new sponsors feature via the patterns '!src/app/sponsors/**' and '!src/components/sponsors/**' in frontend/jest.config.ts; remove those two exclusion patterns from the coverage ignore list so the sponsors code is tracked by coverage (or if there is a specific, justified subdirectory to exclude, replace each broad pattern with a narrow one such as excluding only generated/assets or styles within those folders, e.g. '!src/app/sponsors/**/styles/**', and keep the rest included).backend/tests/unit/apps/api/rest/v0/sponsor_test.py (1)
107-127: Add an explicit inactive-sponsor test case.You now validate the active success path, but there’s still no direct assertion that an existing inactive sponsor returns
404. Adding that case would lock in the active-only contract.🤖 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 - 127, Add a unit test that asserts an existing sponsor with an inactive status returns HTTP 404: in the test file add a new test (similar to test_get_sponsor_success) that patches SponsorModel (or reuses mock_sponsor_model), sets mock_sponsor_model.objects.filter.return_value.first.return_value to a mock_sponsor whose .status is set to SponsorModel.Status.INACTIVE (or equivalent inactive enum), call get_sponsor(mock_request, "<key>") and assert the result.status_code == HTTPStatus.NOT_FOUND; reference SponsorModel and get_sponsor to locate where to add this case.frontend/src/utils/constants.ts (1)
11-12: Reuse the new Slack constant infooterIconsto avoid divergence.You introduced
OWASP_NEST_SLACK_CHANNEL_URL; wiring it intofooterIconskeeps one source of truth.Proposed change
{ icon: FaSlack, - href: 'https://owasp.slack.com/archives/project-nest', + href: OWASP_NEST_SLACK_CHANNEL_URL, label: 'Slack', },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/utils/constants.ts` around lines 11 - 12, The footer currently hardcodes the OWASP Slack URL instead of reusing the new constant; update the footerIcons usage to reference the exported OWASP_NEST_SLACK_CHANNEL_URL constant (from constants.ts) wherever the Slack link is defined (e.g., the Slack entry in footerIcons) so there is a single source of truth and avoid duplicate strings; ensure you import OWASP_NEST_SLACK_CHANNEL_URL in the file that defines footerIcons and replace the literal URL with that constant.frontend/src/components/sponsors/SponsorsFaqSection.tsx (1)
25-28: Attach punctuation directly to the link text node.This avoids ambiguous whitespace around the inline anchor and resolves the Sonar warning.
Proposed change
- </a> - . Tax treatment depends on your jurisdiction and OWASP Foundation policies—consult a tax + </a>. Tax treatment depends on your jurisdiction and OWASP Foundation policies—consult a tax professional if needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/sponsors/SponsorsFaqSection.tsx` around lines 25 - 28, The inline anchor in SponsorsFaqSection.tsx currently has punctuation outside the <a> anchor text which creates ambiguous whitespace and triggers a Sonar warning; update the anchor's text node so the punctuation (e.g., the period or comma following "official donate link") is included inside the <a> element instead of as a separate text node, ensuring the link text becomes "official donate link." (or similar) and preserving surrounding spacing and sentence flow.frontend/src/components/sponsors/SponsorHero.tsx (1)
24-27: Make text spacing before the arrow explicit.This removes the Sonar ambiguity warning and keeps rendering deterministic.
Proposed change
- Become a sponsor + Become a sponsor{' '} <span aria-hidden className="opacity-70"> → </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/sponsors/SponsorHero.tsx` around lines 24 - 27, The spacing before the arrow is implicit and triggers Sonar ambiguity; update the SponsorHero JSX where the arrow span is rendered (the span with aria-hidden and className="opacity-70") to include an explicit space token before the span (e.g., add {' '} or in the text node immediately before the span) so the space is deterministic and Sonar warning is resolved.frontend/src/components/sponsors/BecomeSponsorCTA.tsx (1)
19-22: Use explicit spacing before decorative arrows.This clears the Sonar spacing warnings in both CTA buttons.
Proposed change
- Apply to sponsor + Apply to sponsor{' '} <span aria-hidden className="opacity-70"> → </span> ... - Donate once + Donate once{' '} <span aria-hidden className="opacity-70"> → </span>Also applies to: 40-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/sponsors/BecomeSponsorCTA.tsx` around lines 19 - 22, The decorative arrow span (the span with aria-hidden and className="opacity-70") in the BecomeSponsorCTA component lacks explicit spacing from the preceding text; update both CTA occurrences in BecomeSponsorCTA to add explicit spacing (either a non-breaking space character before the span or a small utility margin such as adding "ml-2" to the span) so the arrow is visually separated and Sonar spacing warnings are resolved.frontend/src/app/sponsors/apply/page.tsx (1)
1-1: Remove unnecessary'use client'directive from this page.The page component doesn't use client-only hooks or APIs, so it can remain a Server Component. Child components (
SponsorApplicationForm,SponsorApplyHero) properly define their own client boundaries when needed. Removing'use client'here reduces the client bundle size.Proposed change
-'use client' - import SponsorApplicationForm from 'components/sponsors/SponsorApplicationForm' import SponsorApplyHero from 'components/sponsors/SponsorApplyHero'🤖 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` at line 1, Remove the top-level 'use client' directive from this page component so it remains a Server Component; locate the page component (the default-exported page function, e.g., SponsorApplyPage or the exported default React component) and delete the "'use client'" line at the top, leaving child components like SponsorApplicationForm and SponsorApplyHero unchanged since they declare their own client boundaries when needed.frontend/src/components/sponsors/SponsorDiamondCard.tsx (2)
14-24: Consider adding meaningful alt text for the logo image.The
alt=""attribute marks the image as decorative, but sponsor logos convey meaningful information. Consider using the sponsor name for better accessibility.♿ Proposed fix
<Image src={sponsor.imageUrl} - alt="" + alt={`${sponsor.name} logo`} width={220} height={110}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/sponsors/SponsorDiamondCard.tsx` around lines 14 - 24, The image for sponsor logos in SponsorDiamondCard currently uses alt="" making it inaccessible; update the logoInner Image element to provide meaningful alt text using the sponsor's name (e.g., alt={sponsor.name || 'Sponsor logo'}) so screen readers receive context, and keep the fallback <span> when sponsor.imageUrl is missing; ensure you reference the sponsor object in SponsorDiamondCard and handle null/undefined sponsor.name safely.
7-9: Consider marking props interface asReadonly.Per SonarCloud hint, marking props as read-only is a TypeScript best practice.
♻️ Proposed fix
-interface SponsorDiamondCardProps { - sponsor: SponsorData -} +interface SponsorDiamondCardProps { + readonly sponsor: SponsorData +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/sponsors/SponsorDiamondCard.tsx` around lines 7 - 9, Change the props interface to make properties immutable: update SponsorDiamondCardProps so its sponsor field is readonly (e.g., declare interface SponsorDiamondCardProps { readonly sponsor: SponsorData }) or replace the interface with a Readonly mapped type (e.g., type SponsorDiamondCardProps = Readonly<{ sponsor: SponsorData }>), and ensure any usages of SponsorDiamondCardProps / SponsorDiamondCard component still type-check.frontend/src/app/sponsors/page.tsx (1)
50-51: Use nullish coalescing assignment (??=) for cleaner syntax.Per SonarCloud suggestion, the nullish coalescing assignment operator is more concise.
♻️ Proposed fix
for (const sponsor of data.sponsors) { const tier = normalizeTier(sponsor.sponsorType) - if (!grouped[tier]) grouped[tier] = [] + grouped[tier] ??= [] grouped[tier].push(mapSponsor(sponsor)) }🤖 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 50 - 51, Replace the explicit existence check and assignment for grouped[tier] with the nullish coalescing assignment operator: instead of "if (!grouped[tier]) grouped[tier] = []" use "grouped[tier] ??= []" before calling grouped[tier].push(mapSponsor(sponsor)); this change should be made where grouped, tier, mapSponsor, and sponsor are used so grouped[tier] is initialized only if nullish.frontend/src/components/sponsors/SponsorTierSection.tsx (1)
6-9: Consider marking props interface asReadonly.SonarCloud flagged this as a best practice. Marking props as read-only prevents accidental mutation and improves type safety.
♻️ Proposed fix
-interface SponsorTierSectionProps { - tier: SponsorTier - sponsors: SponsorData[] -} +interface SponsorTierSectionProps { + readonly tier: SponsorTier + readonly sponsors: readonly SponsorData[] +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/sponsors/SponsorTierSection.tsx` around lines 6 - 9, The props interface SponsorTierSectionProps should be made immutable to prevent accidental mutation: update SponsorTierSectionProps so its properties are readonly (e.g., make tier readonly and sponsors a ReadonlyArray or mark the whole shape Readonly<...>), and ensure the component signature (where SponsorTierSectionProps is used) still types props accordingly; target the SponsorTierSectionProps interface and any use sites that construct/receive these props to apply Readonly semantics for SponsorTier and SponsorData references.backend/apps/owasp/migrations/0074_backfill_sponsor_status_not_null.py (1)
11-25: Migration is irreversible - ensure this is intentional.Using
migrations.RunSQL.noopforreverse_sqlmeans this migration cannot be rolled back automatically. If you need to revert, you'll have to manually drop the NOT NULL constraint and default. This is acceptable for a one-way data normalization, but consider adding a comment documenting this decision.Additionally, for very large tables, the
UPDATEstatement without batching could cause extended table locks. Ifowasp_sponsorsis expected to remain small, this is fine.📝 Suggested documentation
operations = [ migrations.RunSQL( + # Note: This migration is intentionally irreversible. + # Legacy NULL status values are normalized to 'active'. sql=""" UPDATE public.owasp_sponsors🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/owasp/migrations/0074_backfill_sponsor_status_not_null.py` around lines 11 - 25, The migration uses migrations.RunSQL with reverse_sql=migrations.RunSQL.noop which makes it irreversible; either document that decision clearly in the migration file (add a concise comment above the RunSQL block referencing the intentional one-way data normalization for public.owasp_sponsors.status) or provide a proper reverse_sql that reverts the schema changes (for example run SQL that ALTER TABLE public.owasp_sponsors ALTER COLUMN status DROP DEFAULT; ALTER TABLE public.owasp_sponsors ALTER COLUMN status DROP NOT NULL; and optionally UPDATE public.owasp_sponsors SET status = NULL WHERE /* criteria to target rows created by this migration if you can identify them */), and replace migrations.RunSQL.noop with that SQL in the migrations.RunSQL(reverse_sql=...) argument so the migration can be rolled back.frontend/src/components/sponsors/SponsorApplicationForm.tsx (2)
93-104: Simplify by using controlled state directly instead of re-reading from FormData.SonarCloud warns that
fd.get()can returnFile | string | null, and wrapping inString()on aFileyields[object Object]. Since this form uses controlled inputs, theformstate already contains the current values. Re-extracting fromFormDatais redundant and introduces this edge case.♻️ Proposed fix: use controlled state directly
setTouched({ organizationName: true, website: true, contactEmail: true, message: true, }) - const fd = new FormData(e.currentTarget) - const current: FormState = { - organizationName: String(fd.get('organizationName') ?? ''), - website: String(fd.get('website') ?? ''), - contactEmail: String(fd.get('contactEmail') ?? ''), - message: String(fd.get('message') ?? ''), - } - setForm(current) - - if (Object.keys(validate(current)).length > 0) { + if (Object.keys(validate(form)).length > 0) { return }Then update the
JSON.stringifycall to useformdirectly:body: JSON.stringify( - Object.fromEntries([ - ['organization_name', current.organizationName.trim()], - ['website', current.website.trim()], - ['contact_email', current.contactEmail.trim()], - ['message', current.message.trim()], - ]) + { + organization_name: form.organizationName.trim(), + website: form.website.trim(), + contact_email: form.contactEmail.trim(), + message: form.message.trim(), + } ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/sponsors/SponsorApplicationForm.tsx` around lines 93 - 104, The handler re-reads inputs via FormData causing File|string|null issues; instead use the existing controlled state: remove the FormData(fd) and local current FormState creation, stop calling setForm(current), and call validate(form) (not validate(current)) to gate submission; also update the payload/JSON.stringify call to use form directly. Reference: FormState, form, setForm, and validate.
240-251: Inconsistent event handler pattern between FormTextInput and FormTextarea.
FormTextInputusesonValueChange(receiving the string value directly), whileFormTextareauses a rawonChangehandler withe.target.value. Consider usinghandleFieldChange('message')for consistency ifFormTextareasupports anonValueChangeprop, or extract a helper if it doesn't.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/sponsors/SponsorApplicationForm.tsx` around lines 240 - 251, The FormTextarea usage is inconsistent with FormTextInput: replace the inline onChange={(e) => setForm((prev) => ({ ...prev, message: e.target.value }))} with the same handler pattern used by FormTextInput (handleFieldChange('message')) to keep event handling consistent; if FormTextarea exposes an onValueChange prop, pass handleFieldChange('message') there (so it receives the string directly), otherwise refactor FormTextarea to support onValueChange or extract a small adapter helper (e.g., a wrapper that calls handleFieldChange('message') with e.target.value)—make the change around the FormTextarea component where id/name/message, value={form.message}, onBlur={handleBlur('message')} and error/touched props are set.frontend/src/components/sponsors/SponsorTierCard.tsx (1)
37-40: Consider marking props as read-only per SonarCloud hint.The static analysis suggests marking component props as read-only. This is a TypeScript best practice for React components to prevent accidental mutation.
♻️ Proposed fix
-export default function SponsorTierCard({ sponsor, size, tier }: SponsorTierCardProps) { +export default function SponsorTierCard({ sponsor, size, tier }: Readonly<SponsorTierCardProps>) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/sponsors/SponsorTierCard.tsx` around lines 37 - 40, The component props for SponsorTierCard should be marked read-only to satisfy the TypeScript/SonarCloud hint; update the function signature to take a Readonly<SponsorTierCardProps> (e.g., change the parameter annotation to Readonly<SponsorTierCardProps> or accept props: Readonly<SponsorTierCardProps> and then destructure), so sponsor, size, and tier cannot be mutated inside SponsorTierCard; ensure the rest of the file (and any typings for SponsorTierCardProps) remain compatible with the Readonly wrapper.backend/apps/api/rest/v0/sponsor.py (1)
123-128: Optimize by filtering status in the query.The current implementation fetches the sponsor first, then checks status in Python. This could return a sponsor to memory that won't be used. Consider filtering in the query:
♻️ Proposed fix
def get_sponsor( request: HttpRequest, sponsor_id: str = Path(..., example="adobe"), ) -> SponsorDetail | SponsorError: """Get a single active sponsor.""" - sponsor = SponsorModel.objects.filter(key__iexact=sponsor_id).first() - if sponsor and sponsor.status == SponsorModel.Status.ACTIVE: + sponsor = SponsorModel.objects.filter( + key__iexact=sponsor_id, + status=SponsorModel.Status.ACTIVE, + ).first() + if sponsor: return sponsor return Response({"message": "Sponsor not found"}, status=HTTPStatus.NOT_FOUND)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/api/rest/v0/sponsor.py` around lines 123 - 128, Filter by status at the database level instead of checking it in Python: change the query that fetches SponsorModel (currently using SponsorModel.objects.filter(key__iexact=sponsor_id).first()) to include the active status filter (e.g., filter(..., status=SponsorModel.Status.ACTIVE).first()) so the returned sponsor is already active before the function returns, then keep the existing Response({"message": "Sponsor not found"}, status=HTTPStatus.NOT_FOUND) fallback.
🤖 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/FormTextarea.tsx`:
- Line 12: The onBlur prop on FormTextarea is currently typed as onBlur?: () =>
void which is too narrow; update the prop type in the FormTextarea
props/interface to accept React's standard blur handler (e.g.,
React.FocusEventHandler<HTMLTextAreaElement> or (e:
React.FocusEvent<HTMLTextAreaElement>) => void) so consumers receive the
FocusEvent object like onChange does; adjust any usages of FormTextarea.onBlur
to match the new signature and ensure the component forwards the event to the
provided handler.
In `@frontend/src/components/Header.tsx`:
- Around line 120-128: The desktop nav Link currently sets aria-current
unconditionally, causing every link to be announced as the current page; update
the Link in Header.tsx (the Link element that uses cn, pathname and link.href)
to set aria-current only when pathname === link.href (e.g.,
aria-current={pathname === link.href ? 'page' : undefined}) so that only the
active route has aria-current; ensure you use the same pathname and link.href
comparison used for the active class logic.
In `@frontend/src/server/queries/sponsorQueries.ts`:
- Around line 3-12: The current GET_SPONSORS_PAGE_DATA GraphQL query requests
the unfiltered sponsors field and can return non-public entries; change the
query to use an active-only source or add an active filter: update the frontend
query named GetSponsorsPageData (constant GET_SPONSORS_PAGE_DATA) to call the
public/filtered field (e.g., sponsorsPublic or sponsors(active: true)) that only
returns active sponsors, or if you prefer server-side, update the backend
resolver in sponsor.py to enforce active-only filtering and keep the frontend
query pointing to the filtered field; ensure the returned fields (id, imageUrl,
name, sponsorType, url) remain the same.
---
Nitpick comments:
In `@backend/apps/api/rest/v0/sponsor.py`:
- Around line 123-128: Filter by status at the database level instead of
checking it in Python: change the query that fetches SponsorModel (currently
using SponsorModel.objects.filter(key__iexact=sponsor_id).first()) to include
the active status filter (e.g., filter(...,
status=SponsorModel.Status.ACTIVE).first()) so the returned sponsor is already
active before the function returns, then keep the existing Response({"message":
"Sponsor not found"}, status=HTTPStatus.NOT_FOUND) fallback.
In `@backend/apps/owasp/migrations/0074_backfill_sponsor_status_not_null.py`:
- Around line 11-25: The migration uses migrations.RunSQL with
reverse_sql=migrations.RunSQL.noop which makes it irreversible; either document
that decision clearly in the migration file (add a concise comment above the
RunSQL block referencing the intentional one-way data normalization for
public.owasp_sponsors.status) or provide a proper reverse_sql that reverts the
schema changes (for example run SQL that ALTER TABLE public.owasp_sponsors ALTER
COLUMN status DROP DEFAULT; ALTER TABLE public.owasp_sponsors ALTER COLUMN
status DROP NOT NULL; and optionally UPDATE public.owasp_sponsors SET status =
NULL WHERE /* criteria to target rows created by this migration if you can
identify them */), and replace migrations.RunSQL.noop with that SQL in the
migrations.RunSQL(reverse_sql=...) argument so the migration can be rolled back.
In `@backend/tests/unit/apps/api/rest/v0/sponsor_test.py`:
- Around line 107-127: Add a unit test that asserts an existing sponsor with an
inactive status returns HTTP 404: in the test file add a new test (similar to
test_get_sponsor_success) that patches SponsorModel (or reuses
mock_sponsor_model), sets
mock_sponsor_model.objects.filter.return_value.first.return_value to a
mock_sponsor whose .status is set to SponsorModel.Status.INACTIVE (or equivalent
inactive enum), call get_sponsor(mock_request, "<key>") and assert the
result.status_code == HTTPStatus.NOT_FOUND; reference SponsorModel and
get_sponsor to locate where to add this case.
In `@frontend/__tests__/unit/components/forms/shared/FormTextarea.test.tsx`:
- Line 7: Add an assertion in the FormTextarea unit test to verify the required
name prop is passed through to the rendered <textarea>: after rendering the
FormTextarea in
frontend/__tests__/unit/components/forms/shared/FormTextarea.test.tsx (the test
that supplies name: 'test-textarea'), query the rendered textarea (e.g.,
getByRole('textbox') or container.querySelector('textarea')) and assert its name
attribute equals 'test-textarea' so the fixture's name prop is explicitly
verified.
In `@frontend/jest.config.ts`:
- Around line 11-13: The coverage config currently excludes the new sponsors
feature via the patterns '!src/app/sponsors/**' and
'!src/components/sponsors/**' in frontend/jest.config.ts; remove those two
exclusion patterns from the coverage ignore list so the sponsors code is tracked
by coverage (or if there is a specific, justified subdirectory to exclude,
replace each broad pattern with a narrow one such as excluding only
generated/assets or styles within those folders, e.g.
'!src/app/sponsors/**/styles/**', and keep the rest included).
In `@frontend/src/app/sponsors/apply/page.tsx`:
- Line 1: Remove the top-level 'use client' directive from this page component
so it remains a Server Component; locate the page component (the
default-exported page function, e.g., SponsorApplyPage or the exported default
React component) and delete the "'use client'" line at the top, leaving child
components like SponsorApplicationForm and SponsorApplyHero unchanged since they
declare their own client boundaries when needed.
In `@frontend/src/app/sponsors/page.tsx`:
- Around line 50-51: Replace the explicit existence check and assignment for
grouped[tier] with the nullish coalescing assignment operator: instead of "if
(!grouped[tier]) grouped[tier] = []" use "grouped[tier] ??= []" before calling
grouped[tier].push(mapSponsor(sponsor)); this change should be made where
grouped, tier, mapSponsor, and sponsor are used so grouped[tier] is initialized
only if nullish.
In `@frontend/src/components/sponsors/BecomeSponsorCTA.tsx`:
- Around line 19-22: The decorative arrow span (the span with aria-hidden and
className="opacity-70") in the BecomeSponsorCTA component lacks explicit spacing
from the preceding text; update both CTA occurrences in BecomeSponsorCTA to add
explicit spacing (either a non-breaking space character before the span or a
small utility margin such as adding "ml-2" to the span) so the arrow is visually
separated and Sonar spacing warnings are resolved.
In `@frontend/src/components/sponsors/SponsorApplicationForm.tsx`:
- Around line 93-104: The handler re-reads inputs via FormData causing
File|string|null issues; instead use the existing controlled state: remove the
FormData(fd) and local current FormState creation, stop calling
setForm(current), and call validate(form) (not validate(current)) to gate
submission; also update the payload/JSON.stringify call to use form directly.
Reference: FormState, form, setForm, and validate.
- Around line 240-251: The FormTextarea usage is inconsistent with
FormTextInput: replace the inline onChange={(e) => setForm((prev) => ({ ...prev,
message: e.target.value }))} with the same handler pattern used by FormTextInput
(handleFieldChange('message')) to keep event handling consistent; if
FormTextarea exposes an onValueChange prop, pass handleFieldChange('message')
there (so it receives the string directly), otherwise refactor FormTextarea to
support onValueChange or extract a small adapter helper (e.g., a wrapper that
calls handleFieldChange('message') with e.target.value)—make the change around
the FormTextarea component where id/name/message, value={form.message},
onBlur={handleBlur('message')} and error/touched props are set.
In `@frontend/src/components/sponsors/SponsorDiamondCard.tsx`:
- Around line 14-24: The image for sponsor logos in SponsorDiamondCard currently
uses alt="" making it inaccessible; update the logoInner Image element to
provide meaningful alt text using the sponsor's name (e.g., alt={sponsor.name ||
'Sponsor logo'}) so screen readers receive context, and keep the fallback <span>
when sponsor.imageUrl is missing; ensure you reference the sponsor object in
SponsorDiamondCard and handle null/undefined sponsor.name safely.
- Around line 7-9: Change the props interface to make properties immutable:
update SponsorDiamondCardProps so its sponsor field is readonly (e.g., declare
interface SponsorDiamondCardProps { readonly sponsor: SponsorData }) or replace
the interface with a Readonly mapped type (e.g., type SponsorDiamondCardProps =
Readonly<{ sponsor: SponsorData }>), and ensure any usages of
SponsorDiamondCardProps / SponsorDiamondCard component still type-check.
In `@frontend/src/components/sponsors/SponsorHero.tsx`:
- Around line 24-27: The spacing before the arrow is implicit and triggers Sonar
ambiguity; update the SponsorHero JSX where the arrow span is rendered (the span
with aria-hidden and className="opacity-70") to include an explicit space token
before the span (e.g., add {' '} or in the text node immediately before
the span) so the space is deterministic and Sonar warning is resolved.
In `@frontend/src/components/sponsors/SponsorsFaqSection.tsx`:
- Around line 25-28: The inline anchor in SponsorsFaqSection.tsx currently has
punctuation outside the <a> anchor text which creates ambiguous whitespace and
triggers a Sonar warning; update the anchor's text node so the punctuation
(e.g., the period or comma following "official donate link") is included inside
the <a> element instead of as a separate text node, ensuring the link text
becomes "official donate link." (or similar) and preserving surrounding spacing
and sentence flow.
In `@frontend/src/components/sponsors/SponsorTierCard.tsx`:
- Around line 37-40: The component props for SponsorTierCard should be marked
read-only to satisfy the TypeScript/SonarCloud hint; update the function
signature to take a Readonly<SponsorTierCardProps> (e.g., change the parameter
annotation to Readonly<SponsorTierCardProps> or accept props:
Readonly<SponsorTierCardProps> and then destructure), so sponsor, size, and tier
cannot be mutated inside SponsorTierCard; ensure the rest of the file (and any
typings for SponsorTierCardProps) remain compatible with the Readonly wrapper.
In `@frontend/src/components/sponsors/SponsorTierSection.tsx`:
- Around line 6-9: The props interface SponsorTierSectionProps should be made
immutable to prevent accidental mutation: update SponsorTierSectionProps so its
properties are readonly (e.g., make tier readonly and sponsors a ReadonlyArray
or mark the whole shape Readonly<...>), and ensure the component signature
(where SponsorTierSectionProps is used) still types props accordingly; target
the SponsorTierSectionProps interface and any use sites that construct/receive
these props to apply Readonly semantics for SponsorTier and SponsorData
references.
In `@frontend/src/utils/constants.ts`:
- Around line 11-12: The footer currently hardcodes the OWASP Slack URL instead
of reusing the new constant; update the footerIcons usage to reference the
exported OWASP_NEST_SLACK_CHANNEL_URL constant (from constants.ts) wherever the
Slack link is defined (e.g., the Slack entry in footerIcons) so there is a
single source of truth and avoid duplicate strings; ensure you import
OWASP_NEST_SLACK_CHANNEL_URL in the file that defines footerIcons and replace
the literal URL with that constant.
🪄 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: ba6636ed-4900-4ee3-ab1e-3362e080c73e
⛔ Files ignored due to path filters (1)
frontend/src/types/__generated__/sponsorQueries.generated.tsis excluded by!**/__generated__/**
📒 Files selected for processing (37)
backend/apps/api/rest/v0/sponsor.pybackend/apps/common/management/commands/dump_data.pybackend/apps/owasp/admin/sponsor.pybackend/apps/owasp/migrations/0073_sponsor_status_contact_email_chapter_project.pybackend/apps/owasp/migrations/0074_backfill_sponsor_status_not_null.pybackend/apps/owasp/migrations/0075_backfill_sponsor_contact_email_not_null.pybackend/apps/owasp/models/sponsor.pybackend/tests/unit/apps/api/rest/v0/sponsor_test.pyfrontend/__tests__/unit/components/forms/shared/FormDateInput.test.tsxfrontend/__tests__/unit/components/forms/shared/FormTextarea.test.tsxfrontend/jest.config.tsfrontend/src/app/sponsors/apply/layout.tsxfrontend/src/app/sponsors/apply/page.tsxfrontend/src/app/sponsors/layout.tsxfrontend/src/app/sponsors/page.tsxfrontend/src/components/Header.tsxfrontend/src/components/LogoCarousel.tsxfrontend/src/components/ModuleForm.tsxfrontend/src/components/NavButton.tsxfrontend/src/components/ProgramForm.tsxfrontend/src/components/forms/shared/FormDateInput.tsxfrontend/src/components/forms/shared/FormTextInput.tsxfrontend/src/components/forms/shared/FormTextarea.tsxfrontend/src/components/skeletons/SponsorsSkeleton.tsxfrontend/src/components/sponsors/BecomeSponsorCTA.tsxfrontend/src/components/sponsors/SponsorApplicationForm.tsxfrontend/src/components/sponsors/SponsorApplyHero.tsxfrontend/src/components/sponsors/SponsorDiamondCard.tsxfrontend/src/components/sponsors/SponsorHero.tsxfrontend/src/components/sponsors/SponsorTierCard.tsxfrontend/src/components/sponsors/SponsorTierSection.tsxfrontend/src/components/sponsors/SponsorsFaqSection.tsxfrontend/src/components/sponsors/SponsorsOpenSourceShowcase.tsxfrontend/src/server/queries/sponsorQueries.tsfrontend/src/types/sponsor.tsfrontend/src/utils/constants.tsfrontend/src/utils/metadata.ts
Comment on lines
+141
to
+166
| def apply_sponsor( | ||
| request: HttpRequest, | ||
| payload: SponsorApplyRequest, | ||
| ) -> tuple[int, SponsorApplyResponse | SponsorError]: | ||
| """Create a draft sponsor application.""" | ||
| key = slugify(payload.organization_name) | ||
|
|
||
| if SponsorModel.objects.filter(key=key).exists(): | ||
| return HTTPStatus.BAD_REQUEST, SponsorError( | ||
| message=(f"An application for '{payload.organization_name}' already exists.") | ||
| ) | ||
|
|
||
| SponsorModel.objects.create( | ||
| key=key, | ||
| name=payload.organization_name, | ||
| sort_name=payload.organization_name, | ||
| contact_email=payload.contact_email, | ||
| url=payload.website, | ||
| description=payload.message, | ||
| status=SponsorModel.Status.DRAFT, | ||
| ) | ||
|
|
||
| return HTTPStatus.CREATED, SponsorApplyResponse( | ||
| key=key, | ||
| message=("Application received. The Nest team will review and follow up."), | ||
| ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if rate limiting is configured elsewhere for this router
rg -n "rate.?limit|throttle" backend/apps/api/ --type=py -C2Repository: OWASP/Nest
Length of output: 1332
🏁 Script executed:
# First, check the sponsor.py file to see the apply_sponsor endpoint definition and route
cat -n backend/apps/api/rest/v0/sponsor.py | head -50Repository: OWASP/Nest
Length of output: 1553
🏁 Script executed:
# Look for SponsorApplyRequest schema definition
rg -n "class SponsorApplyRequest" backend/apps/api/ --type=py -A 10Repository: OWASP/Nest
Length of output: 988
🏁 Script executed:
# Check SponsorModel fields and validation
rg -n "class SponsorModel" backend/apps/api/ --type=py -A 30Repository: OWASP/Nest
Length of output: 36
🏁 Script executed:
# Check the sponsor router/endpoint registration to see if apply_sponsor has specific throttle settings
rg -n "apply_sponsor|@router.*sponsor" backend/apps/api/rest/v0/sponsor.py --type=py -B 2 -A 5Repository: OWASP/Nest
Length of output: 638
🏁 Script executed:
# Check SponsorModel definition in the database models
fd -type f "sponsor.py" backend/apps/owasp/models/Repository: OWASP/Nest
Length of output: 224
🏁 Script executed:
# Read the SponsorModel
cat backend/apps/owasp/models/sponsor.pyRepository: OWASP/Nest
Length of output: 6023
🏁 Script executed:
# Check if apply_sponsor endpoint requires authentication
rg -n "apply_sponsor|@router" backend/apps/api/rest/v0/sponsor.py --type=py -B 5 | head -40Repository: OWASP/Nest
Length of output: 731
🏁 Script executed:
# Check the router configuration and if throttle applies to apply_sponsor
rg -n "auth|permission" backend/apps/api/rest/v0/sponsor.py --type=pyRepository: OWASP/Nest
Length of output: 36
Add email and URL format validation to the request schema.
This endpoint is publicly accessible and creates database records without server-side format validation. While the SponsorModel has EmailField and URLField at the database level, the endpoint uses objects.create() which bypasses Django's full_clean() validation. The schema should enforce format constraints:
- Email validation:
contact_emailhasmin_length=1but no format validation; should add an email pattern. - URL validation:
websitefield accepts any string; should add URL format validation.
Rate limiting is configured globally at "10/s" in the router config, but this is environment-dependent (empty in non-production). Verify the production environment applies the throttle or consider endpoint-specific configuration if stricter limits are needed.
♻️ Example: Add format validation to schema
class SponsorApplyRequest(Schema):
"""Request schema for sponsor application."""
organization_name: str = Field(..., min_length=1, description="Organization name")
- website: str = Field("", description="Organization website URL")
- contact_email: str = Field(..., min_length=1, description="Contact email address")
+ website: str = Field(
+ "",
+ pattern=r"^(https?://)?.+",
+ description="Organization website URL",
+ )
+ contact_email: str = Field(
+ ...,
+ min_length=1,
+ pattern=r"^[^\s@]+@[^\s@]+\.[^\s@]+$",
+ description="Contact email address",
+ )
message: str = Field("", description="Sponsorship interest / message")| placeholder: string | ||
| value: string | ||
| onChange: (e: React.ChangeEvent<HTMLTextAreaElement>) => void | ||
| onBlur?: () => void |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify FormTextarea onBlur typing and callsites that may require event access.
rg -n --type=tsx 'interface FormTextareaProps|onBlur\?:' frontend/src/components/forms/shared/FormTextarea.tsx
rg -n --type=tsx '<FormTextarea[^>]*onBlur=' frontend
rg -n --type=tsx 'onBlur=\{\(\w+' frontendRepository: OWASP/Nest
Length of output: 137
🏁 Script executed:
# Verify file exists and read FormTextarea.tsx
cat -n frontend/src/components/forms/shared/FormTextarea.tsxRepository: OWASP/Nest
Length of output: 2218
🏁 Script executed:
# Search for FormTextarea component usage in the codebase
rg -n 'FormTextarea' frontend --type=ts --type=jsRepository: OWASP/Nest
Length of output: 2163
🏁 Script executed:
# Also search without type restriction to capture all usages
rg -n '<FormTextarea' frontend -A 3Repository: OWASP/Nest
Length of output: 4111
🏁 Script executed:
# Check ModuleForm for onBlur usage
rg -n 'onBlur|description' frontend/src/components/ModuleForm.tsx -B 2 -A 5 | head -50Repository: OWASP/Nest
Length of output: 1802
🏁 Script executed:
# Check SponsorApplicationForm for onBlur usage
rg -n 'onBlur|message' frontend/src/components/sponsors/SponsorApplicationForm.tsx -B 2 -A 5 | head -50Repository: OWASP/Nest
Length of output: 1364
🏁 Script executed:
# Check ProgramForm for onBlur usage
rg -n 'onBlur|description' frontend/src/components/ProgramForm.tsx -B 2 -A 5 | head -50Repository: OWASP/Nest
Length of output: 1645
🏁 Script executed:
# Also check test file to see if tests cover onBlur
cat -n frontend/__tests__/unit/components/forms/shared/FormTextarea.test.tsxRepository: OWASP/Nest
Length of output: 2715
🏁 Script executed:
# Check other form input components for onChange/onBlur typing consistency
fd 'Form.*\.tsx' frontend/src/components/forms/shared/ | head -10Repository: OWASP/Nest
Length of output: 325
🏁 Script executed:
# Check FormInput for comparison
cat -n frontend/src/components/forms/shared/FormInput.tsx 2>/dev/null | head -50Repository: OWASP/Nest
Length of output: 36
🏁 Script executed:
# Check FormTextInput for onBlur typing pattern
cat -n frontend/src/components/forms/shared/FormTextInput.tsx | head -60Repository: OWASP/Nest
Length of output: 2224
🏁 Script executed:
# Check FormDateInput for onBlur typing pattern
cat -n frontend/src/components/forms/shared/FormDateInput.tsx | head -60Repository: OWASP/Nest
Length of output: 1657
Widen onBlur prop type to accept React's standard blur handler.
onBlur?: () => void is stricter than necessary and inconsistent with onChange, which accepts a React event. This prevents handlers that need access to the focus event object.
Suggested fix
- onBlur?: () => void
+ onBlur?: React.FocusEventHandler<HTMLTextAreaElement>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| onBlur?: () => void | |
| onBlur?: React.FocusEventHandler<HTMLTextAreaElement> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/forms/shared/FormTextarea.tsx` at line 12, The onBlur
prop on FormTextarea is currently typed as onBlur?: () => void which is too
narrow; update the prop type in the FormTextarea props/interface to accept
React's standard blur handler (e.g.,
React.FocusEventHandler<HTMLTextAreaElement> or (e:
React.FocusEvent<HTMLTextAreaElement>) => void) so consumers receive the
FocusEvent object like onChange does; adjust any usages of FormTextarea.onBlur
to match the new signature and ensure the component forwards the event to the
provided handler.
Comment on lines
+120
to
+128
| <Link | ||
| key={link.text} | ||
| href={link.href || '/'} | ||
| className={cn( | ||
| 'navlink px-3 py-2 text-slate-700 hover:text-slate-800 dark:text-slate-300 dark:hover:text-slate-200', | ||
| pathname === link.href && 'font-bold text-blue-800 dark:text-white' | ||
| )} | ||
| aria-current="page" | ||
| > |
Contributor
There was a problem hiding this comment.
Set aria-current conditionally for the active route only.
At Line 127, every desktop nav link is marked as current page. This breaks screen-reader navigation context.
Suggested fix
- aria-current="page"
+ aria-current={pathname === link.href ? 'page' : undefined}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Link | |
| key={link.text} | |
| href={link.href || '/'} | |
| className={cn( | |
| 'navlink px-3 py-2 text-slate-700 hover:text-slate-800 dark:text-slate-300 dark:hover:text-slate-200', | |
| pathname === link.href && 'font-bold text-blue-800 dark:text-white' | |
| )} | |
| aria-current="page" | |
| > | |
| <Link | |
| key={link.text} | |
| href={link.href || '/'} | |
| className={cn( | |
| 'navlink px-3 py-2 text-slate-700 hover:text-slate-800 dark:text-slate-300 dark:hover:text-slate-200', | |
| pathname === link.href && 'font-bold text-blue-800 dark:text-white' | |
| )} | |
| aria-current={pathname === link.href ? 'page' : undefined} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/Header.tsx` around lines 120 - 128, The desktop nav
Link currently sets aria-current unconditionally, causing every link to be
announced as the current page; update the Link in Header.tsx (the Link element
that uses cn, pathname and link.href) to set aria-current only when pathname ===
link.href (e.g., aria-current={pathname === link.href ? 'page' : undefined}) so
that only the active route has aria-current; ensure you use the same pathname
and link.href comparison used for the active class logic.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/common/management/commands/dump_data.py`:
- Around line 119-122: The UPDATE currently blanks all sponsor emails; modify
the SQL passed to self._execute_sql in dump_data.py so it only updates legacy
NULLs by adding a WHERE contact_email IS NULL clause (i.e., change the statement
used in the call that executes "UPDATE public.owasp_sponsors SET contact_email =
'';" to scope to NULLs only), ensuring only NULL contact_email rows are replaced
while leaving existing non-null emails intact.
In `@frontend/src/components/sponsors/SponsorApplicationForm.tsx`:
- Around line 216-219: The server error paragraph in SponsorApplicationForm (the
JSX block that renders {serverError}) lacks live-region semantics so screen
readers won't announce it; update that <p> to include aria-live="assertive" (or
role="alert") and aria-atomic="true" so assistive tech announces the error
immediately when serverError is set, preserving the existing classes and
content.
- Around line 152-157: In SponsorApplicationForm, the current catch conflates
JSON parse failures with network errors because res.json() can throw for
empty/non-JSON error bodies; modify the error-handling path so you attempt to
safely parse the response body before falling back: when res.ok is false, read
the raw body (e.g., await res.text()), try JSON.parse or a guarded res.json() in
its own try/catch to extract data.message, and if parsing fails use a sensible
fallback like response.statusText or a generic "Something went wrong" message
via setServerError; ensure the outer try/catch only reports network failures
(e.g., when fetch itself throws) so setServerError('Could not reach the
server...') is only used for true network errors.
- Around line 80-82: The website validation treats whitespace-only strings as
present because it checks form.website truthiness; update the check in
SponsorApplicationForm (the block that references form.website and isHttpUrl) to
first trim the value (e.g., use form.website?.trim()) and only run isHttpUrl
when the trimmed string is non-empty, otherwise treat it as empty so the
optional field does not fail validation; ensure errors.website is only set when
the trimmed value is non-empty and fails isHttpUrl.
🪄 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: 2df35d1f-b3ee-46d4-a495-7c1f7a7f66c1
📒 Files selected for processing (5)
backend/apps/api/rest/v0/sponsor.pybackend/apps/common/management/commands/dump_data.pybackend/tests/unit/apps/api/rest/v0/sponsor_test.pybackend/tests/unit/apps/common/management/commands/dump_data_test.pyfrontend/src/components/sponsors/SponsorApplicationForm.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/tests/unit/apps/api/rest/v0/sponsor_test.py
- backend/apps/api/rest/v0/sponsor.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/src/components/sponsors/SponsorApplicationForm.tsx (1)
152-157:⚠️ Potential issue | 🟡 MinorDefensively parse non-OK responses so server errors aren’t mislabeled as network failures.
If
res.json()throws on an empty/non-JSON error body, the outercatchshows “Could not reach the server” even when the server responded.Proposed fix
if (res.ok) { setSubmitted(true) } else { - const data = (await res.json()) as { message?: string } - setServerError(data.message ?? 'Something went wrong. Please try again.') + let message = 'Something went wrong. Please try again.' + try { + const data = (await res.json()) as { message?: string } + if (data?.message) message = data.message + } catch { + // Keep generic server message for non-JSON/empty bodies + } + setServerError(message) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/sponsors/SponsorApplicationForm.tsx` around lines 152 - 157, The current else branch calls await res.json() which can throw and fall into the outer catch (mislabeling server responses as network failures); update the non-OK response handling in SponsorApplicationForm (the block that calls res.json() and setServerError) to defensively parse the body: first try to await res.json() inside its own try/catch, on JSON parse failure fall back to await res.text() (or res.statusText) and construct a meaningful message including res.status/res.statusText, then call setServerError with that message; ensure only true fetch/network exceptions reach the outer catch so "Could not reach the server" remains for real network errors.
🤖 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`:
- Line 19: The test currently patches Sponsor.objects.filter but only checks
sorting; update the test in sponsor_test.py to explicitly assert that
Sponsor.objects.filter was called with the ACTIVE status filter by adding an
assertion that filter was invoked with status=Sponsor.Status.ACTIVE (or the
equivalent enum/member used in the codebase) so regressions removing the status
constraint will fail; reference the mocked call to Sponsor.objects.filter and
use its assert_called_with / assert_any_call (or inspect call_args) to verify
the status argument.
---
Duplicate comments:
In `@frontend/src/components/sponsors/SponsorApplicationForm.tsx`:
- Around line 152-157: The current else branch calls await res.json() which can
throw and fall into the outer catch (mislabeling server responses as network
failures); update the non-OK response handling in SponsorApplicationForm (the
block that calls res.json() and setServerError) to defensively parse the body:
first try to await res.json() inside its own try/catch, on JSON parse failure
fall back to await res.text() (or res.statusText) and construct a meaningful
message including res.status/res.statusText, then call setServerError with that
message; ensure only true fetch/network exceptions reach the outer catch so
"Could not reach the server" remains for real network errors.
🪄 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: 6e0be37d-0ea1-4874-ad0a-5c6def14a638
📒 Files selected for processing (5)
backend/apps/common/management/commands/dump_data.pybackend/apps/owasp/api/internal/queries/sponsor.pybackend/tests/unit/apps/common/management/commands/dump_data_test.pybackend/tests/unit/apps/owasp/api/internal/queries/sponsor_test.pyfrontend/src/components/sponsors/SponsorApplicationForm.tsx
✅ Files skipped from review due to trivial changes (1)
- backend/apps/common/management/commands/dump_data.py
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/tests/unit/apps/common/management/commands/dump_data_test.py
backend/tests/unit/apps/owasp/api/internal/queries/sponsor_test.py
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
1 issue found across 5 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="backend/apps/common/management/commands/dump_data.py">
<violation number="1" location="backend/apps/common/management/commands/dump_data.py:124">
P1: PII masking regression: sponsor `contact_email` is no longer fully scrubbed, so non-null email addresses can leak into generated dumps.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
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.
Resolves #4259
This PR introduces PoC for sponsor management within the Nest platform. It implements both the backend, alongside a clean frontend workflow for sponsors to apply.
Features introduced and features extended
owasp_sponsorsdatabase table to support entity associations , sponsors levels and application status/api/v0/sponsorsthat surfaces active sponsors for external dashboards, and GitHub Actions./sponsorspage and a/sponsors/applypage that has the application form (I am assuming there is no payment gateway needed and sponsorship will be processed externally)Notes
Demo
Checklist
make check-testlocally: all warnings addressed, tests passed