Reindex references on deleting or refreshing a .bib file (#14938)

* Reindex references on deleting or refreshing a .bib file

* Remove rendundant props

* Tweak file refresh payload, send refresh response after update to keys, remove some unnecessary returns

* Tidy up

GitOrigin-RevId: bc0309a54fbfd0eb7d8285032300453d360d6b2f

services/web/app/src/Features/LinkedFiles/LinkedFilesController.js

@@ -37,9 +37,12 @@ const {
 const {
   OutputFileFetchFailedError,
   FileTooLargeError,
+  OError,
 } = require('../Errors/Errors')
 const Modules = require('../../infrastructure/Modules')
 const { plainTextResponse } = require('../../infrastructure/Response')
+const ReferencesHandler = require('../References/ReferencesHandler')
+const EditorRealTimeController = require('../Editor/EditorRealTimeController')
 
 module.exports = LinkedFilesController = {
   Agents: _.extend(
@@ -122,7 +125,7 @@ module.exports = LinkedFilesController = {
           return res.sendStatus(400)
         }
 
-        return Agent.refreshLinkedFile(
+        Agent.refreshLinkedFile(
           projectId,
           linkedFileData,
           name,
@@ -132,7 +135,25 @@ module.exports = LinkedFilesController = {
             if (err != null) {
               return LinkedFilesController.handleError(err, req, res, next)
             }
-            return res.json({ new_file_id: newFileId })
+            if (req.body.shouldReindexReferences) {
+              ReferencesHandler.indexAll(projectId, function (error, data) {
+                if (error) {
+                  OError.tag(error, 'failed to index references', {
+                    projectId,
+                  })
+                  return next(error)
+                }
+                EditorRealTimeController.emitToRoom(
+                  projectId,
+                  'references:keys:updated',
+                  data.keys,
+                  true
+                )
+                res.json({ new_file_id: newFileId })
+              })
+            } else {
+              res.json({ new_file_id: newFileId })
+            }
           }
         )
       }

services/web/app/src/Features/References/ReferencesController.js

@@ -39,6 +39,7 @@ module.exports = ReferencesController = {
         res,
         projectId,
         shouldBroadcast,
+        false,
         data
       )
     })
@@ -57,12 +58,13 @@ module.exports = ReferencesController = {
         res,
         projectId,
         shouldBroadcast,
+        true,
         data
       )
     })
   },
 
