Merge pull request #416 from weseek/fix/415-wrongly-forward-match

fix #415 wrongly-forward-match

lib/models/page.js

@@ -713,6 +713,7 @@ module.exports = function(crowi) {
 
     // ignore other pages than descendants
     path = Page.addSlashOfEnd(path);
+
     // add option to escape the regex strings
     const combinedOption = Object.assign({isRegExpEscapedFromPath: true}, option);
 
@@ -722,8 +723,12 @@ module.exports = function(crowi) {
   /**
    * generate the query to find pages that start with `path`
    *
-   * If `path` has `/` at the end, returns '{path}/*' and '{path}' self.
-   * If `path` doesn't have `/` at the end, returns '{path}*'
+   * (GROWI) If 'isRegExpEscapedFromPath' is true, `path` should have `/` at the end
+   *   -> returns '{path}/*' and '{path}' self.
+   * (Crowi) If 'isRegExpEscapedFromPath' is false and `path` has `/` at the end
+   *   -> returns '{path}*'
+   * (Crowi) If 'isRegExpEscapedFromPath' is false and `path` doesn't have `/` at the end
+   *   -> returns '{path}*'
    *
    * *option*
    *   - includeDeletedPage -- if true, search deleted pages (default: false)
@@ -735,17 +740,22 @@ module.exports = function(crowi) {
     var includeDeletedPage = option.includeDeletedPage || false;
     var isRegExpEscapedFromPath = option.isRegExpEscapedFromPath || false;
 
+    /*
+     * 1. add condition for finding the page completely match with `path` w/o last slash
+     */
     let pathSlashOmitted = path;
     if (path.match(/\/$/)) {
-      // add condition for finding the page completely match with `path`
       pathSlashOmitted = path.substr(0, path.length -1);
       pathCondition.push({path: pathSlashOmitted});
     }
 
-    // create forward match pattern
+    /*
+     * 2. add decendants
+     */
     var pattern = (isRegExpEscapedFromPath)
-      ? escapeStringRegexp(pathSlashOmitted)  // escape
+      ? escapeStringRegexp(path)  // escape
       : pathSlashOmitted;
+
     var queryReg = new RegExp('^' + pattern);
     pathCondition.push({path: queryReg});
 

test/models/page.test.js

@@ -74,6 +74,21 @@ describe('Page', () => {
           grantedUsers: [],
           creator: testUser1,
         },
+        {
+          path: '/page1',
+          grant: Page.GRANT_PUBLIC,
+          creator: testUser0,
+        },
+        {
+          path: '/page1/child1',
+          grant: Page.GRANT_PUBLIC,
+          creator: testUser0,
+        },
+        {
+          path: '/page2',
+          grant: Page.GRANT_PUBLIC,
+          creator: testUser0,
+        },
       ];
 
       return testDBUtil.generateFixture(conn, 'Page', fixture);
@@ -394,4 +409,67 @@ describe('Page', () => {
     });
   });
 
+  context('generateQueryToListByStartWith', () => {
+    it('should return only /page/', done => {
+      const user = createdUsers[0];
+      Page.generateQueryToListByStartWith('/page/', user, { isRegExpEscapedFromPath: true })
+      .then(pages => {
+        // assert length
+        expect(pages.length).to.equal(1);
+        // assert paths
+        const pagePaths = pages.map(page => page.path);
+        expect(pagePaths).to.include.members(['/page/for/extended'])
+        done();
+      })
+      .catch((err) => {
+        done(err);
+      });
+    });
+    it('should return only /page1/', done => {
+      const user = createdUsers[0];
+      Page.generateQueryToListByStartWith('/page1/', user, { isRegExpEscapedFromPath: true })
+      .then(pages => {
+        // assert length
+        expect(pages.length).to.equal(2);
+        // assert paths
+        const pagePaths = pages.map(page => page.path);
+        expect(pagePaths).to.include.members(['/page1', '/page1/child1'])
+        done();
+      })
+      .catch((err) => {
+        done(err);
+      });
+    });
+    it('should return pages which starts with /page', done => {
+      const user = createdUsers[0];
+      Page.generateQueryToListByStartWith('/page', user, {})
+      .then(pages => {
+        // assert length
+        expect(pages.length).to.equal(4);
+        // assert paths
+        const pagePaths = pages.map(page => page.path);
+        expect(pagePaths).to.include.members(['/page/for/extended', '/page1', '/page1/child1', '/page2'])
+        done();
+      })
+      .catch((err) => {
+        done(err);
+      });
+    });
+    it('should process with regexp', done => {
+      const user = createdUsers[0];
+      Page.generateQueryToListByStartWith('/page\\d{1}/', user, {})
+      .then(pages => {
+        // assert length
+        expect(pages.length).to.equal(3);
+        // assert paths
+        const pagePaths = pages.map(page => page.path);
+        expect(pagePaths).to.include.members(['/page1', '/page1/child1', '/page2'])
+        done();
+      })
+      .catch((err) => {
+        done(err);
+      });
+    });
+  });
+
 });