ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Revert "don’t count unread_count or dashcard indicators against newRelic load time" 

This reverts commit aa977ecd2a.

Reverting based on belief that this is surfacing errors in the build:

"something went wrong updating unread count" TypeError: Failed to fetch

Change-Id: I842ad732d75c6ca83f31e140ddf5edb6f10e45fe
Reviewed-on: https://gerrit.instructure.com/201108
Reviewed-by: Ryan Shaw <ryan@instructure.com>
QA-Review: Ryan Shaw <ryan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
Tested-by: Jenkins
This commit is contained in:
gbeckmann 2019-07-12 18:33:06 -06:00 committed by Gentry Beckmann
parent bee1d53a77
commit d754ff5ed1
11 changed files with 205 additions and 229 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
app/jsx/dashboard_card/CourseActivitySummaryStore.js
View File

@ -16,32 +16,34 @@
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import _ from 'underscore'
import createStore from '../shared/helpers/createStore'
import $ from 'jquery'
const CourseActivitySummaryStore = createStore({streams: {}})
CourseActivitySummaryStore.getStateForCourse = function(courseId) {
if (typeof courseId === 'undefined') return CourseActivitySummaryStore.getState()
if (_.isUndefined(courseId)) return CourseActivitySummaryStore.getState()
const {streams} = CourseActivitySummaryStore.getState()
if (!(courseId in streams)) {
streams[courseId] = {}
if (_.has(CourseActivitySummaryStore.getState().streams, courseId)) {
return CourseActivitySummaryStore.getState().streams[courseId]
} else {
CourseActivitySummaryStore.getState().streams[courseId] = {}
CourseActivitySummaryStore._fetchForCourse(courseId)
return {}
}
return streams[courseId]
}
CourseActivitySummaryStore._fetchForCourse = function(courseId) {
const fetch = window.fetchIgnoredByNewRelic || window.fetch // don't let this count against us in newRelic's SPA load time stats
fetch(`/api/v1/courses/${courseId}/activity_stream/summary`, {
headers: {Accept: 'application/json'}
let state
$.getJSON(`/api/v1/courses/${courseId}/activity_stream/summary`, stream => {
state = CourseActivitySummaryStore.getState()
state.streams[courseId] = {
stream
}
CourseActivitySummaryStore.setState(state)
})
.then(res => res.json())
.then(stream => {
const state = CourseActivitySummaryStore.getState()
state.streams[courseId] = {stream}
CourseActivitySummaryStore.setState(state)
})
}
export default CourseActivitySummaryStore

70
app/jsx/dashboard_card/__tests__/CourseActivitySummaryStore.test.js
View File

@ -1,70 +0,0 @@
/*
* Copyright (C) 2019 - present Instructure, Inc.
*
* This file is part of Canvas.
*
* Canvas is free software: you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License as published by the Free
* Software Foundation, version 3 of the License.
*
* Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
* A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
* details.
*
* You should have received a copy of the GNU Affero General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import CourseActivitySummaryStore from '../CourseActivitySummaryStore'
import wait from 'waait'
describe('CourseActivitySummaryStore', () => {
const stream = [
{
type: 'DiscussionTopic',
unread_count: 2,
count: 7
},
{
type: 'Conversation',
unread_count: 0,
count: 3
}
]
beforeEach(() => {
CourseActivitySummaryStore.setState({streams: {}})
})
describe('getStateForCourse', () => {
it('should return root state object when no courseId is provided', () => {
expect(CourseActivitySummaryStore.getStateForCourse().streams).toEqual({})
})
it('should return empty object for course id not already in state', () => {
const spy = jest
.spyOn(CourseActivitySummaryStore, '_fetchForCourse')
.mockImplementation(() => {})
expect(CourseActivitySummaryStore.getStateForCourse(1)).toEqual({})
expect(spy).toHaveBeenCalled()
CourseActivitySummaryStore.setState({streams: {1: {stream}}})
expect(CourseActivitySummaryStore.getStateForCourse(1)).toEqual({stream})
})
})
describe('_fetchForCourse', () => {
it('populates state based on API response', async () => {
expect(CourseActivitySummaryStore.getState().streams[1]).toBeUndefined() // precondition
const spy = jest.spyOn(window, 'fetch').mockImplementation(() =>
Promise.resolve().then(() => ({
json: () => Promise.resolve().then(() => stream)
}))
)
CourseActivitySummaryStore._fetchForCourse(1)
await wait(1)
expect(spy).toHaveBeenCalled()
expect(CourseActivitySummaryStore.getState()).toEqual({streams: {1: {stream}}})
})
})
})

67
app/jsx/navigation_header/Navigation.js
View File

@ -37,8 +37,6 @@ const ACTIVE_ROUTE_REGEX = /^\/(courses|groups|accounts|grades|calendar|conversa
const ACTIVE_CLASS = 'ic-app-header__menu-list-item--active'
const UNREAD_COUNT_POLL_INTERVAL = 60000 // 60 seconds
const UNREAD_COUNT_ALLOWED_AGE = UNREAD_COUNT_POLL_INTERVAL / 2
const UNREAD_COUNT_SESSION_STORAGE_KEY = `unread_count_for_${window.ENV.current_user_id}`
const TYPE_URL_MAP = {
courses: '/api/v1/users/self/favorites/courses?include[]=term&exclude[]=enrollments',
@ -60,6 +58,7 @@ export default class Navigation extends React.Component {
courses: [],
help: [],
unread_count: 0,
unread_count_attempts: 0,
isTrayOpen: false,
type: null,
coursesLoading: false,
@ -114,23 +113,13 @@ export default class Navigation extends React.Component {
componentDidMount() {
if (
!this.unread_count_attempts &&
!this.state.unread_count_attempts &&
window.ENV.current_user_id &&
!window.ENV.current_user_disabled_inbox &&
this.unreadCountElement() &&
this.unreadCountElement().length &&
!(window.ENV.current_user && window.ENV.current_user.fake_student)
) {
let msUntilIShouldStartPolling = 0
const saved = sessionStorage.getItem(UNREAD_COUNT_SESSION_STORAGE_KEY)
if (saved) {
const {updatedAt, unread_count} = JSON.parse(saved)
const millisecondsSinceLastUpdate = new Date() - updatedAt
if (millisecondsSinceLastUpdate < UNREAD_COUNT_ALLOWED_AGE) {
this.updateUnreadCount(unread_count)
msUntilIShouldStartPolling = UNREAD_COUNT_ALLOWED_AGE - millisecondsSinceLastUpdate
}
}
setTimeout(this.pollUnreadCount.bind(this), msUntilIShouldStartPolling)
this.pollUnreadCount()
}
}
@ -181,36 +170,26 @@ export default class Navigation extends React.Component {
return data
}
async pollUnreadCount() {
this.unread_count_attempts = (this.unread_count_attempts || 0) + 1
if (this.unread_count_attempts > 5) return
// don't let this count against us in newRelic's SPA load time stats
const fetch = window.fetchIgnoredByNewRelic || window.fetch
try {
const {unread_count} = await (await fetch('/api/v1/conversations/unread_count', {
headers: {Accept: 'application/json'}
})).json()
sessionStorage.setItem(
UNREAD_COUNT_SESSION_STORAGE_KEY,
JSON.stringify({
updatedAt: +new Date(),
unread_count
})
)
this.updateUnreadCount(unread_count)
} catch (error) {
console.error('something went wrong updating unread count', error)
}
setTimeout(this.pollUnreadCount.bind(this), this.unread_count_attempts * UNREAD_COUNT_POLL_INTERVAL)
pollUnreadCount() {
this.setState({unread_count_attempts: this.state.unread_count_attempts + 1}, function() {
if (this.state.unread_count_attempts <= 5) {
$.ajax('/api/v1/conversations/unread_count')
.then(data => this.updateUnreadCount(data.unread_count))
.then(null, console.log.bind(console, 'something went wrong updating unread count'))
.always(() =>
setTimeout(
() => this.pollUnreadCount(),
this.state.unread_count_attempts * UNREAD_COUNT_POLL_INTERVAL
)
)
}
})
}
unreadCountElement() {
return (
this._unreadCountElement ||
(this._unreadCountElement = $('#global_nav_conversations_link').find('.menu-item__badge')[0])
this.$unreadCount ||
(this.$unreadCount = $('#global_nav_conversations_link').find('.menu-item__badge'))
)
}
@ -229,11 +208,9 @@ export default class Navigation extends React.Component {
</ScreenReaderContent>
<PresentationContent>{count}</PresentationContent>
</React.Fragment>,
this.unreadCountElement()
this.unreadCountElement()[0]
)
if (this.unreadCountElement()) {
this.unreadCountElement().style.display = count > 0 ? '' : 'none'
}
this.unreadCountElement().toggle(count > 0)
}
determineActiveLink() {

74
app/jsx/navigation_header/__tests__/Navigation.test.js
View File

@ -1,74 +0,0 @@
// Copyright (C) 2015 - present Instructure, Inc.
//
// This file is part of Canvas.
//
// Canvas is free software: you can redistribute it and/or modify it under
// the terms of the GNU Affero General Public License as published by the Free
// Software Foundation, version 3 of the License.
//
// Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
// WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
// A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
// details.
//
// You should have received a copy of the GNU Affero General Public License along
// with this program. If not, see <http://www.gnu.org/licenses/>.
import $ from 'jquery'
import React from 'react'
import ReactDOM from 'react-dom'
import Navigation from 'jsx/navigation_header/Navigation'
$(document.body).append('<div id="holder">')
const componentHolder = document.getElementById('holder')
const renderComponent = () => ReactDOM.render(<Navigation />, componentHolder)
let $inbox_data
describe('GlobalNavigation', () => {
beforeEach(() => {
fetch.resetMocks()
// Need to setup the global nav stuff we are testing
$inbox_data = $(`
<a
id="global_nav_conversations_link"
href="/conversations"
class="ic-app-header__menu-list-link"
>
<div class="menu-item-icon-container">
<span class="menu-item__badge" style="display: none">0</span>
</div>
</a>
`).appendTo(document.body)
window.ENV.current_user_id = 10
ENV.current_user_disabled_inbox = false
})
afterEach(() => {
ReactDOM.unmountComponentAtNode(componentHolder)
$('#holder').remove()
$inbox_data.remove()
})
it('renders', () => {
expect(() => renderComponent()).not.toThrow()
})
it('shows the inbox badge when necessary', async () => {
fetch.mockResponse(JSON.stringify({unread_count: 12}))
renderComponent()
await new Promise(resolve => setTimeout(resolve, 100))
const $badge = $('#global_nav_conversations_link').find('.menu-item__badge')
expect($badge.text()).toBe('12 unread messages12')
expect($badge.css('display')).toBe('')
})
it('does not show the inbox badge when the user has opted out of notifications', async () => {
ENV.current_user_disabled_inbox = true
renderComponent()
await new Promise(resolve => setTimeout(resolve, 100))
const $badge = $('#global_nav_conversations_link').find('.menu-item__badge')
expect($badge.text()).toBe('0')
expect($badge.css('display')).toBe('none')
})
})

2
app/views/layouts/_head.html.erb
View File

@ -28,7 +28,7 @@
<link rel="preconnect" href="https://fonts.gstatic.com/" crossorigin>
<link href="https://fonts.googleapis.com/css?family=Lato:300,400,400i,700&amp;subset=latin-ext&amp;display=swap" rel="stylesheet">
<!--[if lte IE 9]> <meta http-equiv=refresh content="0; URL=/ie-9-is-not-supported.html" /> <![endif]-->
<script>window.fetchIgnoredByNewRelic = window.fetch</script><%= browser_performance_monitor_embed %>
<%= browser_performance_monitor_embed %>
<%= favicon_link_tag(favicon) %>
<%= favicon_link_tag(brand_variable('ic-brand-apple-touch-icon'), rel: 'apple-touch-icon', type: nil) %>
<%= yield :auto_discovery %>

2
jest.config.js
View File

@ -26,7 +26,6 @@ module.exports = {
'^jsx/(.*)$': '<rootDir>/app/jsx/$1',
'^jst/(.*)$': '<rootDir>/app/views/jst/$1',
"^timezone$": "<rootDir>/public/javascripts/timezone_core.js",
'node_modules-version-of-backbone': require.resolve('backbone'),
"\\.svg$": "<rootDir>/jest/imageMock.js"
},
roots: ['app/jsx', 'app/coffeescripts'],
@ -56,7 +55,6 @@ module.exports = {
coverageDirectory: '<rootDir>/coverage-jest/',
moduleFileExtensions: [...defaults.moduleFileExtensions, 'coffee', 'handlebars'],
restoreMocks: true,
transform: {
'^i18n': '<rootDir>/jest/i18nTransformer.js',

2
jest/jest-setup.js
View File

@ -21,7 +21,7 @@ import Adapter from 'enzyme-adapter-react-16'
const errorsToIgnore = ["Warning: [Focusable] Exactly one tabbable child is required (0 found)."];
window.fetch = require('jest-fetch-mock')
window.fetch = require('unfetch')
/* eslint-disable-next-line */
const _consoleDotError = console.error

2
package.json
View File

@ -177,7 +177,6 @@
"jest-canvas-mock": "^1",
"jest-config": "^24",
"jest-dom": "^3",
"jest-fetch-mock": "^2.1.2",
"jest-junit": "^6",
"jest-localstorage-mock": "^2",
"jest-moxios-utils": "^1",
@ -213,6 +212,7 @@
"style-loader": "^0.23",
"stylelint": "^9",
"through2": "^2",
"unfetch": "^4.0.1",
"waait": "^1",
"webpack": "^4",
"webpack-cleanup-plugin": "^0.5",

77
spec/coffeescripts/jsx/dashboard_card/CourseActivitySummaryStoreSpec.js Normal file
View File

@ -0,0 +1,77 @@
/*
* Copyright (C) 2015 - present Instructure, Inc.
*
* This file is part of Canvas.
*
* Canvas is free software: you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License as published by the Free
* Software Foundation, version 3 of the License.
*
* Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
* A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
* details.
*
* You should have received a copy of the GNU Affero General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import {has, isEmpty} from 'lodash'
import CourseActivitySummaryStore from 'jsx/dashboard_card/CourseActivitySummaryStore'
QUnit.module('CourseActivitySummaryStore', {
setup() {
CourseActivitySummaryStore.setState({streams: {}})
this.server = sinon.fakeServer.create()
this.stream = [
{
type: 'DiscussionTopic',
unread_count: 2,
count: 7
},
{
type: 'Conversation',
unread_count: 0,
count: 3
}
]
},
teardown() {
return this.server.restore()
}
})
test('getStateForCourse', function() {
ok(
has(CourseActivitySummaryStore.getStateForCourse(), 'streams'),
'should return root state object when no courseId is provided'
)
const spy = sandbox.stub(CourseActivitySummaryStore, '_fetchForCourse').returns(true)
ok(
isEmpty(CourseActivitySummaryStore.getStateForCourse(1)),
'should return empty object for course id not already in state'
)
ok(spy.called, 'should call _fetchForCourse to fetch stream info for course')
CourseActivitySummaryStore.setState({streams: {1: {stream: this.stream}}})
deepEqual(
CourseActivitySummaryStore.getStateForCourse(1),
{stream: this.stream},
'should return stream if present'
)
})
test('_fetchForCourse', function() {
ok(isEmpty(CourseActivitySummaryStore.getState().streams[1]), 'precondition')
this.server.respondWith('GET', '/api/v1/courses/1/activity_stream/summary', [
200,
{'Content-Type': 'application/json'},
JSON.stringify(this.stream)
])
CourseActivitySummaryStore._fetchForCourse(1)
this.server.respond()
deepEqual(
CourseActivitySummaryStore.getState().streams[1].stream,
this.stream,
'should populate state based on API response'
)
})

82
spec/javascripts/jsx/navigation_header/NavigationHeaderSpec.js Normal file
View File

@ -0,0 +1,82 @@
// Copyright (C) 2015 - present Instructure, Inc.
//
// This file is part of Canvas.
//
// Canvas is free software: you can redistribute it and/or modify it under
// the terms of the GNU Affero General Public License as published by the Free
// Software Foundation, version 3 of the License.
//
// Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
// WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
// A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
// details.
//
// You should have received a copy of the GNU Affero General Public License along
// with this program. If not, see <http://www.gnu.org/licenses/>.
import $ from 'jquery'
import React from 'react'
import ReactDOM from 'react-dom'
import Navigation from 'jsx/navigation_header/Navigation'
const wrapper = document.getElementById('fixtures')
$(wrapper).append('<div id="holder">')
const componentHolder = document.getElementById('holder')
const renderComponent = function() {
return ReactDOM.render(<Navigation />, componentHolder)
}
QUnit.module('GlobalNavigation', {
setup() {
// Need to setup the global nav stuff we are testing
this.$inbox_data = $(`
<a
id="global_nav_conversations_link"
href="/conversations"
class="ic-app-header__menu-list-link"
>
<div class="menu-item-icon-container">
<span class="menu-item__badge" style="display: none">0</span>
</div>
</a>
`).appendTo(wrapper)
this.server = sinon.fakeServer.create()
window.ENV.current_user_id = 10
ENV.current_user_disabled_inbox = false
const response = {unread_count: 10}
this.server.respondWith('GET', /unread/, [
200,
{'Content-Type': 'application/json'},
JSON.stringify(response)
])
},
teardown() {
this.server.restore()
ReactDOM.unmountComponentAtNode(componentHolder)
$('#holder').remove()
this.$inbox_data.remove()
}
})
test('it renders', function() {
this.component = renderComponent()
ok(this.component)
})
test('shows the inbox badge when necessary', function() {
this.component = renderComponent()
this.server.respond()
const $badge = $('#global_nav_conversations_link').find('.menu-item__badge')
ok($badge.is(':visible'))
})
test('does not show the inbox badge when the user has opted out of notifications', function() {
ENV.current_user_disabled_inbox = true
this.component = renderComponent()
this.server.respond()
const $badge = $('#global_nav_conversations_link').find('.menu-item__badge')
notOk($badge.is(':visible'))
})

26
yarn.lock
View File

@ -6116,14 +6116,6 @@ cross-fetch@2.2.2:
node-fetch "2.1.2"
whatwg-fetch "2.0.4"
cross-fetch@^2.2.2:
version "2.2.3"
resolved "https://registry.yarnpkg.com/cross-fetch/-/cross-fetch-2.2.3.tgz#e8a0b3c54598136e037f8650f8e823ccdfac198e"
integrity sha512-PrWWNH3yL2NYIb/7WF/5vFG3DCQiXDOVf8k3ijatbrtnwNuhMWLC7YF7uqf53tbTFDzHIUD8oITw4Bxt8ST3Nw==
dependencies:
node-fetch "2.1.2"
whatwg-fetch "2.0.4"
cross-spawn@^3.0.0:
version "3.0.1"
resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-3.0.1.tgz#1256037ecb9f0c5f79e3d6ef135e30770184b982"
@ -11516,14 +11508,6 @@ jest-environment-node@^24.8.0:
jest-mock "^24.8.0"
jest-util "^24.8.0"
jest-fetch-mock@^2.1.2:
version "2.1.2"
resolved "https://registry.yarnpkg.com/jest-fetch-mock/-/jest-fetch-mock-2.1.2.tgz#1260b347918e3931c4ec743ceaf60433da661bd0"
integrity sha512-tcSR4Lh2bWLe1+0w/IwvNxeDocMI/6yIA2bijZ0fyWxC4kQ18lckQ1n7Yd40NKuisGmcGBRFPandRXrW/ti/Bw==
dependencies:
cross-fetch "^2.2.2"
promise-polyfill "^7.1.1"
jest-get-type@^24.8.0:
version "24.8.0"
resolved "https://registry.yarnpkg.com/jest-get-type/-/jest-get-type-24.8.0.tgz#a7440de30b651f5a70ea3ed7ff073a32dfe646fc"
@ -15687,11 +15671,6 @@ promise-inflight@^1.0.1:
resolved "https://registry.yarnpkg.com/promise-inflight/-/promise-inflight-1.0.1.tgz#98472870bf228132fcbdd868129bad12c3c029e3"
integrity sha1-mEcocL8igTL8vdhoEputEsPAKeM=
promise-polyfill@^7.1.1:
version "7.1.2"
resolved "https://registry.yarnpkg.com/promise-polyfill/-/promise-polyfill-7.1.2.tgz#ab05301d8c28536301622d69227632269a70ca3b"
integrity sha512-FuEc12/eKqqoRYIGBrUptCBRhobL19PS2U31vMNTfyck1FxPyMfgsXyW4Mav85y/ZN1hop3hOwRlUDok23oYfQ==
promise@^7.0.3, promise@^7.1.1:
version "7.3.1"
resolved "https://registry.yarnpkg.com/promise/-/promise-7.3.1.tgz#064b72602b18f90f29192b8b1bc418ffd1ebd3bf"
@ -19207,6 +19186,11 @@ underscore@~1.7.0:
resolved "https://registry.yarnpkg.com/underscore/-/underscore-1.7.0.tgz#6bbaf0877500d36be34ecaa584e0db9fef035209"
integrity sha1-a7rwh3UA02vjTsqlhODbn+8DUgk=
unfetch@^4.0.1:
version "4.1.0"
resolved "https://registry.yarnpkg.com/unfetch/-/unfetch-4.1.0.tgz#6ec2dd0de887e58a4dee83a050ded80ffc4137db"
integrity sha512-crP/n3eAPUJxZXM9T80/yv0YhkTEx2K1D3h7D1AJM6fzsWZrxdyRuLN0JH/dkZh1LNH8LxCnBzoPFCPbb2iGpg==
unherit@^1.0.4:
version "1.1.2"
resolved "https://registry.yarnpkg.com/unherit/-/unherit-1.1.2.tgz#14f1f397253ee4ec95cec167762e77df83678449"