Skip to content

Commit d3ce160

Browse files
authored
feat(ui): Add project id to URL when visiting Issue Details di… (#13875)
There is still a problem when visiting an issue details page with a direct URL and when a user does not have access to multiple projects. When this happens the global selection header tries to select a seemingly random project. This PR makes a few changes -- if there is no existing `project` URL param and you visit an issue details page, it will add the issues project id to the URL. This way when you go back to the issues stream it will have the proper project in context (instead of selecting a random project). This also fixes the project selector on issue details: there is an async loading state that we are not handling well. The project selector flashes from "All Projects" (and unlocked) to a locked state with project slug. This will make the project selector aware of its "loading" state and show an empty string until we have the project slug.
1 parent eef253e commit d3ce160

File tree

4 files changed

+280
-41
lines changed

4 files changed

+280
-41
lines changed

src/sentry/static/sentry/app/components/organizations/globalSelectionHeader/index.jsx

+108-36
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ import withProjects from 'app/utils/withProjects';
3333

3434
import {getStateFromQuery} from './utils';
3535

36+
function getProjectIdFromProject(project) {
37+
return parseInt(project.id, 10);
38+
}
39+
3640
class GlobalSelectionHeader extends React.Component {
3741
static propTypes = {
3842
organization: SentryTypes.Organization,
@@ -42,6 +46,15 @@ class GlobalSelectionHeader extends React.Component {
4246
* List of projects to display in project selector
4347
*/
4448
projects: PropTypes.arrayOf(SentryTypes.Project).isRequired,
49+
50+
/**
51+
* A project will be forced from parent component (selection is disabled, and if user
52+
* does not have multi-project support enabled, it will not try to auto select a project).
53+
*
54+
* Project will be specified in the prop `forceProject` (since its data is async)
55+
*/
56+
shouldForceProject: PropTypes.bool,
57+
4558
/**
4659
* If a forced project is passed, selection is disabled
4760
*/
@@ -52,20 +65,40 @@ class GlobalSelectionHeader extends React.Component {
5265
*/
5366
selection: SentryTypes.GlobalSelection,
5467

55-
// Display Environment selector?
68+
/**
69+
* Display Environment selector?
70+
*/
5671
showEnvironmentSelector: PropTypes.bool,
5772

58-
// Display Environment selector?
73+
/**
74+
* Display Environment selector?
75+
*/
5976
showDateSelector: PropTypes.bool,
6077

61-
// Disable automatic routing
78+
/**
79+
* Disable automatic routing
80+
*/
6281
hasCustomRouting: PropTypes.bool,
6382

64-
// Reset these URL params when we fire actions
65-
// (custom routing only)
83+
/**
84+
* Reset these URL params when we fire actions
85+
* (custom routing only)
86+
*/
6687
resetParamsOnChange: PropTypes.arrayOf(PropTypes.string),
6788

68-
// Props passed to child components //
89+
/**
90+
* GlobalSelectionStore is not always initialized (e.g. Group Details) before this is rendered
91+
*
92+
* This component intentionally attempts to sync store --> URL Parameter
93+
* only when mounted, except when this prop changes.
94+
*
95+
* XXX: This comes from GlobalSelectionStore and currently does not reset,
96+
* so it happens at most once. Can add a reset as needed.
97+
*/
98+
forceUrlSync: PropTypes.bool,
99+
100+
/// Props passed to child components ///
101+
69102
/**
70103
* Show absolute date selectors
71104
*/
@@ -75,15 +108,6 @@ class GlobalSelectionHeader extends React.Component {
75108
*/
76109
showRelative: PropTypes.bool,
77110

78-
// GlobalSelectionStore is not always initialized (e.g. Group Details) before this is rendered
79-
//
80-
// This component intentionally attempts to sync store --> URL Parameter
81-
// only when mounted, except when this prop changes.
82-
//
83-
// XXX: This comes from GlobalSelectionStore and currently does not reset,
84-
// so it happens at most once. Can add a reset as needed.
85-
forceUrlSync: PropTypes.bool,
86-
87111
// Callbacks //
88112
onChangeProjects: PropTypes.func,
89113
onUpdateProjects: PropTypes.func,
@@ -110,7 +134,14 @@ class GlobalSelectionHeader extends React.Component {
110134
return;
111135
}
112136

113-
const {location, params, organization, selection} = this.props;
137+
const {
138+
location,
139+
params,
140+
organization,
141+
selection,
142+
shouldForceProject,
143+
forceProject,
144+
} = this.props;
114145

115146
const hasMultipleProjectFeature = this.hasMultipleProjectSelection();
116147

@@ -133,12 +164,7 @@ class GlobalSelectionHeader extends React.Component {
133164
if (hasMultipleProjectFeature) {
134165
updateProjects(requestedProjects);
135166
} else {
136-
const allowedProjects =
137-
requestedProjects.length > 0
138-
? requestedProjects.slice(0, 1)
139-
: this.getFirstProject();
140-
updateProjects(allowedProjects);
141-
updateParams({project: allowedProjects}, this.getRouter());
167+
this.enforceSingleProject({requestedProjects, shouldForceProject, forceProject});
142168
}
143169
} else if (params && params.orgId === organization.slug) {
144170
// Otherwise, if organization has NOT changed,
@@ -147,19 +173,12 @@ class GlobalSelectionHeader extends React.Component {
147173
// e.g. when switching to a new view that uses this component,
148174
// update URL parameters to reflect current store
149175
const {datetime, environments, projects} = selection;
176+
const otherParams = {environment: environments, ...datetime};
150177

151178
if (hasMultipleProjectFeature || projects.length === 1) {
152-
updateParamsWithoutHistory(
153-
{project: projects, environment: environments, ...datetime},
154-
this.getRouter()
155-
);
179+
updateParamsWithoutHistory({project: projects, ...otherParams}, this.getRouter());
156180
} else {
157-
const allowedProjects = this.getFirstProject();
158-
updateProjects(allowedProjects);
159-
updateParams(
160-
{project: allowedProjects, environment: environments, ...datetime},
161-
this.getRouter()
162-
);
181+
this.enforceSingleProject({shouldForceProject, forceProject}, otherParams);
163182
}
164183
}
165184
}
@@ -216,13 +235,32 @@ class GlobalSelectionHeader extends React.Component {
216235
}
217236

218237
componentDidUpdate(prevProps) {
219-
const {hasCustomRouting, location, forceUrlSync, selection} = this.props;
238+
const {
239+
hasCustomRouting,
240+
location,
241+
selection,
242+
forceUrlSync,
243+
forceProject,
244+
} = this.props;
220245

221246
if (hasCustomRouting) {
222247
return;
223248
}
224249

225-
// Kind of gross
250+
// This means that previously forceProject was falsey (e.g. loading) and now
251+
// we have the project to force.
252+
//
253+
// If user does not have multiple project selection, we need to save the forced
254+
// project into the store (if project is not in URL params), otherwise
255+
// there will be weird behavior in this component since it just picks a project
256+
if (!this.hasMultipleProjectSelection() && forceProject && !prevProps.forceProject) {
257+
// Make sure a project isn't specified in query param already, since it should take precendence
258+
const {project} = getStateFromQuery(location.query);
259+
if (!project) {
260+
this.enforceSingleProject({forceProject});
261+
}
262+
}
263+
226264
if (forceUrlSync && !prevProps.forceUrlSync) {
227265
const {project, environment} = getStateFromQuery(location.query);
228266

@@ -249,6 +287,38 @@ class GlobalSelectionHeader extends React.Component {
249287
return new Set(this.props.organization.features).has('global-views');
250288
};
251289

290+
/**
291+
* If user does not have access to `global-views` (e.g. multi project select), then
292+
* we update URL params with 1) `props.forceProject`, 2) requested projects from URL params,
293+
* 3) first project user is a member of from org
294+
*/
295+
enforceSingleProject = (
296+
{requestedProjects, shouldForceProject, forceProject} = {},
297+
otherParams
298+
) => {
299+
let newProject;
300+
301+
// This is the case where we *want* to force project, but we are still loading
302+
// the forced project's details
303+
if (shouldForceProject && !forceProject) {
304+
return;
305+
}
306+
307+
if (forceProject) {
308+
// this takes precendence over the other options
309+
newProject = [getProjectIdFromProject(forceProject)];
310+
} else if (requestedProjects && requestedProjects.length > 0) {
311+
// If there is a list of projects from URL params, select first project from that list
312+
newProject = [requestedProjects[0]];
313+
} else {
314+
// Otherwise, get first project from org that the user is a member of
315+
newProject = this.getFirstProject();
316+
}
317+
318+
updateProjects(newProject);
319+
updateParamsWithoutHistory({project: newProject, ...otherParams}, this.getRouter());
320+
};
321+
252322
/**
253323
* Identifies the query params (that are relevant to this component) that have changed
254324
*
@@ -390,7 +460,7 @@ class GlobalSelectionHeader extends React.Component {
390460

391461
getFirstProject = () => {
392462
return flatten(this.getProjects())
393-
.map(p => parseInt(p.id, 10))
463+
.map(getProjectIdFromProject)
394464
.slice(0, 1);
395465
};
396466

@@ -412,6 +482,7 @@ class GlobalSelectionHeader extends React.Component {
412482
render() {
413483
const {
414484
className,
485+
shouldForceProject,
415486
forceProject,
416487
organization,
417488
showAbsolute,
@@ -430,9 +501,10 @@ class GlobalSelectionHeader extends React.Component {
430501
return (
431502
<Header className={className}>
432503
<HeaderItemPosition>
433-
{forceProject && this.getBackButton()}
504+
{shouldForceProject && this.getBackButton()}
434505
<MultipleProjectSelector
435506
organization={organization}
507+
shouldForceProject={shouldForceProject}
436508
forceProject={forceProject}
437509
projects={projects}
438510
nonMemberProjects={nonMemberProjects}

src/sentry/static/sentry/app/components/organizations/multipleProjectSelector.jsx

+15-4
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ export default class MultipleProjectSelector extends React.PureComponent {
2525
onChange: PropTypes.func,
2626
onUpdate: PropTypes.func,
2727
multi: PropTypes.bool,
28+
shouldForceProject: PropTypes.bool,
2829
forceProject: SentryTypes.Project,
2930
};
3031

@@ -135,6 +136,7 @@ export default class MultipleProjectSelector extends React.PureComponent {
135136
nonMemberProjects,
136137
multi,
137138
organization,
139+
shouldForceProject,
138140
forceProject,
139141
} = this.props;
140142
const selectedProjectIds = new Set(value);
@@ -145,14 +147,23 @@ export default class MultipleProjectSelector extends React.PureComponent {
145147
selectedProjectIds.has(parseInt(project.id, 10))
146148
);
147149

148-
return forceProject ? (
150+
// `forceProject` can be undefined if it is loading the project
151+
// We are intentionally using an empty string as its "loading" state
152+
153+
return shouldForceProject ? (
149154
<StyledHeaderItem
150155
icon={<StyledInlineSvg src="icon-project" />}
151156
locked={true}
152-
lockedMessage={t(`This issue is unique to the ${forceProject.slug} project`)}
153-
settingsLink={`/settings/${organization.slug}/projects/${forceProject.slug}/`}
157+
lockedMessage={
158+
forceProject
159+
? t(`This issue is unique to the ${forceProject.slug} project`)
160+
: t('This issue is unique to a project')
161+
}
162+
settingsLink={
163+
forceProject && `/settings/${organization.slug}/projects/${forceProject.slug}/`
164+
}
154165
>
155-
{forceProject.slug}
166+
{forceProject ? forceProject.slug : ''}
156167
</StyledHeaderItem>
157168
) : (
158169
<StyledProjectSelector

src/sentry/static/sentry/app/views/organizationGroupDetails/groupDetails.jsx

+1
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,7 @@ const GroupDetails = createReactClass({
254254
organization={organization}
255255
forceProject={project}
256256
showDateSelector={false}
257+
shouldForceProject
257258
/>
258259
)}
259260
{isLoading ? (

0 commit comments

Comments
 (0)