-  _handleIndexResponse(req, res, projectId, shouldBroadcast, data) {
+  _handleIndexResponse(req, res, projectId, shouldBroadcast, isAllDocs, data) {
     if (data == null || data.keys == null) {
       return res.json({ projectId, keys: [] })
     }
@@ -70,7 +72,8 @@ module.exports = ReferencesController = {
       EditorRealTimeController.emitToRoom(
         projectId,
         'references:keys:updated',
-        data.keys
+        data.keys,
+        isAllDocs
       )
     }
     return res.json(data)

services/web/frontend/js/features/file-tree/components/file-tree-context.js

@@ -26,7 +26,7 @@ function FileTreeContext({
       reindexReferences={reindexReferences}
     >
       <FileTreeSelectableProvider onSelect={onSelect}>
-        <FileTreeActionableProvider>
+        <FileTreeActionableProvider reindexReferences={reindexReferences}>
           <FileTreeDraggableProvider>{children}</FileTreeDraggableProvider>
         </FileTreeActionableProvider>
       </FileTreeSelectableProvider>

services/web/frontend/js/features/file-tree/contexts/file-tree-actionable.js

@@ -119,7 +119,7 @@ function fileTreeActionableReducer(state, action) {
   }
 }
 
-export function FileTreeActionableProvider({ children }) {
+export function FileTreeActionableProvider({ reindexReferences, children }) {
   const { _id: projectId } = useProjectContext(projectContextPropTypes)
   const { permissionsLevel } = useEditorContext(editorContextPropTypes)
 
@@ -187,9 +187,12 @@ export function FileTreeActionableProvider({ children }) {
   // deletes entities in series. Tree will be updated via the socket event
   const finishDeleting = useCallback(() => {
     dispatch({ type: ACTION_TYPES.DELETING })
+    let shouldReindexReferences = false
 
     return mapSeries(Array.from(selectedEntityIds), id => {
       const found = findInTreeOrThrow(fileTreeData, id)
+      shouldReindexReferences =
+        shouldReindexReferences || /\.bib$/.test(found.entity.name)
       return syncDelete(projectId, found.type, found.entity._id).catch(
         error => {
           // throw unless 404
@@ -200,13 +203,16 @@ export function FileTreeActionableProvider({ children }) {
       )
     })
       .then(() => {
+        if (shouldReindexReferences) {
+          reindexReferences()
+        }
         dispatch({ type: ACTION_TYPES.CLEAR })
       })
       .catch(error => {
         // set an error and allow user to retry
         dispatch({ type: ACTION_TYPES.ERROR, error })
       })
-  }, [fileTreeData, projectId, selectedEntityIds])
+  }, [fileTreeData, projectId, selectedEntityIds, reindexReferences])
 
   // moves entities. Tree is updated immediately and data are sync'd after.
   const finishMoving = useCallback(
@@ -404,6 +410,7 @@ export function FileTreeActionableProvider({ children }) {
 }
 
 FileTreeActionableProvider.propTypes = {
+  reindexReferences: PropTypes.func.isRequired,
   children: PropTypes.oneOfType([
     PropTypes.arrayOf(PropTypes.node),
     PropTypes.node,

services/web/frontend/js/features/file-view/components/file-view-header.tsx

@@ -12,7 +12,6 @@ import importOverleafModules from '../../../../macros/import-overleaf-module.mac
 import useAbortController from '../../../shared/hooks/use-abort-controller'
 import { LinkedFileIcon } from './file-view-icons'
 import { BinaryFile, hasProvider, LinkedFile } from '../types/binary-file'
-import { debugConsole } from '@/utils/debugging'
 
 const tprLinkedFileInfo = importOverleafModules('tprLinkedFileInfo') as {
   import: { LinkedFileInfo: ElementType }
@@ -45,13 +44,9 @@ function shortenedUrl(url: string) {
 
 type FileViewHeaderProps = {
   file: BinaryFile
-  storeReferencesKeys: (keys: string[]) => void
 }
 
-export default function FileViewHeader({
-  file,
-  storeReferencesKeys,
-}: FileViewHeaderProps) {
+export default function FileViewHeader({ file }: FileViewHeaderProps) {
   const { _id: projectId } = useProjectContext({
     _id: PropTypes.string.isRequired,
   })
@@ -92,7 +87,16 @@ export default function FileViewHeader({
     setRefreshing(true)
     // Replacement of the file handled by the file tree
     window.expectingLinkedFileRefreshedSocketFor = file.name
-    postJSON(`/project/${projectId}/linked_file/${file.id}/refresh`, { signal })
+    const body = {
+      shouldReindexReferences:
+        file.linkedFileData?.provider === 'mendeley' ||
+        file.linkedFileData?.provider === 'zotero' ||
+        /\.bib$/.test(file.name),
+    }
+    postJSON(`/project/${projectId}/linked_file/${file.id}/refresh`, {
+      signal,
+      body,
+    })
       .then(() => {
         setRefreshing(false)
       })
@@ -100,29 +104,7 @@ export default function FileViewHeader({
         setRefreshing(false)
         setRefreshError(err.data?.message || err.message)
       })
-      .finally(() => {
-        if (
-          hasProvider(file, 'mendeley') ||
-          hasProvider(file, 'zotero') ||
-          file.name.match(/^.*\.bib$/)
-        ) {
-          reindexReferences()
-        }
-      })
-
-    function reindexReferences() {
-      const opts = {
-        body: { shouldBroadcast: true },
-      }
-
-      postJSON(`/project/${projectId}/references/indexAll`, opts)
-        .then(response => {
-          // Later updated by the socket but also updated here for immediate use
-          storeReferencesKeys(response.keys)
-        })
-        .catch(debugConsole.error)
-    }
-  }, [file, projectId, signal, storeReferencesKeys])
+  }, [file, projectId, signal])
 
   return (
     <div>

services/web/frontend/js/features/file-view/components/file-view.js

@@ -9,7 +9,7 @@ import Icon from '../../../shared/components/icon'
 
 const imageExtensions = ['png', 'jpg', 'jpeg', 'gif']
 
-export default function FileView({ file, storeReferencesKeys }) {
+export default function FileView({ file }) {
   const [contentLoading, setContentLoading] = useState(true)
   const [hasError, setHasError] = useState(false)
 
@@ -34,7 +34,7 @@ export default function FileView({ file, storeReferencesKeys }) {
 
   const content = (
     <>
-      <FileViewHeader file={file} storeReferencesKeys={storeReferencesKeys} />
+      <FileViewHeader file={file} />
       {imageExtensions.includes(extension) && (
         <FileViewImage
           fileName={file.name}
@@ -77,5 +77,4 @@ FileView.propTypes = {
     id: PropTypes.string,
     name: PropTypes.string,
   }).isRequired,
-  storeReferencesKeys: PropTypes.func.isRequired,
 }

services/web/frontend/js/ide/references/ReferencesManager.js

@@ -50,17 +50,17 @@ export default ReferencesManager = class ReferencesManager {
       // not on every reconnect
       if (!this.inited) {
         this.inited = true
-        this.ide.socket.on('references:keys:updated', keys =>
-          this._storeReferencesKeys(keys)
+        this.ide.socket.on('references:keys:updated', (keys, allDocs) =>
+          this._storeReferencesKeys(keys, allDocs)
         )
         this.indexAllReferences(false)
       }
     })
   }
 
-  _storeReferencesKeys(newKeys) {
+  _storeReferencesKeys(newKeys, replaceExistingKeys) {
     const oldKeys = this.$scope.$root._references.keys
-    const keys = _.union(oldKeys, newKeys)
+    const keys = replaceExistingKeys ? newKeys : _.union(oldKeys, newKeys)
     window.dispatchEvent(
       new CustomEvent('project:references', {
         detail: keys,
@@ -99,7 +99,7 @@ export default ReferencesManager = class ReferencesManager {
     return this.ide.$http
       .post(`/project/${this.$scope.project_id}/references/index`, opts)
       .then(response => {
-        return this._storeReferencesKeys(response.data.keys)
+        return this._storeReferencesKeys(response.data.keys, false)
       })
   }
 
@@ -111,7 +111,7 @@ export default ReferencesManager = class ReferencesManager {
     return this.ide.$http
       .post(`/project/${this.$scope.project_id}/references/indexAll`, opts)
       .then(response => {
-        return this._storeReferencesKeys(response.data.keys)
+        return this._storeReferencesKeys(response.data.keys, true)
       })
   }
 }

services/web/test/frontend/features/file-tree/flows/delete-entity.test.js

@@ -13,6 +13,7 @@ import FileTreeRoot from '../../../../../frontend/js/features/file-tree/componen
 describe('FileTree Delete Entity Flow', function () {
   const onSelect = sinon.stub()
   const onInit = sinon.stub()
+  const reindexReferences = sinon.stub()
 
   beforeEach(function () {
     window.metaAttributesCache = new Map()
@@ -23,6 +24,7 @@ describe('FileTree Delete Entity Flow', function () {
     fetchMock.restore()
     onSelect.reset()
     onInit.reset()
+    reindexReferences.reset()
     cleanUpContext()
     window.metaAttributesCache = new Map()
   })
@@ -41,7 +43,7 @@ describe('FileTree Delete Entity Flow', function () {
       renderWithEditorContext(
         <FileTreeRoot
           refProviders={{}}
-          reindexReferences={() => null}
+          reindexReferences={reindexReferences}
           setRefProviderEnabled={() => null}
           setStartedFreeTrial={() => null}
           onSelect={onSelect}
@@ -94,6 +96,7 @@ describe('FileTree Delete Entity Flow', function () {
 
       const [lastFetchPath] = fetchMock.lastCall(fetchMatcher)
       expect(lastFetchPath).to.equal('/project/123abc/doc/456def')
+      expect(reindexReferences).not.to.have.been.called
     })
 
     it('continues delete on 404s', async function () {
@@ -220,7 +223,7 @@ describe('FileTree Delete Entity Flow', function () {
       renderWithEditorContext(
         <FileTreeRoot
           refProviders={{}}
-          reindexReferences={() => null}
+          reindexReferences={reindexReferences}
           setRefProviderEnabled={() => null}
           setStartedFreeTrial={() => null}
           onSelect={onSelect}
@@ -254,7 +257,7 @@ describe('FileTree Delete Entity Flow', function () {
       fireEvent.click(deleteButton)
     })
 
-    it('removes all items', async function () {
+    it('removes all items and reindexes references after deleting .bib file', async function () {
       const fetchMatcher = /\/project\/\w+\/(doc|file)\/\w+/
       fetchMock.delete(fetchMatcher, 204)
 
@@ -289,6 +292,7 @@ describe('FileTree Delete Entity Flow', function () {
         .map(([url]) => url)
       expect(firstFetchPath).to.equal('/project/123abc/doc/456def')
       expect(secondFetchPath).to.equal('/project/123abc/file/789ghi')
+      expect(reindexReferences).to.have.been.called
     })
   })
 

services/web/test/frontend/features/file-view/components/file-view-header.test.js

@@ -5,7 +5,6 @@ import {
 } from '@testing-library/react'
 import { expect } from 'chai'
 import fetchMock from 'fetch-mock'
-import sinon from 'sinon'
 
 import { renderWithEditorContext } from '../../../helpers/render-with-context'
 import FileViewHeader from '../../../../../frontend/js/features/file-view/components/file-view-header'
@@ -55,9 +54,7 @@ describe('<FileViewHeader/>', function () {
 
   describe('header text', function () {
     it('Renders the correct text for a file with the url provider', function () {
-      renderWithEditorContext(
-        <FileViewHeader file={urlFile} storeReferencesKeys={() => {}} />
-      )
+      renderWithEditorContext(<FileViewHeader file={urlFile} />)
       screen.getByText('Imported from', { exact: false })
       screen.getByText('at 3:24 am Wed, 17th Feb 21', {
         exact: false,
@@ -65,9 +62,7 @@ describe('<FileViewHeader/>', function () {
     })
 
     it('Renders the correct text for a file with the project_file provider', function () {
-      renderWithEditorContext(
-        <FileViewHeader file={projectFile} storeReferencesKeys={() => {}} />
-      )
+      renderWithEditorContext(<FileViewHeader file={projectFile} />)
       screen.getByText('Imported from', { exact: false })
       screen.getByText('Another project', { exact: false })
       screen.getByText('/source-entity-path.ext, at 3:24 am Wed, 17th Feb 21', {
@@ -99,9 +94,7 @@ describe('<FileViewHeader/>', function () {
         }
       )
 
-      renderWithEditorContext(
-        <FileViewHeader file={projectFile} storeReferencesKeys={() => {}} />
-      )
+      renderWithEditorContext(<FileViewHeader file={projectFile} />)
 
       fireEvent.click(screen.getByRole('button', { name: 'Refresh' }))
 
@@ -119,23 +112,7 @@ describe('<FileViewHeader/>', function () {
         }
       )
 
-      const reindexResponse = {
-        projectId: '123abc',
-        keys: ['reference1', 'reference2', 'reference3', 'reference4'],
-      }
-      fetchMock.post(
-        'express:/project/:project_id/references/indexAll',
-        reindexResponse
-      )
-
-      const storeReferencesKeys = sinon.stub()
-
-      renderWithEditorContext(
-        <FileViewHeader
-          file={thirdPartyReferenceFile}
-          storeReferencesKeys={storeReferencesKeys}
-        />
-      )
+      renderWithEditorContext(<FileViewHeader file={thirdPartyReferenceFile} />)
 
       fireEvent.click(screen.getByRole('button', { name: 'Refresh' }))
 
@@ -144,15 +121,15 @@ describe('<FileViewHeader/>', function () {
       )
 
       expect(fetchMock.done()).to.be.true
-      expect(storeReferencesKeys).to.be.calledWith(reindexResponse.keys)
+
+      const lastCallBody = JSON.parse(fetchMock.lastCall()[1].body)
+      expect(lastCallBody.shouldReindexReferences).to.be.true
     })
   })
 
   describe('The download button', function () {
     it('exists', function () {
-      renderWithEditorContext(
-        <FileViewHeader file={urlFile} storeReferencesKeys={() => {}} />
-      )
+      renderWithEditorContext(<FileViewHeader file={urlFile} />)
 
       screen.getByText('Download', { exact: false })
     })

services/web/test/frontend/features/file-view/components/file-view.test.js

@@ -45,9 +45,7 @@ describe('<FileView/>', function () {
         'Text file content'
       )
 
-      renderWithEditorContext(
-        <FileView file={textFile} storeReferencesKeys={() => {}} />
-      )
+      renderWithEditorContext(<FileView file={textFile} />)
 
       await waitForElementToBeRemoved(() =>
         screen.getByText('Loading', { exact: false })
@@ -60,9 +58,7 @@ describe('<FileView/>', function () {
         name: 'example.not-tex',
       }
 
-      renderWithEditorContext(
-        <FileView file={unpreviewableTextFile} storeReferencesKeys={() => {}} />
-      )
+      renderWithEditorContext(<FileView file={unpreviewableTextFile} />)
 
       await screen.findByText('Sorry, no preview is available', {
         exact: false,
@@ -72,17 +68,13 @@ describe('<FileView/>', function () {
 
   describe('for an image file', function () {
     it('shows a loading indicator while the file is loading', async function () {
-      renderWithEditorContext(
-        <FileView file={imageFile} storeReferencesKeys={() => {}} />
-      )
+      renderWithEditorContext(<FileView file={imageFile} />)
 
       screen.getByText('Loading', { exact: false })
     })
 
     it('shows messaging if the image could not be loaded', async function () {
-      renderWithEditorContext(
-        <FileView file={imageFile} storeReferencesKeys={() => {}} />
-      )
+      renderWithEditorContext(<FileView file={imageFile} />)
 
       // Fake the image request failing as the request is handled by the browser
       fireEvent.error(screen.getByRole('img'))