diff --git a/contentcuration/contentcuration/decorators.py b/contentcuration/contentcuration/decorators.py index e8a2dd5a0d..9c51e83b7a 100644 --- a/contentcuration/contentcuration/decorators.py +++ b/contentcuration/contentcuration/decorators.py @@ -76,6 +76,10 @@ class DelayUserStorageCalculation(ContextDecorator): def is_active(self): return self.depth > 0 + def add(self, user_id): + if user_id not in self.queue: + self.queue.append(user_id) + def __enter__(self): self.depth += 1 diff --git a/contentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue b/contentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue index ec6c56d582..e319fc452a 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue @@ -705,10 +705,11 @@ audioVideoFiles = this.nodeFiles.filter(file => this.allowedFileType(file)); // return the last item in the array const file = audioVideoFiles[audioVideoFiles.length - 1]; - return file.duration; - } else { - return null; + if (file) { + return file.duration; + } } + return null; }, videoSelected() { return this.oneSelected && this.firstNode.kind === ContentKindsNames.VIDEO; diff --git a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue index 685232138c..21f5fd82b0 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/pages/StagingTreePage/index.vue @@ -204,6 +204,8 @@ data-test="deploy-dialog" :title="$tr('deployChannel')" :submitText="$tr('confirmDeployBtn')" + :submitDisabled="submitDisabled" + :cancelDisabled="submitDisabled" :cancelText="$tr('cancelDeployBtn')" @submit="onDeployChannelClick" @cancel="displayDeployDialog = false" @@ -293,6 +295,7 @@ displayDeployDialog: false, drawer: false, elevated: false, + submitDisabled: false, }; }, computed: { @@ -504,7 +507,13 @@ this.elevated = e.target.scrollTop > 0; }, async onDeployChannelClick() { - await this.deployCurrentChannel(); + this.submitDisabled = true; + try { + await this.deployCurrentChannel(); + } catch (e) { + this.submitDisabled = false; + throw e; + } await this.loadChannel(this.currentChannel.id); this.$router.push(this.rootTreeRoute); diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/currentChannel/actions.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/currentChannel/actions.js index 1036f23a5b..dd5bb6139a 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/currentChannel/actions.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/currentChannel/actions.js @@ -51,7 +51,12 @@ export function deployCurrentChannel(context) { let payload = { channel_id: context.state.currentChannelId, }; - return client.post(window.Urls.activate_channel(), payload); + return client.post(window.Urls.activate_channel(), payload).catch(e => { + // If response is 'Bad request', channel must already be activated + if (e.response && e.response.status === 400) { + return Promise.resolve(); + } + }); } export function publishChannel(context, version_notes) { diff --git a/contentcuration/contentcuration/frontend/shared/client.js b/contentcuration/contentcuration/frontend/shared/client.js index 056f120827..e962e46dd4 100644 --- a/contentcuration/contentcuration/frontend/shared/client.js +++ b/contentcuration/contentcuration/frontend/shared/client.js @@ -1,3 +1,4 @@ +import omit from 'lodash/omit'; import axios from 'axios'; import qs from 'qs'; import * as Sentry from '@sentry/vue'; @@ -26,7 +27,15 @@ export function paramsSerializer(params) { const client = axios.create({ xsrfCookieName: 'csrftoken', xsrfHeaderName: 'X-CSRFToken', - paramsSerializer, + paramsSerializer: { + serialize: paramsSerializer, + }, +}); + +// Track when the browser was last offline for error reporting purposes +let lastOffline = null; +window.addEventListener('offline', () => { + lastOffline = Date.now(); }); client.interceptors.response.use( @@ -55,11 +64,12 @@ client.interceptors.response.use( // In dev build log warnings to console for developer use console.warn('AJAX Request Error: ' + message); // eslint-disable-line no-console console.warn('Error data: ', error); // eslint-disable-line no-console - } else { + } else if (error.code !== 'ECONNABORTED') { Sentry.withScope(function(scope) { scope.addAttachment({ filename: 'error.json', - data: JSON.stringify(error), + // strip csrf token from headers + data: JSON.stringify(omit(error, ['config.headers.X-CSRFToken'])), contentType: 'application/json', }); Sentry.captureException(new Error(message), { @@ -69,6 +79,10 @@ client.interceptors.response.use( method: error.config.method, url, }, + Network: { + lastOffline: lastOffline ? `${Date.now() - lastOffline}ms ago` : 'never', + online: navigator.onLine, + }, }, }); }); diff --git a/contentcuration/contentcuration/frontend/shared/data/resources.js b/contentcuration/contentcuration/frontend/shared/data/resources.js index bd8796a8fe..ac51f15330 100644 --- a/contentcuration/contentcuration/frontend/shared/data/resources.js +++ b/contentcuration/contentcuration/frontend/shared/data/resources.js @@ -264,65 +264,82 @@ class IndexedDBResource { CHANGES_TABLE, () => { // Get any relevant changes that would be overwritten by this bulkPut - return db[CHANGES_TABLE].where('[table+key]') + const changesPromise = db[CHANGES_TABLE].where('[table+key]') .anyOf(itemData.map(datum => [this.tableName, this.getIdValue(datum)])) - .sortBy('rev', changes => { - changes = mergeAllChanges(changes, true); - const collectedChanges = collectChanges(changes)[this.tableName] || {}; - for (let changeType of Object.keys(collectedChanges)) { - const map = {}; - for (let change of collectedChanges[changeType]) { - map[change.key] = change; - } - collectedChanges[changeType] = map; + .sortBy('rev'); + const currentPromise = this.table + .where(this.idField) + .anyOf(itemData.map(datum => this.getIdValue(datum))) + .toArray(); + + return Promise.all([changesPromise, currentPromise]).then(([changes, currents]) => { + changes = mergeAllChanges(changes, true); + const collectedChanges = collectChanges(changes)[this.tableName] || {}; + for (let changeType of Object.keys(collectedChanges)) { + const map = {}; + for (let change of collectedChanges[changeType]) { + map[change.key] = change; } - const data = itemData - .map(datum => { - datum[LAST_FETCHED] = now; - const id = this.getIdValue(datum); - // If we have an updated change, apply the modifications here - if ( - collectedChanges[CHANGE_TYPES.UPDATED] && - collectedChanges[CHANGE_TYPES.UPDATED][id] - ) { - applyMods(datum, collectedChanges[CHANGE_TYPES.UPDATED][id].mods); + collectedChanges[changeType] = map; + } + const currentMap = {}; + for (let currentObj of currents) { + currentMap[this.getIdValue(currentObj)] = currentObj; + } + const data = itemData + .map(datum => { + const id = this.getIdValue(datum); + datum[LAST_FETCHED] = now; + // Persist TASK_ID and COPYING_FLAG attributes when directly fetching from the server + if (currentMap[id] && currentMap[id][TASK_ID]) { + datum[TASK_ID] = currentMap[id][TASK_ID]; + } + if (currentMap[id] && currentMap[id][COPYING_FLAG]) { + datum[COPYING_FLAG] = currentMap[id][COPYING_FLAG]; + } + // If we have an updated change, apply the modifications here + if ( + collectedChanges[CHANGE_TYPES.UPDATED] && + collectedChanges[CHANGE_TYPES.UPDATED][id] + ) { + applyMods(datum, collectedChanges[CHANGE_TYPES.UPDATED][id].mods); + } + return datum; + // If we have a deleted change, just filter out this object so we don't reput it + }) + .filter( + datum => + !collectedChanges[CHANGE_TYPES.DELETED] || + !collectedChanges[CHANGE_TYPES.DELETED][this.getIdValue(datum)] + ); + return this.table.bulkPut(data).then(() => { + // Move changes need to be reapplied on top of fetched data in case anything + // has happened on the backend. + return applyChanges(Object.values(collectedChanges[CHANGE_TYPES.MOVED] || {})).then( + results => { + if (!results || !results.length) { + return data; } - return datum; - // If we have a deleted change, just filter out this object so we don't reput it - }) - .filter( - datum => - !collectedChanges[CHANGE_TYPES.DELETED] || - !collectedChanges[CHANGE_TYPES.DELETED][this.getIdValue(datum)] - ); - return this.table.bulkPut(data).then(() => { - // Move changes need to be reapplied on top of fetched data in case anything - // has happened on the backend. - return applyChanges(Object.values(collectedChanges[CHANGE_TYPES.MOVED] || {})).then( - results => { - if (!results || !results.length) { - return data; - } - const resultsMap = {}; - for (let result of results) { - const id = this.getIdValue(result); - resultsMap[id] = result; - } - return data - .map(datum => { - const id = this.getIdValue(datum); - if (resultsMap[id]) { - applyMods(datum, resultsMap[id]); - } - return datum; - // Concatenate any unsynced created objects onto - // the end of the returned objects - }) - .concat(Object.values(collectedChanges[CHANGE_TYPES.CREATED]).map(c => c.obj)); + const resultsMap = {}; + for (let result of results) { + const id = this.getIdValue(result); + resultsMap[id] = result; } - ); - }); + return data + .map(datum => { + const id = this.getIdValue(datum); + if (resultsMap[id]) { + applyMods(datum, resultsMap[id]); + } + return datum; + // Concatenate any unsynced created objects onto + // the end of the returned objects + }) + .concat(Object.values(collectedChanges[CHANGE_TYPES.CREATED]).map(c => c.obj)); + } + ); }); + }); } ); } diff --git a/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/MarkdownEditor/MarkdownEditor.vue b/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/MarkdownEditor/MarkdownEditor.vue index 722c44f4f0..f8b779ca96 100644 --- a/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/MarkdownEditor/MarkdownEditor.vue +++ b/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/MarkdownEditor/MarkdownEditor.vue @@ -57,25 +57,19 @@ import '../mathquill/mathquill.js'; import 'codemirror/lib/codemirror.css'; import '@toast-ui/editor/dist/toastui-editor.css'; - import * as Showdown from 'showdown'; import Editor from '@toast-ui/editor'; - import debounce from 'lodash/debounce'; - import { stripHtml } from 'string-strip-html'; import imageUpload, { paramsToImageFieldHTML } from '../plugins/image-upload'; import formulas from '../plugins/formulas'; import minimize from '../plugins/minimize'; - import formulaHtmlToMd from '../plugins/formulas/formula-html-to-md'; import formulaMdToHtml from '../plugins/formulas/formula-md-to-html'; - import imagesHtmlToMd from '../plugins/image-upload/image-html-to-md'; import imagesMdToHtml from '../plugins/image-upload/image-md-to-html'; import { CLASS_MATH_FIELD_ACTIVE } from '../constants'; import { registerMarkdownFormulaField } from '../plugins/formulas/MarkdownFormulaField'; import { registerMarkdownImageField } from '../plugins/image-upload/MarkdownImageField'; - import { clearNodeFormat, getExtensionMenuPosition } from './utils'; - import keyHandlers from './keyHandlers'; + import { clearNodeFormat, generateCustomConverter, getExtensionMenuPosition } from './utils'; import FormulasMenu from './FormulasMenu/FormulasMenu'; import ImagesMenu from './ImagesMenu/ImagesMenu'; import ClickOutside from 'shared/directives/click-outside'; @@ -83,8 +77,6 @@ registerMarkdownFormulaField(); registerMarkdownImageField(); - const wrapWithSpaces = html => ` ${html} `; - const AnalyticsActionMap = { Bold: 'Bold', Italic: 'Italicize', @@ -164,7 +156,6 @@ markdown(newMd, previousMd) { if (newMd !== previousMd && newMd !== this.editor.getMarkdown()) { this.editor.setMarkdown(newMd); - this.updateCustomNodeSpacers(); this.initImageFields(); } }, @@ -172,36 +163,7 @@ mounted() { this.mathQuill = MathQuill.getInterface(2); - // This is currently the only way of inheriting and adjusting - // default TUI's convertor methods - // see https://github.com/nhn/tui.editor/issues/615 - const tmpEditor = new Editor({ - el: this.$refs.editor, - }); - const showdown = new Showdown.Converter(); - const Convertor = tmpEditor.convertor.constructor; - class CustomConvertor extends Convertor { - toMarkdown(content) { - content = showdown.makeMarkdown(content); - content = imagesHtmlToMd(content); - content = formulaHtmlToMd(content); - content = content.replaceAll(' ', ' '); - - // TUI.editor sprinkles in extra `
` tags that Kolibri renders literally - // any copy pasted rich text that renders as HTML but does not get converted - // will linger here, so remove it as Kolibri will render it literally also. - content = stripHtml(content).result; - return content; - } - toHTML(content) { - // Kolibri and showdown assume double newlines for a single line break, - // wheras TUI.editor prefers single newline characters. - content = content.replaceAll('\n\n', '\n'); - content = super.toHTML(content); - return content; - } - } - tmpEditor.remove(); + const CustomConvertor = generateCustomConverter(this.$refs.editor); const createBoldButton = () => { { @@ -271,8 +233,8 @@ // https://github.com/nhn/tui.editor/blob/master/apps/editor/docs/custom-html-renderer.md customHTMLRenderer: { text(node) { - let content = formulaMdToHtml(node.literal); - content = imagesMdToHtml(content); + let content = formulaMdToHtml(node.literal, true); + content = imagesMdToHtml(content, true); return { type: 'html', content, @@ -306,35 +268,6 @@ this.keyDownEventListener = this.$el.addEventListener('keydown', this.onKeyDown, true); this.clickEventListener = this.$el.addEventListener('click', this.onClick); this.editImageEventListener = this.$el.addEventListener('editImage', this.handleEditImage); - - // Make sure all custom nodes have spacers around them. - // Note: this is debounced because it's called every keystroke - const editorEl = this.$refs.editor; - this.updateCustomNodeSpacers = debounce(() => { - editorEl.querySelectorAll('span[is]').forEach(el => { - el.editing = true; - const hasLeftwardSpace = el => { - return ( - el.previousSibling && - el.previousSibling.textContent && - /\s$/.test(el.previousSibling.textContent) - ); - }; - const hasRightwardSpace = el => { - return ( - el.nextSibling && el.nextSibling.textContent && /^\s/.test(el.nextSibling.textContent) - ); - }; - if (!hasLeftwardSpace(el)) { - el.insertAdjacentText('beforebegin', '\xa0'); - } - if (!hasRightwardSpace(el)) { - el.insertAdjacentText('afterend', '\xa0'); - } - }); - }, 150); - - this.updateCustomNodeSpacers(); }, activated() { this.editor.focus(); @@ -358,15 +291,9 @@ * a recommended solution here https://github.com/neilj/Squire/issues/107 */ onKeyDown(event) { - const squire = this.editor.getSquire(); - // Apply squire selection workarounds this.fixSquireSelectionOnKeyDown(event); - if (event.key in keyHandlers) { - keyHandlers[event.key](squire); - } - // ESC should close menus if any are open // or close the editor if none are open if (event.key === 'Escape') { @@ -413,8 +340,6 @@ event.preventDefault(); event.stopPropagation(); } - - this.updateCustomNodeSpacers(); }, onPaste(event) { const fragment = clearNodeFormat({ @@ -507,7 +432,7 @@ const getRightwardElement = selection => getElementAtRelativeOffset(selection, 1); const getCharacterAtRelativeOffset = (selection, relativeOffset) => { - let { element, offset } = squire.getSelectionInfoByOffset( + const { element, offset } = squire.getSelectionInfoByOffset( selection.startContainer, selection.startOffset + relativeOffset ); @@ -529,27 +454,31 @@ /\s$/.test(getCharacterAtRelativeOffset(selection, 0)); const moveCursor = (selection, amount) => { - let { element, offset } = squire.getSelectionInfoByOffset( - selection.startContainer, - selection.startOffset + amount - ); - if (amount > 0) { - selection.setStart(element, offset); - } else { - selection.setEnd(element, offset); - } + const element = getElementAtRelativeOffset(selection, amount); + selection.setStart(element, 0); + selection.setEnd(element, 0); return selection; }; - // make sure Squire doesn't delete rightward custom nodes when 'backspace' is pressed - if (event.key !== 'ArrowRight' && event.key !== 'Delete') { - if (isCustomNode(getRightwardElement(selection))) { + const rightwardElement = getRightwardElement(selection); + const leftwardElement = getLeftwardElement(selection); + + if (event.key === 'ArrowRight') { + if (isCustomNode(rightwardElement)) { + squire.setSelection(moveCursor(selection, 1)); + } else if (spacerAndCustomElementAreRightward(selection)) { + squire.setSelection(moveCursor(selection, 2)); + } + } + if (event.key === 'ArrowLeft') { + if (isCustomNode(leftwardElement)) { squire.setSelection(moveCursor(selection, -1)); + } else if (spacerAndCustomElementAreLeftward(selection)) { + squire.setSelection(moveCursor(selection, -2)); } } // make sure Squire doesn't get stuck with a broken cursor position when deleting // elements with `contenteditable="false"` in FireFox - let leftwardElement = getLeftwardElement(selection); if (event.key === 'Backspace') { if (selection.startContainer.tagName === 'DIV') { // This happens normally when deleting from the beginning of an empty line... @@ -791,7 +720,6 @@ } else { let squire = this.editor.getSquire(); squire.insertHTML(formulaHTML); - this.updateCustomNodeSpacers(); } }, resetFormulasMenu() { @@ -876,8 +804,7 @@ const mdImageEl = template.content.firstElementChild; mdImageEl.setAttribute('editing', true); - // insert non-breaking spaces to allow users to write text before and after - this.editor.getSquire().insertHTML(wrapWithSpaces(mdImageEl.outerHTML)); + this.editor.getSquire().insertHTML(mdImageEl.outerHTML); this.initImageFields(); } diff --git a/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/MarkdownEditor/keyHandlers.js b/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/MarkdownEditor/keyHandlers.js deleted file mode 100644 index 7cb2b93597..0000000000 --- a/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/MarkdownEditor/keyHandlers.js +++ /dev/null @@ -1,74 +0,0 @@ -import { CLASS_MATH_FIELD, KEY_ARROW_RIGHT, KEY_ARROW_LEFT, KEY_BACKSPACE } from '../constants'; - -/** - * When arrow right pressed and next element is a math field - * then move cursor to a first position after the math field - * - * @param {Object} squire Squire instance - */ -const onArrowRight = squire => { - const selection = squire.getSelection(); - - if ( - selection && - selection.startContainer.nextSibling && - selection.startOffset === selection.startContainer.length && - selection.startContainer.nextSibling.classList.contains(CLASS_MATH_FIELD) - ) { - const rangeAfterMathField = new Range(); - rangeAfterMathField.setStartAfter(selection.startContainer.nextSibling); - - squire.setSelection(rangeAfterMathField); - } -}; - -/** - * When arrow left pressed and previous element is a math field - * then move cursor to a last position before the math field - * - * @param {Object} squire Squire instance - */ -const onArrowLeft = squire => { - const selection = squire.getSelection(); - - if ( - selection && - selection.startContainer.previousSibling && - selection.startOffset === 1 && - selection.startContainer.previousSibling.classList.contains(CLASS_MATH_FIELD) - ) { - const rangeBeforeMathField = new Range(); - rangeBeforeMathField.setStartBefore(selection.startContainer.previousSibling); - - squire.setSelection(rangeBeforeMathField); - } -}; - -/** - * When backspace pressed and previous element is a math field - * then remove the math field - * - * @param {Object} squire Squire instance - */ -const onBackspace = squire => { - const selection = squire.getSelection(); - - if ( - selection && - selection.startContainer.previousSibling && - selection.startOffset === 1 && - selection.startContainer.previousSibling.classList.contains(CLASS_MATH_FIELD) - ) { - const mathFieldRange = new Range(); - mathFieldRange.setStartBefore(selection.startContainer.previousSibling); - mathFieldRange.setEndBefore(selection.startContainer); - - mathFieldRange.deleteContents(); - } -}; - -export default { - [KEY_ARROW_RIGHT]: onArrowRight, - [KEY_ARROW_LEFT]: onArrowLeft, - [KEY_BACKSPACE]: onBackspace, -}; diff --git a/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/MarkdownEditor/utils.js b/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/MarkdownEditor/utils.js index 5894845273..f5ce62e98e 100644 --- a/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/MarkdownEditor/utils.js +++ b/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/MarkdownEditor/utils.js @@ -1,3 +1,10 @@ +import * as Showdown from 'showdown'; +import Editor from '@toast-ui/editor'; +import { stripHtml } from 'string-strip-html'; + +import imagesHtmlToMd from '../plugins/image-upload/image-html-to-md'; +import formulaHtmlToMd from '../plugins/formulas/formula-html-to-md'; + /** * Clear DOM node by keeping only its text content. * @@ -74,3 +81,37 @@ export const getExtensionMenuPosition = ({ editorEl, targetX, targetY }) => { right: menuRight, }; }; + +export const generateCustomConverter = el => { + // This is currently the only way of inheriting and adjusting + // default TUI's convertor methods + // see https://github.com/nhn/tui.editor/issues/615 + const tmpEditor = new Editor({ el }); + const showdown = new Showdown.Converter(); + const Convertor = tmpEditor.convertor.constructor; + class CustomConvertor extends Convertor { + toMarkdown(content) { + content = showdown.makeMarkdown(content); + content = imagesHtmlToMd(content); + content = formulaHtmlToMd(content); + // TUI.editor sprinkles in extra `
` tags that Kolibri renders literally + // When showdown has already added linebreaks to render these in markdown + // so we just remove these here. + content = content.replaceAll('
', ''); + + // any copy pasted rich text that renders as HTML but does not get converted + // will linger here, so remove it as Kolibri will render it literally also. + content = stripHtml(content).result; + return content; + } + toHTML(content) { + // Kolibri and showdown assume double newlines for a single line break, + // wheras TUI.editor prefers single newline characters. + content = content.replaceAll('\n\n', '\n'); + content = super.toHTML(content); + return content; + } + } + tmpEditor.remove(); + return CustomConvertor; +}; diff --git a/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/__tests__/utils.spec.js b/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/__tests__/utils.spec.js index d201e81b33..1a55e0b179 100644 --- a/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/__tests__/utils.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/__tests__/utils.spec.js @@ -1,4 +1,9 @@ -import { clearNodeFormat } from '../MarkdownEditor/utils'; +/** + * @jest-environment jest-environment-jsdom-sixteen + */ +// Jsdom@^16 is required to test toast UI, as it relies on the Range API. + +import { clearNodeFormat, generateCustomConverter } from '../MarkdownEditor/utils'; const htmlStringToFragment = htmlString => { const template = document.createElement('template'); @@ -56,3 +61,17 @@ describe('clearNodeFormat', () => { ); }); }); + +describe('markdown conversion', () => { + it('converts image tags to markdown without escaping them', () => { + const el = document.createElement('div'); + const CustomConvertor = generateCustomConverter(el); + const converter = new CustomConvertor(); + const html = + '![](${☣ CONTENTSTORAGE}/bc1c5a86e1e46f20a6b4ee2c1bb6d6ff.png =485.453125x394)'; + + expect(converter.toMarkdown(html)).toBe( + '![](${☣ CONTENTSTORAGE}/bc1c5a86e1e46f20a6b4ee2c1bb6d6ff.png =485.453125x394)' + ); + }); +}); diff --git a/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/formulas/formula-md-to-html.js b/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/formulas/formula-md-to-html.js index f99fcd2f09..61669e5eac 100644 --- a/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/formulas/formula-md-to-html.js +++ b/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/formulas/formula-md-to-html.js @@ -12,6 +12,10 @@ * */ -export default markdown => { - return markdown.replace(/\$\$(.*?)\$\$/g, '$1'); +export default (markdown, editing) => { + const editAttr = editing ? ' editing="true"' : ''; + return markdown.replace( + /\$\$(.*?)\$\$/g, + `$1` + ); }; diff --git a/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/image-upload/MarkdownImageField.vue b/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/image-upload/MarkdownImageField.vue index d5308e83dc..a58aad5705 100644 --- a/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/image-upload/MarkdownImageField.vue +++ b/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/image-upload/MarkdownImageField.vue @@ -136,8 +136,12 @@ ); }, handleRemove() { - this.$destroy(); - this.$el.parentNode.removeChild(this.$el); + this.editorField.dispatchEvent( + new CustomEvent('remove', { + bubbles: true, + cancelable: true, + }) + ); }, handleResize() { this.resizing = true; diff --git a/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/image-upload/image-md-to-html.js b/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/image-upload/image-md-to-html.js index c452c5a82a..1f61c121d1 100644 --- a/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/image-upload/image-md-to-html.js +++ b/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/image-upload/image-md-to-html.js @@ -14,6 +14,6 @@ import { IMAGE_REGEX, imageMdToImageFieldHTML } from './index'; // convert markdown images to image editor field custom elements -export default markdown => { - return markdown.replace(IMAGE_REGEX, imageMd => imageMdToImageFieldHTML(imageMd)); +export default (markdown, editing) => { + return markdown.replace(IMAGE_REGEX, imageMd => imageMdToImageFieldHTML(imageMd, editing)); }; diff --git a/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/image-upload/index.js b/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/image-upload/index.js index b4b13a7201..0e33f894c4 100644 --- a/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/image-upload/index.js +++ b/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/image-upload/index.js @@ -30,8 +30,10 @@ export const paramsToImageMd = ({ src, alt, width, height }) => { } }; -export const imageMdToImageFieldHTML = imageMd => - `${imageMd}`; +export const imageMdToImageFieldHTML = (imageMd, editing) => { + const editAttr = editing ? ' editing="true"' : ''; + return `${imageMd}`; +}; export const paramsToImageFieldHTML = params => imageMdToImageFieldHTML(paramsToImageMd(params)); export default imageUploadExtension; diff --git a/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/registerCustomMarkdownField.js b/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/registerCustomMarkdownField.js index 250ae5b30f..da581955be 100644 --- a/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/registerCustomMarkdownField.js +++ b/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/plugins/registerCustomMarkdownField.js @@ -2,6 +2,54 @@ import Vue from 'vue'; import vueCustomElement from 'vue-custom-element'; import { v4 as uuidv4 } from 'uuid'; +const leftwardSpaceRegex = /\s$/; + +const leftwardDoubleSpaceRegex = /\s\s$/; + +const hasLeftwardSpace = el => { + return ( + // Has a previous sibling + el.previousSibling && + // Which has text content + el.previousSibling.textContent && + // The text content has white space right before this element + leftwardSpaceRegex.test(el.previousSibling.textContent) && + // And either this sibling doesn't have a previous sibling + (!el.previousSibling.previousSibling || + // Or it doesn't have a hasAttribute function + typeof el.previousSibling.previousSibling.hasAttribute !== 'function' || + // Or the previous sibling is not another custom field + !el.previousSibling.previousSibling.hasAttribute('is') || + // Or the previous sibling has two white spaces, one for each + // of the custom fields on either side. + leftwardDoubleSpaceRegex.test(el.previousSibling.textContent)) + ); +}; + +const rightwardSpaceRegex = /^\s/; + +const rightwardDoubleSpaceRegex = /^\s\s/; + +const hasRightwardSpace = el => { + return ( + // Has a next sibling + el.nextSibling && + // Which has text content + el.nextSibling.textContent && + // The text content has white space right after this element + rightwardSpaceRegex.test(el.nextSibling.textContent) && + // And either this sibling doesn't have a next sibling + (!el.nextSibling.nextSibling || + // Or it doesn't have a hasAttribute function + typeof el.nextSibling.nextSibling.hasAttribute !== 'function' || + // Or the next sibling is not another custom field + !el.nextSibling.nextSibling.hasAttribute('is') || + // Or the next sibling has two white spaces, one for each + // of the custom fields on either side. + rightwardDoubleSpaceRegex.test(el.nextSibling.textContent)) + ); +}; + export default VueComponent => { const dashed = camel => camel.replace(/([a-zA-Z])(?=[A-Z])/g, '$1-').toLowerCase(); const name = dashed(VueComponent.name); @@ -15,10 +63,10 @@ export default VueComponent => { vueInstanceCreatedCallback() { // by default, `contenteditable` will be false this.setAttribute('contenteditable', Boolean(VueComponent.contentEditable)); - + const id = `markdown-field-${uuidv4()}`; // a hack to prevent squire from merging custom element spans // see here: https://github.com/nhn/tui.editor/blob/master/libs/squire/source/Node.js#L92-L101 - this.classList.add(`markdown-field-${uuidv4()}`); + this.classList.add(id); // pass innerHTML of host element as the `markdown` property this.observer = new MutationObserver(mutations => { @@ -35,14 +83,38 @@ export default VueComponent => { this.innerHTML = textNodesRemoved.map(n => n.nodeValue).join(); } else { // otherwise, pass the innerHTML to inner Vue component as `markdown` prop - this.getVueInstance().markdown = this.innerHTML; + this.markdown = this.innerHTML; } }); }); this.observer.observe(this, { characterData: true, childList: true }); + this.addEventListener('remove', () => { + if (hasLeftwardSpace(this)) { + this.previousSibling.textContent = this.previousSibling.textContent.replace( + leftwardSpaceRegex, + '' + ); + } + if (hasRightwardSpace(this)) { + this.nextSibling.textContent = this.nextSibling.textContent.replace( + rightwardSpaceRegex, + '' + ); + } + if (this.parentNode) { + this.parentNode.removeChild(this); + } + }); + + if (!hasLeftwardSpace(this)) { + this.insertAdjacentText('beforebegin', '\xa0'); + } + if (!hasRightwardSpace(this)) { + this.insertAdjacentText('afterend', '\xa0'); + } // initialize the `markdown` property - this.getVueInstance().$root.markdown = this.innerHTML; + this.markdown = this.innerHTML; }, shadowCss: VueComponent.shadowCSS, shadow: true, diff --git a/contentcuration/contentcuration/frontend/shared/views/files/Uploader.vue b/contentcuration/contentcuration/frontend/shared/views/files/Uploader.vue index 4615b6d6e3..cc0214b8d4 100644 --- a/contentcuration/contentcuration/frontend/shared/views/files/Uploader.vue +++ b/contentcuration/contentcuration/frontend/shared/views/files/Uploader.vue @@ -63,7 +63,7 @@ import FileDropzone from './FileDropzone'; import FileStorage from './FileStorage'; - import { fileErrors, MAX_FILE_SIZE } from 'shared/constants'; + import { MAX_FILE_SIZE } from 'shared/constants'; import { fileSizeMixin } from 'shared/mixins'; import Alert from 'shared/views/Alert'; import { FormatPresetsList } from 'shared/leUtils/FormatPresets'; @@ -193,16 +193,17 @@ } else if (this.tooLargeFiles.length) { this.showTooLargeFilesAlert = true; } - return this.handleUploads(files).then(fileObjects => { - const objects = fileObjects.map(f => f.fileObject); - if (fileObjects.length) { - for (let ret of fileObjects) { - const fileObject = ret.fileObject; - ret.promise.then(err => { - if (err !== fileErrors.UPLOAD_FAILED && isFunction(this.uploadCompleteHandler)) { - this.uploadCompleteHandler(this.getFileUpload(fileObject.id)); - } - }); + return this.handleUploads(files).then(fileUploads => { + const objects = fileUploads.map(f => f.fileObject).filter(f => !f.error); + if (fileUploads.length) { + for (let fileUpload of fileUploads) { + fileUpload.uploadPromise + .then(fileObject => { + if (isFunction(this.uploadCompleteHandler)) { + this.uploadCompleteHandler(this.getFileUpload(fileObject.id)); + } + }) + .catch(() => {}); } if (isFunction(this.uploadingHandler)) { this.uploadingHandler(this.allowMultiple ? objects : objects[0]); @@ -220,9 +221,9 @@ // need to distinguish between presets with same extension // (e.g. high res vs. low res videos) [...files].map(file => this.uploadFile({ file, preset: this.presetID }).catch(() => null)) - ).then(fileObjects => { + ).then(fileUploads => { // Filter out any null values here - return fileObjects.filter(Boolean); + return fileUploads.filter(Boolean); }); }, }, diff --git a/contentcuration/contentcuration/frontend/shared/vuex/file/actions.js b/contentcuration/contentcuration/frontend/shared/vuex/file/actions.js index 1a63391c67..dce95e5431 100644 --- a/contentcuration/contentcuration/frontend/shared/vuex/file/actions.js +++ b/contentcuration/contentcuration/frontend/shared/vuex/file/actions.js @@ -164,90 +164,103 @@ export function uploadFileToStorage( }); } +/** + * @return {Promise<{uploadPromise: Promise, fileObject: Object}>} + */ export function uploadFile(context, { file, preset = null } = {}) { - return new Promise((resolve, reject) => { - // 1. Get the checksum of the file - Promise.all([getHash(file), extractMetadata(file, preset)]) - .then(([checksum, metadata]) => { - const file_format = file.name - .split('.') - .pop() - .toLowerCase(); - // 2. Get the upload url - File.uploadUrl({ + const file_format = file.name + .split('.') + .pop() + .toLowerCase(); + const hashPromise = getHash(file).catch(() => Promise.reject(fileErrors.CHECKSUM_HASH_FAILED)); + let checksum, + metadata = {}; + + return Promise.all([hashPromise, extractMetadata(file, preset)]) + .then(([fileChecksum, fileMetadata]) => { + checksum = fileChecksum; + metadata = fileMetadata; + + // 2. Get the upload url + return File.uploadUrl({ + checksum, + size: file.size, + type: file.type, + name: file.name, + file_format, + ...metadata, + }).catch(error => { + let errorType = fileErrors.UPLOAD_FAILED; + if (error.response && error.response.status === 412) { + errorType = fileErrors.NO_STORAGE; + } + return Promise.reject(errorType); + }); // End get upload url + }) + .then(data => { + const fileObject = { + ...data.file, + loaded: 0, + total: file.size, + }; + context.commit('ADD_FILE', fileObject); + + // Asynchronously generate file preview + setTimeout(() => { + const reader = new FileReader(); + reader.readAsDataURL(file); + reader.onloadend = () => { + if (reader.result) { + context.commit('ADD_FILE', { + id: data.file.id, + previewSrc: reader.result, + }); + } + }; + }, 0); + + // 3. Upload file + const uploadPromise = context + .dispatch('uploadFileToStorage', { + id: fileObject.id, checksum, - size: file.size, - type: file.type, - name: file.name, + file, file_format, - ...metadata, + url: data['uploadURL'], + contentType: data['mimetype'], + mightSkip: data['might_skip'], }) - .then(data => { - const fileObject = { - ...data.file, - loaded: 0, - total: file.size, - }; - context.commit('ADD_FILE', fileObject); - // 3. Upload file - const promise = context - .dispatch('uploadFileToStorage', { - id: fileObject.id, - checksum, - file, - file_format, - url: data['uploadURL'], - contentType: data['mimetype'], - mightSkip: data['might_skip'], - }) - .catch(() => { - context.commit('ADD_FILE', { - id: fileObject.id, - loaded: 0, - error: fileErrors.UPLOAD_FAILED, - }); - return fileErrors.UPLOAD_FAILED; - }); // End upload file - // Resolve with a summary of the uploaded file - // and a promise that can be chained from for file - // upload completion - resolve({ fileObject, promise }); - // Asynchronously generate file preview - const reader = new FileReader(); - reader.readAsDataURL(file); - reader.onloadend = () => { - if (reader.result) { - context.commit('ADD_FILE', { - id: data.file.id, - previewSrc: reader.result, - }); - } - }; - }) - .catch(error => { - let errorType = fileErrors.UPLOAD_FAILED; - if (error.response && error.response.status === 412) { - errorType = fileErrors.NO_STORAGE; - } - const fileObject = { - checksum, - loaded: 0, - total: file.size, - file_size: file.size, - original_filename: file.name, - file_format, - preset: metadata.preset, - error: errorType, - }; - context.commit('ADD_FILE', fileObject); - // Resolve with a summary of the uploaded file - resolve(fileObject); - }); // End get upload url - }) - .catch(() => { - reject(fileErrors.CHECKSUM_HASH_FAILED); - }); // End get hash - }); + .then(() => fileObject) + .catch(() => { + // Update vuex with failure + context.commit('ADD_FILE', { + id: fileObject.id, + loaded: 0, + error: fileErrors.UPLOAD_FAILED, + }); + return Promise.reject(fileErrors.UPLOAD_FAILED); + }); + // End upload file + return { fileObject, uploadPromise }; + }) + .catch(error => { + // If error isn't one of defined error constants, raise it + if (!Object.values(fileErrors).includes(error)) { + throw error; + } + const fileObject = { + checksum, + loaded: 0, + total: file.size, + file_size: file.size, + original_filename: file.name, + file_format, + preset: metadata.preset, + error, + }; + context.commit('ADD_FILE', fileObject); + return { fileObject, uploadPromise: Promise.reject(error) }; + }); } export function getAudioData(context, url) { diff --git a/contentcuration/contentcuration/frontend/shared/vuex/indexedDBPlugin/index.js b/contentcuration/contentcuration/frontend/shared/vuex/indexedDBPlugin/index.js index 52a1475260..9f4fddfb57 100644 --- a/contentcuration/contentcuration/frontend/shared/vuex/indexedDBPlugin/index.js +++ b/contentcuration/contentcuration/frontend/shared/vuex/indexedDBPlugin/index.js @@ -114,11 +114,15 @@ export default function IndexedDBPlugin(db, listeners = []) { if (change.type === CHANGE_TYPES.UPDATED) { obj = change.mods; } + // omit the last fetched attribute used only in resource layer + const mods = omit(obj, [LAST_FETCHED]); + if (Object.keys(mods).length === 0) { + return; + } events.emit(getEventName(change.table, change.type), { // we ensure we invoke the listeners with an object that has the PK [db[change.table].schema.primKey.keyPath]: change.key, - // spread the object, omitting the last fetched attribute used only in resource layer - ...omit(obj, [LAST_FETCHED]), + ...mods, }); }); }); diff --git a/contentcuration/contentcuration/management/commands/reconcile_change_tasks.py b/contentcuration/contentcuration/management/commands/reconcile_change_tasks.py new file mode 100644 index 0000000000..6fde77c781 --- /dev/null +++ b/contentcuration/contentcuration/management/commands/reconcile_change_tasks.py @@ -0,0 +1,44 @@ +import logging + +from django.core.management.base import BaseCommand + +from contentcuration.celery import app +from contentcuration.models import Change +from contentcuration.models import User + +logging.basicConfig() +logger = logging.getLogger('command') + + +class Command(BaseCommand): + """ + Reconciles that unready tasks are marked as reserved or active according to celery control + """ + + def handle(self, *args, **options): + from contentcuration.tasks import apply_channel_changes_task + from contentcuration.tasks import apply_user_changes_task + + active_task_ids = [task['id'] for task in app.get_active_and_reserved_tasks()] + + channel_changes = Change.objects.filter(channel_id__isnull=False, applied=False, errored=False) \ + .order_by('channel_id', 'created_by_id') \ + .values('channel_id', 'created_by_id') \ + .distinct() + for channel_change in channel_changes: + apply_channel_changes_task.revoke(exclude_task_ids=active_task_ids, channel_id=channel_change['channel_id']) + apply_channel_changes_task.fetch_or_enqueue( + User.objects.get(pk=channel_change['created_by_id']), + channel_id=channel_change['channel_id'] + ) + + user_changes = Change.objects.filter(channel_id__isnull=True, user_id__isnull=False, applied=False, errored=False) \ + .order_by('user_id', 'created_by_id') \ + .values('user_id', 'created_by_id') \ + .distinct() + for user_change in user_changes: + apply_user_changes_task.revoke(exclude_task_ids=active_task_ids, user_id=user_change['user_id']) + apply_user_changes_task.fetch_or_enqueue( + User.objects.get(pk=user_change['created_by_id']), + user_id=user_change['user_id'] + ) diff --git a/contentcuration/contentcuration/tests/helpers.py b/contentcuration/contentcuration/tests/helpers.py index 8e3172fcd4..eeddd11c5d 100644 --- a/contentcuration/contentcuration/tests/helpers.py +++ b/contentcuration/contentcuration/tests/helpers.py @@ -2,6 +2,7 @@ from importlib import import_module import mock +from celery import states from contentcuration.models import TaskResult @@ -20,6 +21,7 @@ def clear_tasks(except_task_id=None): qs = qs.exclude(task_id=except_task_id) for task_id in qs.values_list("task_id", flat=True): app.control.revoke(task_id, terminate=True) + qs.update(status=states.REVOKED) def mock_class_instance(target): diff --git a/contentcuration/contentcuration/tests/test_asynctask.py b/contentcuration/contentcuration/tests/test_asynctask.py index 85b40b6237..e24e04933e 100644 --- a/contentcuration/contentcuration/tests/test_asynctask.py +++ b/contentcuration/contentcuration/tests/test_asynctask.py @@ -142,6 +142,9 @@ def _wait_for(self, async_result, timeout=30): with allow_join_result(): return async_result.get(timeout=timeout) + def test_app_count_queued_tasks(self): + self.assertIsInstance(app.count_queued_tasks(), int) + def test_asynctask_reports_success(self): """ Tests that when an async task is created and completed, the Task object has a status of 'SUCCESS' and @@ -245,3 +248,17 @@ def test_requeue_task(self): second_result = self._wait_for(second_async_result) self.assertIsNone(second_result) self.assertTrue(second_async_result.successful()) + + def test_revoke_task(self): + channel_id = uuid.uuid4() + async_result = test_task.enqueue(self.user, channel_id=channel_id) + test_task.revoke(channel_id=channel_id) + + # this should raise an exception, even though revoked, because the task is in ready state but not success + with self.assertRaises(Exception): + self._wait_for(async_result) + + try: + TaskResult.objects.get(task_id=async_result.task_id, status=states.REVOKED) + except TaskResult.DoesNotExist: + self.fail('Missing revoked task result') diff --git a/contentcuration/contentcuration/tests/test_decorators.py b/contentcuration/contentcuration/tests/test_decorators.py index c3fb08e0eb..2c795716d7 100644 --- a/contentcuration/contentcuration/tests/test_decorators.py +++ b/contentcuration/contentcuration/tests/test_decorators.py @@ -2,17 +2,22 @@ from contentcuration.decorators import delay_user_storage_calculation from contentcuration.tests.base import StudioTestCase +from contentcuration.tests.base import testdata from contentcuration.utils.user import calculate_user_storage class DecoratorsTestCase(StudioTestCase): + def setUp(self): + super(DecoratorsTestCase, self).setUp() + self.user = testdata.user() + @mock.patch("contentcuration.utils.user.calculate_user_storage_task") def test_delay_storage_calculation(self, mock_task): @delay_user_storage_calculation def do_test(): - calculate_user_storage(self.admin_user.id) - calculate_user_storage(self.admin_user.id) + calculate_user_storage(self.user.id) + calculate_user_storage(self.user.id) mock_task.fetch_or_enqueue.assert_not_called() do_test() - mock_task.fetch_or_enqueue.assert_called_once_with(self.admin_user, user_id=self.admin_user.id) + mock_task.fetch_or_enqueue.assert_called_once_with(self.user, user_id=self.user.id) diff --git a/contentcuration/contentcuration/tests/viewsets/base.py b/contentcuration/contentcuration/tests/viewsets/base.py index 5eca0415fe..48ddd1c995 100644 --- a/contentcuration/contentcuration/tests/viewsets/base.py +++ b/contentcuration/contentcuration/tests/viewsets/base.py @@ -5,6 +5,9 @@ from contentcuration.celery import app from contentcuration.models import Change from contentcuration.tests.helpers import clear_tasks +from contentcuration.viewsets.sync.constants import CHANNEL +from contentcuration.viewsets.sync.constants import SYNCED +from contentcuration.viewsets.sync.utils import _generate_event as base_generate_event from contentcuration.viewsets.sync.utils import generate_copy_event as base_generate_copy_event from contentcuration.viewsets.sync.utils import generate_create_event as base_generate_create_event from contentcuration.viewsets.sync.utils import generate_delete_event as base_generate_delete_event @@ -35,6 +38,16 @@ def generate_update_event(*args, **kwargs): return event +def generate_sync_channel_event(channel_id, attributes, tags, files, assessment_items): + event = base_generate_event(key=channel_id, table=CHANNEL, event_type=SYNCED, channel_id=channel_id, user_id=None) + event["rev"] = random.randint(1, 10000000) + event["attributes"] = attributes + event["tags"] = tags + event["files"] = files + event["assessment_items"] = assessment_items + return event + + class SyncTestMixin(object): celery_task_always_eager = None diff --git a/contentcuration/contentcuration/tests/viewsets/test_channel.py b/contentcuration/contentcuration/tests/viewsets/test_channel.py index 63b0940ae4..b88f54ad98 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_channel.py +++ b/contentcuration/contentcuration/tests/viewsets/test_channel.py @@ -2,6 +2,7 @@ import uuid +import mock from django.urls import reverse from contentcuration import models @@ -9,6 +10,7 @@ from contentcuration.tests.base import StudioAPITestCase from contentcuration.tests.viewsets.base import generate_create_event from contentcuration.tests.viewsets.base import generate_delete_event +from contentcuration.tests.viewsets.base import generate_sync_channel_event from contentcuration.tests.viewsets.base import generate_update_event from contentcuration.tests.viewsets.base import SyncTestMixin from contentcuration.viewsets.sync.constants import CHANNEL @@ -273,6 +275,30 @@ def test_cannot_delete_some_channels(self): self.assertTrue(models.Channel.objects.get(id=channel1.id).deleted) self.assertFalse(models.Channel.objects.get(id=channel2.id).deleted) + @mock.patch("contentcuration.viewsets.channel.sync_channel") + def test_sync_channel_called_correctly(self, sync_channel_mock): + user = testdata.user() + channel = testdata.channel() + channel.editors.add(user) + channel_node = channel.main_tree.get_descendants().first() + channel_node.copy_to(target=channel.main_tree) + + self.client.force_authenticate(user=user) + for i in range(1, 5): + sync_channel_mock.reset_mock() + args = [channel.id, False, False, False, False] + args[i] = True + + response = self.sync_changes( + [ + generate_sync_channel_event(*args) + ] + ) + + self.assertEqual(response.status_code, 200) + self.assertEqual(sync_channel_mock.call_args.args[i], True) + sync_channel_mock.assert_called_once() + class CRUDTestCase(StudioAPITestCase): @property diff --git a/contentcuration/contentcuration/urls.py b/contentcuration/contentcuration/urls.py index 4f0e5358e6..a39f750883 100644 --- a/contentcuration/contentcuration/urls.py +++ b/contentcuration/contentcuration/urls.py @@ -78,6 +78,8 @@ def get_redirect_url(self, *args, **kwargs): re_path(r'^api/probers/get_prober_channel', views.get_prober_channel, name='get_prober_channel'), re_path(r'^api/probers/publishing_status', views.publishing_status, name='publishing_status'), re_path(r'^api/probers/celery_worker_status', views.celery_worker_status, name='celery_worker_status'), + re_path(r'^api/probers/task_queue_status', views.task_queue_status, name='task_queue_status'), + re_path(r'^api/probers/unapplied_changes_status', views.unapplied_changes_status, name='unapplied_changes_status'), re_path(r'^api/sync/$', SyncView.as_view(), name="sync"), ] diff --git a/contentcuration/contentcuration/utils/celery/app.py b/contentcuration/contentcuration/utils/celery/app.py index 5870947bad..9d8c260700 100644 --- a/contentcuration/contentcuration/utils/celery/app.py +++ b/contentcuration/contentcuration/utils/celery/app.py @@ -1,9 +1,11 @@ import base64 import json +import redis.exceptions from celery import Celery from contentcuration.utils.celery.tasks import CeleryTask +from contentcuration.utils.sentry import report_exception class CeleryApp(Celery): @@ -45,6 +47,35 @@ def get_queued_tasks(self, queue_name="celery"): return decoded_tasks + def count_queued_tasks(self, queue_name="celery"): + """ + :param queue_name: The queue name, defaults to the default "celery" queue + :return: int + """ + count = 0 + try: + with self.pool.acquire(block=True) as conn: + count = conn.default_channel.client.llen(queue_name) + except redis.exceptions.RedisError as e: + # log these so we can get visibility into the reliability of the redis connection + report_exception(e) + pass + return count + + def get_active_and_reserved_tasks(self): + """ + Iterate over active and reserved tasks + :return: A list of dictionaries + """ + active = self.control.inspect().active() or {} + for _, tasks in active.items(): + for task in tasks: + yield task + reserved = self.control.inspect().reserved() or {} + for _, tasks in reserved.items(): + for task in tasks: + yield task + def decode_result(self, result, status=None): """ Decodes the celery result, like the raw result from the database, using celery tools diff --git a/contentcuration/contentcuration/utils/celery/tasks.py b/contentcuration/contentcuration/utils/celery/tasks.py index 0d066d3329..630fa92d0b 100644 --- a/contentcuration/contentcuration/utils/celery/tasks.py +++ b/contentcuration/contentcuration/utils/celery/tasks.py @@ -246,6 +246,25 @@ def requeue(self, **kwargs): logging.info(f"Re-queuing task {self.name} for user {task_result.user.pk} from {request.id} | {task_kwargs}") return self.enqueue(task_result.user, **task_kwargs) + def revoke(self, exclude_task_ids=None, **kwargs): + """ + Revokes and terminates all unready tasks matching the kwargs + :param exclude_task_ids: Any task ids to exclude from this behavior + :param kwargs: Task keyword arguments that will be used to match against tasks + :return: The number of tasks revoked + """ + task_ids = self.find_incomplete_ids(**self.backend.decode(self._prepare_kwargs(kwargs))) + if exclude_task_ids is not None: + task_ids = task_ids.exclude(task_id__in=task_ids) + count = 0 + for task_id in task_ids: + logging.info(f"Revoking task {task_id}") + self.app.control.revoke(task_id, terminate=True) + count += 1 + # be sure the database backend has these marked appropriately + self.TaskModel.objects.filter(task_id__in=task_ids).update(status=states.REVOKED) + return count + class CeleryAsyncResult(AsyncResult): """ diff --git a/contentcuration/contentcuration/utils/user.py b/contentcuration/contentcuration/utils/user.py index 673b0634c8..aeeffbf5b5 100644 --- a/contentcuration/contentcuration/utils/user.py +++ b/contentcuration/contentcuration/utils/user.py @@ -9,13 +9,14 @@ def calculate_user_storage(user_id): from contentcuration.decorators import delay_user_storage_calculation if delay_user_storage_calculation.is_active: - delay_user_storage_calculation.queue.append(user_id) + delay_user_storage_calculation.add(user_id) return try: if user_id is None: raise User.DoesNotExist user = User.objects.get(pk=user_id) - calculate_user_storage_task.fetch_or_enqueue(user, user_id=user_id) + if not user.is_admin: + calculate_user_storage_task.fetch_or_enqueue(user, user_id=user_id) except User.DoesNotExist: logging.error("Tried to calculate user storage for user with id {} but they do not exist".format(user_id)) diff --git a/contentcuration/contentcuration/views/base.py b/contentcuration/contentcuration/views/base.py index 3347513239..ae522d53dc 100644 --- a/contentcuration/contentcuration/views/base.py +++ b/contentcuration/contentcuration/views/base.py @@ -14,6 +14,7 @@ from django.db.models import UUIDField from django.db.models.functions import Cast from django.http import HttpResponse +from django.http import HttpResponseBadRequest from django.http import HttpResponseForbidden from django.http import HttpResponseNotFound from django.shortcuts import redirect @@ -173,6 +174,39 @@ def celery_worker_status(request): return Response(app.control.inspect().ping() or {}) +@api_view(["GET"]) +@authentication_classes((TokenAuthentication, SessionAuthentication)) +@permission_classes((IsAuthenticated,)) +def task_queue_status(request): + if not request.user.is_admin: + return HttpResponseForbidden() + + from contentcuration.celery import app + + return Response({ + 'queued_task_count': app.count_queued_tasks(), + }) + + +@api_view(["GET"]) +@authentication_classes((TokenAuthentication, SessionAuthentication)) +@permission_classes((IsAuthenticated,)) +def unapplied_changes_status(request): + if not request.user.is_admin: + return HttpResponseForbidden() + + from contentcuration.celery import app + + active_task_count = 0 + for _ in app.get_active_and_reserved_tasks(): + active_task_count += 1 + + return Response({ + 'active_task_count': active_task_count, + 'unapplied_changes_count': Change.objects.filter(applied=False, errored=False).count(), + }) + + """ END HEALTH CHECKS """ @@ -334,6 +368,8 @@ def activate_channel_endpoint(request): channel = Channel.filter_edit_queryset(Channel.objects.all(), request.user).get(pk=data["channel_id"]) except Channel.DoesNotExist: return HttpResponseNotFound("Channel not found") + if channel.staging_tree is None: + return HttpResponseBadRequest('Channel is not staged') try: activate_channel(channel, request.user) except PermissionDenied as e: diff --git a/contentcuration/contentcuration/views/internal.py b/contentcuration/contentcuration/views/internal.py index 754c8fb740..ccbc358e5e 100644 --- a/contentcuration/contentcuration/views/internal.py +++ b/contentcuration/contentcuration/views/internal.py @@ -37,6 +37,7 @@ from contentcuration.api import activate_channel from contentcuration.api import write_file_to_storage from contentcuration.constants import completion_criteria +from contentcuration.decorators import delay_user_storage_calculation from contentcuration.models import AssessmentItem from contentcuration.models import Change from contentcuration.models import Channel @@ -54,7 +55,6 @@ from contentcuration.utils.nodes import map_files_to_node from contentcuration.utils.nodes import map_files_to_slideshow_slide_item from contentcuration.utils.sentry import report_exception -from contentcuration.utils.tracing import trace from contentcuration.viewsets.sync.constants import CHANNEL from contentcuration.viewsets.sync.utils import generate_publish_event from contentcuration.viewsets.sync.utils import generate_update_event @@ -565,7 +565,7 @@ def __init__(self, node, errors): super(IncompleteNodeError, self).__init__(self.message) -@trace +@delay_user_storage_calculation def convert_data_to_nodes(user, content_data, parent_node): """ Parse dict and create nodes accordingly """ try: diff --git a/contentcuration/contentcuration/viewsets/channel.py b/contentcuration/contentcuration/viewsets/channel.py index 9c739cbdfe..81646d0626 100644 --- a/contentcuration/contentcuration/viewsets/channel.py +++ b/contentcuration/contentcuration/viewsets/channel.py @@ -559,7 +559,7 @@ def sync(self, pk, attributes=False, tags=False, files=False, assessment_items=F attributes, tags, files, - tags, + assessment_items, progress_tracker=progress_tracker, ) diff --git a/deploy/cloudprober.cfg b/deploy/cloudprober.cfg index 57bd0084db..c5a129455e 100644 --- a/deploy/cloudprober.cfg +++ b/deploy/cloudprober.cfg @@ -160,4 +160,28 @@ probe { timeout_msec: 10000 # 10s } +probe { + name: "unapplied_changes_status" + type: EXTERNAL + targets { dummy_targets {} } + external_probe { + mode: ONCE + command: "./probers/unapplied_changes_probe.py" + } + interval_msec: 1800000 # 30 minutes + timeout_msec: 20000 # 20s +} + +probe { + name: "task_queue_status" + type: EXTERNAL + targets { dummy_targets {} } + external_probe { + mode: ONCE + command: "./probers/task_queue_probe.py" + } + interval_msec: 600000 # 10 minutes + timeout_msec: 10000 # 10s +} + # Note: When deploying on GKE, the error logs can be found under GCE VM instance. diff --git a/deploy/probers/task_queue_probe.py b/deploy/probers/task_queue_probe.py new file mode 100755 index 0000000000..3a3b02cfed --- /dev/null +++ b/deploy/probers/task_queue_probe.py @@ -0,0 +1,21 @@ +#!/usr/bin/env python +from base import BaseProbe + + +class TaskQueueProbe(BaseProbe): + + metric = "task_queue_ping_latency_msec" + threshold = 50 + + def do_probe(self): + r = self.request("api/probers/task_queue_status/") + r.raise_for_status() + results = r.json() + + task_count = results.get('queued_task_count', 0) + if task_count >= self.threshold: + raise Exception("Task queue length is over threshold! {} > {}".format(task_count, self.threshold)) + + +if __name__ == "__main__": + TaskQueueProbe().run() diff --git a/deploy/probers/unapplied_changes_probe.py b/deploy/probers/unapplied_changes_probe.py new file mode 100755 index 0000000000..a3ceee915e --- /dev/null +++ b/deploy/probers/unapplied_changes_probe.py @@ -0,0 +1,22 @@ +#!/usr/bin/env python +from base import BaseProbe + + +class UnappliedChangesProbe(BaseProbe): + + metric = "unapplied__changes_ping_latency_msec" + + def do_probe(self): + r = self.request("api/probers/unapplied_changes_status/") + r.raise_for_status() + results = r.json() + + active_task_count = results.get('active_task_count', 0) + unapplied_changes_count = results.get('unapplied_changes_count', 0) + + if active_task_count == 0 and unapplied_changes_count > 0: + raise Exception("There are unapplied changes and no active tasks! {} unapplied changes".format(unapplied_changes_count)) + + +if __name__ == "__main__": + UnappliedChangesProbe().run() diff --git a/jest_config/jest.conf.js b/jest_config/jest.conf.js index eb4849b797..ff89cf2c24 100644 --- a/jest_config/jest.conf.js +++ b/jest_config/jest.conf.js @@ -25,7 +25,7 @@ module.exports = { '^.+\\.js$': '/node_modules/babel-jest', '.*\\.(vue)$': '/node_modules/vue-jest', }, - transformIgnorePatterns: ['/node_modules/(?!vuetify|epubjs|kolibri-design-system|kolibri-constants)'], + transformIgnorePatterns: ['/node_modules/(?!vuetify|epubjs|kolibri-design-system|kolibri-constants|axios)'], snapshotSerializers: ['/node_modules/jest-serializer-vue'], setupFilesAfterEnv: [path.resolve(__dirname, './setup')], coverageDirectory: '/coverage', diff --git a/package.json b/package.json index 6457bfe0ed..2bfb0743af 100644 --- a/package.json +++ b/package.json @@ -57,7 +57,7 @@ "@sentry/vue": "^7.11.1", "@toast-ui/editor": "^2.3.1", "ajv": "^8.9.0", - "axios": "^0.27.2", + "axios": "^1.2.0", "broadcast-channel": "^4.17.0", "codemirror": "5.58.2", "core-js": "^3.25.1", diff --git a/yarn.lock b/yarn.lock index a12cc9970d..e89b9eec52 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2739,13 +2739,14 @@ aws4@^1.8.0: resolved "https://registry.yarnpkg.com/aws4/-/aws4-1.11.0.tgz#d61f46d83b2519250e2784daf5b09479a8b41c59" integrity sha512-xh1Rl34h6Fi1DC2WWKfxUTVqRsNnr6LsKz2+hfwDxQJWmrx8+c7ylaqBMcHfl1U1r2dsifOvKX3LQuLNZ+XSvA== -axios@^0.27.2: - version "0.27.2" - resolved "https://registry.yarnpkg.com/axios/-/axios-0.27.2.tgz#207658cc8621606e586c85db4b41a750e756d972" - integrity sha512-t+yRIyySRTp/wua5xEr+z1q60QmLq8ABsS5O9Me1AsE5dfKqgnCFzwiCZZ/cGNd1lq4/7akDWMxdhVlucjmnOQ== +axios@^1.2.0: + version "1.2.0" + resolved "https://registry.yarnpkg.com/axios/-/axios-1.2.0.tgz#1cb65bd75162c70e9f8d118a905126c4a201d383" + integrity sha512-zT7wZyNYu3N5Bu0wuZ6QccIf93Qk1eV8LOewxgjOZFd2DenOs98cJ7+Y6703d0wkaXGY6/nZd4EweJaHz9uzQw== dependencies: - follow-redirects "^1.14.9" + follow-redirects "^1.15.0" form-data "^4.0.0" + proxy-from-env "^1.1.0" babel-code-frame@^6.26.0: version "6.26.0" @@ -5352,11 +5353,16 @@ flush-promises@^1.0.2: resolved "https://registry.yarnpkg.com/flush-promises/-/flush-promises-1.0.2.tgz#4948fd58f15281fed79cbafc86293d5bb09b2ced" integrity sha512-G0sYfLQERwKz4+4iOZYQEZVpOt9zQrlItIxQAAYAWpfby3gbHrx0osCHz5RLl/XoXevXk0xoN4hDFky/VV9TrA== -follow-redirects@^1.0.0, follow-redirects@^1.14.9: +follow-redirects@^1.0.0: version "1.15.1" resolved "https://registry.yarnpkg.com/follow-redirects/-/follow-redirects-1.15.1.tgz#0ca6a452306c9b276e4d3127483e29575e207ad5" integrity sha512-yLAMQs+k0b2m7cVxpS1VKJVvoz7SS9Td1zss3XRwXj+ZDH00RJgnuLx7E44wx02kQLrdM3aOOy+FpzS7+8OizA== +follow-redirects@^1.15.0: + version "1.15.2" + resolved "https://registry.yarnpkg.com/follow-redirects/-/follow-redirects-1.15.2.tgz#b460864144ba63f2681096f274c4e57026da2c13" + integrity sha512-VQLG33o04KaQ8uYi2tVNbdrWp1QWxNNea+nmIB4EVM28v0hmP17z7aG1+wAkNzVq4KeXTq3221ye5qTJP91JwA== + for-each@^0.3.3: version "0.3.3" resolved "https://registry.yarnpkg.com/for-each/-/for-each-0.3.3.tgz#69b447e88a0a5d32c3e7084f3f1710034b21376e" @@ -9951,6 +9957,11 @@ proxy-addr@~2.0.7: forwarded "0.2.0" ipaddr.js "1.9.1" +proxy-from-env@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/proxy-from-env/-/proxy-from-env-1.1.0.tgz#e102f16ca355424865755d2c9e8ea4f24d58c3e2" + integrity sha512-D+zkORCbA9f1tdWRK0RaCR3GPv50cMxcrz4X8k5LTSUD1Dkw47mKJEZQNunItRTkWwgtaUSo1RVFRIG9ZXiFYg== + prr@~1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/prr/-/prr-1.0.1.tgz#d3fc114ba06995a45ec6893f484ceb1d78f5f476"