- Notifications
You must be signed in to change notification settings - Fork474
Added entityName property#762
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:dev
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
@A-Vamshi is attempting to deploy a commit to theStack Team onVercel. A member of the Team first needs toauthorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
PR Summary
Added configurable entity naming across team-related components, allowing customization of team terminology (e.g., 'Team' to 'Course') throughout the UI.
- Added
entityNameprop to key components (TeamCreation,SelectedTeamSwitcher,TeamInvitation,AccountSettings) with pluralization support - Uses a new
pluralizepackage dependency for handling entity name pluralization consistently - Inconsistent casing and type handling for
entityNameprop across components needs standardization - Potential prop type mismatches in
packages/template/src/components-page/team-invitation.tsxrequire attention - Consider using a
Mapinstead of plain objects for dynamic string templating to prevent prototype pollution
5 files reviewed, 6 comments
Edit PR Review Bot Settings |Greptile
| constentityName=props.entityName??"Team"; | ||
| constpluralEntity=pluralize(entityName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
style: Consider caching pluralized value in useMemo to prevent unnecessary recalculations on re-renders
| exportfunctionTeamInvitation(props:{ | ||
| fullPage?:boolean, | ||
| searchParams:Record<string,string> | ||
| entityName:string, | ||
| }){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
logic: Type mismatch between TeamInvitation and TeamInvitationInner components. entityName is required here but optional in TeamInvitationInner.
| exportfunctionTeamInvitation(props:{ | |
| fullPage?:boolean, | |
| searchParams:Record<string,string> | |
| entityName:string, | |
| }){ | |
| exportfunctionTeamInvitation(props:{ | |
| fullPage?:boolean, | |
| searchParams:Record<string,string> | |
| entityName?:string, | |
| }){ |
| <MessageCardtitle={t('Expired{entity} Invitation Link',{entity:entityName})}fullPage={fullPage}> | ||
| <Typography>{t('Your team invitation link has expired. Please request a new{entity} invitation link ',{entity:entityName.toLowerCase()})}</Typography> | ||
| </MessageCard> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
logic: Inconsistent string: 'team' is hardcoded here while entityName is used for the title
| <MessageCardtitle={t('Expired {entity} Invitation Link',{entity:entityName})}fullPage={fullPage}> | |
| <Typography>{t('Yourteam invitation link has expired. Please request a new {entity} invitation link ',{entity:entityName.toLowerCase()})}</Typography> | |
| </MessageCard> | |
| <MessageCardtitle={t('Expired {entity} Invitation Link',{entity:entityName})}fullPage={fullPage}> | |
| <Typography>{t('Your{entity} invitation link has expired. Please request a new {entity} invitation link ',{entity:entityName.toLowerCase()})}</Typography> | |
| </MessageCard> |
Uh oh!
There was an error while loading.Please reload this page.
| </div>, | ||
| type:'item', | ||
| id:`team-${team.id}`, | ||
| id:`${entityName.toLowerCase()}-${team.id}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
logic: Keep team-related IDs consistent. Other IDs like 'team-creation' still use hardcoded 'team-' prefix while this line uses dynamic entityName
Uh oh!
There was an error while loading.Please reload this page.
| exportfunctionTeamInvitation(props:{ | ||
| fullPage?:boolean, | ||
| searchParams:Record<string,string> | ||
| entityName:string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Breaking change in TeamInvitation component's props interface - 'entityName' is marked as required but existing usages in stack-handler.tsx and potentially other places don't provide this prop. This will cause TypeScript compilation errors and potential runtime issues if not provided. The props should maintain backward compatibility by making entityName optional with a default value of 'Team'.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
😱 Found 1 issue. Time to roll up your sleeves! 😱 🗒️ View all ignored comments in this repo
Need help? Join our Discord for support! |
| team=teams?.find(team=>team.id===value)||null; | ||
| if(!team){ | ||
| thrownewStackAssertionError('Team not found, this should not happen'); | ||
| thrownewStackAssertionError('{entity} not found, this should not happen',{entity:entityName}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The error message in the StackAssertionError contains spaces and is not machine‐readable. Consider using a unique, identifier-like string (e.g. "selected-team-not-found") as the first argument.
| thrownewStackAssertionError('{entity}notfound, this should not happen',{entity:entityName}); | |
| thrownewStackAssertionError('selected-team-not-found',{entity:entityName}); |
This comment was generated because it violated a code review rule:mrule_dFi2eJA7OgSpYeiv.
Documentation Changes Required1. docs/templates/components/account-settings.mdx
2. docs/templates/sdk/types/project.mdxUpdate the description of
3. docs/templates/components/selected-team-switcher.mdx
<SelectedTeamSwitcherurlMap={(team)=>`/team/${team.id}`}selectedTeam={currentTeam}noUpdateSelectedTeam={false}entityName="Organization"/> Please ensure these changes are reflected in the relevant documentation files. |
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Uh oh!
There was an error while loading.Please reload this page.
This PR adds the entityName prop to the following components/pages as requested in#655
selected-team-switcher
account-settings
team-creation
team-invitation
Important
Add
entityNameprop to customize entity name display in team-related components usingpluralize.entityNameprop toAccountSettings,TeamCreation,TeamInvitation, andSelectedTeamSwitchercomponents to customize entity name display.pluralizelibrary inaccount-settings.tsxandselected-team-switcher.tsxto handle plural forms ofentityName.AccountSettings: UsesentityNameto customize team-related titles and IDs.TeamCreation: UsesentityNamefor form validation messages and UI text.TeamInvitation: UsesentityNamefor invitation messages and error handling.SelectedTeamSwitcher: UsesentityNamefor dropdown labels and error messages.pluralizetopackage.jsonfor handling pluralization.This description was created by
for4ba52c3. You cancustomize this summary. It will automatically update as commits are pushed.