Enhance performance for the security tab

This makes it so we show spinners until things have loaded.  It also
makes it so that we begin to fetch the data sooner and rely on our
cache a bit more after.  Not only should this increase speed perception,
but should also really enhance the speed and experience.

closes CORE-2324

Test Plan:
  - Enable CSP feature flag
  - Go to the security tab on an account/subaccount
  - You should initially see a spinner, then the page should show
    fully loaded once it disappears.

Change-Id: Ia0c4b556cfd6e1a364443fe8f800e8c9ac4e10c9
Reviewed-on: https://gerrit.instructure.com/182384
Tested-by: Jenkins
QA-Review: Tucker Mcknight <tmcknight@instructure.com>
Product-Review: Clay Diffrient <cdiffrient@instructure.com>
Reviewed-by: Brent Burgoyne <bburgoyne@instructure.com>
This commit is contained in:
Clay Diffrient 2019-02-21 14:47:03 -07:00
parent efe203c1b7
commit 698f60be32
11 changed files with 126 additions and 39 deletions

View File

@ -986,6 +986,11 @@ class AccountsController < ApplicationController
MASKED_APP_CENTER_ACCESS_TOKEN: @account.settings[:app_center_access_token].try(:[], 0...5),
PERMISSIONS: {
:create_tool_manually => @account.grants_right?(@current_user, session, :create_tool_manually),
},
CSP: {
:enabled => @account.csp_enabled?,
:inherited => @account.csp_inherited?,
:settings_locked => @account.csp_locked?,
}
})
js_env(edit_help_links_env, true)

View File

@ -27,10 +27,17 @@ describe('start', () => {
const fixtures = document.createElement('div')
fixtures.setAttribute('id', 'fixtures')
document.body.appendChild(fixtures)
const fakeAxios = {
put: jest.fn(() => ({then() {}})),
get: jest.fn(() => ({then() {}}))
}
expect(() => {
start(fixtures, {
context: 'account',
contextId: '1'
contextId: '1',
api: fakeAxios
})
}).not.toThrowError()
})

View File

@ -23,6 +23,11 @@ function pluralize(word) {
return word
}
export const WHITELISTS_LOADED = 'WHITELISTS_LOADED'
export function setWhitelistsLoaded(value) {
return {type: WHITELISTS_LOADED, payload: value}
}
export const SET_DIRTY = 'SET_DIRTY'
export function setDirtyAction(value) {
if (typeof value !== 'boolean') {
@ -211,6 +216,7 @@ export function getCurrentWhitelist(context, contextId) {
tools: response.data.tools_whitelist || {}
}
dispatch(addDomainBulkAction(addDomainMap))
dispatch(setWhitelistsLoaded(true))
})
}

View File

