From 04dbb7d2f24f81b8b210f6c934db994d9e815b24 Mon Sep 17 00:00:00 2001 From: Alf Eaton Date: Mon, 21 Oct 2024 10:58:34 +0100 Subject: [PATCH] Remove spell check languages that are only available on the server (#21056) GitOrigin-RevId: cfe10a18af8149327754b3a2e62883c7ebc04bfc --- .../Features/Editor/EditorHttpController.js | 116 +++++++++++++----- .../src/Features/Project/ProjectController.js | 1 + services/web/config/settings.defaults.js | 4 +- .../settings-spell-check-language.tsx | 22 ++-- ...mirror-editor-spellchecker-client.spec.tsx | 89 ++++++++++++-- .../src/Editor/EditorHttpControllerTests.js | 1 + 6 files changed, 183 insertions(+), 50 deletions(-) diff --git a/services/web/app/src/Features/Editor/EditorHttpController.js b/services/web/app/src/Features/Editor/EditorHttpController.js index 5cdc205320..8efdc23d89 100644 --- a/services/web/app/src/Features/Editor/EditorHttpController.js +++ b/services/web/app/src/Features/Editor/EditorHttpController.js @@ -13,6 +13,8 @@ const Errors = require('../Errors/Errors') const DocstoreManager = require('../Docstore/DocstoreManager') const logger = require('@overleaf/logger') const { expressify } = require('@overleaf/promise-utils') +const Settings = require('@overleaf/settings') +const SplitTestHandler = require('../SplitTests/SplitTestHandler') module.exports = { joinProject: expressify(joinProject), @@ -27,28 +29,6 @@ module.exports = { _nameIsAcceptableLength, } -const unsupportedSpellcheckLanguages = [ - 'am', - 'hy', - 'bn', - 'gu', - 'he', - 'hi', - 'hu', - 'is', - 'kn', - 'ml', - 'mr', - 'or', - 'ss', - 'ta', - 'te', - 'uk', - 'uz', - 'zu', - 'fi', -] - async function joinProject(req, res, next) { const projectId = req.params.Project_id let userId = req.body.userId // keep schema in sync with router @@ -76,15 +56,39 @@ async function joinProject(req, res, next) { if (project.deletedByExternalDataSource) { await ProjectDeleter.promises.unmarkAsDeletedByExternalSource(projectId) } - // disable spellchecking for currently unsupported spell check languages - // preserve the value in the db so they can use it again once we add back - // support. - // TODO: allow these if in client-side spell check split test - if ( - unsupportedSpellcheckLanguages.indexOf(project.spellCheckLanguage) !== -1 - ) { - project.spellCheckLanguage = '' + + let spellCheckClient + try { + spellCheckClient = + (await SplitTestHandler.promises.getAssignment( + req, + res, + 'spell-check-client' + )?.variant) === 'enabled' + } catch { + spellCheckClient = false } + + let spellCheckNoServer + try { + spellCheckNoServer = + (await SplitTestHandler.promises.getAssignment( + req, + res, + 'spell-check-no-server' + )?.variant) === 'enabled' + } catch { + spellCheckNoServer = false + } + + if (project.spellCheckLanguage) { + project.spellCheckLanguage = await chooseSpellCheckLanguage( + project.spellCheckLanguage, + spellCheckClient, + spellCheckNoServer + ) + } + res.json({ project, privilegeLevel, @@ -286,3 +290,55 @@ async function deleteEntity(req, res, next) { ) res.sendStatus(204) } + +async function chooseSpellCheckLanguage( + spellCheckLanguage, + spellCheckClient = false, + spellCheckNoServer = false +) { + const supportedSpellCheckLanguages = new Set( + Settings.languages + // optionally only include languages which support client-side spell checking + .filter(language => { + if (!spellCheckClient) { + // only include spell-check languages that are available on the server + return language.server !== false + } + + if (spellCheckNoServer) { + // only include spell-check languages that are available in the client + return language.dic !== undefined + } + + return true + }) + .map(language => language.code) + ) + + if (supportedSpellCheckLanguages.has(spellCheckLanguage)) { + return spellCheckLanguage + } + + // Disable spell checking for currently unsupported spell check languages. + // Preserve the value in the database so they can use it again once we add back support. + + // existing behaviour: map all unsupported languages to "off" + if (!spellCheckNoServer) { + return '' + } + + // new behaviour: map some server-only languages to a specific variant + switch (spellCheckLanguage) { + case 'en': + // map "English" to "English (American)" + return 'en_US' + + case 'no': + // map "Norwegian" to "Norwegian (Bokmål)" + return 'nb_NO' + + default: + // map anything else to "off" + return '' + } +} diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index e152a60f10..a836dac80d 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/services/web/app/src/Features/Project/ProjectController.js @@ -342,6 +342,7 @@ const _ProjectController = { 'default-visual-for-beginners', 'hotjar', 'spell-check-client', + 'spell-check-no-server', ].filter(Boolean) const getUserValues = async userId => diff --git a/services/web/config/settings.defaults.js b/services/web/config/settings.defaults.js index fe040ac952..78d253bb4b 100644 --- a/services/web/config/settings.defaults.js +++ b/services/web/config/settings.defaults.js @@ -453,7 +453,6 @@ module.exports = { { code: 'an', dic: 'an_ES', name: 'Aragonese', server: false }, { code: 'ar', dic: 'ar', name: 'Arabic' }, { code: 'be_BY', dic: 'be_BY', name: 'Belarusian', server: false }, - { code: 'gl', dic: 'gl_ES', name: 'Galician' }, { code: 'eu', dic: 'eu', name: 'Basque' }, { code: 'bn_BD', dic: 'bn_BD', name: 'Bengali', server: false }, { code: 'bs_BA', dic: 'bs_BA', name: 'Bosnian', server: false }, @@ -469,7 +468,7 @@ module.exports = { { code: 'et', dic: 'et_EE', name: 'Estonian' }, { code: 'fo', dic: 'fo', name: 'Faroese' }, { code: 'fr', dic: 'fr', name: 'French' }, - { code: 'gl_ES', dic: 'gl_ES', name: 'Galician', server: false }, + { code: 'gl', dic: 'gl_ES', name: 'Galician', server: false }, { code: 'de', dic: 'de_DE', name: 'German' }, { code: 'de_AT', dic: 'de_AT', name: 'German (Austria)', server: false }, { @@ -511,7 +510,6 @@ module.exports = { code: 'pt_PT', dic: 'pt_PT', name: 'Portuguese (European)', - server: true, }, { code: 'pa', name: 'Punjabi' }, { code: 'ro', dic: 'ro_RO', name: 'Romanian' }, diff --git a/services/web/frontend/js/features/editor-left-menu/components/settings/settings-spell-check-language.tsx b/services/web/frontend/js/features/editor-left-menu/components/settings/settings-spell-check-language.tsx index 896ca5ab62..0d52ab4b49 100644 --- a/services/web/frontend/js/features/editor-left-menu/components/settings/settings-spell-check-language.tsx +++ b/services/web/frontend/js/features/editor-left-menu/components/settings/settings-spell-check-language.tsx @@ -6,27 +6,29 @@ import SettingsMenuSelect from './settings-menu-select' import type { Optgroup } from './settings-menu-select' import { useFeatureFlag } from '@/shared/context/split-test-context' -// allow selection of spell-check languages that are only supported in the client-side spell checker -const showClientOnlyLanguages = true - export default function SettingsSpellCheckLanguage() { const { t } = useTranslation() const languages = getMeta('ol-languages') const spellCheckClientEnabled = useFeatureFlag('spell-check-client') + const spellCheckNoServer = useFeatureFlag('spell-check-no-server') const { spellCheckLanguage, setSpellCheckLanguage } = useProjectSettingsContext() const optgroup: Optgroup = useMemo(() => { - const options = (languages ?? []).filter(lang => { - const clientOnly = lang.server === false - - if (clientOnly && !showClientOnlyLanguages) { - return false + const options = (languages ?? []).filter(language => { + if (!spellCheckClientEnabled) { + // only include spell-check languages that are available on the server + return language.server !== false } - return spellCheckClientEnabled || !clientOnly + if (spellCheckNoServer) { + // only include spell-check languages that are available in the client + return language.dic !== undefined + } + + return true }) return { @@ -36,7 +38,7 @@ export default function SettingsSpellCheckLanguage() { label: language.name, })), } - }, [languages, spellCheckClientEnabled]) + }, [languages, spellCheckClientEnabled, spellCheckNoServer]) return (