Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions src/sentry/static/sentry/app/stores/environmentStore.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,7 @@ const EnvironmentStore = Reflux.createStore({

let prodEnvs = allEnvs.filter(e => PRODUCTION_ENV_NAMES.has(e.name));

return (
defaultEnv ||
(prodEnvs.length && prodEnvs[0]) ||
(allEnvs.length && allEnvs[0]) ||
null
);
return defaultEnv || (prodEnvs.length && prodEnvs[0]) || null;
},
});

Expand Down
104 changes: 93 additions & 11 deletions src/sentry/static/sentry/app/views/stream.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ import _ from 'lodash';

import ApiMixin from '../mixins/apiMixin';
import GroupStore from '../stores/groupStore';
import LatestContextStore from '../stores/latestContextStore';
import StreamTagStore from '../stores/streamTagStore';
import EnvironmentStore from '../stores/environmentStore';
import LoadingError from '../components/loadingError';
import LoadingIndicator from '../components/loadingIndicator';
import ProjectState from '../mixins/projectState';
Expand All @@ -18,7 +21,6 @@ import StreamGroup from '../components/stream/group';
import StreamActions from './stream/actions';
import StreamTagActions from '../actions/streamTagActions';
import AlertActions from '../actions/alertActions';
import StreamTagStore from '../stores/streamTagStore';
import StreamFilters from './stream/filters';
import StreamSidebar from './stream/sidebar';
import TimeSince from '../components/timeSince';
Expand All @@ -27,6 +29,10 @@ import {logAjaxError} from '../utils/logging';
import parseLinkHeader from '../utils/parseLinkHeader';
import {t, tn, tct} from '../locale';

import {objToQuery, queryToObj} from '../utils/stream';

import {setActiveEnvironment} from '../actionCreators/environments';

const MAX_TAGS = 500;

