-
-
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 all 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,5 @@ const GroupEvents = createReactClass({ | |
}, | ||
}); | ||
|
||
export default GroupEvents; | ||
export {GroupEvents}; // For tests | ||
export default withEnvironment(GroupEvents); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
// Jest Snapshot v1, https://goo.gl/fbAQLP | ||
|
||
exports[`groupEvents renders 1`] = ` | ||
<div> | ||
<div | ||
style={ | ||
Object { | ||
"marginBottom": 20, | ||
} | ||
} | ||
> | ||
<SearchBar | ||
defaultQuery="" | ||
onSearch={[Function]} | ||
placeholder="search event id, message, or tags" | ||
query="" | ||
/> | ||
</div> | ||
<div> | ||
<div | ||
className="event-list" | ||
> | ||
<EventsTable | ||
events={ | ||
Array [ | ||
Object { | ||
"eventID": "12345", | ||
"groupID": "1", | ||
"id": "1", | ||
"message": "ApiException", | ||
}, | ||
Object { | ||
"eventID": "12346", | ||
"groupID": "1", | ||
"id": "2", | ||
"message": "TestException", | ||
}, | ||
] | ||
} | ||
fixedDimensions={false} | ||
params={ | ||
Object { | ||
"groupId": "1", | ||
"orgId": "orgId", | ||
"projectId": "projectId", | ||
} | ||
} | ||
tagList={Array []} | ||
/> | ||
</div> | ||
<Pagination | ||
onCursor={[Function]} | ||
/> | ||
</div> | ||
</div> | ||
`; |
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