-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
feat(environments): Add group events environment filtering #7366
Changes from 2 commits
45194dc
e48ff71
efba6cb
daca22d
e1695e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import React from 'react'; | ||
import PropTypes from 'prop-types'; | ||
import createReactClass from 'create-react-class'; | ||
import {browserHistory} from 'react-router'; | ||
|
||
|
@@ -10,42 +11,86 @@ import Pagination from '../components/pagination'; | |
import SearchBar from '../components/searchBar'; | ||
import EventsTable from '../components/eventsTable/eventsTable'; | ||
import {t} from '../locale'; | ||
import withEnvironment from '../utils/withEnvironment'; | ||
import {getQueryEnvironment, getQueryStringWithEnvironment} from '../utils/queryString'; | ||
import EnvironmentStore from '../stores/environmentStore'; | ||
import {setActiveEnvironment} from '../actionCreators/environments'; | ||
|
||
const GroupEvents = createReactClass({ | ||
displayName: 'GroupEvents', | ||
|
||
propTypes: { | ||
environment: PropTypes.object, | ||
}, | ||
|
||
mixins: [ApiMixin, GroupState], | ||
|
||
getInitialState() { | ||
let queryParams = this.props.location.query; | ||
return { | ||
const queryParams = this.props.location.query; | ||
|
||
const initialState = { | ||
eventList: [], | ||
loading: true, | ||
error: false, | ||
pageLinks: '', | ||
query: queryParams.query || '', | ||
}; | ||
|
||
// If an environment is specified in the query, update the global environment | ||
// Otherwise if a global environment is present update the query | ||
const queryEnvironment = EnvironmentStore.getByName( | ||
getQueryEnvironment(queryParams.query || '') | ||
); | ||
|
||
if (queryEnvironment) { | ||
setActiveEnvironment(queryEnvironment); | ||
} else if (this.props.environment) { | ||
const newQuery = getQueryStringWithEnvironment( | ||
initialState.query, | ||
this.props.environment.name | ||
); | ||
this.handleSearch(newQuery); | ||
} | ||
|
||
return initialState; | ||
}, | ||
|
||
componentWillMount() { | ||
this.fetchData(); | ||
}, | ||
|
||
componentWillReceiveProps(nextProps) { | ||
if ( | ||
nextProps.params.groupId !== this.props.params.groupId || | ||
nextProps.location.search !== this.props.location.search | ||
) { | ||
let queryParams = nextProps.location.query; | ||
// If query has changed, update the environment with the query environment | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it intentional that this is no longer handling the case where There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it's guarding against a situation that I can't see ever arising. We don't change group ID within the group details view. To change group ID you'd have to navigate to a different route and the component will be remounted. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems I added it when upgrading to react-router 1.0 here. I don't really remember what I was doing; maybe I wasn't using routes correctly. I agree it should be re-mounted. |
||
if (nextProps.location.search !== this.props.location.search) { | ||
const queryParams = nextProps.location.query; | ||
|
||
const queryEnvironment = EnvironmentStore.getByName( | ||
getQueryEnvironment(queryParams.query || '') | ||
); | ||
|
||
if (queryEnvironment) { | ||
setActiveEnvironment(queryEnvironment); | ||
} | ||
|
||
this.setState( | ||
{ | ||
query: queryParams.query, | ||
}, | ||
this.fetchData | ||
); | ||
} | ||
|
||
// If environment has changed, update query with new environment | ||
if (nextProps.environment !== this.props.environment) { | ||
const newQueryString = getQueryStringWithEnvironment( | ||
nextProps.location.query.query || '', | ||
nextProps.environment ? nextProps.environment.name : null | ||
); | ||
this.handleSearch(newQueryString); | ||
} | ||
}, | ||
|
||
onSearch(query) { | ||
handleSearch(query) { | ||
let targetQueryParams = {}; | ||
if (query !== '') targetQueryParams.query = query; | ||
|
||
|
@@ -56,28 +101,17 @@ const GroupEvents = createReactClass({ | |
}); | ||
}, | ||
|
||
getEndpoint() { | ||
let params = this.props.params; | ||
let queryParams = { | ||
...this.props.location.query, | ||
limit: 50, | ||
query: this.state.query, | ||
}; | ||
|
||
return `/issues/${params.groupId}/events/?${jQuery.param(queryParams)}`; | ||
}, | ||
|
||
fetchData() { | ||
let queryParams = this.props.location.query; | ||
|
||
this.setState({ | ||
loading: true, | ||
error: false, | ||
}); | ||
|
||
this.api.request(this.getEndpoint(), { | ||
const query = {limit: 50, query: this.state.query}; | ||
|
||
this.api.request(`/issues/${this.props.params.groupId}/events/`, { | ||
query, | ||
method: 'GET', | ||
data: queryParams, | ||
success: (data, _, jqXHR) => { | ||
this.setState({ | ||
eventList: data, | ||
|
@@ -155,7 +189,7 @@ const GroupEvents = createReactClass({ | |
defaultQuery="" | ||
placeholder={t('search event id, message, or tags')} | ||
query={this.state.query} | ||
onSearch={this.onSearch} | ||
onSearch={this.handleSearch} | ||
/> | ||
</div> | ||
{this.renderBody()} | ||
|
@@ -164,4 +198,4 @@ const GroupEvents = createReactClass({ | |
}, | ||
}); | ||
|
||
export default GroupEvents; | ||
export default withEnvironment(GroupEvents); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,7 +25,7 @@ import StreamFilters from './stream/filters'; | |
import StreamSidebar from './stream/sidebar'; | ||
import TimeSince from '../components/timeSince'; | ||
import utils from '../utils'; | ||
import streamUtils from './stream/utils'; | ||
import qsUtils from '../utils/queryString'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: let's try to keep import name the same as module name |
||
import {logAjaxError} from '../utils/logging'; | ||
import parseLinkHeader from '../utils/parseLinkHeader'; | ||
import {t, tn, tct} from '../locale'; | ||
|
@@ -370,7 +370,7 @@ const Stream = createReactClass({ | |
let url = this.getGroupListEndpoint(); | ||
|
||
// Remove leading and trailing whitespace | ||
let query = streamUtils.formatQueryString(this.state.query); | ||
let query = qsUtils.formatQueryString(this.state.query); | ||
|
||
let activeEnvironment = this.state.activeEnvironment; | ||
let activeEnvName = activeEnvironment ? activeEnvironment.name : null; | ||
|
@@ -385,7 +385,7 @@ const Stream = createReactClass({ | |
|
||
// 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 | ||
const queryEnvironment = streamUtils.getQueryEnvironment(query); | ||
const queryEnvironment = qsUtils.getQueryEnvironment(query); | ||
|
||
if (queryEnvironment !== null) { | ||
// Set the global environment to the one specified by the saved search | ||
|
@@ -396,7 +396,7 @@ const Stream = createReactClass({ | |
requestParams.environment = queryEnvironment; | ||
} else if (activeEnvironment) { | ||
// Set the environment of the query to match the global settings | ||
query = streamUtils.getQueryStringWithEnvironment(query, activeEnvironment.name); | ||
query = qsUtils.getQueryStringWithEnvironment(query, activeEnvironment.name); | ||
requestParams.query = query; | ||
requestParams.environment = activeEnvironment.name; | ||
} | ||
|
@@ -536,7 +536,7 @@ const Stream = createReactClass({ | |
// the environment parameter is part of the saved search | ||
let environment = context.environment; | ||
|
||
let query = streamUtils.getQueryStringWithEnvironment( | ||
let query = qsUtils.getQueryStringWithEnvironment( | ||
this.state.query, | ||
environment === null ? null : environment.name | ||
); | ||
|
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.
What about the component passing
onQueryChange
to this? Was it not being passed?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.
It wasn't passed or used anywhere