-
Notifications
You must be signed in to change notification settings - Fork 16
PM-1612 Update visible columns on copilot applications #1185
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 our terms of service and privacy 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?
Conversation
@@ -291,12 +291,13 @@ const CopilotOpportunityDetails: FC<{}> = () => { | |||
) | |||
} | |||
{activeTab === CopilotDetailsTabViews.details && <OpportunityDetails opportunity={opportunity} />} | |||
{activeTab === CopilotDetailsTabViews.applications && isAdminOrPM && opportunity && ( | |||
{activeTab === CopilotDetailsTabViews.applications && opportunity && ( |
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 isAdminOrPM
prop is being passed to CopilotApplications
, but the condition isAdminOrPM
has been removed from the check for rendering this component. Ensure that the logic for selectively visible columns is correctly implemented within CopilotApplications
based on this prop.
@@ -96,12 +97,18 @@ const CopilotApplications: FC<{ | |||
|
|||
const tableData = useMemo(getData, [props.copilotApplications, props.members]) | |||
|
|||
const visibleColumns = props.isAdminOrPM |
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.
Consider renaming visibleColumns
to filteredColumns
or userSpecificColumns
for clarity, as it indicates that the columns are filtered based on user roles.
@@ -96,12 +97,18 @@ const CopilotApplications: FC<{ | |||
|
|||
const tableData = useMemo(getData, [props.copilotApplications, props.members]) | |||
|
|||
const visibleColumns = props.isAdminOrPM | |||
? tableColumns | |||
: tableColumns.filter(col => ![ |
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 filter logic could be extracted into a separate function for better readability and maintainability. For example, const getVisibleColumns = (isAdminOrPM, columns) => { return isAdminOrPM ? columns : columns.filter(...); }
.
Related JIRA Ticket:
https://topcoder.atlassian.net/browse/PM-1612
What's in this PR?
The Applications tab is now visible to all
The columns however have been made selectively visible to Admins/PMs