From baa16b8d84c2fd1162c810142d5db0a0797c7639 Mon Sep 17 00:00:00 2001 From: Lyn Nagara Date: Thu, 13 Jun 2019 15:40:52 -0700 Subject: [PATCH] fix(environments): Fix environments list loading on alert rules pages Fixes loading of environment lists on the alert rules pages. Also completely removes the EnvironmentStore as we don't need this throughout the app anymore. --- .../app/actionCreators/environments.jsx | 14 ---- .../sentry/app/actions/environmentActions.jsx | 9 --- .../sentry/app/stores/environmentStore.jsx | 78 ------------------- .../app/views/projects/projectContext.jsx | 17 +--- .../projectAlerts/projectAlertRules.jsx | 8 +- .../projectAlerts/ruleEditor/index.jsx | 56 ++++++------- .../projectAlertRuleDetails.spec.jsx | 22 +++--- .../{ => settings}/projectAlertRules.spec.jsx | 0 .../projectGeneralSettings.spec.jsx | 8 +- 9 files changed, 49 insertions(+), 163 deletions(-) delete mode 100644 src/sentry/static/sentry/app/actions/environmentActions.jsx delete mode 100644 src/sentry/static/sentry/app/stores/environmentStore.jsx rename tests/js/spec/views/{ => settings}/projectAlertRuleDetails.spec.jsx (91%) rename tests/js/spec/views/{ => settings}/projectAlertRules.spec.jsx (100%) rename tests/js/spec/views/{ => settings}/projectGeneralSettings.spec.jsx (98%) diff --git a/src/sentry/static/sentry/app/actionCreators/environments.jsx b/src/sentry/static/sentry/app/actionCreators/environments.jsx index 6ba32061c31b6a..35c04260a59308 100644 --- a/src/sentry/static/sentry/app/actionCreators/environments.jsx +++ b/src/sentry/static/sentry/app/actionCreators/environments.jsx @@ -1,17 +1,3 @@ -import EnvironmentActions from 'app/actions/environmentActions'; - -export function loadEnvironments(data, envName) { - EnvironmentActions.loadData(data, envName); -} - -export function loadActiveEnvironments(data) { - EnvironmentActions.loadActiveData(data); -} - -export function loadHiddenEnvironments(data) { - EnvironmentActions.loadHiddenData(data); -} - /** * Fetches all environments for an organization * diff --git a/src/sentry/static/sentry/app/actions/environmentActions.jsx b/src/sentry/static/sentry/app/actions/environmentActions.jsx deleted file mode 100644 index bb5a8ef54dd16f..00000000000000 --- a/src/sentry/static/sentry/app/actions/environmentActions.jsx +++ /dev/null @@ -1,9 +0,0 @@ -import Reflux from 'reflux'; - -const EnvironmentActions = Reflux.createActions([ - 'loadData', - 'loadActiveData', - 'loadHiddenData', -]); - -export default EnvironmentActions; diff --git a/src/sentry/static/sentry/app/stores/environmentStore.jsx b/src/sentry/static/sentry/app/stores/environmentStore.jsx deleted file mode 100644 index e77f5070d47466..00000000000000 --- a/src/sentry/static/sentry/app/stores/environmentStore.jsx +++ /dev/null @@ -1,78 +0,0 @@ -import Reflux from 'reflux'; -import {toTitleCase} from 'app/utils'; - -import ProjectActions from 'app/actions/projectActions'; -import EnvironmentActions from 'app/actions/environmentActions'; - -const DEFAULT_EMPTY_ENV_NAME = '(No Environment)'; -const DEFAULT_EMPTY_ROUTING_NAME = 'none'; - -const EnvironmentStore = Reflux.createStore({ - init() { - this.items = []; - this.hidden = null; - this.defaultEnvironment = null; - this.listenTo(EnvironmentActions.loadData, this.loadInitialData); - this.listenTo(EnvironmentActions.loadActiveData, this.loadActiveData); - this.listenTo(EnvironmentActions.loadHiddenData, this.loadHiddenData); - this.listenTo(ProjectActions.setActive, this.onSetActiveProject); - }, - - loadInitialData(items, activeEnvironmentName) { - this.loadActiveData(items); - }, - - loadHiddenData(items) { - this.loadData('hidden', items); - }, - - loadActiveData(items) { - this.loadData('items', items); - }, - - loadData(key, items) { - items = items || []; - this[key] = items.map(item => ({ - id: item.id, - name: item.name, - get displayName() { - return toTitleCase(item.name) || DEFAULT_EMPTY_ENV_NAME; - }, - get urlRoutingName() { - return encodeURIComponent(item.name) || DEFAULT_EMPTY_ROUTING_NAME; - }, - })); - this.trigger(this.items); - }, - - getByName(name) { - const envs = this.items; - return envs.find(item => item.name === name) || null; - }, - - getActive() { - return this.items; - }, - - getHidden() { - return this.hidden; - }, - - onSetActiveProject(project) { - if (project) { - this.defaultEnvironment = project.defaultEnvironment || null; - } - }, - - // Default environment is either the first based on the set of common names - // or the first in the environment list if none match - getDefault() { - const allEnvs = this.items; - - const defaultEnv = allEnvs.find(e => e.name === this.defaultEnvironment); - - return defaultEnv || null; - }, -}); - -export default EnvironmentStore; diff --git a/src/sentry/static/sentry/app/views/projects/projectContext.jsx b/src/sentry/static/sentry/app/views/projects/projectContext.jsx index bbfc129f4c1674..60ae32f843ecd0 100644 --- a/src/sentry/static/sentry/app/views/projects/projectContext.jsx +++ b/src/sentry/static/sentry/app/views/projects/projectContext.jsx @@ -5,7 +5,6 @@ import React from 'react'; import Reflux from 'reflux'; import createReactClass from 'create-react-class'; -import {loadEnvironments} from 'app/actionCreators/environments'; import {fetchOrgMembers} from 'app/actionCreators/members'; import {setActiveProject} from 'app/actionCreators/projects'; import {t} from 'app/locale'; @@ -46,7 +45,6 @@ const ProjectContext = createReactClass({ projects: PropTypes.arrayOf(SentryTypes.Project), projectId: PropTypes.string, orgId: PropTypes.string, - location: PropTypes.object, }, childContextTypes: { @@ -159,7 +157,7 @@ const ProjectContext = createReactClass({ }, async fetchData() { - const {orgId, projectId, location, skipReload} = this.props; + const {orgId, projectId, skipReload} = this.props; // we fetch core access/information from the global organization data const activeProject = this.identifyProject(); const hasAccess = activeProject && activeProject.hasAccess; @@ -177,12 +175,8 @@ const ProjectContext = createReactClass({ `/projects/${orgId}/${projectId}/` ); - const environmentRequest = this.props.api.requestPromise( - `/projects/${orgId}/${projectId}/environments/` - ); - try { - const [project, envs] = await Promise.all([projectRequest, environmentRequest]); + const project = await projectRequest; this.setState({ loading: false, project, @@ -192,13 +186,6 @@ const ProjectContext = createReactClass({ // assuming here that this means the project is considered the active project setActiveProject(project); - - // If an environment is specified in the query string, load it instead of default - const queryEnv = location.query.environment; - // The default environment cannot be "" (No Environment) - const {defaultEnvironment} = project; - const envName = typeof queryEnv === 'undefined' ? defaultEnvironment : queryEnv; - loadEnvironments(envs, envName); } catch (error) { this.setState({ loading: false, diff --git a/src/sentry/static/sentry/app/views/settings/projectAlerts/projectAlertRules.jsx b/src/sentry/static/sentry/app/views/settings/projectAlerts/projectAlertRules.jsx index 4b0520c7ecdebe..8338cad51850b6 100644 --- a/src/sentry/static/sentry/app/views/settings/projectAlerts/projectAlertRules.jsx +++ b/src/sentry/static/sentry/app/views/settings/projectAlerts/projectAlertRules.jsx @@ -18,12 +18,12 @@ import Button from 'app/components/button'; import Confirm from 'app/components/confirm'; import Duration from 'app/components/duration'; import EmptyStateWarning from 'app/components/emptyStateWarning'; -import EnvironmentStore from 'app/stores/environmentStore'; import PermissionAlert from 'app/views/settings/project/permissionAlert'; import SentryTypes from 'app/sentryTypes'; import Tooltip from 'app/components/tooltip'; import recreateRoute from 'app/utils/recreateRoute'; import withApi from 'app/utils/withApi'; +import {getDisplayName} from 'app/utils/environment'; import ProjectAlertHeader from './projectAlertHeader'; @@ -92,9 +92,9 @@ const RuleRow = withApi( const {data, canEdit} = this.props; const editLink = recreateRoute(`${data.id}/`, this.props); - const env = EnvironmentStore.getByName(data.environment); - - const environmentName = env ? env.displayName : t('All Environments'); + const environmentName = data.environment + ? getDisplayName({name: data.environment}) + : t('All Environments'); return ( diff --git a/src/sentry/static/sentry/app/views/settings/projectAlerts/ruleEditor/index.jsx b/src/sentry/static/sentry/app/views/settings/projectAlerts/ruleEditor/index.jsx index 8621f004afcd5d..1b9d4e774b079c 100644 --- a/src/sentry/static/sentry/app/views/settings/projectAlerts/ruleEditor/index.jsx +++ b/src/sentry/static/sentry/app/views/settings/projectAlerts/ruleEditor/index.jsx @@ -16,11 +16,11 @@ import { import {t} from 'app/locale'; import withApi from 'app/utils/withApi'; import Button from 'app/components/button'; -import EnvironmentStore from 'app/stores/environmentStore'; import LoadingIndicator from 'app/components/loadingIndicator'; import RuleNodeList from 'app/views/settings/projectAlerts/ruleEditor/ruleNodeList'; import recreateRoute from 'app/utils/recreateRoute'; import space from 'app/styles/space'; +import {getDisplayName} from 'app/utils/environment'; const FREQUENCY_CHOICES = [ ['5', t('5 minutes')], @@ -57,11 +57,12 @@ const RuleEditor = createReactClass({ rule: null, loading: false, error: null, + environments: [], }; }, componentDidMount() { - this.fetchRule(); + this.fetchData(); }, componentDidUpdate() { @@ -70,30 +71,31 @@ const RuleEditor = createReactClass({ } }, - fetchRule() { - const {ruleId, projectId, orgId} = this.props.params; - - if (ruleId) { - const endpoint = `/projects/${orgId}/${projectId}/rules/${ruleId}/`; - this.props.api.request(endpoint, { - success: rule => { - this.setState({ - rule, - }); - }, - }); - } else { - const defaultRule = { - actionMatch: 'all', - actions: [], - conditions: [], - name: '', - frequency: 30, - environment: ALL_ENVIRONMENTS_KEY, - }; + fetchData() { + const { + api, + params: {ruleId, projectId, orgId}, + } = this.props; + + const defaultRule = { + actionMatch: 'all', + actions: [], + conditions: [], + name: '', + frequency: 30, + environment: ALL_ENVIRONMENTS_KEY, + }; - this.setState({rule: defaultRule}); - } + const promises = [ + api.requestPromise(`/projects/${orgId}/${projectId}/environments/`), + ruleId + ? api.requestPromise(`/projects/${orgId}/${projectId}/rules/${ruleId}/`) + : Promise.resolve(defaultRule), + ]; + + Promise.all(promises).then(([environments, rule]) => { + this.setState({environments, rule}); + }); }, handleSubmit(e) { @@ -195,10 +197,10 @@ const RuleEditor = createReactClass({ }, render() { - const activeEnvs = EnvironmentStore.getActive() || []; + const {environments} = this.state; const environmentChoices = [ [ALL_ENVIRONMENTS_KEY, t('All Environments')], - ...activeEnvs.map(env => [env.name, env.displayName]), + ...environments.map(env => [env.name, getDisplayName(env)]), ]; if (!this.state.rule) { diff --git a/tests/js/spec/views/projectAlertRuleDetails.spec.jsx b/tests/js/spec/views/settings/projectAlertRuleDetails.spec.jsx similarity index 91% rename from tests/js/spec/views/projectAlertRuleDetails.spec.jsx rename to tests/js/spec/views/settings/projectAlertRuleDetails.spec.jsx index a3b2be28147fb7..54dbbaa1503d6c 100644 --- a/tests/js/spec/views/projectAlertRuleDetails.spec.jsx +++ b/tests/js/spec/views/settings/projectAlertRuleDetails.spec.jsx @@ -3,9 +3,8 @@ import {mount} from 'enzyme'; import {browserHistory} from 'react-router'; import ProjectAlertRuleDetails from 'app/views/settings/projectAlerts/projectAlertRuleDetails'; -import EnvironmentStore from 'app/stores/environmentStore'; -import {selectByValue} from '../../helpers/select'; +import {selectByValue} from 'app-test/helpers/select'; jest.mock('jquery'); jest.unmock('app/utils/recreateRoute'); @@ -47,19 +46,20 @@ describe('ProjectAlertRuleDetails', function() { {path: ':ruleId/', name: 'Edit'}, ]; - beforeEach(function() { + beforeEach(async function() { browserHistory.replace = jest.fn(); MockApiClient.addMockResponse({ url: '/projects/org-slug/project-slug/rules/configuration/', - method: 'GET', body: TestStubs.ProjectAlertRuleConfiguration(), }); MockApiClient.addMockResponse({ url: '/projects/org-slug/project-slug/rules/1/', - method: 'GET', body: TestStubs.ProjectAlertRule(), }); - EnvironmentStore.loadActiveData(TestStubs.Environments()); + MockApiClient.addMockResponse({ + url: '/projects/org-slug/project-slug/environments/', + body: TestStubs.Environments(), + }); }); afterEach(function() { @@ -68,7 +68,7 @@ describe('ProjectAlertRuleDetails', function() { describe('New alert rule', function() { let wrapper, mock; - beforeEach(function() { + beforeEach(async function() { mock = MockApiClient.addMockResponse({ url: '/projects/org-slug/project-slug/rules/', method: 'POST', @@ -82,10 +82,12 @@ describe('ProjectAlertRuleDetails', function() { />, TestStubs.routerContext() ); + await tick(); + wrapper.update(); }); it('sets defaults', function() { - const selects = wrapper.find('SelectField Select'); + const selects = wrapper.find('SelectControl'); expect(selects.first().props().value).toBe('all'); expect(selects.last().props().value).toBe(30); }); @@ -119,7 +121,7 @@ describe('ProjectAlertRuleDetails', function() { describe('Edit alert rule', function() { let wrapper, mock; const endpoint = '/projects/org-slug/project-slug/rules/1/'; - beforeEach(function() { + beforeEach(async function() { mock = MockApiClient.addMockResponse({ url: endpoint, method: 'PUT', @@ -133,6 +135,8 @@ describe('ProjectAlertRuleDetails', function() { />, TestStubs.routerContext() ); + await tick(); + wrapper.update(); }); it('updates', function() { diff --git a/tests/js/spec/views/projectAlertRules.spec.jsx b/tests/js/spec/views/settings/projectAlertRules.spec.jsx similarity index 100% rename from tests/js/spec/views/projectAlertRules.spec.jsx rename to tests/js/spec/views/settings/projectAlertRules.spec.jsx diff --git a/tests/js/spec/views/projectGeneralSettings.spec.jsx b/tests/js/spec/views/settings/projectGeneralSettings.spec.jsx similarity index 98% rename from tests/js/spec/views/projectGeneralSettings.spec.jsx rename to tests/js/spec/views/settings/projectGeneralSettings.spec.jsx index 1e22fb754abee5..8a0926ec872684 100644 --- a/tests/js/spec/views/projectGeneralSettings.spec.jsx +++ b/tests/js/spec/views/settings/projectGeneralSettings.spec.jsx @@ -6,7 +6,7 @@ import ProjectContext from 'app/views/projects/projectContext'; import ProjectGeneralSettings from 'app/views/settings/projectGeneralSettings'; import ProjectsStore from 'app/stores/projectsStore'; -import {selectByValue} from '../../helpers/select'; +import {selectByValue} from 'app-test/helpers/select'; jest.mock('jquery'); @@ -294,11 +294,6 @@ describe('projectGeneralSettings', function() { method: 'GET', body: {...project, slug: 'new-project'}, }); - const newProjectEnv = MockApiClient.addMockResponse({ - url: `/projects/${org.slug}/new-project/environments/`, - method: 'GET', - body: [], - }); const newProjectMembers = MockApiClient.addMockResponse({ url: `/organizations/${org.slug}/users/`, method: 'GET', @@ -320,7 +315,6 @@ describe('projectGeneralSettings', function() { await tick(); wrapper.update(); expect(newProjectGet).toHaveBeenCalled(); - expect(newProjectEnv).toHaveBeenCalled(); expect(newProjectMembers).toHaveBeenCalled(); });