search

strongloop / strong-remoting

Communicate between objects in servers, mobile apps, and other servers.
www.strongloop.com
Other
105 stars 93 forks source link

To allow user to customize the root element of xml output #272

Closed davidcheung closed 8 years ago

davidcheung commented 8 years ago

To take wrapperElement as an option in remoteMethod setup 

{
  returns: { 
    root: true,
      xml: {wrapperElement: 'a'}
    },
    http: { path: '/' }
}

Objective is to allow user to produce output as follow 

<a>1</a>

Fixes #262

connected to strongloop/strong-remoting/issues/262

davidcheung commented 8 years ago

also added declaration=false flag 

{
  "returns": { 
    "root": true,
    "xml": { "declaration": false }
  },
  "http": { "path": "/" }
}

to not include the declaration of

<?xml version="1.0" encoding="UTF-8"?>

To address this https://github.com/strongloop-internal/scrum-cs/issues/396

Everytime I return something like:

cb(null, '{a:blah}');

I receive xml like: <response><a>blah</a></response> But what I want is xml like: <a>blah</a>

@bajtos, @ritch not sure if this feature is needed, and not sure where to add documentation for such, any advice?

ritch commented 8 years ago

To address this strongloop-internal/scrum-cs#396

I don't think this exactly addresses the issue, but it might be a welcome feature.

davidcheung commented 8 years ago

@ritch, I have addressed the 2 suggestions you made, please review

davidcheung commented 8 years ago

@bajtos review please :)

davidcheung commented 8 years ago

@slnode test please

bajtos commented 8 years ago

@davidcheung reviewed, see my comments above.

davidcheung commented 8 years ago

hi @bajtos, have added unit test for the changes. Please review again :)

bajtos commented 8 years ago

@davidcheung great, this is almost ready. Please address my comments above and squash the commits into a single one. Make sure the commit message mentions all new features (declaration: false, wrapperElement: 'name').

davidcheung commented 8 years ago

@bajtos i have addressed the comments above, I believe the PR is good to go. Please review again

bajtos commented 8 years ago

@slnode test please

bajtos commented 8 years ago

@davidcheung I have fixed the test name myself and improved the commit message a bit. The patch LGTM, I'll land it as soon as I get CI build results.