Merge pull request #86 from tomtiao/feat/support-empty-string-title

Updated: allows empty string as topic title

Author: danielsss

src/core/base.ts

@@ -1,6 +1,8 @@
 import * as TreeModel from 'tree-model';
 import { Node } from 'tree-model';
 
+import { isString } from '../utils/common';
+
 const v4 = require('uuid/v4');
 const Debug = require('debug');
 
@@ -96,11 +98,12 @@ export default class Base {
   protected getConflictedComponentId(
     options: ConflictedOnDifferentBranchOptions & ConflictedOnSameBranchOptions
   ): string | null {
-    if (options.title && options.parentId) {
+    const validString = isString(options.title);
+    if (validString && options.parentId) {
       return this.different(options);
     }
 
-    if (options.title && options.customId) {
+    if (validString && options.customId) {
       return this.identical(options);
     }
 

src/core/topic.ts

@@ -5,7 +5,7 @@ import {
 import { SummaryOptions } from '../abstracts/summary.abstract';
 import { Summary } from './summary';
 import { Note } from './note';
-import { isEmpty, isObject, isRuntime } from '../utils/common';
+import { isEmpty, isObject, isRuntime, isString } from '../utils/common';
 
 import * as Model from '../common/model';
 import * as Core from 'xmind-model';
@@ -73,7 +73,7 @@ export class Topic extends Base implements AbstractTopic {
   public add(topic: Model.Topic & {
     customId?: string | number, parentId?: string
   } = {} as any, options?: { index: number }): Topic {
-    if (!topic.title || typeof topic.title !== 'string') {
+    if (!isString(topic.title)) {
       throw new Error('topic.title should be a valid string');
     }
     topic.id = topic.id || this.id;
@@ -183,7 +183,8 @@ export class Topic extends Base implements AbstractTopic {
   public cid(title?: string, dependencies: {
     parentId?: number | string, customId?: number | string
   } = {}): string | null {
-    if (title && dependencies) {
+    const validTitle = isString(title);
+    if (validTitle && dependencies) {
       if (dependencies.parentId) {
         return this.getConflictedComponentId({
           title, parentId: dependencies.parentId
@@ -196,7 +197,7 @@ export class Topic extends Base implements AbstractTopic {
       }
     }
 
-    if (title && typeof title === 'string') {
+    if (validTitle) {
       return this.findComponentIdBy(title);
     }
     return this.lastId;

src/utils/common.ts

@@ -35,5 +35,9 @@ const isRuntime = function(): boolean {
   return typeof global === 'object' && typeof window === 'undefined';
 };
 
-export {isEmpty, isObject, isRuntime};
+const isString = function(v: any): v is string {
+  return typeof v === 'string';
+};
+
+export {isEmpty, isObject, isRuntime, isString};
 

test/functional/index.test.ts

@@ -44,12 +44,13 @@ describe('# Functional Test', () => {
 
   describe('# Topic', () => {
     it('should be a lots of main topic added to rootTopic', done => {
-      const topics = ['main topic 1', 'main topic 2', 'main topic 3', 'main topic 4'];
+      const topics = ['main topic 1', 'main topic 2', 'main topic 3', 'main topic 4', ''];
       const {workbook, topic} = getComponents();
       topic.add({title: 'main topic 1'})
         .add({title: 'main topic 2'})
         .add({title: 'main topic 3'})
-        .add({title: 'main topic 4'});
+        .add({title: 'main topic 4'})
+        .add({title: ''});
 
       const children = workbook.toJSON()[0].rootTopic.children.attached;
       for (let i = 0; i < children.length; i++) {
@@ -72,11 +73,12 @@ describe('# Functional Test', () => {
         .on(topic.cid())
         .add({title: 'subtopic 1'})
         .add({title: 'subtopic 2'})
-        .add({title: 'subtopic 3'});
+        .add({title: 'subtopic 3'})
+        .add({title: ''});
 
       const subtopics = workbook.toJSON()[0].rootTopic.children.attached[0].children.attached;
       for(let i = 0; i < subtopics.length; i++) {
-        expect(subtopics[i].title.startsWith('subtopic')).to.be.true;
+        expect(subtopics[i].title.startsWith('subtopic') || subtopics[i].title === '').to.be.true;
         expect(subtopics[i].id).to.not.be.empty;
       }
       done();
@@ -97,14 +99,15 @@ describe('# Functional Test', () => {
       const subTopic2Id = topic.cid();
 
       topic.add({title: 'subtopic 3'});
+      topic.add({title: ''});
       topic.destroy(subTopic2Id);
       // Do nothing if you have to remove topic twice
       topic.destroy(subTopic2Id);
 
       const subtopics = workbook.toJSON()[0].rootTopic.children.attached[0].children.attached;
-      expect(subtopics.length).to.be.eq(2);
+      expect(subtopics.length).to.be.eq(3);
       for(let i = 0; i < subtopics.length; i++) {
-        expect(subtopics[i].title.startsWith('subtopic')).to.be.true;
+        expect(subtopics[i].title.startsWith('subtopic') || subtopics[i].title === '').to.be.true;
         expect(subtopics[i].id).to.not.be.empty;
       }
       done();
@@ -159,9 +162,11 @@ describe('# Functional Test', () => {
       topic
         .add({title: 'main topic 1'})
         .add({title: 'main topic 2'})
-        .add({title: 'main topic 3'});
+        .add({title: 'main topic 3'})
+        .add({title: ''});
 
       topic.destroy(topic.cid('main topic 2'));
+      topic.destroy(topic.cid(''));
       zip.save().then(async status => {
         expect(status).to.be.true;
         const p = getBuildTemporaryPath('default.xmind');
@@ -173,6 +178,7 @@ describe('# Functional Test', () => {
           const {attached} = map.rootTopic.children;
           expect(attached.length).to.be.eq(2);
           expect(attached.find(child => child.title === topic.cid('main topic 2'))).to.be.undefined;
+          expect(attached.find(child => child.title === topic.cid(''))).to.be.undefined;
           fs.unlinkSync(p);
           done();
         });

test/units/common.test.ts

@@ -1,4 +1,4 @@
-import { isEmpty, isObject } from '../../src/utils/common';
+import { isEmpty, isObject, isString } from '../../src/utils/common';
 import { expect } from 'chai';
 
 
@@ -54,4 +54,21 @@ describe('# Common Utils Test', function() {
     expect(isObject(() => {})).to.eq(true);
     done();
   });
+  
+  it('isString(), should return true if given any string', done => {
+    expect(isString('')).to.eq(true);
+    expect(isString('foo')).to.eq(true);
+    done();
+  })
+  
+  it('isString(), should return false if given any non string value', done => {
+    expect(isString(42)).to.eq(false);
+    expect(isString(null)).to.eq(false);
+    expect(isString(undefined)).to.eq(false);
+    expect(isString({})).to.eq(false);
+    expect(isString(() => {})).to.eq(false);
+    expect(isString([])).to.eq(false);
+    expect(isString(new String)).to.eq(false);
+    done();
+  })
 });

test/units/topic.test.ts

@@ -26,7 +26,7 @@ describe('# Topic Unit Test', () => {
     }
   });
 
-  it('should be failed to add topic with empty title', done => {
+  it('should be failed to add topic with non string title', done => {
     const {topic} = getComponents();
     try {
       topic.on().add();
@@ -36,6 +36,18 @@ describe('# Topic Unit Test', () => {
     }
   });
 
+  it('should be able to add topic with empty title', done => {
+    const {topic} = getComponents();
+
+    topic.on().add({ title: '' });
+
+    const topics = topic.cids();
+    expect(topics).to.have.property(topic.cid(''));
+    expect(topics).to.have.property(topic.cid('Central Topic'));
+    expect(topics).to.have.property(topic.rootTopicId);
+    done();
+  });
+
   it('should be failed if call .on() with an invalid componentId', done => {
     try {
       const {topic} = getComponents();
@@ -226,11 +238,32 @@ describe('# Topic Unit Test', () => {
     expect(topic.cid('abc', { parentId: a2})).to.eq(abc1);
     expect(topic.cid('abc', { parentId: a1})).to.eq(abc2);
 
+    topic.add({ title: 'main topic - 1', customId: 3 });
+    const a3 = topic.cid();
+    topic.add({ title: 'main topic - 1', customId: 4 });
+    const a4 = topic.cid();
+    expect(topic.cid('main topic - 1', { customId: 3 })).to.eq(a3);
+    expect(topic.cid('main topic - 1', { customId: 4 })).to.eq(a4);
+
+    topic.on(a4).add({title: ''});
+    const emptyString1 = topic.cid();
+    topic.on(a3).add({title: ''});
+    const emptyString2 = topic.cid();
+
+    expect(topic.cid('', { parentId: a4 })).to.eq(emptyString1);
+    expect(topic.cid('', { parentId: a3 })).to.eq(emptyString2);
+
     topic.destroy(topic.cid('abc', { parentId: a1}));
     topic.destroy(topic.cid('abc', { parentId: a2}));
 
     expect(topic.cid('abc', { parentId: a1})).to.eq(null);
     expect(topic.cid('abc', { parentId: a2})).to.eq(null);
+
+    topic.destroy(topic.cid('', { parentId: a3}));
+    topic.destroy(topic.cid('', { parentId: a4}));
+
+    expect(topic.cid('', { parentId: a4})).to.eq(null);
+    expect(topic.cid('', { parentId: a3})).to.eq(null);
     done();
   });
 });