feat(cow-fi): add Resources section for programmatic CMS content - #7846
feat(cow-fi): add Resources section for programmatic CMS content#7846riboflavin wants to merge 10 commits into
Conversation
Add typed fetch helpers for resources, campaign summaries, and slug lookup without touching the existing Article/Learn pipeline.
Introduce /resources hub, campaign listings, and detail pages for programmatic CMS content.
Expose the new /resources hub from the Help section without adding main nav.
Keep programmatic pages fresh and discoverable alongside existing Learn routes.
Point cow-fi at the locally built CMS client package until the published @cowprotocol/cms release with /resources is available.
Use a concrete font-size so campaign hub and listing pages render.
Avoid BigInt exponentiation that Webpack turns into Math.pow, and instantiate the CMS client locally so content pages do not import the trading/wallet core barrel.
permanentRedirect throws a Next control-flow error; catching it turned wrong-campaign URLs into soft failures instead of redirects.
Use BigInt('...') instead of a BigInt literal, and require browsers that
support bigint so maxAmountSpend no longer crashes Resources pages.
Local file: linking was only for development. Bump after cowprotocol/cms Resource collection is published.
|
@riboflavin is attempting to deploy a commit to the cow-dev Team on Vercel. A member of the Team first needs to authorize it. |
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
WalkthroughAdds CMS-backed resources hub, campaign, and detail pages with ISR, metadata, rich-text rendering, sitemap timestamps, cache refresh, tracking cleanup, footer navigation, and CMS image configuration. It also replaces selected BigInt literals with constructor calls for production build compatibility. ChangesResources feature
BigInt compatibility
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant ResourceRoute
participant CmsService
participant ResourcePageComponent
Browser->>ResourceRoute: request campaign and slug route
ResourceRoute->>CmsService: getResourceBySlug(slug)
CmsService-->>ResourceRoute: resource attributes
ResourceRoute->>ResourcePageComponent: render resource
ResourcePageComponent-->>Browser: resource content and share controls
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
apps/cow-fi/components/ResourcePageComponent.tsx (1)
51-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove unused
contentvariable.
contentis computed by mapping blocks and joining bodies, but is never referenced in the JSX. The blocks are rendered directly on lines 92-94. This is dead code that adds unnecessary computation on every render.♻️ Proposed fix
const description = attributes?.description || '' - const content = - blocks?.map((block: SharedRichTextComponent) => (isRichTextComponent(block) ? block.body : '')).join(' ') || '' const pathname = usePathname()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cow-fi/components/ResourcePageComponent.tsx` around lines 51 - 52, Remove the unused content variable and its associated blocks mapping/joining computation from ResourcePageComponent; keep the existing direct blocks rendering unchanged.apps/cow-fi/services/cms/index.ts (1)
362-389: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
getAllResourceSlugsfetches only the first page — resources beyond 100 items are silently missed.
getAllResourceSlugsissues a single CMS request withpagination[pageSize] = DEFAULT_PAGE_SIZE(100) and no pagination loop. If more than 100 resources exist,generateStaticParamswon't pre-render those pages andgetCampaignSummarieswill undercount or omit campaigns. The sitemap'sgetAllResourceSlugsWithDatescorrectly loops through all pages — consider applying the same pattern here.♻️ Proposed fix: paginate through all pages
export async function getAllResourceSlugs(): Promise<ResourceSlugParam[]> { try { + let allSlugs: ResourceSlugParam[] = [] + let page = 1 + let hasMore = true + + while (hasMore) { const { data, error, response } = await client.GET('/resources', { params: { query: { fields: ['slug', 'campaign'], - 'pagination[pageSize]': DEFAULT_PAGE_SIZE, + 'pagination[page]': page, + 'pagination[pageSize]': DEFAULT_PAGE_SIZE, }, }, querySerializer, ...clientAddons, }) if (error) { console.error(`Error ${response.status} getting resource slugs: ${response.url}`, error) throw error } - return data.data - .filter((resource: Resource) => resource.attributes?.slug && resource.attributes?.campaign) - .map((resource: Resource) => ({ - campaign: resource.attributes!.campaign!, - slug: resource.attributes!.slug!, - })) + const pageSlugs = data.data + .flatMap((resource: Resource) => { + const { slug, campaign } = resource.attributes ?? {} + if (!slug || !campaign) return [] + return [{ campaign, slug }] + }) + allSlugs = allSlugs.concat(pageSlugs) + hasMore = data.meta.pagination.page < data.meta.pagination.pageCount + page++ + } + + return allSlugs } catch (error) { return handleCmsBuildFailure('getAllResourceSlugs', error, []) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cow-fi/services/cms/index.ts` around lines 362 - 389, Update getAllResourceSlugs to paginate through every CMS results page instead of issuing only the initial request with DEFAULT_PAGE_SIZE. Reuse the pagination approach from getAllResourceSlugsWithDates, accumulating valid slug/campaign pairs until no further pages remain, while preserving the existing error handling through handleCmsBuildFailure.apps/cow-fi/components/ResourcesHubComponent.tsx (1)
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
import typefor type-only imports fromservices/cmsin client components.
CampaignSummaryis only used as a type annotation (campaigns: CampaignSummary[]), but the regularimportrisks pulling server-side CMS code into the client bundle.♻️ Proposed fix
-import { CampaignSummary } from '../services/cms' +import type { CampaignSummary } from '../services/cms'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cow-fi/components/ResourcesHubComponent.tsx` at line 9, Update the CampaignSummary import in ResourcesHubComponent to use a type-only import, since it is only referenced in the campaigns type annotation and must not include services/cms runtime code in the client bundle.apps/cow-fi/components/ResourcesCampaignComponent.tsx (1)
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
import typefor type-only imports fromservices/cmsin client components.
Resourceis only used as a type annotation (resources: Resource[]), but the regularimportrisks pulling the CMS client instantiation and server-side code fromservices/cmsinto the client bundle. This aligns with the PR objective of avoiding importing server/trading code into content pages.♻️ Proposed fix
-import { Resource } from '../services/cms' +import type { Resource } from '../services/cms'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cow-fi/components/ResourcesCampaignComponent.tsx` at line 9, Change the Resource import in ResourcesCampaignComponent to a type-only import, preserving its use in the resources: Resource[] annotation and preventing services/cms runtime code from entering the client bundle.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/cow-fi/app/`(main)/resources/[campaign]/[slug]/page.tsx:
- Around line 90-97: Update the redirect condition in the resource page flow to
handle a missing resource.attributes.campaign before comparing it with campaign,
preventing redirects to an undefined campaign path. Preserve the existing
permanentRedirect behavior for valid campaign values that differ from the URL
campaign.
In `@apps/cow-fi/next-sitemap.config.js`:
- Around line 138-139: Update both getAllResourceSlugsWithDates and
getAllArticleSlugsWithDates to resolve the CMS base URL through the shared
CMS_BASE_URL configuration, or preserve the same precedence by adding
REACT_APP_CMS_BASE_URL between NEXT_PUBLIC_CMS_BASE_URL and the hardcoded
default. Ensure both sitemap fetchers use the configured URL when only
REACT_APP_CMS_BASE_URL is set.
In `@apps/cow-fi/services/cms/index.ts`:
- Around line 383-384: Replace the non-null assertions in the campaign mapping
with values narrowed by the filter callback: extract and validate attributes,
campaign, and slug within that callback, then reuse those validated values when
constructing the result. Preserve the existing truthy filtering behavior while
removing every `!` assertion from the affected `resource.attributes` accesses.
---
Nitpick comments:
In `@apps/cow-fi/components/ResourcePageComponent.tsx`:
- Around line 51-52: Remove the unused content variable and its associated
blocks mapping/joining computation from ResourcePageComponent; keep the existing
direct blocks rendering unchanged.
In `@apps/cow-fi/components/ResourcesCampaignComponent.tsx`:
- Line 9: Change the Resource import in ResourcesCampaignComponent to a
type-only import, preserving its use in the resources: Resource[] annotation and
preventing services/cms runtime code from entering the client bundle.
In `@apps/cow-fi/components/ResourcesHubComponent.tsx`:
- Line 9: Update the CampaignSummary import in ResourcesHubComponent to use a
type-only import, since it is only referenced in the campaigns type annotation
and must not include services/cms runtime code in the client bundle.
In `@apps/cow-fi/services/cms/index.ts`:
- Around line 362-389: Update getAllResourceSlugs to paginate through every CMS
results page instead of issuing only the initial request with DEFAULT_PAGE_SIZE.
Reuse the pagination approach from getAllResourceSlugsWithDates, accumulating
valid slug/campaign pairs until no further pages remain, while preserving the
existing error handling through handleCmsBuildFailure.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b100484d-cdf9-4ba7-af8b-963aac0a4239
📒 Files selected for processing (18)
apps/cow-fi/app/(main)/resources/[campaign]/[slug]/page.tsxapps/cow-fi/app/(main)/resources/[campaign]/page.tsxapps/cow-fi/app/(main)/resources/page.tsxapps/cow-fi/app/api/revalidate/route.tsapps/cow-fi/components/CmsImage/index.tsxapps/cow-fi/components/ResourcePageComponent.tsxapps/cow-fi/components/ResourcesCampaignComponent.tsxapps/cow-fi/components/ResourcesHubComponent.tsxapps/cow-fi/const/resources.tsapps/cow-fi/middleware.tsapps/cow-fi/next-sitemap.config.jsapps/cow-fi/next.config.tsapps/cow-fi/package.jsonapps/cow-fi/services/cms/config.tsapps/cow-fi/services/cms/helpers.tsapps/cow-fi/services/cms/index.tslibs/common-utils/src/maxAmountSpend.tslibs/ui/src/containers/Footer/footer.constants.ts
| if (!resource?.attributes) { | ||
| return notFound() | ||
| } | ||
|
|
||
| // Keep outside try/catch — permanentRedirect throws a control-flow error Next must handle | ||
| if (resource.attributes.campaign !== campaign) { | ||
| permanentRedirect(`/resources/${resource.attributes.campaign}/${slug}`) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Missing null check for resource.attributes.campaign before redirect comparison.
If campaign is undefined or null, the condition resource.attributes.campaign !== campaign evaluates to true (since campaign from the URL is always a string), causing a redirect to /resources/undefined/${slug}. The downstream ResourcePageComponent explicitly guards against falsy campaign (if (!campaign || !attributes?.slug) return null), confirming this case is reachable.
🐛 Proposed fix: guard against missing campaign before redirect
if (!resource?.attributes) {
return notFound()
}
+ if (!resource.attributes.campaign) {
+ return notFound()
+ }
+
// Keep outside try/catch — permanentRedirect throws a control-flow error Next must handle
if (resource.attributes.campaign !== campaign) {
permanentRedirect(`/resources/${resource.attributes.campaign}/${slug}`)
}📝 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.
| if (!resource?.attributes) { | |
| return notFound() | |
| } | |
| // Keep outside try/catch — permanentRedirect throws a control-flow error Next must handle | |
| if (resource.attributes.campaign !== campaign) { | |
| permanentRedirect(`/resources/${resource.attributes.campaign}/${slug}`) | |
| } | |
| if (!resource?.attributes) { | |
| return notFound() | |
| } | |
| if (!resource.attributes.campaign) { | |
| return notFound() | |
| } | |
| // Keep outside try/catch — permanentRedirect throws a control-flow error Next must handle | |
| if (resource.attributes.campaign !== campaign) { | |
| permanentRedirect(`/resources/${resource.attributes.campaign}/${slug}`) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/cow-fi/app/`(main)/resources/[campaign]/[slug]/page.tsx around lines 90
- 97, Update the redirect condition in the resource page flow to handle a
missing resource.attributes.campaign before comparing it with campaign,
preventing redirects to an undefined campaign path. Preserve the existing
permanentRedirect behavior for valid campaign values that differ from the URL
campaign.
| async function getAllResourceSlugsWithDates() { | ||
| const cmsBaseUrl = process.env.NEXT_PUBLIC_CMS_BASE_URL || 'https://cms.cow.fi/api' |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Missing REACT_APP_CMS_BASE_URL fallback in sitemap fetcher.
getAllResourceSlugsWithDates resolves the CMS base URL with only NEXT_PUBLIC_CMS_BASE_URL and a hardcoded default, omitting the REACT_APP_CMS_BASE_URL fallback that config.ts includes. If only REACT_APP_CMS_BASE_URL is configured, the sitemap fetcher would use the wrong CMS URL. Note: the existing getAllArticleSlugsWithDates (line 96) has the same gap, so this is a pre-existing pattern — but it's worth fixing for consistency.
🔧 Proposed fix: import CMS_BASE_URL from config
- const cmsBaseUrl = process.env.NEXT_PUBLIC_CMS_BASE_URL || 'https://cms.cow.fi/api'
+ const cmsBaseUrl = CMS_BASE_URLThis requires importing CMS_BASE_URL from the config at the top of the file. If that import is not feasible in the sitemap config context, add the REACT_APP_CMS_BASE_URL fallback:
- const cmsBaseUrl = process.env.NEXT_PUBLIC_CMS_BASE_URL || 'https://cms.cow.fi/api'
+ const cmsBaseUrl = process.env.NEXT_PUBLIC_CMS_BASE_URL || process.env.REACT_APP_CMS_BASE_URL || 'https://cms.cow.fi/api'📝 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.
| async function getAllResourceSlugsWithDates() { | |
| const cmsBaseUrl = process.env.NEXT_PUBLIC_CMS_BASE_URL || 'https://cms.cow.fi/api' | |
| async function getAllResourceSlugsWithDates() { | |
| const cmsBaseUrl = process.env.NEXT_PUBLIC_CMS_BASE_URL || process.env.REACT_APP_CMS_BASE_URL || 'https://cms.cow.fi/api' |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/cow-fi/next-sitemap.config.js` around lines 138 - 139, Update both
getAllResourceSlugsWithDates and getAllArticleSlugsWithDates to resolve the CMS
base URL through the shared CMS_BASE_URL configuration, or preserve the same
precedence by adding REACT_APP_CMS_BASE_URL between NEXT_PUBLIC_CMS_BASE_URL and
the hardcoded default. Ensure both sitemap fetchers use the configured URL when
only REACT_APP_CMS_BASE_URL is set.
| campaign: resource.attributes!.campaign!, | ||
| slug: resource.attributes!.slug!, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Non-null assertions violate coding guidelines.
Lines 383-384 use resource.attributes!.campaign! and resource.attributes!.slug!. The coding guidelines for **/*.{ts,tsx,js,jsx} state: "MUST NOT use any or non-null assertions (!) in production code." The filter on line 381 guarantees truthiness at runtime, but TypeScript cannot narrow through the callback.
🔧 Proposed fix: extract values in the filter callback
return data.data
- .filter((resource: Resource) => resource.attributes?.slug && resource.attributes?.campaign)
- .map((resource: Resource) => ({
- campaign: resource.attributes!.campaign!,
- slug: resource.attributes!.slug!,
- }))
+ .flatMap((resource: Resource) => {
+ const { slug, campaign } = resource.attributes ?? {}
+ if (!slug || !campaign) return []
+ return [{ campaign, slug }]
+ })📝 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.
| campaign: resource.attributes!.campaign!, | |
| slug: resource.attributes!.slug!, | |
| .flatMap((resource: Resource) => { | |
| const { slug, campaign } = resource.attributes ?? {} | |
| if (!slug || !campaign) return [] | |
| return [{ campaign, slug }] | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/cow-fi/services/cms/index.ts` around lines 383 - 384, Replace the
non-null assertions in the campaign mapping with values narrowed by the filter
callback: extract and validate attributes, campaign, and slug within that
callback, then reuse those validated values when constructing the result.
Preserve the existing truthy filtering behavior while removing every `!`
assertion from the affected `resource.attributes` accesses.
Source: Coding guidelines
fairlighteth
left a comment
fairlighteth
left a comment
There was a problem hiding this comment.
⚠️ AI Review (Codex GPT-5, worked 28m): resource path revalidation is broken; valid campaign labels can crash
Finding: [BLOCKING] Resource-specific revalidation requests are rejected
- Location:
apps/cow-fi/app/api/revalidate/route.ts:39 - This one is important: an authenticated payload containing
/resources/<campaign>/<slug>is rejected by the/learn-only allowlist beforerevalidateTagorrevalidatePathruns. I reproducedUnsupported revalidation path "/resources/tokens/example". - The new dynamic calls at lines 56–57 also omit the
page/layoutargument; Next 15.5.18 warns that dynamic patterns without it have no effect by default. - Receipt: PR #7618 deliberately introduced the narrow allowlist as a Medium-security remediation, so it should be extended precisely rather than bypassed.
Suggested fix
- Allow exactly
/resources,/resources/<campaign>, and/resources/<campaign>/<slug>using the canonical CMS slug grammar. - Revalidate the accepted concrete path, and either pass
'page'for dynamic patterns or invalidate the/resourceslayout. - Add validator and authenticated route-handler tests for resource payloads.
Finding: [NON-BLOCKING] A schema-valid constructor campaign crashes metadata generation
- Location:
apps/cow-fi/const/resources.ts:6 - CMS PR #94 permits lowercase campaign slugs, so
constructoris valid. The plain-object lookup returns the inheritedObjectconstructor instead of a string;generateMetadatathen throws atlabel.toLowerCase(). - I reproduced the returned value as a function and the resulting
label.toLowerCase is not a function. - This is separate from the existing missing-campaign guard comment because the campaign is present and schema-valid.
Suggested fix
- Use a
Map<string, string>or an own-property check before returning a configured label. - Add coverage for known, unknown, hyphenated, and
constructorcampaign names.
Review scope and related context
- Reviewed current head
4f15eb3against base1ab0190. - Harness used: DeepSec PR mode over the exact 18 changed files. Source: DeepSec output verified against current code. Command used
pnpm dlx deepsec@2.0.12 process --files-from … --comment-out /tmp/deepsec-comment-cowswap-7846-head.md; exit1, run20260714151805-29e774a8c9a75754. - Security result: no
verified-vulnerabilityfindings. DeepSec produced three BUG candidates; two became the correctness findings above, while the native-symbol ERC-20 candidate predates this PR. - Automated verification:
cow-fi:lintpassed; the focused CMS validation/Markdown suites passed, 18/18 tests. - Targeted typecheck is not green:
@cowprotocol/cms0.11.0 lacksResourceListResponseDataItemand/resources, matching the dependency already disclosed in the PR body. Re-run after the CMS release and dependency bump. - Existing CodeRabbit feedback already covers the missing campaign guard, sitemap environment fallback, prohibited non-null assertions, unused content, type-only imports, and first-page slug pagination.
- When resolving that pagination feedback, also cover
ResourcesCampaignPage -> getResources; otherwise the rendered campaign listing still stops at 100 items. - No author replies or resolved threads required follow-up verification.
🤖 Prompt for AI agents
Verify these findings against the current PR head and keep the fixes minimal.
1. Extend the existing revalidation allowlist with exact resource route patterns without weakening the security boundary introduced in PR #7618.
2. Ensure dynamic revalidatePath calls pass the required page/layout type, or invalidate the resources layout.
3. Replace inherited-property-prone campaign label lookup with a Map or own-property-safe lookup.
4. Add focused tests for resource revalidation payloads and the "constructor" campaign case.
5. While addressing the existing pagination feedback, verify that the campaign listing itself can render beyond DEFAULT_PAGE_SIZE.
Generated using the pr-review and security-review skills from the CoW Protocol skills repo.
2 participants
Summary
/resources— campaign hub/resources/[campaign]— campaign listing/resources/[campaign]/[slug]— detail (redirects if campaign mismatches)/resources(no main nav).lastmod, middleware tracking-param stripping, and cache headers for/resources.@cowprotocol/coretrading code into content pages).maxAmountSpendso Webpack/Babel does not crash content pages that import@cowprotocol/common-utils.Depends on
@cowprotocol/cmsis published, bumpapps/cow-fi+libs/coreoff0.11.0to the new version (typed/resourcesendpoints). Until then, typecheck against Resource schemas may fail.Summary by CodeRabbit
New Features
Bug Fixes