Преглед на файлове

Remove spell check languages that are only available on the server (#21056)

GitOrigin-RevId: cfe10a18af8149327754b3a2e62883c7ebc04bfc
Alf Eaton преди 1 година
родител
ревизия
04dbb7d2f2

+ 86 - 30
services/web/app/src/Features/Editor/EditorHttpController.js

@@ -13,6 +13,8 @@ const Errors = require('../Errors/Errors')
 const DocstoreManager = require('../Docstore/DocstoreManager')
 const DocstoreManager = require('../Docstore/DocstoreManager')
 const logger = require('@overleaf/logger')
 const logger = require('@overleaf/logger')
 const { expressify } = require('@overleaf/promise-utils')
 const { expressify } = require('@overleaf/promise-utils')
+const Settings = require('@overleaf/settings')
+const SplitTestHandler = require('../SplitTests/SplitTestHandler')
 
 
 module.exports = {
 module.exports = {
   joinProject: expressify(joinProject),
   joinProject: expressify(joinProject),
@@ -27,28 +29,6 @@ module.exports = {
   _nameIsAcceptableLength,
   _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) {
 async function joinProject(req, res, next) {
   const projectId = req.params.Project_id
   const projectId = req.params.Project_id
   let userId = req.body.userId // keep schema in sync with router
   let userId = req.body.userId // keep schema in sync with router
@@ -76,15 +56,39 @@ async function joinProject(req, res, next) {
   if (project.deletedByExternalDataSource) {
   if (project.deletedByExternalDataSource) {
     await ProjectDeleter.promises.unmarkAsDeletedByExternalSource(projectId)
     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({
   res.json({
     project,
     project,
     privilegeLevel,
     privilegeLevel,
@@ -286,3 +290,55 @@ async function deleteEntity(req, res, next) {
   )
   )
   res.sendStatus(204)
   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 ''
+  }
+}

+ 1 - 0
services/web/app/src/Features/Project/ProjectController.js

@@ -342,6 +342,7 @@ const _ProjectController = {
       'default-visual-for-beginners',
       'default-visual-for-beginners',
       'hotjar',
       'hotjar',
       'spell-check-client',
       'spell-check-client',
+      'spell-check-no-server',
     ].filter(Boolean)
     ].filter(Boolean)
 
 
     const getUserValues = async userId =>
     const getUserValues = async userId =>

+ 1 - 3
services/web/config/settings.defaults.js

@@ -453,7 +453,6 @@ module.exports = {
     { code: 'an', dic: 'an_ES', name: 'Aragonese', server: false },
     { code: 'an', dic: 'an_ES', name: 'Aragonese', server: false },
     { code: 'ar', dic: 'ar', name: 'Arabic' },
     { code: 'ar', dic: 'ar', name: 'Arabic' },
     { code: 'be_BY', dic: 'be_BY', name: 'Belarusian', server: false },
     { code: 'be_BY', dic: 'be_BY', name: 'Belarusian', server: false },
-    { code: 'gl', dic: 'gl_ES', name: 'Galician' },
     { code: 'eu', dic: 'eu', name: 'Basque' },
     { code: 'eu', dic: 'eu', name: 'Basque' },
     { code: 'bn_BD', dic: 'bn_BD', name: 'Bengali', server: false },
     { code: 'bn_BD', dic: 'bn_BD', name: 'Bengali', server: false },
     { code: 'bs_BA', dic: 'bs_BA', name: 'Bosnian', 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: 'et', dic: 'et_EE', name: 'Estonian' },
     { code: 'fo', dic: 'fo', name: 'Faroese' },
     { code: 'fo', dic: 'fo', name: 'Faroese' },
     { code: 'fr', dic: 'fr', name: 'French' },
     { 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', dic: 'de_DE', name: 'German' },
     { code: 'de_AT', dic: 'de_AT', name: 'German (Austria)', server: false },
     { code: 'de_AT', dic: 'de_AT', name: 'German (Austria)', server: false },
     {
     {
@@ -511,7 +510,6 @@ module.exports = {
       code: 'pt_PT',
       code: 'pt_PT',
       dic: 'pt_PT',
       dic: 'pt_PT',
       name: 'Portuguese (European)',
       name: 'Portuguese (European)',
-      server: true,
     },
     },
     { code: 'pa', name: 'Punjabi' },
     { code: 'pa', name: 'Punjabi' },
     { code: 'ro', dic: 'ro_RO', name: 'Romanian' },
     { code: 'ro', dic: 'ro_RO', name: 'Romanian' },

+ 11 - 9
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 type { Optgroup } from './settings-menu-select'
 import { useFeatureFlag } from '@/shared/context/split-test-context'
 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() {
 export default function SettingsSpellCheckLanguage() {
   const { t } = useTranslation()
   const { t } = useTranslation()
   const languages = getMeta('ol-languages')
   const languages = getMeta('ol-languages')
 
 
   const spellCheckClientEnabled = useFeatureFlag('spell-check-client')
   const spellCheckClientEnabled = useFeatureFlag('spell-check-client')
+  const spellCheckNoServer = useFeatureFlag('spell-check-no-server')
 
 
   const { spellCheckLanguage, setSpellCheckLanguage } =
   const { spellCheckLanguage, setSpellCheckLanguage } =
     useProjectSettingsContext()
     useProjectSettingsContext()
 
 
   const optgroup: Optgroup = useMemo(() => {
   const optgroup: Optgroup = useMemo(() => {
-    const options = (languages ?? []).filter(lang => {
-      const clientOnly = lang.server === false
+    const options = (languages ?? []).filter(language => {
+      if (!spellCheckClientEnabled) {
+        // only include spell-check languages that are available on the server
+        return language.server !== false
+      }
 
 
-      if (clientOnly && !showClientOnlyLanguages) {
-        return false
+      if (spellCheckNoServer) {
+        // only include spell-check languages that are available in the client
+        return language.dic !== undefined
       }
       }
 
 
-      return spellCheckClientEnabled || !clientOnly
+      return true
     })
     })
 
 
     return {
     return {
@@ -36,7 +38,7 @@ export default function SettingsSpellCheckLanguage() {
         label: language.name,
         label: language.name,
       })),
       })),
     }
     }
-  }, [languages, spellCheckClientEnabled])
+  }, [languages, spellCheckClientEnabled, spellCheckNoServer])
 
 
   return (
   return (
     <SettingsMenuSelect
     <SettingsMenuSelect

+ 82 - 7
services/web/test/frontend/features/source-editor/components/codemirror-editor-spellchecker-client.spec.tsx

@@ -7,13 +7,87 @@ import forEach from 'mocha-each'
 import PackageVersions from '../../../../../app/src/infrastructure/PackageVersions'
 import PackageVersions from '../../../../../app/src/infrastructure/PackageVersions'
 
 
 const languages = [
 const languages = [
+  { code: 'af', dic: 'af_ZA', name: 'Afrikaans' },
+  { code: 'an', dic: 'an_ES', name: 'Aragonese' },
+  { code: 'ar', dic: 'ar', name: 'Arabic' },
+  { code: 'be_BY', dic: 'be_BY', name: 'Belarusian' },
+  { code: 'bg', dic: 'bg_BG', name: 'Bulgarian' },
+  { code: 'bn_BD', dic: 'bn_BD', name: 'Bengali' },
+  { code: 'bo', dic: 'bo', name: 'Tibetan' },
+  { code: 'br', dic: 'br_FR', name: 'Breton' },
+  { code: 'bs_BA', dic: 'bs_BA', name: 'Bosnian' },
+  { code: 'ca', dic: 'ca', name: 'Catalan' },
+  { code: 'cs', dic: 'cs_CZ', name: 'Czech' },
+  { code: 'de', dic: 'de_DE', name: 'German' },
+  { code: 'de_AT', dic: 'de_AT', name: 'German (Austria)' },
+  { code: 'de_CH', dic: 'de_CH', name: 'German (Switzerland)' },
+  { code: 'dz', dic: 'dz', name: 'Dzongkha' },
+  { code: 'el', dic: 'el_GR', name: 'Greek' },
+  { code: 'en_AU', dic: 'en_AU', name: 'English (Australian)' },
+  { code: 'en_CA', dic: 'en_CA', name: 'English (Canadian)' },
   { code: 'en_GB', dic: 'en_GB', name: 'English (British)' },
   { code: 'en_GB', dic: 'en_GB', name: 'English (British)' },
+  { code: 'en_US', dic: 'en_US', name: 'English (American)' },
+  { code: 'en_ZA', dic: 'en_ZA', name: 'English (South African)' },
+  { code: 'eo', dic: 'eo', name: 'Esperanto' },
+  { code: 'es', dic: 'es_ES', name: 'Spanish' },
+  { code: 'et', dic: 'et_EE', name: 'Estonian' },
+  { code: 'eu', dic: 'eu', name: 'Basque' },
+  { code: 'fa', dic: 'fa_IR', name: 'Persian' },
+  { code: 'fo', dic: 'fo', name: 'Faroese' },
   { code: 'fr', dic: 'fr', name: 'French' },
   { code: 'fr', dic: 'fr', name: 'French' },
+  { code: 'ga', dic: 'ga_IE', name: 'Irish' },
+  { code: 'gd_GB', dic: 'gd_GB', name: 'Scottish Gaelic' },
+  { code: 'gl', dic: 'gl_ES', name: 'Galician' },
+  { code: 'gu_IN', dic: 'gu_IN', name: 'Gujarati' },
+  { code: 'gug_PY', dic: 'gug_PY', name: 'Guarani' },
+  { code: 'he_IL', dic: 'he_IL', name: 'Hebrew' },
+  { code: 'hi_IN', dic: 'hi_IN', name: 'Hindi' },
+  { code: 'hr', dic: 'hr_HR', name: 'Croatian' },
+  { code: 'hu_HU', dic: 'hu_HU', name: 'Hungarian' },
+  { code: 'id', dic: 'id_ID', name: 'Indonesian' },
+  { code: 'is_IS', dic: 'is_IS', name: 'Icelandic' },
+  { code: 'it', dic: 'it_IT', name: 'Italian' },
+  { code: 'kk', dic: 'kk_KZ', name: 'Kazakh' },
+  { code: 'kmr', dic: 'kmr_Latn', name: 'Kurmanji' },
+  { code: 'ko', dic: 'ko', name: 'Korean' },
+  { code: 'lo_LA', dic: 'lo_LA', name: 'Laotian' },
+  { code: 'lt', dic: 'lt_LT', name: 'Lithuanian' },
+  { code: 'lv', dic: 'lv_LV', name: 'Latvian' },
+  { code: 'ml_IN', dic: 'ml_IN', name: 'Malayalam' },
+  { code: 'mn_MN', dic: 'mn_MN', name: 'Mongolian' },
+  { code: 'nb_NO', dic: 'nb_NO', name: 'Norwegian (Bokmål)' },
+  { code: 'ne_NP', dic: 'ne_NP', name: 'Nepali' },
+  { code: 'nl', dic: 'nl', name: 'Dutch' },
+  { code: 'nn_NO', dic: 'nn_NO', name: 'Norwegian (Nynorsk)' },
+  { code: 'oc_FR', dic: 'oc_FR', name: 'Occitan' },
+  { code: 'pl', dic: 'pl_PL', name: 'Polish' },
+  { code: 'pt_BR', dic: 'pt_BR', name: 'Portuguese (Brazilian)' },
+  { code: 'pt_PT', dic: 'pt_PT', name: 'Portuguese (European)' },
+  { code: 'ro', dic: 'ro_RO', name: 'Romanian' },
+  { code: 'ru', dic: 'ru_RU', name: 'Russian' },
+  { code: 'si_LK', dic: 'si_LK', name: 'Sinhala' },
+  { code: 'sk', dic: 'sk_SK', name: 'Slovak' },
+  { code: 'sl', dic: 'sl_SI', name: 'Slovenian' },
+  { code: 'sr_RS', dic: 'sr_RS', name: 'Serbian' },
   { code: 'sv', dic: 'sv_SE', name: 'Swedish' },
   { code: 'sv', dic: 'sv_SE', name: 'Swedish' },
+  { code: 'sw_TZ', dic: 'sw_TZ', name: 'Swahili' },
+  { code: 'te_IN', dic: 'te_IN', name: 'Telugu' },
+  { code: 'th_TH', dic: 'th_TH', name: 'Thai' },
+  { code: 'tl', dic: 'tl', name: 'Tagalog' },
+  { code: 'tr_TR', dic: 'tr_TR', name: 'Turkish' },
+  { code: 'uz_UZ', dic: 'uz_UZ', name: 'Uzbek' },
+  { code: 'vi_VN', dic: 'vi_VN', name: 'Vietnamese' },
 ]
 ]
 
 
 const suggestions = {
 const suggestions = {
+  af: ['medicyne', 'medisyne'],
+  be_BY: ['лекi', 'лекі'],
+  bg: ['лекарствo', 'лекарство'],
+  de: ['Medicin', 'Medizin'],
+  en_CA: ['theatr', 'theatre'],
   en_GB: ['medecine', 'medicine'],
   en_GB: ['medecine', 'medicine'],
+  en_US: ['theatr', 'theater'],
+  es: ['medicaminto', 'medicamento'],
   fr: ['medecin', 'médecin'],
   fr: ['medecin', 'médecin'],
   sv: ['medecin', 'medicin'],
   sv: ['medecin', 'medicin'],
 }
 }
@@ -42,11 +116,11 @@ forEach(Object.keys(suggestions)).describe(
         win.metaAttributesCache.set('ol-preventCompileOnLoad', true)
         win.metaAttributesCache.set('ol-preventCompileOnLoad', true)
         win.metaAttributesCache.set('ol-splitTestVariants', {
         win.metaAttributesCache.set('ol-splitTestVariants', {
           'spell-check-client': 'enabled',
           'spell-check-client': 'enabled',
+          'spell-check-no-server': 'enabled',
         })
         })
         win.metaAttributesCache.set('ol-splitTestInfo', {
         win.metaAttributesCache.set('ol-splitTestInfo', {
-          'spell-check-client': {
-            phase: 'release',
-          },
+          'spell-check-client': { phase: 'release' },
+          'spell-check-no-server': { phase: 'release' },
         })
         })
         win.metaAttributesCache.set('ol-learnedWords', ['baz'])
         win.metaAttributesCache.set('ol-learnedWords', ['baz'])
         win.metaAttributesCache.set(
         win.metaAttributesCache.set(
@@ -77,12 +151,13 @@ forEach(Object.keys(suggestions)).describe(
     it('shows suggestions for misspelled word', function () {
     it('shows suggestions for misspelled word', function () {
       const [from, to] = suggestions[spellCheckLanguage]
       const [from, to] = suggestions[spellCheckLanguage]
 
 
-      cy.get('@line').type(from)
-      cy.get('@line').get('.ol-cm-spelling-error').contains(from)
+      cy.get('@line').type(`${from} ${to}`)
+      cy.get('@line').get('.ol-cm-spelling-error').should('have.length', 1)
+      cy.get('@line').get('.ol-cm-spelling-error').should('have.text', from)
 
 
       cy.get('@line').get('.ol-cm-spelling-error').rightclick()
       cy.get('@line').get('.ol-cm-spelling-error').rightclick()
-      cy.findByText(to).click()
-      cy.get('@line').contains(to)
+      cy.findByRole('menuitem', { name: to }).click()
+      cy.get('@line').contains(`${to} ${to}`)
       cy.get('@line').find('.ol-cm-spelling-error').should('not.exist')
       cy.get('@line').find('.ol-cm-spelling-error').should('not.exist')
     })
     })
   }
   }

+ 1 - 0
services/web/test/unit/src/Editor/EditorHttpControllerTests.js

@@ -131,6 +131,7 @@ describe('EditorHttpController', function () {
     }
     }
     this.SplitTestHandler = {
     this.SplitTestHandler = {
       promises: {
       promises: {
+        getAssignment: sinon.stub().resolves({ variant: 'default' }),
         getAssignmentForMongoUser: sinon
         getAssignmentForMongoUser: sinon
           .stub()
           .stub()
           .resolves({ variant: 'default' }),
           .resolves({ variant: 'default' }),