Resolve Circular Dependency (#360)

* Resolve utils --> list curcular dependency

* Remove comment about circular dependency

Co-authored-by: Isaac White <[email protected]>

---------

Co-authored-by: Isaac White <[email protected]>

Author: afischer

package-lock.json

@@ -2,37 +2,20 @@
 
 const {google} = require('googleapis')
 const cheerio = require('cheerio')
-const slugify = require('slugify')
 const xlsx = require('xlsx')
 
 const cache = require('./cache')
 const formatter = require('./formatter')
 const log = require('./logger')
 const {getAuth} = require('./auth')
+const {slugify} = require('./utils')
 
 const supportedTypes = new Set(['document', 'spreadsheet', 'text/html'])
 
 const revisionSupportedArr = ['document', 'spreadsheet', 'presentation']
 const revisionSupported = new Set(revisionSupportedArr)
 const revisionMimeSupported = new Set(revisionSupportedArr.map((x) => `application/vnd.google-apps.${x}`))
 
-exports.cleanName = (name = '') => {
-  return name
-    .trim()
-    // eslint-disable-next-line no-useless-escape
-    .replace(/^(\d+[-–—_\s]*)([^\d\/\-^\s]+)/, '$2') // remove leading numbers and delimiters
-    .replace(/\s*\|\s*([^|]+)$/i, '') // remove trailing pipe and tags
-    .replace(/\.[^.]+$/, '') // remove file extensions
-}
-
-exports.slugify = (text = '') => {
-  // convert non alpha numeric into whitespace, rather than removing
-  const alphaNumeric = text.replace(/[^\p{L}\p{N}]+/ug, ' ')
-  return slugify(alphaNumeric, {
-    lower: true
-  })
-}
-
 exports.fetchDoc = async (id, resourceType, req) => {
   const data = await cache.get(id)
   if (data && data.content) {

server/docs.js

@@ -8,7 +8,7 @@ const cache = require('./cache')
 const log = require('./logger')
 const {getAuth} = require('./auth')
 const {isSupported} = require('./utils')
-const docs = require('./docs')
+const {slugify, cleanName} = require('./utils')
 
 const driveType = process.env.DRIVE_TYPE
 const driveId = process.env.DRIVE_ID
@@ -170,9 +170,8 @@ function produceTree(files, firstParent) {
     const {parents, id, name, mimeType} = resource
 
     // prepare data for the individual file and store later for reference
-    // FIXME: consider how to remove circular dependency here.
-    const prettyName = docs.cleanName(name)
-    const slug = docs.slugify(prettyName)
+    const prettyName = cleanName(name)
+    const slug = slugify(prettyName)
     const tagString = (name.match(/\|\s*([^|]+)$/i) || [])[1] || ''
     const tags = tagString.split(',')
       .map((t) => t.trim().toLowerCase())

server/list.js

@@ -4,7 +4,8 @@ const router = require('express-promise-router')()
 
 const log = require('../logger')
 const {getMeta} = require('../list')
-const {fetchDoc, cleanName} = require('../docs')
+const {fetchDoc} = require('../docs')
+const {cleanName} = require('../utils')
 const {getTemplates, sortDocs, stringTemplate, formatUrl, pathPrefix} = require('../utils')
 const {parseUrl} = require('../urlParser')
 

server/routes/categories.js

@@ -4,8 +4,8 @@ const router = require('express-promise-router')()
 
 const log = require('../logger')
 const {getMeta, getPlaylist} = require('../list')
-const {fetchDoc, cleanName} = require('../docs')
-const {stringTemplate, formatUrl, pathPrefix} = require('../utils')
+const {fetchDoc} = require('../docs')
+const {stringTemplate, formatUrl, pathPrefix, cleanName} = require('../utils')
 const {parseUrl} = require('../urlParser')
 
 router.get('*', handlePlaylist)

server/routes/playlists.js

@@ -2,6 +2,7 @@
 const fs = require('fs')
 const path = require('path')
 const {promisify} = require('util')
+const slugify = require('slugify')
 const yaml = require('js-yaml')
 const {get: deepProp} = require('lodash')
 const merge = require('deepmerge')
@@ -45,6 +46,23 @@ exports.sortDocs = (a, b) => {
   return b.resourceType === 'folder' ? 1 : -1
 }
 
+exports.cleanName = (name = '') => {
+  return name
+    .trim()
+    // eslint-disable-next-line no-useless-escape
+    .replace(/^(\d+[-–—_\s]*)([^\d\/\-^\s]+)/, '$2') // remove leading numbers and delimiters
+    .replace(/\s*\|\s*([^|]+)$/i, '') // remove trailing pipe and tags
+    .replace(/\.[^.]+$/, '') // remove file extensions
+}
+
+exports.slugify = (text = '') => {
+  // convert non alpha numeric into whitespace, rather than removing
+  const alphaNumeric = text.replace(/[^\p{L}\p{N}]+/ug, ' ')
+  return slugify(alphaNumeric, {
+    lower: true
+  })
+}
+
 // attempts to require from attemptPath. If file isn't present, looks for a
 // file of the same name in the server dir
 exports.requireWithFallback = (attemptPath) => {

server/utils.js

@@ -2,70 +2,11 @@
 
 const {expect} = require('chai')
 
-const {cleanName, slugify, fetchDoc} = require('../../server/docs')
+const {fetchDoc} = require('../../server/docs')
 
 const PAYLOAD_KEYS = ['html', 'byline', 'createdBy', 'sections']
 
 describe('Docs', () => {
-  describe('Name Cleaner', () => {
-    it('should remove leading numbers and delimeters', () => {
-      expect(cleanName('0000123abc12345')).equals('abc12345')
-      expect(cleanName('   abc     ')).equals('abc')
-      expect(cleanName('123-abc')).equals('abc') // hyphen
-      expect(cleanName('123–abc')).equals('abc') // en dash
-      expect(cleanName('123—abc')).equals('abc') // em dash
-    })
-
-    it('should remove trailing delimeters', () => {
-      expect(cleanName('foo | thing')).equals('foo')
-      expect(cleanName('one   |      two')).equals('one')
-      expect(cleanName('one | two | three')).equals('one | two')
-    })
-
-    it('should remove file extensions', () => {
-      expect(cleanName('foo.html')).equals('foo')
-      expect(cleanName('foo.txt')).equals('foo')
-      expect(cleanName('nytimes.com.txt')).equals('nytimes.com')
-    })
-
-    it('should not remove numbers when no text in the document name', () => {
-      expect(cleanName('2018')).equals('2018')
-      expect(cleanName('3/28')).equals('3/28')
-      expect(cleanName('3-28-2018')).equals('3-28-2018')
-    })
-
-    it('should only remove keywords preceded by a pipe', () => {
-      expect(cleanName('Page foo | home')).equals('Page foo')
-      expect(cleanName('Page foo home | home')).equals('Page foo home')
-      expect(cleanName('Page foo hidden | home')).equals('Page foo hidden')
-      expect(cleanName('Page foo home | home, hidden')).equals('Page foo home')
-      expect(cleanName('Page foo home | hidden, home')).equals('Page foo home')
-    })
-
-    it('should only remove words after the last pipe pipe', () => {
-      expect(cleanName('I | love | pipes | home')).equals('I | love | pipes')
-      expect(cleanName('I | love | pipes | foobar')).equals('I | love | pipes')
-    })
-  })
-
-  describe('Slugification', () => {
-    it('should slugify simple phrases', () => {
-      expect(slugify('this is a slug')).equals('this-is-a-slug')
-      expect(slugify(' this   is a     slug   ')).equals('this-is-a-slug')
-      expect(slugify('this-is a slug')).equals('this-is-a-slug')
-      expect(slugify('this... is a slug!')).equals('this-is-a-slug')
-      expect(slugify('2018 this is a slug')).equals('2018-this-is-a-slug')
-    })
-
-    it('should strip spacing', () => {
-      expect(slugify('  slugify-  me please ')).equals('slugify-me-please')
-    })
-
-    it('should support diacritics', () => {
-      expect(slugify('Öğretmenelere Öneriler')).equals('ogretmenelere-oneriler')
-    })
-  })
-
   describe('Fetching Docs', () => {
     it('should fetch document data with expected structure', async () => {
       const doc = await fetchDoc('id-doc', 'document', {})

test/unit/docs.test.js

@@ -0,0 +1,66 @@
+'use strict'
+
+const {expect} = require('chai')
+
+const {cleanName, slugify} = require('../../server/utils')
+
+describe('Utils', () => {
+  describe('Name Cleaner', () => {
+    it('should remove leading numbers and delimeters', () => {
+      expect(cleanName('0000123abc12345')).equals('abc12345')
+      expect(cleanName('   abc     ')).equals('abc')
+      expect(cleanName('123-abc')).equals('abc') // hyphen
+      expect(cleanName('123–abc')).equals('abc') // en dash
+      expect(cleanName('123—abc')).equals('abc') // em dash
+    })
+
+    it('should remove trailing delimeters', () => {
+      expect(cleanName('foo | thing')).equals('foo')
+      expect(cleanName('one   |      two')).equals('one')
+      expect(cleanName('one | two | three')).equals('one | two')
+    })
+
+    it('should remove file extensions', () => {
+      expect(cleanName('foo.html')).equals('foo')
+      expect(cleanName('foo.txt')).equals('foo')
+      expect(cleanName('nytimes.com.txt')).equals('nytimes.com')
+    })
+
+    it('should not remove numbers when no text in the document name', () => {
+      expect(cleanName('2018')).equals('2018')
+      expect(cleanName('3/28')).equals('3/28')
+      expect(cleanName('3-28-2018')).equals('3-28-2018')
+    })
+
+    it('should only remove keywords preceded by a pipe', () => {
+      expect(cleanName('Page foo | home')).equals('Page foo')
+      expect(cleanName('Page foo home | home')).equals('Page foo home')
+      expect(cleanName('Page foo hidden | home')).equals('Page foo hidden')
+      expect(cleanName('Page foo home | home, hidden')).equals('Page foo home')
+      expect(cleanName('Page foo home | hidden, home')).equals('Page foo home')
+    })
+
+    it('should only remove words after the last pipe pipe', () => {
+      expect(cleanName('I | love | pipes | home')).equals('I | love | pipes')
+      expect(cleanName('I | love | pipes | foobar')).equals('I | love | pipes')
+    })
+  })
+
+  describe('Slugification', () => {
+    it('should slugify simple phrases', () => {
+      expect(slugify('this is a slug')).equals('this-is-a-slug')
+      expect(slugify(' this   is a     slug   ')).equals('this-is-a-slug')
+      expect(slugify('this-is a slug')).equals('this-is-a-slug')
+      expect(slugify('this... is a slug!')).equals('this-is-a-slug')
+      expect(slugify('2018 this is a slug')).equals('2018-this-is-a-slug')
+    })
+
+    it('should strip spacing', () => {
+      expect(slugify('  slugify-  me please ')).equals('slugify-me-please')
+    })
+
+    it('should support diacritics', () => {
+      expect(slugify('Öğretmenelere Öneriler')).equals('ogretmenelere-oneriler')
+    })
+  })
+})