You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by BenJanecke <gi...@git.apache.org> on 2016/07/13 13:19:36 UTC

[GitHub] couchdb-fauxton pull request #745: #3030 Centralise ajax requests

GitHub user BenJanecke opened a pull request:

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

    #3030 Centralise ajax requests

    The api for the ajax module is as follows
    
    ```
    import { get, post } from 'core/ajax';
    
    // GET: /some_enpoint?page=1&order=name
    get({
     path: '/some_enpoint',
    }, {
     page: 1,
     order: 'name',
    }).then((response) => {
     // do stuff
    }, (err) => {
     // something broke
    }); 
    
    // POST: /some_enpoint
             body={ foo: 'bar' }
    get({
     path: '/some_enpoint',
     data: { foo: 'bar' },
    }).then((response) => {
     // do stuff
    }, (err) => {
     // something broke
    }); 
    ```

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

    $ git pull https://github.com/BenJanecke/couchdb-fauxton ajax

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

    https://github.com/apache/couchdb-fauxton/pull/745.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 #745
    
----
commit a7419e4d404757c5b75a64c9b3fbd778b04f870b
Author: Ben Janecke <be...@gmail.com>
Date:   2016-07-13T13:17:17Z

    #3030 Centralise ajax requests
    
    The api for the ajax module is as follows
    
    ```
    import { get, post } from 'core/ajax';
    
    // GET: /some_enpoint?page=1&order=name
    get({
     path: '/some_enpoint',
    }, {
     page: 1,
     order: 'name',
    }).then((response) => {
     // do stuff
    }, (err) => {
     // something broke
    }); 
    
    // POST: /some_enpoint
             body={ foo: 'bar' }
    get({
     path: '/some_enpoint',
     data: { foo: 'bar' },
    }).then((response) => {
     // do stuff
    }, (err) => {
     // something broke
    }); 
    ```

----


---
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 #745: #3030 Centralise ajax requests

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

    https://github.com/apache/couchdb-fauxton/pull/745#discussion_r70819861
  
    --- Diff: app/addons/auth/resources.js ---
    @@ -136,7 +133,7 @@ Auth.Session = CouchdbSession.Session.extend({
         }
       },
     
    -  createAdmin: function (username, password, login, node) {
    +  createAdmin(username, password, login, node) {
    --- End diff --
    
    i would like to see those style changes in a separate commit, so the diff for the actual change is easier to reason about


---
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 #745: #3030 Centralise ajax requests

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

    https://github.com/apache/couchdb-fauxton/pull/745#discussion_r70819553
  
    --- Diff: app/core/ajax.js ---
    @@ -0,0 +1,188 @@
    +import $ from 'jquery';
    +
    +export const buildQuery = (query) => (
    +  `?${Object.keys(query).reduce((items, item) => (
    +    query[item] ? [...items, `${item}=${encodeURIComponent(query[item])}`] : items
    +  ), []).join('&')}`
    +);
    +
    +export const url = (path, query) => (
    +  `${path}${!emptyObject(query) ? buildQuery(query) : ''}`
    +);
    +
    +const emptyObject = (object) => (
    +  Object.keys(object || {}).length == 0
    +);
    +
    +//  ----------------------------------------
    +//  makes a GET request to the provided path
    +//  arguments: (options, query)
    +//  options: {
    +//    path: '/foo', // the location you want to send the request to
    +//  }
    +//  Any optional query paramaters you want to pass
    +//  query: {}
    +//  for example
    +//  query: {
    +//    page: 1,
    +//    term: 'foo',
    +//  }
    +//  will generate
    +//  /${path}?page=1&term=foo
    +export const request = (options, query) => (
    +  $.ajax(Object.assign(
    +    emptyObject(options.data)
    --- End diff --
    
    can we rename it to something like `isSendingData`?


---
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 #745: #3030 Centralise ajax requests

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

    https://github.com/apache/couchdb-fauxton/pull/745#discussion_r70820672
  
    --- Diff: app/core/ajax.js ---
    @@ -0,0 +1,188 @@
    +import $ from 'jquery';
    +
    +export const buildQuery = (query) => (
    +  `?${Object.keys(query).reduce((items, item) => (
    +    query[item] ? [...items, `${item}=${encodeURIComponent(query[item])}`] : items
    +  ), []).join('&')}`
    --- End diff --
    
    if we decide to keep it, the needs a few unit tests


---
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 #745: #3030 Centralise ajax requests

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

    https://github.com/apache/couchdb-fauxton/pull/745
  
    @robertkowalski Haven't forgotten about this, going to spend some time on it this weekend. 


---
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 #745: #3030 Centralise ajax requests

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

    https://github.com/apache/couchdb-fauxton/pull/745
  
    thought: can we use something like http://visionmedia.github.io/superagent/ instead of rolling our own?


---
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 #745: #3030 Centralise ajax requests

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

    https://github.com/apache/couchdb-fauxton/pull/745
  
    Unfortunately this PR is stale now. I'm going to close it. Feel free to open it if you want to work on it and bring it up to date.


---
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 #745: #3030 Centralise ajax requests

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

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


---
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 #745: #3030 Centralise ajax requests

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

    https://github.com/apache/couchdb-fauxton/pull/745#discussion_r70818487
  
    --- Diff: app/addons/auth/resources.js ---
    @@ -12,49 +12,46 @@
     
     import app from "../../app";
     import FauxtonAPI from "../../core/api";
    +import { get, post, put, del } from "../../core/ajax";
     import CouchdbSession from "../../core/couchdbSession";
     
     var Auth = new FauxtonAPI.addon();
     
     
     var Admin = Backbone.Model.extend({
     
    -  initialize: function (props, options) {
    +  initialize(props, options) {
         this.node = options.node;
       },
     
    -  url: function () {
    +  url() {
    --- End diff --
    
    style: `url()` -> `url ()`


---
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 #745: #3030 Centralise ajax requests

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

    https://github.com/apache/couchdb-fauxton/pull/745#discussion_r70819061
  
    --- Diff: app/core/ajax.js ---
    @@ -0,0 +1,188 @@
    +import $ from 'jquery';
    +
    +export const buildQuery = (query) => (
    +  `?${Object.keys(query).reduce((items, item) => (
    +    query[item] ? [...items, `${item}=${encodeURIComponent(query[item])}`] : items
    +  ), []).join('&')}`
    --- End diff --
    
    is that equivalent to http://api.jquery.com/jquery.param/ ?


---
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 #745: #3030 Centralise ajax requests

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

    https://github.com/apache/couchdb-fauxton/pull/745
  
    ben that is really nice and cool stuff. i really like api sugar!
    
    i guess most of the changed code is already covered by other tests, right?
    
    the travis run failed. you can run the unit tests with:
    
    `npm t`


---
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.
---