Merge pull request #21251 from overleaf/ae-revert-no-session

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

GitOrigin-RevId: 6f77aa92f1b7d7181cec7d470c8e6604b1e90fe3

services/web/app/src/Features/Editor/EditorHttpController.js

@@ -13,8 +13,6 @@ 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),
@@ -29,6 +27,28 @@ 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
@@ -56,39 +76,15 @@ async function joinProject(req, res, next) {
   if (project.deletedByExternalDataSource) {
     await ProjectDeleter.promises.unmarkAsDeletedByExternalSource(projectId)
   }
-
-  let spellCheckClient
-  try {
-    spellCheckClient =
-      (await SplitTestHandler.promises.getAssignment(
-        req,
-        res,
-        'spell-check-client'
-      )?.variant) === 'enabled'
-  } catch {
-    spellCheckClient = false
+  // 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 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,
@@ -290,55 +286,3 @@ 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 ''
-  }
-}

services/web/app/src/Features/Project/ProjectController.js

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

services/web/config/settings.defaults.js

@@ -453,6 +453,7 @@ 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 },
@@ -468,7 +469,7 @@ module.exports = {
     { code: 'et', dic: 'et_EE', name: 'Estonian' },
     { code: 'fo', dic: 'fo', name: 'Faroese' },
     { code: 'fr', dic: 'fr', name: 'French' },
-    { code: 'gl', dic: 'gl_ES', name: 'Galician', server: false },
+    { code: 'gl_ES', 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 },
     {
@@ -510,6 +511,7 @@ module.exports = {
       code: 'pt_PT',
       dic: 'pt_PT',
       name: 'Portuguese (European)',
+      server: true,
     },
     { code: 'pa', name: 'Punjabi' },
     { code: 'ro', dic: 'ro_RO', name: 'Romanian' },

services/web/frontend/js/features/editor-left-menu/components/settings/settings-spell-check-language.tsx

@@ -6,29 +6,27 @@ 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(language => {
-      if (!spellCheckClientEnabled) {
-        // only include spell-check languages that are available on the server
-        return language.server !== false
+    const options = (languages ?? []).filter(lang => {
+      const clientOnly = lang.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 true
+      return spellCheckClientEnabled || !clientOnly
     })
 
     return {
@@ -38,7 +36,7 @@ export default function SettingsSpellCheckLanguage() {
         label: language.name,
       })),
     }
-  }, [languages, spellCheckClientEnabled, spellCheckNoServer])
+  }, [languages, spellCheckClientEnabled])
 
   return (
     <SettingsMenuSelect

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

@@ -7,87 +7,13 @@ import forEach from 'mocha-each'
 import PackageVersions from '../../../../../app/src/infrastructure/PackageVersions'
 
 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_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: '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: '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 = {
-  af: ['medicyne', 'medisyne'],
-  be_BY: ['лекi', 'лекі'],
-  bg: ['лекарствo', 'лекарство'],
-  de: ['Medicin', 'Medizin'],
-  en_CA: ['theatr', 'theatre'],
   en_GB: ['medecine', 'medicine'],
-  en_US: ['theatr', 'theater'],
-  es: ['medicaminto', 'medicamento'],
   fr: ['medecin', 'médecin'],
   sv: ['medecin', 'medicin'],
 }
@@ -116,11 +42,11 @@ forEach(Object.keys(suggestions)).describe(
         win.metaAttributesCache.set('ol-preventCompileOnLoad', true)
         win.metaAttributesCache.set('ol-splitTestVariants', {
           'spell-check-client': 'enabled',
-          'spell-check-no-server': 'enabled',
         })
         win.metaAttributesCache.set('ol-splitTestInfo', {
-          'spell-check-client': { phase: 'release' },
-          'spell-check-no-server': { phase: 'release' },
+          'spell-check-client': {
+            phase: 'release',
+          },
         })
         win.metaAttributesCache.set('ol-learnedWords', ['baz'])
         win.metaAttributesCache.set(
@@ -151,13 +77,12 @@ forEach(Object.keys(suggestions)).describe(
     it('shows suggestions for misspelled word', function () {
       const [from, to] = suggestions[spellCheckLanguage]
 
-      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').type(from)
+      cy.get('@line').get('.ol-cm-spelling-error').contains(from)
 
       cy.get('@line').get('.ol-cm-spelling-error').rightclick()
-      cy.findByRole('menuitem', { name: to }).click()
-      cy.get('@line').contains(`${to} ${to}`)
+      cy.findByText(to).click()
+      cy.get('@line').contains(to)
       cy.get('@line').find('.ol-cm-spelling-error').should('not.exist')
     })
   }

services/web/test/unit/src/Editor/EditorHttpControllerTests.js

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