You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by garrensmith <gi...@git.apache.org> on 2017/03/28 12:06:52 UTC

[GitHub] couchdb-fauxton pull request #889: Shown database doesn't exist notification

GitHub user garrensmith opened a pull request:

    https://github.com/apache/couchdb-fauxton/pull/889

    Shown database doesn't exist notification

    This fixes #888

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/garrensmith/couchdb-fauxton db-not-found

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/couchdb-fauxton/pull/889.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #889
    
----
commit a5cf65d149facee891cffed7850b8a3d3bc6502b
Author: Garren Smith <ga...@gmail.com>
Date:   2017-03-28T12:05:40Z

    Shown database doesn't exist notification
    
    This fixes #888

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #889: Shown database doesn't exist notification

Posted by millayr <gi...@git.apache.org>.
Github user millayr commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/889#discussion_r108420139
  
    --- Diff: app/addons/documents/index-results/actions.js ---
    @@ -50,20 +50,23 @@ export default {
     
         if (!options.collection.fetch) { return; }
     
    -    return options.collection.fetch({reset: true}).then(function () {
    +    return options.collection.fetch({reset: true}).then(() => {
           this.resultsListReset();
           this.sendMessageNewResultList(options);
     
    -    }.bind(this), function (collection, _xhr) {
    +    }, function (collection, _xhr) {
    --- End diff --
    
    `(collection, _xhr) => {`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #889: Shown database doesn't exist notification

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/889#discussion_r108425965
  
    --- Diff: app/addons/documents/mango/__tests__/mango.actions.test.js ---
    @@ -0,0 +1,60 @@
    +// Licensed under the Apache License, Version 2.0 (the "License"); you may not
    +// use this file except in compliance with the License. You may obtain a copy of
    +// the License at
    +//
    +//   http://www.apache.org/licenses/LICENSE-2.0
    +//
    +// Unless required by applicable law or agreed to in writing, software
    +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
    +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
    +// License for the specific language governing permissions and limitations under
    +// the License.
    +
    +import utils from '../../../../../test/mocha/testUtils';
    +import FauxtonAPI from "../../../../core/api";
    +import Actions from '../mango.actions';
    +import sinon from 'sinon';
    +
    +const {restore, assert} = utils;
    +FauxtonAPI.router = new FauxtonAPI.Router([]);
    +
    +describe('Mango actions', () => {
    +
    +  afterEach(() => {
    +    restore(FauxtonAPI.navigate);
    +    restore(FauxtonAPI.addNotification);
    +  });
    +
    +  it("should show a notification and redirect if database doesn't exist", (done) => {
    +    const navigateSpy = sinon.spy(FauxtonAPI, 'navigate');
    +    const notificationSpy = sinon.spy(FauxtonAPI, 'addNotification');
    +
    +    const database = {
    +      safeID : () => 'safe-id-db'
    +    };
    +
    +    const options = {
    +      database,
    +      indexList: {
    +        database,
    +        fetch: () => {
    +          return Promise.reject({
    +            responseJSON: {
    +              error: 'not_found'
    +            }
    +          });
    +        },
    +      }
    +    };
    +
    +    Actions.getIndexList(options);
    +    process.nextTick(() => {
    +      assert.ok(notificationSpy.calledOnce);
    --- End diff --
    
    I prefer assert.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #889: Shown database doesn't exist notification

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith closed the pull request at:

    https://github.com/apache/couchdb-fauxton/pull/889


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #889: Shown database doesn't exist notification

Posted by millayr <gi...@git.apache.org>.
Github user millayr commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/889#discussion_r108423500
  
    --- Diff: app/addons/documents/index-results/actions.js ---
    @@ -50,20 +50,23 @@ export default {
     
         if (!options.collection.fetch) { return; }
     
    -    return options.collection.fetch({reset: true}).then(function () {
    +    return options.collection.fetch({reset: true}).then(() => {
           this.resultsListReset();
           this.sendMessageNewResultList(options);
     
    -    }.bind(this), function (collection, _xhr) {
    +    }, function (collection, _xhr) {
           //Make this more robust as sometimes the colection is passed through here.
           var xhr = collection.responseText ? collection : _xhr;
           var errorMsg = 'Bad Request';
     
           try {
    -        var responseText = JSON.parse(xhr.responseText);
    +        const responseText = JSON.parse(xhr.responseText);
             if (responseText.reason) {
               errorMsg = responseText.reason;
             }
    +        if (responseText.error && responseText.error === 'not_found') {
    +          return;
    --- End diff --
    
    Won't this bail out of the function and skip the `addNotification` call?  Is that your intent here?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #889: Shown database doesn't exist notification

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/889#discussion_r108426438
  
    --- Diff: app/addons/documents/index-results/actions.js ---
    @@ -50,20 +50,23 @@ export default {
     
         if (!options.collection.fetch) { return; }
     
    -    return options.collection.fetch({reset: true}).then(function () {
    +    return options.collection.fetch({reset: true}).then(() => {
           this.resultsListReset();
           this.sendMessageNewResultList(options);
     
    -    }.bind(this), function (collection, _xhr) {
    +    }, function (collection, _xhr) {
           //Make this more robust as sometimes the colection is passed through here.
           var xhr = collection.responseText ? collection : _xhr;
           var errorMsg = 'Bad Request';
     
           try {
    -        var responseText = JSON.parse(xhr.responseText);
    +        const responseText = JSON.parse(xhr.responseText);
             if (responseText.reason) {
               errorMsg = responseText.reason;
             }
    +        if (responseText.error && responseText.error === 'not_found') {
    +          return;
    --- End diff --
    
    Yes, that is intentional. We use the check in the sidebar action to validate if the database exists or not. If this shows a message here we will get two notifications that say the same thing.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton issue #889: Shown database doesn't exist notification

Posted by millayr <gi...@git.apache.org>.
Github user millayr commented on the issue:

    https://github.com/apache/couchdb-fauxton/pull/889
  
    Cool +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #889: Shown database doesn't exist notification

Posted by millayr <gi...@git.apache.org>.
Github user millayr commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/889#discussion_r108424752
  
    --- Diff: app/addons/documents/mango/__tests__/mango.actions.test.js ---
    @@ -0,0 +1,60 @@
    +// Licensed under the Apache License, Version 2.0 (the "License"); you may not
    +// use this file except in compliance with the License. You may obtain a copy of
    +// the License at
    +//
    +//   http://www.apache.org/licenses/LICENSE-2.0
    +//
    +// Unless required by applicable law or agreed to in writing, software
    +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
    +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
    +// License for the specific language governing permissions and limitations under
    +// the License.
    +
    +import utils from '../../../../../test/mocha/testUtils';
    +import FauxtonAPI from "../../../../core/api";
    +import Actions from '../mango.actions';
    +import sinon from 'sinon';
    +
    +const {restore, assert} = utils;
    +FauxtonAPI.router = new FauxtonAPI.Router([]);
    +
    +describe('Mango actions', () => {
    +
    +  afterEach(() => {
    +    restore(FauxtonAPI.navigate);
    +    restore(FauxtonAPI.addNotification);
    +  });
    +
    +  it("should show a notification and redirect if database doesn't exist", (done) => {
    +    const navigateSpy = sinon.spy(FauxtonAPI, 'navigate');
    +    const notificationSpy = sinon.spy(FauxtonAPI, 'addNotification');
    +
    +    const database = {
    +      safeID : () => 'safe-id-db'
    +    };
    +
    +    const options = {
    +      database,
    +      indexList: {
    +        database,
    +        fetch: () => {
    +          return Promise.reject({
    +            responseJSON: {
    +              error: 'not_found'
    +            }
    +          });
    +        },
    +      }
    +    };
    +
    +    Actions.getIndexList(options);
    +    process.nextTick(() => {
    +      assert.ok(notificationSpy.calledOnce);
    --- End diff --
    
    I've been trying to use Jest's `expect` when doing these checks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #889: Shown database doesn't exist notification

Posted by millayr <gi...@git.apache.org>.
Github user millayr commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/889#discussion_r108425211
  
    --- Diff: app/addons/documents/sidebar/__tests__/sidebar.actions.test.js ---
    @@ -0,0 +1,60 @@
    +// Licensed under the Apache License, Version 2.0 (the "License"); you may not
    +// use this file except in compliance with the License. You may obtain a copy of
    +// the License at
    +//
    +//   http://www.apache.org/licenses/LICENSE-2.0
    +//
    +// Unless required by applicable law or agreed to in writing, software
    +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
    +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
    +// License for the specific language governing permissions and limitations under
    +// the License.
    +
    +import utils from '../../../../../test/mocha/testUtils';
    +import FauxtonAPI from "../../../../core/api";
    +import Actions from '../actions';
    +import sinon from 'sinon';
    +
    +const {restore, assert} = utils;
    +FauxtonAPI.router = new FauxtonAPI.Router([]);
    +
    +describe('Sidebar actions', () => {
    +
    +  afterEach(() => {
    +    restore(FauxtonAPI.navigate);
    +    restore(FauxtonAPI.addNotification);
    +  });
    +
    +  it("should show a notification and redirect if database doesn't exist", (done) => {
    +    const navigateSpy = sinon.spy(FauxtonAPI, 'navigate');
    +    const notificationSpy = sinon.spy(FauxtonAPI, 'addNotification');
    +
    +    const database = {
    +      safeID : () => 'safe-id-db'
    +    };
    +
    +    const options = {
    +      database,
    +      designDocs: {
    +        database,
    +        fetch: () => {
    +          return Promise.reject({
    +            responseJSON: {
    +              error: 'not_found'
    +            }
    +          });
    +        },
    +      }
    +    };
    +
    +    Actions.newOptions(options);
    +    process.nextTick(() => {
    +      assert.ok(notificationSpy.calledOnce);
    --- End diff --
    
    similar to above, I've been using `expect` for these.  We should probably decide on a common approach here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---