Merge pull request #431 from specklesystems/dim/filename-cleanup

Dim/filename cleanup

packages/frontend/src/components/dialogs/BranchEditDialog.vue

@@ -1,6 +1,11 @@
 <template>
-  <v-dialog v-model="show" width="500" @keydown.esc="cancel" :fullscreen="$vuetify.breakpoint.smAndDown">
-    <v-card :loading="loading" v-if="branch && branch.name !== 'main'">
+  <v-dialog
+    v-model="show"
+    width="500"
+    :fullscreen="$vuetify.breakpoint.smAndDown"
+    @keydown.esc="cancel"
+  >
+    <v-card v-if="branch && branch.name !== 'main'" :loading="loading">
       <v-toolbar color="primary" dark flat>
         <v-app-bar-nav-icon style="pointer-events: none">
           <v-icon>mdi-pencil</v-icon>
@@ -64,6 +69,8 @@ export default {
       showDelete: false,
       nameRules: [
         (v) => !!v || 'Branches need a name too!',
+        (v) =>
+          !(v.startsWith('#') || v.startsWith('/')) || 'Branch names cannot start with "#" or "/"',
         (v) =>
           (v && this.allBranchNames.findIndex((e) => e === v) === -1) ||
           'A branch with this name already exists',

packages/frontend/src/components/dialogs/BranchNewDialog.vue

@@ -42,18 +42,17 @@ export default {
       showError: false,
       error: null,
       streamId: null,
-      branchNames: ['main', 'globals'],
+      reservedBranchNames: ['main', 'globals'],
       valid: false,
       loading: false,
       name: null,
       nameRules: [
         (v) => !!v || 'Branches need a name too!',
         (v) =>
-          (v && !v.startsWith('globals')) ||
-          'Globals is a reserved branch name. Please choose a different name.',
+          !(v.startsWith('#') || v.startsWith('/')) || 'Branch names cannot start with "#" or "/"',
         (v) =>
-          (v && this.branchNames.findIndex((e) => e === v) === -1) ||
-          'A branch with this name already exists',
+          (v && this.reservedBranchNames.findIndex((e) => e === v) === -1) ||
+          'This is a reserved branch name',
         (v) => (v && v.length <= 100) || 'Name must be less than 100 characters',
         (v) => (v && v.length >= 3) || 'Name must be at least 3 characters'
       ],

packages/frontend/src/router/index.js

@@ -57,7 +57,7 @@ const routes = [
     meta: {
       title: 'Home | Speckle'
     },
-    component: () => import('@/views/Frontend_re.vue'),
+    component: () => import('@/views/Frontend.vue'),
     children: [
       {
         path: '',
@@ -80,7 +80,7 @@ const routes = [
         meta: {
           title: 'Stream | Speckle'
         },
-        component: () => import('@/views/stream/Stream_re_re.vue'),
+        component: () => import('@/views/stream/Stream.vue'),
         children: [
           {
             path: '',
@@ -88,7 +88,7 @@ const routes = [
             meta: {
               title: 'Stream | Speckle'
             },
-            component: () => import('@/views/stream/Details_re.vue')
+            component: () => import('@/views/stream/Details.vue')
           },
 
           {

packages/server/modules/core/services/branches.js

@@ -18,6 +18,8 @@ module.exports = {
     branch.name = name.toLowerCase( )
     branch.description = description
 
+    if(name) module.exports.validateBranchName( { name } )
+
     let [ id ] = await Branches( ).returning( 'id' ).insert( branch )
 
     // update stream updated at
@@ -27,9 +29,14 @@ module.exports = {
   },
 
   async updateBranch( { id, name, description } ) {
+    if ( name ) module.exports.validateBranchName( { name } )
     return await Branches( ).where( { id: id } ).update( { name: name ? name.toLowerCase( ) : name, description: description } )
   },
 
+  validateBranchName( { name } ) {
+    if ( name.startsWith( '/' ) || name.startsWith( '#' ) ) throw new Error( 'Branch names cannot start with # or /.' )
+  },
+
   async getBranchById( { id } ) {
     return await Branches( ).where( { id: id } ).first( ).select( '*' )
   },
@@ -54,7 +61,7 @@ module.exports = {
   },
 
   async getBranchByNameAndStreamId( { streamId, name } ) {
-    let query = Branches( ).select( '*' ).where( { streamId: streamId } ).andWhere( knex.raw( 'LOWER(name) = ?', [name]) ).first( )
+    let query = Branches( ).select( '*' ).where( { streamId: streamId } ).andWhere( knex.raw( 'LOWER(name) = ?', [ name.toLowerCase() ] ) ).first( )
     return await query
   },
 

packages/server/modules/core/tests/branches.spec.js

@@ -24,7 +24,6 @@ const {
 } = require( '../services/branches' )
 
 describe( 'Branches @core-branches', ( ) => {
-
   let user = {
     name: 'Dimitrie Stefanescu',
     email: 'didimitrie4342@gmail.com',
@@ -73,6 +72,52 @@ describe( 'Branches @core-branches', ( ) => {
     }
   } )
 
+  it( 'Should not allow branch names starting with # or /', async ( ) => {
+    try {
+      await createBranch( { name: '/pasta', streamId: stream.id, authorId: user.id } )
+      assert.fail( 'Illegal branch name passed through.' )
+    } catch ( err ) {
+      expect( err.message ).to.contain( 'names cannot start with # or /' )
+    }
+
+    try {
+      await createBranch( { name: '#rice', streamId: stream.id, authorId: user.id } )
+      assert.fail( 'Illegal branch name passed through.' )
+    } catch ( err ) {
+      expect( err.message ).to.contain( 'names cannot start with # or /' )
+    }
+
+    try {
+      await updateBranch( { id: branch.id, name: '/super/part/two' } )
+      assert.fail( 'Illegal branch name passed through in update operation.' )
+    } catch ( err ) {
+      expect( err.message ).to.contain( 'names cannot start with # or /' )
+    }
+
+    try {
+      await updateBranch( { id: branch.id, name: '#super#part#three' } )
+      assert.fail( 'Illegal branch name passed through in update operation.' )
+    } catch ( err ) {
+      expect( err.message ).to.contain( 'names cannot start with # or /' )
+    }
+  } )
+
+  it( 'Branch names should be case insensitive (always lowercase)', async ( ) => {
+    let id = await createBranch( { name: 'CaseSensitive', streamId: stream.id, authorId: user.id } )
+    
+    let b = await getBranchByNameAndStreamId( { streamId: stream.id, name:'casesensitive' } )
+    expect( b.name ).to.equal( 'casesensitive' )
+
+    let bb = await getBranchByNameAndStreamId( { streamId: stream.id, name:'CaseSensitive' } )
+    expect( bb.name ).to.equal( 'casesensitive' )
+
+    let bbb = await getBranchByNameAndStreamId( { streamId: stream.id, name:'CASESENSITIVE' } )
+    expect( bbb.name ).to.equal( 'casesensitive' )
+
+    // cleanup
+    await deleteBranchById( { id, streamId: stream.id } )
+  } )
+
   it( 'Should get a branch', async ( ) => {
     let myBranch = await getBranchById( { id: branch.id } )
     expect( myBranch.authorId ).to.equal( user.id )
@@ -87,7 +132,6 @@ describe( 'Branches @core-branches', ( ) => {
   } )
 
   it( 'Should get all stream branches', async ( ) => {
-
     await createBranch( { name: 'main-faster', streamId: stream.id, authorId: user.id } )
     await createBranch( { name: 'main-blaster', streamId: stream.id, authorId: user.id } )
     await createBranch( { name: 'blaster-farter', streamId: stream.id, authorId: user.id } )
@@ -112,7 +156,5 @@ describe( 'Branches @core-branches', ( ) => {
     } catch ( e ){
       // pass
     }
-
   } )
-
 } )