const Stream = createReactClass({
Expand All @@ -42,6 +48,7 @@ const Stream = createReactClass({
mixins: [
Reflux.listenTo(GroupStore, 'onGroupChange'),
Reflux.listenTo(StreamTagStore, 'onStreamTagChange'),
Reflux.listenTo(LatestContextStore, 'onLatestContextChange'),
ApiMixin,
ProjectState,
],
Expand All @@ -63,6 +70,10 @@ const Stream = createReactClass({
? project && !project.firstEvent
: realtimeActiveCookie === 'true';

let hasEnvironmentsFeature = new Set(this.getOrganization().features).has(
'environments'
);

return {
groupIds: [],
isDefaultSearch: null,
Expand All @@ -86,6 +97,11 @@ const Stream = createReactClass({
tagsLoading: true,
isSidebarVisible: false,
processingIssues: null,
activeEnvironment: hasEnvironmentsFeature
? LatestContextStore.getInitialState().environment
: null,
// TODO(lyn): remove when feature is rolled out
hasEnvironmentsFeature,
...this.getQueryState(),
};
},
Expand All @@ -101,6 +117,9 @@ const Stream = createReactClass({
this.fetchSavedSearches();
this.fetchProcessingIssues();
this.fetchTags();

// Make sure it gets called on mount
this.onLatestContextChange(LatestContextStore.getInitialState());
},

componentWillReceiveProps(nextProps) {
Expand All @@ -109,15 +128,18 @@ const Stream = createReactClass({
}

// Do not make new API request if props haven't actually changed
if (!_.isEqual(this.props, nextProps)) {
// Unless no request has been performed yet
if (!_.isEqual(this.props, nextProps) || !this.lastRequest) {
this.fetchData();
}

// you cannot apply both a query and a saved search (our routes do not
// support it), so the searchId takes priority
let nextSearchId = nextProps.params.searchId || null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you make this comment a tiny bit more verbose as well? this logic is complicated and i think we'll be glad in the future :o


let searchIdChanged = this.state.isDefaultSearch
? nextProps.params.searchId
: nextProps.params.searchId !== this.state.searchId;
? nextSearchId
: nextSearchId !== this.state.searchId;

if (searchIdChanged || nextProps.location.search !== this.props.location.search) {
// TODO(dcramer): handle 404 from popState on searchId
Expand Down Expand Up @@ -159,8 +181,8 @@ const Stream = createReactClass({
savedSearchList: data,
loading: false,
};
let needsData = this.state.loading;
let searchId = this.state.searchId;
let needsData = this.state.loading;
if (searchId) {
let match = data.filter(search => {
return search.id === searchId;
Expand All @@ -182,17 +204,20 @@ const Stream = createReactClass({
let defaultResults = data.filter(search => {
return search.isUserDefault;
});

if (!defaultResults.length) {
defaultResults = data.filter(search => {
return search.isDefault;
});
}

if (defaultResults.length) {
newState.searchId = defaultResults[0].id;
newState.query = defaultResults[0].query;
newState.isDefaultSearch = true;
}
}

return void this.setState(newState, needsData ? this.fetchData : null);
},
error: error => {
Expand Down Expand Up @@ -345,14 +370,40 @@ const Stream = createReactClass({

let url = this.getGroupListEndpoint();

// Remove leading and trailing whitespace
let query = this.state.query.replace(/^\s+|\s+$/g, '');

let activeEnvironment = this.state.activeEnvironment;
let activeEnvName = activeEnvironment ? activeEnvironment.name : null;

let requestParams = {
query: this.state.query.replace(/^\s+|\s+$/g, ''),
query,
limit: this.props.maxItems,
sort: this.state.sort,
statsPeriod: this.state.statsPeriod,
shortIdLookup: '1',
};

// Always keep the global active environment in sync with the queried environment
// The global environment wins unless there one is specified by the saved search
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

helpful comment, thank you 🗣

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

although you you mind labeling the branches of the if? like moving your comment
The global environment wins unless there one is specified by the saved search
inside one of them to show explicity which branch it is. (nit )

let queryObj = queryToObj(query);
if ('environment' in queryObj) {
// Set the global environment to the one specified by the saved search
if (queryObj.environment !== activeEnvName) {
let env = EnvironmentStore.getByName(queryObj.environment);
setActiveEnvironment(env);
}
requestParams.environment = queryObj.environment;
} else if (activeEnvironment) {
// Set the environment of the query to match the global settings
query = objToQuery({
...queryObj,
environment: activeEnvironment.name,
});
requestParams.query = query;
requestParams.environment = activeEnvironment.name;
}

let currentQuery = this.props.location.query || {};
if ('cursor' in currentQuery) {
requestParams.cursor = currentQuery.cursor;
Expand Down Expand Up @@ -392,6 +443,7 @@ const Stream = createReactClass({
return void this.setState({
error: false,
dataLoading: false,
query,
queryCount:
typeof queryCount !== 'undefined' ? parseInt(queryCount, 10) || 0 : 0,
queryMaxCount:
Expand Down Expand Up @@ -478,6 +530,39 @@ const Stream = createReactClass({
});
},

onLatestContextChange(context) {
if (this.state.hasEnvironmentsFeature) {
// Always query the currently active environment selection unless
// the environment parameter is part of the saved search
let environment = context.environment;

let query = this.state.query;

if (environment) {
// Environment has changed: append the new environment to the query
query = objToQuery({
...queryToObj(this.state.query),
environment: environment.name,
});
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, maybe label these branches explicitly? i know the comment above is doing it, but labeled if branches help a lot when reading dense logic like this IMO

// Environment is null, remove it from the query
let queryObj = queryToObj(this.state.query);
delete queryObj.environment;
query = objToQuery({
...queryObj,
});
}

this.setState(
{
activeEnvironment: environment,
query,
},
this.fetchData
);
}
},

onSearch(query) {
if (query === this.state.query) {
// if query is the same, just re-fetch data
Expand Down Expand Up @@ -534,6 +619,7 @@ const Stream = createReactClass({
}

let params = this.props.params;

let path = this.state.searchId
? `/${params.orgId}/${params.projectId}/searches/${this.state.searchId}/`
: `/${params.orgId}/${params.projectId}/`;
Expand Down Expand Up @@ -635,11 +721,7 @@ const Stream = createReactClass({
/>
);
});
return (
<ul className="group-list" ref="groupList">
{groupNodes}
</ul>
);
return <ul className="group-list">{groupNodes}</ul>;
},

renderAwaitingEvents() {
Expand Down
1 change: 1 addition & 0 deletions tests/acceptance/test_project_issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ def setUp(self):
teams=[self.team],
name='Bengal',
)
self.environment = self.create_environment(name="staging")
self.login_as(self.user)
self.path = '/{}/{}/'.format(self.org.slug, self.project.slug)

Expand Down
21 changes: 21 additions & 0 deletions tests/js/spec/views/stream.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,27 @@ describe('Stream', function() {

expect(stream.lastRequest).toBeNull();
});

it('sends environment attribute if one is set', function() {
stubbedApiRequest.restore();

let requestCancel = sandbox.stub();
let requestOptions;
sandbox.stub(Client.prototype, 'request', function(url, options) {
requestOptions = options;
return {
cancel: requestCancel,
};
});

let stream = wrapper.instance();
stream.state.activeEnvironment = {name: 'prod'};
stream.state.query = 'is:unresolved environment:prod';
stream.fetchData();

expect(requestOptions.data.query).toContain('environment:prod');
expect(requestOptions.data.environment).toBe('prod');
});
});

describe('render()', function() {
Expand Down