@ -24,6 +24,7 @@ import Heading from '@instructure/ui-elements/lib/components/Heading'
import Text from '@instructure/ui-elements/lib/components/Text'
import View from '@instructure/ui-layout/lib/components/View'
import Checkbox from '@instructure/ui-forms/lib/components/Checkbox'
import Spinner from '@instructure/ui-elements/lib/components/Spinner'
import Grid, {GridCol, GridRow} from '@instructure/ui-layout/lib/components/Grid'
import {
getCspEnabled,
@ -47,7 +48,8 @@ export class SecurityPanel extends Component {
getCspInherited: func.isRequired,
setCspInherited: func.isRequired,
getCurrentWhitelist: func.isRequired,
isSubAccount: bool
isSubAccount: bool,
whitelistsHaveLoaded: bool
}
static defaultProps = {
@ -114,12 +116,18 @@ export class SecurityPanel extends Component {
</GridRow>
<GridRow>
<GridCol>
<ConnectedWhitelist
context={this.props.context}
contextId={this.props.contextId}
isSubAccount={this.props.isSubAccount}
inherited={this.props.cspInherited}
/>
{!this.props.whitelistsHaveLoaded ? (
<View as="div" margin="large" padding="large" textAlign="center">
<Spinner size="large" title={I18n.t('Loading')} />
</View>
) : (
<ConnectedWhitelist
context={this.props.context}
contextId={this.props.contextId}
isSubAccount={this.props.isSubAccount}
inherited={this.props.cspInherited}
/>
)}
</GridCol>
</GridRow>
</Grid>
@ -132,7 +140,8 @@ function mapStateToProps(state, ownProps) {
return {
...ownProps,
cspEnabled: state.cspEnabled,
cspInherited: state.cspInherited
cspInherited: state.cspInherited,
whitelistsHaveLoaded: state.whitelistsHaveLoaded
}
}

View File

@ -193,7 +193,7 @@ export class Whitelist extends Component {
</form>
{whitelistToShow.length <= 0 ? (
<Billboard
size="x-small"
size="small"
heading={I18n.t('No domains whitelisted')}
hero={<EmptyDesert />}
/>

View File

@ -21,13 +21,20 @@ import {Provider} from 'react-redux'
import {render} from 'react-testing-library'
import {configStore} from '../../store'
const fakeAxios = {
delete: jest.fn(() => ({then() {}})),
get: jest.fn(() => ({then() {}})),
post: jest.fn(() => ({then() {}})),
put: jest.fn(() => ({then() {}}))
}
// This is modified from a version by Kent C. Dodds described here:
// https://github.com/kentcdodds/react-testing-library/blob/master/examples/__tests__/react-redux.js
export function renderWithRedux(
ui,
{
initialState,
store = configStore(initialState, {
store = configStore(initialState, fakeAxios, {
disableLogger: true
})
} = {}

View File

@ -27,7 +27,12 @@ export const CONFIG = {
}
export function start(element, props = {}, state = defaultState) {
const store = configStore(state)
const initialState = {...state}
if (props.initialCspSettings) {
initialState.cspEnabled = props.initialCspSettings.enabled
initialState.cspInherited = props.initialCspSettings.inherited
}
const store = configStore(initialState, props.api)
render(
<Provider store={store}>

View File

@ -28,7 +28,8 @@ import {
SET_CSP_INHERITED,
SET_CSP_INHERITED_OPTIMISTIC,
SET_DIRTY,
COPY_INHERITED_SUCCESS
COPY_INHERITED_SUCCESS,
WHITELISTS_LOADED
} from './actions'
export function cspEnabled(state = false, action) {
@ -60,6 +61,15 @@ export function isDirty(state = false, action) {
}
}
export function whitelistsHaveLoaded(state = false, action) {
switch (action.type) {
case WHITELISTS_LOADED:
return action.payload
default:
return state
}
}
function getInheritedList(toolsWhiteList, effectiveWhitelist) {
const toolsKeys = Object.keys(toolsWhiteList)
return effectiveWhitelist.filter(domain => !toolsKeys.includes(domain))
@ -128,5 +138,6 @@ export default combineReducers({
cspEnabled,
cspInherited,
isDirty,
whitelistedDomains
whitelistedDomains,
whitelistsHaveLoaded
})

View File

@ -19,29 +19,21 @@
import {createStore, applyMiddleware} from 'redux'
import ReduxThunk from 'redux-thunk'
import rootReducer from './reducers'
import axios from 'axios'
import {setupCache} from 'axios-cache-adapter'
export const defaultState = {
cspEnabled: false,
cspInherited: false,
isDirty: false,
whitelistsHaveLoaded: false,
whitelistedDomains: {
account: [],
effective: [],
inherited: [],
tools: {}
}
}
const cache = setupCache({
maxAge: 0.5 * 60 * 1000 // Hold onto the data for 30 seconds
})
const api = axios.create({
adapter: cache.adapter
})
export function configStore(initialState, options = {}) {
export function configStore(initialState, api, options = {}) {
const middleware = [
ReduxThunk.withExtraArgument({axios: api}),
process.env.NODE_ENV !== 'production' &&

View File

@ -18,8 +18,11 @@
import React from 'react'
import ReactDOM from 'react-dom'
import I18n from 'i18n!account_settings_jsx_bundle'
import FeatureFlagAdminView from 'compiled/views/feature_flags/FeatureFlagAdminView'
import CustomHelpLinkSettings from '../custom_help_link_settings/CustomHelpLinkSettings'
import Spinner from '@instructure/ui-elements/lib/components/Spinner'
import View from '@instructure/ui-layout/lib/components/View'
import 'account_settings'
import 'compiled/bundles/modules/account_quota_settings'
@ -40,3 +43,15 @@ if (document.getElementById('custom_help_link_settings')) {
)
}
if (document.getElementById('tab-security')) {
ReactDOM.render(
<View
as="div"
margin="large"
padding="large"
textAlign="center"
>
<Spinner size="large" title={I18n.t('Loading')} />
</View>, document.getElementById('tab-security'))
}

View File

@ -21,6 +21,8 @@ import I18n from 'i18n!account_settings'
import $ from 'jquery'
import htmlEscape from 'str/htmlEscape'
import RichContentEditor from 'jsx/shared/rce/RichContentEditor'
import axios from 'axios'
import {setupCache} from 'axios-cache-adapter'
import 'jqueryui/tabs'
import globalAnnouncements from './global_announcements'
import './jquery.ajaxJSON'
@ -117,25 +119,53 @@ import './vendor/jquery.scrollTo'
globalAnnouncements.augmentView()
globalAnnouncements.bindDomEvents()
$("#account_settings_tabs").tabs({
$('#account_settings_tabs')
.tabs({
beforeActivate: (event, ui) => {
if (ui.newTab.context.id === 'tab-security-link') {
import('jsx/account_settings')
.then(({start}) => {
const splitContext = window.ENV.context_asset_string.split('_')
start(document.getElementById('tab-security'), {
context: splitContext[0],
contextId: splitContext[1],
isSubAccount: !ENV.ACCOUNT.root_account
});
}).catch(() => {
// We really should never get here... but if we do... do something.
const $message = $('<div />').text('Security Tab failed to load')
$('tab-security').append($message)
// Set up axios and send a prefetch request to get the data we need,
// this should make things appear to be much quicker once the bundle
// loads in.
const cache = setupCache({
maxAge: 0.5 * 60 * 1000, // Hold onto the data for 30 seconds
debug: true
})
const api = axios.create({
adapter: cache.adapter
})
const splitContext = window.ENV.context_asset_string.split('_')
api
.get(`/api/v1/${splitContext[0]}s/${splitContext[1]}/csp_settings`)
.then(() => {
// Bring in the actual bundle of files to use
import('jsx/account_settings')
.then(({ start }) => {
start(document.getElementById('tab-security'), {
context: splitContext[0],
contextId: splitContext[1],
isSubAccount: !ENV.ACCOUNT.root_account,
initialCspSettings: ENV.CSP,
api
})
})
.catch(() => {
// We really should never get here... but if we do... do something.
$('#tab-security').text(I18n.t('Security Tab failed to load'))
})
})
.catch(() => {
// We really should never get here... but if we do... do something.
$('#tab-security').text(I18n.t('Security Tab failed to load'))
})
}
}
}).show();
})
.show()
$(".add_ip_filter_link").click(function(event) {
event.preventDefault();
var $filter = $(".ip_filter.blank:first").clone(true).removeClass('blank');