Add additional fields to campaign model #946
Conversation
andrashee
requested review from
mkue,
ssandino and
socialincome-dev
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
This comment was marked as resolved.
This comment was marked as resolved.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
shared/src/types/campaign.ts (1)
21-22: Add JSDoc comments to document the new properties.While the property names are somewhat self-explanatory, adding documentation would help other developers understand their exact purpose and impact.
Consider adding JSDoc comments:
+/** Indicates if the campaign is visible to the public. Defaults to false in the collection. */ public?: boolean; +/** Indicates if the campaign should be highlighted/featured on the campaign overview. Defaults to false in the collection. */ featured?: boolean;admin/src/collections/Campaigns.ts (1)
107-118: Consider adding validation for featured campaignsThe implementation of the
publicandfeaturedflags looks good, with appropriate defaults for security. However, consider adding validation to ensure that featured campaigns must also be public, as it wouldn't make sense to feature a private campaign.featured: { dataType: 'boolean', name: 'Featured', description: 'Featured on campaign overview', defaultValue: false, + validation: { + beforeSave: (value: boolean, values: Campaign) => { + if (value && !values.public) { + throw Error('Featured campaigns must be public'); + } + return value; + } + }, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
admin/src/collections/Campaigns.ts(1 hunks)shared/src/types/campaign.ts(1 hunks)
🔇 Additional comments (2)
shared/src/types/campaign.ts (1)
21-22: Verify the impact of these new flags.
The addition of these optional flags looks good. Let's verify their usage across the codebase to ensure proper handling of undefined values.
admin/src/collections/Campaigns.ts (1)
107-118: Verify type definitions alignment
Let's verify that these new properties are properly reflected in the Campaign type definition.
✅ Verification successful
Type definitions are properly aligned ✅
The Campaign type definition in shared/src/types/campaign.ts already includes both public and featured properties as optional boolean fields, which perfectly aligns with the collection schema definition.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Campaign type includes the new properties
# Expected: Find type definition with public and featured properties
rg -A 10 "type Campaign = {" shared/src/types/campaign.ts
Length of output: 157
Script:
#!/bin/bash
# Let's try a different approach to find Campaign type definition
# First, let's find the relevant type file
fd "campaign.ts|Campaign.ts" --type f
# Then let's use ast-grep to find type definitions
ast-grep --pattern 'type Campaign = {
$$$
}'
# As a fallback, let's also try searching with ripgrep without special chars
rg -l "type Campaign"
Length of output: 1516
|
Visit the preview URL for this PR (updated for commit bb0bc60): https://si-admin-staging--pr946-ahee-campaign-model-3l4u4c3d.web.app (expires Fri, 29 Nov 2024 10:12:32 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: b7b0969384059dce6ea8fad1ee1d1737e54e6676 |
andrashee
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
admin/src/collections/Campaigns.ts(2 hunks)shared/src/types/campaign.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- shared/src/types/campaign.ts
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
admin/src/collections/Campaigns.ts (2)
71-100: Consider implementing URL sanitization for social media linksWhile the implementation structure is good, consider adding URL sanitization to prevent potential security issues like XSS attacks when these URLs are rendered in the frontend.
Consider implementing a URL sanitization utility that:
- Validates protocol (https preferred)
- Sanitizes URL parameters
- Validates against allowed domains for each platform
Would you like me to provide an example implementation of such a utility?
137-148: Enhance visibility flag descriptionsThe current descriptions could be more detailed to help administrators understand the implications of these flags.
Consider updating the descriptions like this:
public: { dataType: 'boolean', name: 'Public', - description: 'Listed on campaign overview', + description: 'When enabled, the campaign will be visible in public campaign listings and searchable by users', defaultValue: false, }, featured: { dataType: 'boolean', name: 'Featured', - description: 'Featured on campaign overview', + description: 'When enabled, the campaign will be highlighted in prominent positions like the homepage or featured sections', defaultValue: false, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
admin/src/collections/Campaigns.ts(2 hunks)
🔇 Additional comments (1)
admin/src/collections/Campaigns.ts (1)
149-159: Implement database-level uniqueness constraint for slugs
While the description mentions that slugs must be unique, there's no mechanism to enforce this at the database level.
Let's check if there are any existing unique constraints or indexes on the slug field:
Consider implementing:
- A Firestore security rule to enforce uniqueness
- A Cloud Function to validate uniqueness before write
- An index on the slug field for efficient lookups
Would you like me to provide implementation details for any of these suggestions?
Summary by CodeRabbit
link_website,link_instagram,link_tiktok,link_facebook, andlink_xfor social media and website links.Public: Indicates if the campaign is listed on the campaign overview.Featured: Indicates if the campaign is highlighted on the campaign overview.Slug: For creating user-friendly URLs.Linktype for better link representation.These enhancements allow for improved visibility and promotion of campaigns within the application.