feat(insights): Remove Projects from Insights navigation #112535
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| import {Fragment} from 'react'; | ||
|
|
||
| import {t} from 'sentry/locale'; | ||
| import {SecondaryNavigation} from 'sentry/views/navigation/secondary/components'; | ||
| import {ProjectsNavigationItems} from 'sentry/views/navigation/secondary/sections/projects/starredProjectsList'; | ||
|
|
||
| export function ProjectsSecondaryNavigation() { | ||
| return ( | ||
| <Fragment> | ||
| <SecondaryNavigation.Header>{t('Projects')}</SecondaryNavigation.Header> | ||
| <SecondaryNavigation.Body> | ||
| <ProjectsNavigationItems /> | ||
| </SecondaryNavigation.Body> | ||
| </Fragment> | ||
| ); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| import {Fragment, useMemo} from 'react'; | ||
| import partition from 'lodash/partition'; | ||
|
|
||
| import {t} from 'sentry/locale'; | ||
| import {useOrganization} from 'sentry/utils/useOrganization'; | ||
| import {useProjects} from 'sentry/utils/useProjects'; | ||
| import {SecondaryNavigation} from 'sentry/views/navigation/secondary/components'; | ||
| import {makeProjectsPathname} from 'sentry/views/projects/pathname'; | ||
|
|
||
| interface ProjectsNavigationItemsProps { | ||
| allProjectsAnalyticsItemName?: string; | ||
| starredAnalyticsItemName?: string; | ||
| } | ||
|
|
||
| export function ProjectsNavigationItems({ | ||
| allProjectsAnalyticsItemName = 'projects_all', | ||
| starredAnalyticsItemName = 'project_starred', | ||
| }: ProjectsNavigationItemsProps) { | ||
| const organization = useOrganization(); | ||
| const {projects} = useProjects(); | ||
|
|
||
| const [starredProjects, nonStarredProjects] = useMemo(() => { | ||
| return partition(projects, project => project.isBookmarked); | ||
| }, [projects]); | ||
|
|
||
| const displayStarredProjects = starredProjects.length > 0; | ||
| const projectsToDisplay = displayStarredProjects | ||
| ? starredProjects.slice(0, 8) | ||
| : nonStarredProjects.filter(project => project.isMember).slice(0, 8); | ||
|
|
||
| return ( | ||
| <Fragment> | ||
| <SecondaryNavigation.Section id="projects-all"> | ||
| <SecondaryNavigation.List> | ||
| <SecondaryNavigation.ListItem> | ||
| <SecondaryNavigation.Link | ||
| to={makeProjectsPathname({path: '/', organization})} | ||
| end | ||
| analyticsItemName={allProjectsAnalyticsItemName} | ||
| > | ||
| {t('All Projects')} | ||
| </SecondaryNavigation.Link> | ||
| </SecondaryNavigation.ListItem> | ||
| </SecondaryNavigation.List> | ||
| </SecondaryNavigation.Section> | ||
| {projectsToDisplay.length > 0 ? ( | ||
| <Fragment> | ||
| <SecondaryNavigation.Separator /> | ||
| <SecondaryNavigation.Section | ||
| id="starred-projects" | ||
| title={displayStarredProjects ? t('Starred Projects') : t('Projects')} | ||
| > | ||
| <SecondaryNavigation.List> | ||
| {projectsToDisplay.map(project => ( | ||
| <SecondaryNavigation.ListItem key={project.id}> | ||
| <SecondaryNavigation.Link | ||
| to={makeProjectsPathname({ | ||
| path: `/${project.slug}/`, | ||
| organization, | ||
| })} | ||
| leadingItems={ | ||
| <SecondaryNavigation.ProjectIcon | ||
| projectPlatforms={ | ||
| project.platform ? [project.platform] : ['default'] | ||
| } | ||
| /> | ||
| } | ||
| analyticsItemName={starredAnalyticsItemName} | ||
| > | ||
| {project.slug} | ||
| </SecondaryNavigation.Link> | ||
| </SecondaryNavigation.ListItem> | ||
| ))} | ||
| </SecondaryNavigation.List> | ||
| </SecondaryNavigation.Section> | ||
| </Fragment> | ||
| ) : null} | ||
| </Fragment> | ||
| ); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,26 @@ | ||
| import {Outlet} from 'react-router-dom'; | ||
|
|
||
| import {Redirect} from 'sentry/components/redirect'; | ||
| import {useOrganization} from 'sentry/utils/useOrganization'; | ||
| import {useRedirectNavigationV2Routes} from 'sentry/views/navigation/useRedirectNavigationV2Routes'; | ||
|
|
||
| export default function Projects() { | ||
| const organization = useOrganization(); | ||
| // Both hooks are called unconditionally (React rules of hooks), but only one | ||
| // can match at a time since the current URL can't start with both prefixes. | ||
| // We select which result to act on based on the feature flag. | ||
| const forwardRedirect = useRedirectNavigationV2Routes({ | ||
| oldPathPrefix: '/projects/', | ||
| newPathPrefix: '/insights/projects/', | ||
| }); | ||
| const reverseRedirect = useRedirectNavigationV2Routes({ | ||
| oldPathPrefix: '/insights/projects/', | ||
| newPathPrefix: '/projects/', | ||
| }); | ||
|
|
||
| const redirectPath = organization.features.includes('workflow-engine-ui') | ||
| ? reverseRedirect | ||
| : forwardRedirect; | ||
|
Contributor
There was a problem hiding this comment. Spurious redirect warning logged on canonical project URLsMedium Severity Unconditionally calling both   Reviewed by Cursor Bugbot for commit baa4036. Configure here.
Member
Author
There was a problem hiding this comment. This is somewhat of a bummer but doesn't feel that important? Not sure! It's a very rare situation that we have two of those redirect hook in the same place, due to this conditional redirect! |
||
|
|
||
| if (redirectPath) { | ||
| return <Redirect to={redirectPath} />; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,15 @@ | ||
| import type {Organization} from 'sentry/types/organization'; | ||
| import {normalizeUrl} from 'sentry/utils/url/normalizeUrl'; | ||
|
|
||
| export function makeProjectsPathname({ | ||
| path, | ||
| organization, | ||
| }: { | ||
| organization: Organization; | ||
| path: '/' | `/${string}/`; | ||
| }) { | ||
| const base = organization.features.includes('workflow-engine-ui') | ||
| ? 'projects' | ||
| : 'insights/projects'; | ||
| return normalizeUrl(`/organizations/${organization.slug}/${base}${path}`); | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@JonasBa you were trying to remove this redirect hook - do you think we should use something else for this particular behavior?
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.
Yeah, it's fine to use. I'd probably rather add a root level redirect as I think that is where we keep some other similar logic.
@gggritso the redirect hook sends a log line to sentry so that we can track if or what routes it fires for. I may message you in the future about removing this if we see that's ok :)?
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.
Of course! I'm hoping that this redirection logic only stays around for a week or two before we just rip this code right out because Insights (and Projects) ar going away in favour of dashboards 🤞🏻