refactor(api/web): use cuhacking logo on cms instead of local asset #360
Conversation
WalkthroughThis PR updates several components and configurations across the codebase. The layout component now uses a dynamic logo URL from loader data instead of a static import. A new Shell icon is added for the portal in the nav icon map. Global configurations have been updated by removing the old Hackathon config and introducing new Website and Portal configurations, which are then re-exported appropriately. Numerous database collection files have refined their access controls and field configurations, and new access functions have been added to streamline permission checks. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant AccessCtrl
participant DB
Client->>API: Submit data update request
API->>AccessCtrl: Evaluate permissions (isSelf, isOrganizer, isSuperAdmin, etc.)
AccessCtrl-->>API: Permission granted/denied
API->>DB: Execute operation if permitted
DB-->>API: Return operation result
API-->>Client: Response with result
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
Progress: resolved 0, reused 0, downloaded 1, added 0 This error happened while installing a direct dependency of /tmp/eslint Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
HasithDeAlwis
force-pushed
the
hasith/refactor/web/359-use-cuhacking-logo-on-cms-instead-of-local-asset
branch
from
79331c3 to
71f18ad
Compare
|
Take a look at the first commit. Important change |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
apps/axiom/src/components/Nav/navIconMap.ts(2 hunks)libs/cms/configs/server.ts(2 hunks)libs/db/globals/Portal.ts(1 hunks)libs/db/globals/Website.ts(4 hunks)libs/db/globals/index.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (22.13.1)
- GitHub Check: test
🔇 Additional comments (13)
apps/axiom/src/components/Nav/navIconMap.ts (2)
15-15: Added Shell icon import from lucide-reactClean addition of the Shell icon import to support the new portal feature.
27-27: Added portal mapping to Shell iconGood implementation of the icon mapping for the new portal feature. This correctly maps the "portal" slug to the Shell icon.
libs/db/globals/index.ts (1)
1-2: Updated global exportsClean replacement of the previous Hackathon export with more granular Website and Portal exports. This refactoring improves modularity.
libs/cms/configs/server.ts (2)
20-20: Updated imports to use Website and Portal modulesThe import statement now correctly references the new Website and Portal modules instead of the previous Hackathon2025 module.
32-33: Updated globals configuration to use Website and PortalThe globals array has been properly updated to use the new Website and Portal modules, which aligns with the overall refactoring approach.
libs/db/globals/Portal.ts (4)
6-9: Access control configuration with commented-out update rulesAccess control is set to allow read for all users, but the update rule is commented out. This might indicate incomplete implementation.
Is the access control implementation for update operations intentionally omitted? Consider either implementing proper update rules or removing the commented code if not needed.
14-24: Live preview configuration with appropriate breakpointsWell-structured live preview configuration that correctly uses environment variables to determine the URL dynamically.
29-57: Dashboard cards configurationGood implementation of the dashboard cards with appropriate default values. The collapsible structure makes the admin interface cleaner.
138-146: Basic profile and registration fieldsThe profile and registration fields are very basic (simple text fields). Consider whether these need more complex configuration based on your requirements.
Are these placeholder fields intended to be expanded later? If these are meant to be functional fields, they might need more detailed configuration including proper field types, validation, etc.
libs/db/globals/Website.ts (4)
2-2: Import looks correct.
No issues observed with this import statement.
4-4: Confirm all references to the renamed global.
Ensure there are no references still pointing to the oldHackathon2025export name.
15-15: Validate environment variables.
Double-check that bothCUHACKING_2025_WEBSITE_LOCAL_URLandCUHACKING_2025_WEBSITE_PUBLIC_URLare defined for all relevant environments.
90-90: No issues here.
The field is clearly named and aligns with existing naming.
libs/db/globals/Portal.ts
Outdated
| defaultValue: [ | ||
| { | ||
| title: "MLH Code of Conduct", | ||
| text: | ||
| "TL;DR. Be respectful. Harassment and abuse are never tolerated. If you are in a situation that makes you uncomfortable at " + | ||
| "an MLH Member Event, if the event itself creates an unsafe or inappropriate environment, or if interacting with an MLH " + | ||
| "representative or event organizer makes you uncomfortable, please report it using the procedures included in this document.\n\n" + | ||
| "Major League Hacking (MLH) stands for inclusivity. We believe that every single person has the right to hack in a safe and " + | ||
| "welcoming environment.", | ||
| checkbox: false, | ||
| checkboxLabel: "I have read MLH Code of Conduct *", | ||
| }, | ||
| { | ||
| title: "MLH Terms & Conditions", | ||
| text: "", | ||
| checkbox: false, | ||
| checkboxLabel: "I have read MLH Terms & Conditions", | ||
| }, | ||
| { | ||
| title: "MLH Privacy Policy", | ||
| text: "", | ||
| checkbox: false, | ||
| checkboxLabel: "I have read MLH Privacy Policy", | ||
| }, | ||
| { | ||
| title: "cuHacking Terms & Conditions", | ||
| text: "", | ||
| checkbox: false, | ||
| checkboxLabel: "I have read cuHacking Terms & Conditions", | ||
| }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Terms & Conditions implementation
The Terms & Conditions section has a few issues:
- The MLH Code of Conduct text is hardcoded as a long string
- Several terms sections (MLH Terms & Conditions, MLH Privacy Policy, cuHacking Terms & Conditions) have empty text fields
Consider these improvements:
- Move the hardcoded text to constants or external files for better maintainability
- Fill in the missing content for the empty text fields or implement a mechanism to load them dynamically
- text:
- "TL;DR. Be respectful. Harassment and abuse are never tolerated. If you are in a situation that makes you uncomfortable at " +
- "an MLH Member Event, if the event itself creates an unsafe or inappropriate environment, or if interacting with an MLH " +
- "representative or event organizer makes you uncomfortable, please report it using the procedures included in this document.\n\n" +
- "Major League Hacking (MLH) stands for inclusivity. We believe that every single person has the right to hack in a safe and " +
- "welcoming environment.",
+ text: MLH_CODE_OF_CONDUCT_TEXT, // Define this constant at the top of the file or import from a constants file📝 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.
| defaultValue: [ | |
| { | |
| title: "MLH Code of Conduct", | |
| text: | |
| "TL;DR. Be respectful. Harassment and abuse are never tolerated. If you are in a situation that makes you uncomfortable at " + | |
| "an MLH Member Event, if the event itself creates an unsafe or inappropriate environment, or if interacting with an MLH " + | |
| "representative or event organizer makes you uncomfortable, please report it using the procedures included in this document.\n\n" + | |
| "Major League Hacking (MLH) stands for inclusivity. We believe that every single person has the right to hack in a safe and " + | |
| "welcoming environment.", | |
| checkbox: false, | |
| checkboxLabel: "I have read MLH Code of Conduct *", | |
| }, | |
| { | |
| title: "MLH Terms & Conditions", | |
| text: "", | |
| checkbox: false, | |
| checkboxLabel: "I have read MLH Terms & Conditions", | |
| }, | |
| { | |
| title: "MLH Privacy Policy", | |
| text: "", | |
| checkbox: false, | |
| checkboxLabel: "I have read MLH Privacy Policy", | |
| }, | |
| { | |
| title: "cuHacking Terms & Conditions", | |
| text: "", | |
| checkbox: false, | |
| checkboxLabel: "I have read cuHacking Terms & Conditions", | |
| }, | |
| ], | |
| }, | |
| defaultValue: [ | |
| { | |
| title: "MLH Code of Conduct", | |
| text: MLH_CODE_OF_CONDUCT_TEXT, // Define this constant at the top of the file or import from a constants file | |
| checkbox: false, | |
| checkboxLabel: "I have read MLH Code of Conduct *", | |
| }, | |
| { | |
| title: "MLH Terms & Conditions", | |
| text: "", | |
| checkbox: false, | |
| checkboxLabel: "I have read MLH Terms & Conditions", | |
| }, | |
| { | |
| title: "MLH Privacy Policy", | |
| text: "", | |
| checkbox: false, | |
| checkboxLabel: "I have read MLH Privacy Policy", | |
| }, | |
| { | |
| title: "cuHacking Terms & Conditions", | |
| text: "", | |
| checkbox: false, | |
| checkboxLabel: "I have read cuHacking Terms & Conditions", | |
| }, | |
| ], | |
| }, |
MFarabi619
force-pushed
the
hasith/refactor/web/359-use-cuhacking-logo-on-cms-instead-of-local-asset
branch
from
e0d0bbf to
b82a895
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
apps/axiom/src/components/Nav/navIconMap.ts(2 hunks)libs/cms/configs/server.ts(2 hunks)libs/db/globals/Portal.ts(1 hunks)libs/db/globals/Website.ts(4 hunks)libs/db/globals/index.ts(1 hunks)libs/website/layouts/base.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (22.13.1)
- GitHub Check: test
🔇 Additional comments (8)
apps/axiom/src/components/Nav/navIconMap.ts (2)
15-15: Added Shell icon import correctlyThe Shell icon is imported from lucide-react library to support the new portal navigation item.
27-27: New portal navigation mapping addedThis entry correctly maps the "portal" slug to the Shell icon for the navigation system.
libs/db/globals/index.ts (1)
1-2: Updated global exports to reflect new architectureReplaced the Hackathon export with Website and Portal exports, which aligns with the PR objective of using CMS-based assets instead of local ones.
libs/cms/configs/server.ts (2)
20-20: Updated imports for new global configurationReplaced Hackathon2025 import with Website and Portal imports, which aligns with the changes in the globals index file.
32-33: Updated globals configuration in baseConfigReplaced Hackathon2025 with Website and Portal in the globals array, which correctly implements the server-side changes needed for the new CMS structure.
libs/website/layouts/base.tsx (2)
11-11: Updated loader data type to include logo informationThe type definition now includes the logo object containing a URL, which is needed to support loading the logo from CMS.
16-16: Replaced hardcoded logo with CMS-sourced logoThis change implements the main objective of the PR by using the logo URL from the CMS instead of a local asset.
Consider adding a fallback in case the logo URL is undefined:
- logo={header.logo.url} + logo={header.logo?.url || '/default-logo.png'}libs/db/globals/Portal.ts (1)
105-133: Terms & Conditions text is hardcoded.
libs/db/globals/Portal.ts
Outdated
| // update: ({ req }) => req.user?.roles?.some(role => role.admin === true || role.name === 'owner'), | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Clean up commented out access rule.
Consider removing or clarifying this commented out access rule to avoid confusion. If it's intended for future use, add a brief comment or a TODO note explaining its purpose.
MFarabi619
force-pushed
the
hasith/refactor/web/359-use-cuhacking-logo-on-cms-instead-of-local-asset
branch
from
b82a895 to
c70a984
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
libs/db/globals/Portal.ts (3)
8-8: 🧹 Nitpick (assertive)Clean up commented out access rule
Consider removing or clarifying this commented out access rule to avoid confusion. If it's intended for future use, add a brief comment or a TODO note explaining its purpose.
15-15: 🛠️ Refactor suggestionAdd fallback for environment variables in live preview URL
The live preview URL lacks fallbacks for undefined environment variables which could cause runtime issues.
- url: process.env.NODE_ENV === "development" ? process.env.CUHACKING_2025_PORTAL_LOCAL_URL : process.env.CUHACKING_2025_PORTAL_PUBLIC_URL, + url: process.env.NODE_ENV === "development" + ? process.env.CUHACKING_2025_PORTAL_LOCAL_URL || "http://localhost:3000" + : process.env.CUHACKING_2025_PORTAL_PUBLIC_URL || "https://portal.cuhacking.ca",
103-134: 🧹 Nitpick (assertive)Improve Terms & Conditions implementation
The Terms & Conditions section has several issues:
- MLH Code of Conduct text is hardcoded as a long string
- Several terms sections have empty text fields
Consider moving the hardcoded text to constants or external files for better maintainability.
+const MLH_CODE_OF_CONDUCT_TEXT = + "TL;DR. Be respectful. Harassment and abuse are never tolerated. If you are in a situation that makes you uncomfortable at " + + "an MLH Member Event, if the event itself creates an unsafe or inappropriate environment, or if interacting with an MLH " + + "representative or event organizer makes you uncomfortable, please report it using the procedures included in this document.\n\n" + + "Major League Hacking (MLH) stands for inclusivity. We believe that every single person has the right to hack in a safe and " + + "welcoming environment."; // In the defaultValue array { title: "MLH Code of Conduct", - text: - "TL;DR. Be respectful. Harassment and abuse are never tolerated. If you are in a situation that makes you uncomfortable at " + - "an MLH Member Event, if the event itself creates an unsafe or inappropriate environment, or if interacting with an MLH " + - "representative or event organizer makes you uncomfortable, please report it using the procedures included in this document.\n\n" + - "Major League Hacking (MLH) stands for inclusivity. We believe that every single person has the right to hack in a safe and " + - "welcoming environment.", + text: MLH_CODE_OF_CONDUCT_TEXT,libs/db/globals/Website.ts (3)
8-8: 🧹 Nitpick (assertive)Clean up commented out access rule
Consider removing or clarifying this commented out access rule to avoid confusion. If it's intended for future use, add a brief comment or a TODO note explaining its purpose.
15-15: 🛠️ Refactor suggestionAdd fallback for environment variables in live preview URL
The live preview URL lacks fallbacks for undefined environment variables which could cause runtime issues.
- url: process.env.NODE_ENV === "development" ? process.env.CUHACKING_2025_WEBSITE_LOCAL_URL : process.env.CUHACKING_2025_WEBSITE_PUBLIC_URL, + url: process.env.NODE_ENV === "development" + ? process.env.CUHACKING_2025_WEBSITE_LOCAL_URL || "http://localhost:3000" + : process.env.CUHACKING_2025_WEBSITE_PUBLIC_URL || "https://cuhacking.ca",
41-50: 🧹 Nitpick (assertive)Handle multiple or missing media documents
The current implementation assumes the first matching document is available, but doesn't validate if no document or multiple documents match the query.
defaultValue: async ({ req }) => { const media = await req.payload.find({ collection: "media", where: { filename: { equals: "cuHacking_2025_primary_logo.svg" } } }); - return media?.docs?.[0]?.id || null; + if (!media?.docs?.length) { + // Either return a default asset ID or null + return null; + } + // Optionally handle multiple results + if (media.docs.length > 1) { + console.warn(`Multiple logos found with filename: cuHacking_2025_primary_logo.svg`); + } + return media.docs[0].id; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
apps/axiom/src/components/Nav/navIconMap.ts(2 hunks)libs/cms/configs/server.ts(2 hunks)libs/db/collections/OrganizerTeams/index.ts(3 hunks)libs/db/collections/models/Groups.ts(1 hunks)libs/db/globals/Hackathon.ts(0 hunks)libs/db/globals/Portal.ts(1 hunks)libs/db/globals/Website.ts(1 hunks)libs/db/globals/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- libs/db/globals/Hackathon.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (8)
apps/axiom/src/components/Nav/navIconMap.ts (2)
15-15: New Shell icon import added correctlyThe Shell icon import is properly added to the existing lucide-react imports.
27-27: Portal navigation icon mapping looks goodThe new entry for the portal slug using the Shell icon follows the existing pattern in the navigation icon map.
libs/db/collections/models/Groups.ts (1)
11-11: Enhanced admin columns with timestampsAdding the updatedAt and createdAt columns to the default view provides better context for each group record in the admin interface.
libs/db/collections/OrganizerTeams/index.ts (3)
9-9: Enhanced admin columns with timestampsAdding the updatedAt and createdAt columns to the default view provides better context for each organizer team in the admin interface.
18-21: Field definitions formatting changeThe change condenses multi-line field definitions to single-line. Functionally equivalent, just a formatting preference.
30-34: Team data formatting changeThe change condenses multi-line team definitions to single-line. Maintains the same functionality with more compact syntax.
libs/cms/configs/server.ts (2)
20-20: Clean import statement added for Website and PortalThe addition of these new global configs from
@/db/globalsis well structured and aligns with the refactoring to use CMS-based assets instead of local ones.
32-33: Updated globals configurationReplacing
Hackathon2025with the newWebsiteandPortalglobals is appropriate. This change supports the PR objective to use CMS-managed assets.
libs/db/globals/index.ts
Outdated
| export * from './Website' | ||
| export * from './Portal' |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Exports updated to use new Website and Portal modules
The change replaces the previous Hackathon export with the new Website and Portal exports, aligning with the refactoring to use CMS logo instead of local assets.
🏁 Script executed:
#!/bin/bash
# Check if there are any remaining references to Hackathon that might need updating
rg -l "import.*Hackathon" --type tsLength of output: 236
Action Required: Update Remaining Hackathon References
- The updated exports in
libs/db/globals/index.tscorrectly now re-export from./Websiteand./Portalas part of the refactoring. - However, the following files still reference
Hackathon, which may indicate outdated imports:apps/portal-e2e/src/registration/hacker/index.spec.tsapps/portal-e2e/src/index.spec.tslibs/portal/ui/dashboard/hackathon-countdown/hackathon-countdown.stories.tsxlibs/portal/pages/index/index.tsx
- Please verify whether these references are still valid (perhaps for legacy tests or stories) or if they need to be updated to align with the new Website and Portal modules.
libs/db/globals/Website.ts
Outdated
| // export async const seedPlatforms ={ | ||
| // // [ | ||
| // // { | ||
| // // "website":{ | ||
| // // logoAlt: "cuhacking-symbol-green" | ||
| // // }, | ||
| // // } | ||
| // // ] | ||
| // } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove commented out code
The commented-out seedPlatforms section at the end of the file should be removed if it's not needed, or properly implemented if it is.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
libs/db/access/index.ts(2 hunks)libs/db/collections/OrganizerTeams/index.ts(2 hunks)libs/db/collections/models/Groups.ts(1 hunks)libs/db/globals/Portal.ts(1 hunks)libs/db/globals/Website.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (14)
libs/db/collections/OrganizerTeams/index.ts (3)
5-9: More specific imports improve maintainabilityThe change from importing the generic
authenticatedaccess control to specific functions (isSelf,isOrganizer,isSuperAdmin) better reflects the actual usage in the code and follows best practices.
18-23: Improved security with granular access controlsThe access control changes enhance security by:
- Limiting read access to organizers instead of any authenticated user
- Restricting create/delete operations to super admins
- Allowing users to update their own records with the new
isSelfcheckThis provides better security boundaries for the application.
24-29: Better field organization with sidebar positioningAdding the
admin: {position: 'sidebar'}property improves the CMS UI organization by moving these fields to the sidebar for better visibility and user experience.libs/db/globals/Portal.ts (2)
20-20: Add fallback for environment variablesThe live preview URL configuration doesn't handle cases where environment variables might be undefined.
108-139: Consider refactoring Terms & Conditions implementationThe current implementation has a few issues:
- The MLH Code of Conduct text is hardcoded as a long string
- Several terms sections (MLH Terms & Conditions, MLH Privacy Policy, cuHacking Terms & Conditions) have empty text fields
libs/db/access/index.ts (1)
11-12: Simplified return structure in anyone functionThe function now returns the user ID directly rather than wrapping it in an equality condition. This aligns with how it's used in access controls.
libs/db/globals/Website.ts (4)
20-20: Add fallback for environment variables in live preview URL
46-55: Handle multiple or missing media documentsThe logo defaultValue function doesn't handle the case where multiple matching documents might be found.
45-56: Dynamically fetching logo from CMSGood implementation of fetching the logo from the CMS instead of using a local asset, aligning with the PR's main objective. This makes logo management more centralized and easier to update.
188-196: Remove commented out codeThe commented-out
seedPlatformssection at the end of the file should be removed if it's not needed.libs/db/collections/models/Groups.ts (4)
3-7: Good modular import approach.Clean import structure for access control functions. This follows best practices by importing only the specific functions needed.
12-15: Improved granular access controls.Replacing the generic admin function with specific access controls improves security by clearly defining permissions for each operation. This aligns with the principle of least privilege.
19-19: Enhanced admin view with timestamp columns.Adding
updatedAtandcreatedAtto the default columns provides better visibility into the data timeline for administrators.
27-27: UI improvement with sidebar positioning.Moving the symbol field to the sidebar enhances the admin interface layout.
libs/db/globals/Portal.ts
Outdated
| name: "description", | ||
| type: "text", | ||
| label: "Description", | ||
| defaultValue: "YO! Before we get into it, read these please.", |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider more professional wording for legal content
The default description "YO! Before we get into it, read these please." is very informal for legal content. Consider using more professional language for this section.
- defaultValue: "YO! Before we get into it, read these please.",
+ defaultValue: "Please review the following legal agreements before proceeding.",📝 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.
| defaultValue: "YO! Before we get into it, read these please.", | |
| defaultValue: "Please review the following legal agreements before proceeding.", |
libs/db/access/index.ts
Outdated
| export const isSelf = ({ req: {user} }) => { | ||
| if (!user) return false; | ||
| if (user){ | ||
| return { | ||
| id: { | ||
| equals: user.id | ||
| } | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Fix code formatting and structure in isSelf function
The new isSelf function has inconsistent formatting and structure issues:
- Inconsistent indentation
- Unnecessary nested condition and braces
- Extra closing brace at line 35
export const isSelf = ({ req: {user} }) => {
if (!user) return false;
- if (user){
return {
id: {
equals: user.id
}
}
- };
}📝 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.
| export const isSelf = ({ req: {user} }) => { | |
| if (!user) return false; | |
| if (user){ | |
| return { | |
| id: { | |
| equals: user.id | |
| } | |
| } | |
| }; | |
| } | |
| export const isSelf = ({ req: {user} }) => { | |
| if (!user) return false; | |
| return { | |
| id: { | |
| equals: user.id | |
| } | |
| } | |
| } |
libs/db/collections/models/Groups.ts
Outdated
| { | ||
| type: "row", | ||
| admin:{position:'sidebar'}, | ||
| fields:[ | ||
| { | ||
| name: "name", | ||
| type: "text", | ||
| }, | ||
| { | ||
| name: "event", | ||
| type: "relationship", | ||
| relationTo: "hackathons", | ||
| }, | ||
| ] | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improved field grouping with row layout.
Restructuring the fields with a row layout and sidebar positioning creates a more organized admin interface. However, there's inconsistent indentation in this section.
Consider fixing the indentation for better readability:
{
type: "row",
admin:{position:'sidebar'},
fields:[
- {
- name: "name",
- type: "text",
- },
- {
- name: "event",
- type: "relationship",
- relationTo: "hackathons",
- },
+ {
+ name: "name",
+ type: "text",
+ },
+ {
+ name: "event",
+ type: "relationship",
+ relationTo: "hackathons",
+ },
]
},📝 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.
| { | |
| type: "row", | |
| admin:{position:'sidebar'}, | |
| fields:[ | |
| { | |
| name: "name", | |
| type: "text", | |
| }, | |
| { | |
| name: "event", | |
| type: "relationship", | |
| relationTo: "hackathons", | |
| }, | |
| ] | |
| }, | |
| { | |
| type: "row", | |
| admin:{position:'sidebar'}, | |
| fields:[ | |
| { | |
| name: "name", | |
| type: "text", | |
| }, | |
| { | |
| name: "event", | |
| type: "relationship", | |
| relationTo: "hackathons", | |
| }, | |
| ] | |
| }, |
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
libs/cms/configs/server.ts (1)
30-139: 💡 Verification agent🧩 Analysis chain
Verify all references to old Hackathon2025 global are updated
The change from Hackathon2025 to Website and Portal is significant. Ensure all code that previously accessed properties from Hackathon2025 has been updated to use the appropriate properties from Website or Portal.
🏁 Script executed:
#!/bin/bash # Search for any remaining references to Hackathon2025 that might need updating rg -i "Hackathon2025" --type ts --type tsxLength of output: 70
I'll need to further verify that there are no remaining references to the old
Hackathon2025global because the initialrgcommand failed to properly search TSX files. Please run the following script to check both *.ts and *.tsx files:Once you confirm that there are no remaining references (or update any that still exist to use
WebsiteorPortal), you can finalize the changes.Action: Make sure all code previously referring to
Hackathon2025now uses the appropriate properties fromWebsiteorPortal.
🏁 Script executed:
#!/bin/bash # Search for any remaining references to Hackathon2025 in TypeScript and TSX files using glob patterns rg -i "Hackathon2025" -g "*.ts" -g "*.tsx"Length of output: 355
Action Required: Update Remaining
Hackathon2025ReferencesIt appears that although references in the server configuration have been updated, the type definitions in
libs/db/payload-types.tsstill mentionHackathon2025andHackathon2025Select. Please update these to reflect the new naming (i.e., usingWebsiteorPortalas appropriate) or confirm that they remain valid for your intended usage.
- File:
libs/db/payload-types.ts
- Update type definitions so that any use of
Hackathon2025andHackathon2025Selectis either removed or renamed to align with the new structure.- Ensure that any logic depending on these types also reflects the updated naming.
♻️ Duplicate comments (1)
libs/db/access/index.ts (1)
26-35:⚠️ Potential issueFix code formatting and structure in isSelf function.
The new
isSelffunction has inconsistent formatting and structure issues:
- Inconsistent indentation
- Unnecessary nested condition and braces
- Extra closing brace at line 35
export const isSelf = ({ req: {user} }) => { if (!user) return false; - if (user){ return { id: { equals: user.id } } - }; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
libs/cms/configs/server.ts(3 hunks)libs/db/access/index.ts(4 hunks)libs/db/collections/Brands.ts(3 hunks)libs/db/collections/Brands/index.ts(0 hunks)libs/db/collections/OrganizerTeams.ts(2 hunks)libs/db/collections/models/Emails.ts(1 hunks)libs/db/collections/models/Hardware.ts(1 hunks)libs/db/collections/models/Media.ts(1 hunks)libs/db/collections/models/Users.ts(9 hunks)
💤 Files with no reviewable changes (1)
- libs/db/collections/Brands/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (30)
libs/cms/configs/server.ts (2)
18-18: Clean import replacement for global configsThe import changes from Hackathon2025 to separate Website and Portal globals aligns with the PR goal of using CMS-hosted assets instead of local ones. This modular approach provides better separation of concerns.
30-31: Good structural change to globals configurationReplacing Hackathon2025 with separate Website and Portal globals in the CMS configuration is appropriate. This separation allows for more targeted management of website and portal-specific settings, including the logo asset that was previously stored locally.
libs/db/collections/models/Emails.ts (2)
7-11: Good import organization.Clean import of access control functions from the centralized access module.
16-19:❓ Verification inconclusive
Verify logical OR operation in access controls.
The use of the logical OR operator (
||) betweenisOrganizerandisSuperAdminmight not work as expected in Payload CMS access control. Typically, these functions should return a boolean or a filter object, but combining them with OR might lead to unexpected behavior.
🏁 Script executed:
#!/bin/bash # Check if there are examples in the codebase of using || in access controls rg -A 3 -B 3 "access: \{[^}]+(isOrganizer \|\| isSuperAdmin)" --type tsLength of output: 73
Verify Logical OR in Access Controls
Please double-check that both
isOrganizerandisSuperAdminreturn consistent types (either booleans or filter objects) required by Payload CMS. The combined use ofisOrganizer || isSuperAdminin the update access control (atlibs/db/collections/models/Emails.tslines 16-19) may not behave as expected if the return types differ. Since our current search didn’t reveal any similar usage elsewhere in the codebase, please verify this logic against the Payload CMS documentation or through manual testing before finalizing the change.libs/db/collections/models/Hardware.ts (2)
2-6: Good import organization.Clean import of access control functions from the centralized access module.
14-17: Verify logical OR operation in access controls.The use of the logical OR operator (
||) betweenisOrganizerandisSuperAdminmight not work as expected in Payload CMS access control. Access control functions typically return boolean or filter objects that might not combine correctly with OR operations.Consider using a custom function that combines these permissions:
- read: anyone, - create: isOrganizer || isSuperAdmin, - update: isOrganizer || isSuperAdmin, - delete: isSuperAdmin, + read: anyone, + create: ({ req }) => isOrganizer({ req }) || isSuperAdmin({ req }), + update: ({ req }) => isOrganizer({ req }) || isSuperAdmin({ req }), + delete: isSuperAdmin,libs/db/collections/models/Media.ts (3)
4-8: Good import organization.Clean import of access control functions from the centralized access module.
13-14: Verify logical OR operation in access controls.The use of the logical OR operator (
||) betweenisOrganizerandisSuperAdminmight not work as expected in Payload CMS access control.
17-19: 🧹 Nitpick (assertive)Add null check for user and user.group.
The admin hidden function could encounter errors if
useroruser.groupis undefined.- hidden: ({user}) => { - return user?.group?.name === "Hacker" - }, + hidden: ({user}) => { + return user?.group?.name === "Hacker" + },This is already handled correctly with the optional chaining operator, good job!
Likely an incorrect or invalid review comment.
libs/db/access/index.ts (1)
12-12: Simplified return in anyone function.The simplified return structure makes this function cleaner.
libs/db/collections/OrganizerTeams.ts (4)
4-9: Imports look neat.
No issues or suggestions here.
15-15: Default columns updated successfully.
Everything looks aligned with typical admin usage.
19-22: Confirm intended access forread: anyone.
Ensure that publicly exposing all organizer team data has no unintended privacy implications.
25-27: Good move placing these fields in the sidebar.
They’re more discoverable for admins.libs/db/collections/Brands.ts (3)
6-10: New access imports.
Nothing problematic. Great update.
61-64: Broad read access.
Verify that exposing brand data to anyone is deliberate.
79-104: Row-based grouping and new fields.
The field layout is now more structured; looks good for admin usage.libs/db/collections/models/Users.ts (13)
7-11: Additional access control imports.
They appear consistent with the rest of the codebase.
270-274: Revisited user access (admin, read, create, update, delete).
Check if allowing any organizer or sponsor to admin is fully intended.
315-326: Relationship access block.
This ensures only organizers or super admins can create or update these fields. Looks sound.
331-334: Similar protected relationship.
Consistent with the approach in the rest of the file.
340-343: Self-update logic.
Guarantees non-admins can only update their own record. Seems good.
345-347: Condition block fororganizerTeam.
Hard-coded check for group with ID 2. Confirm that the group ID is stable.
467-468: Closing bracket adjustments.
No functional issues here.
470-480:agreedToTermsandresumeLinkas read-only.
Reviewers can’t accidentally alter these user-consent fields. Good approach.
529-531: Hidden emergency contact collapsible.
Ensure that missing this info won’t cause issues if it’s really needed.
536-537: Emergency contact name field.
Straightforward addition; no issues.
562-564: Making “Brand & Socials” collapsible read-only.
Protects brand references from accidental edits.
713-715: Hidden Discord fields.
Same pattern as GitHub. No immediate concerns.
752-754: Hidden Google fields.
Maintaining consistency across all provider collapsibles.
| export const isOrganizerOrSponsor = ({ req: {user} }) => { | ||
| if (user){ | ||
| if (user.organizerTeam?.name){ | ||
| return true | ||
| } | ||
| if (user.group.name === "Sponsor"){ | ||
| return true | ||
| } | ||
| }; | ||
| return false | ||
| } |
There was a problem hiding this comment.
Fix syntax error in isOrganizerOrSponsor function.
The function has a syntax error with a misplaced semicolon after the nested if statements, which will cause runtime errors.
export const isOrganizerOrSponsor = ({ req: {user} }) => {
if (user){
if (user.organizerTeam?.name){
return true
}
if (user.group.name === "Sponsor"){
return true
}
-};
+ }
return false
}📝 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.
| export const isOrganizerOrSponsor = ({ req: {user} }) => { | |
| if (user){ | |
| if (user.organizerTeam?.name){ | |
| return true | |
| } | |
| if (user.group.name === "Sponsor"){ | |
| return true | |
| } | |
| }; | |
| return false | |
| } | |
| export const isOrganizerOrSponsor = ({ req: {user} }) => { | |
| if (user){ | |
| if (user.organizerTeam?.name){ | |
| return true | |
| } | |
| if (user.group.name === "Sponsor"){ | |
| return true | |
| } | |
| } | |
| return false | |
| } |
libs/db/access/index.ts
Outdated
| export const isOrganizer = ({ req: {user} }) => { | ||
| if (user){ | ||
| if (user.organizerTeam?.name){ | ||
| return true | ||
| } | ||
| return false | ||
| }; | ||
| return false | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Standardize function structure and returns.
The isOrganizer function has nested conditions and inconsistent return patterns compared to other similar functions. Consider standardizing the structure for better maintainability.
export const isOrganizer = ({ req: {user} }) => {
if (user){
if (user.organizerTeam?.name){
return true
}
- return false
-};
- return false
+ }
+ return false
}📝 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.
| export const isOrganizer = ({ req: {user} }) => { | |
| if (user){ | |
| if (user.organizerTeam?.name){ | |
| return true | |
| } | |
| return false | |
| }; | |
| return false | |
| } | |
| export const isOrganizer = ({ req: {user} }) => { | |
| if (user) { | |
| if (user.organizerTeam?.name) { | |
| return true | |
| } | |
| } | |
| return false | |
| } |
| { name: "Co-Leads", symbol: { alt: "cuHacking 2025 Symbol Pink" }}, | ||
| { name: "Advisors", symbol: { alt: "cuHacking 2025 Symbol White" }}, | ||
| { name: "Community Engagement", symbol: { alt: "cuHacking 2025 Symbol Blue" }}, | ||
| { name: "Hacker Experience", symbol: { alt: "cuHacking 2025 Symbol Purple" }}, |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Minor alt text consistency check.
If possible, consider a consistent naming pattern for alt text across all symbols to maintain uniform style.
| // hidden: ({user}) => { | ||
| // return user?.group?.name === "Hacker" |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Commented-out hiding logic.
Confirm if it’s no longer required or just temporarily disabled.
| admin: { | ||
| hidden: true | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Hidden GitHub collapsible.
If it’s not used, this is fine. Otherwise re-check if you need it visible.
Checklist
mainbefore creating this PR.Summary by CodeRabbit
New Features
Refactor