Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file removed addon/.gitkeep
Empty file.
20 changes: 17 additions & 3 deletions addon/utils/url-builder.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,22 @@
import param from 'jquery-param';
import URL from 'url';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could add a note about this being a polyfill and that when Ember drop support for IE 11 this won't be needed anymore:

https://caniuse.com/#search=URLSearchParams


export default function(url, path, queryParams) {
let query = param(queryParams);
let pathUrl = url.charAt(url.length - 1) === '/' ? `${url}${path}` : `${url}/${path}`;
let pathUrl;
if (path) {
let maybeMissingSlash = url.endsWith('/') || path.startsWith('/') ? '' : '/'
pathUrl = `${url}${maybeMissingSlash}${path}`;
} else {
pathUrl = url
}

// Use native URL API to generate query
let queryObj = new URL('https://exelord.com/ember-custom-actions/').searchParams;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let queryObj = new URL('https://exelord.com/ember-custom-actions/').searchParams;
// Passing in example.com because a domain is needed, but we're not using it.
// Using `URL` only to access the queryParams API.
let queryObj = new URL('https://example.com').searchParams;

Copy link
Member

@Exelord Exelord May 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://host.test - never use .com it fires DNS lookup, which allows track where the request is coming from.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure? I'm 99% certain that new URL('https://example.com') won't fire any DNS lookups. It's just a bunch of string operations.

But yeah, .test should also work well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case we won't render it that's true. But just to prevent that issue in the future is better to forget about .com's for examples :)

if (queryParams) {
Object.keys(queryParams).forEach(key =>
queryObj.set(key, queryParams[key])
);
}
let query = queryObj.toString();

return query ? `${pathUrl}?${query}` : pathUrl;
}
6 changes: 0 additions & 6 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,5 @@ module.exports = {
{ transformation: 'cjs', as: 'lodash.merge' }
]
});

this.import('node_modules/jquery-param/jquery-param.js', {
using: [
{ transformation: 'amd', as: 'jquery-param' }
]
});
},
};
13 changes: 8 additions & 5 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"dependencies": {
"ember-cli-babel": "6.16.0",
"ember-cli-cjs-transform": "2.0.0",
"jquery-param": "1.0.1",
"ember-url": "0.6.0",
"lodash.merge": "4.6.2"
},
"devDependencies": {
Expand Down
Empty file removed tests/dummy/app/models/.gitkeep
Empty file.
6 changes: 5 additions & 1 deletion tests/dummy/app/models/bike.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,9 @@ import { modelAction } from 'ember-custom-actions';

export default Model.extend({
name: attr(),
ride: modelAction('ride', { method: 'PUT', data: { defaultParam: 'ok' } })
ride: modelAction('ride', { method: 'PUT', data: { defaultParam: 'ok' } }),
clean: modelAction(undefined, {
method: 'PATCH',
data: { defaultParam: 'ok' }
})
});
Empty file removed tests/unit/.gitkeep
Empty file.
25 changes: 25 additions & 0 deletions tests/unit/models/bike-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,28 @@ test('model action set serialized errors in error object', function(assert) {
done();
});
});

test('model action without actionId', function(assert) {
assert.expect(4);

this.server.patch('/bikes/:id', (request) => {
let data = JSON.parse(request.requestBody);

assert.deepEqual(data, { myParam: 'My first param', defaultParam: 'ok' });
assert.deepEqual(request.queryParams, { soap: 'true', include: 'sponge' });
assert.equal(request.url, '/bikes/1?include=sponge&soap=true');

return [200, { }, 'true'];
});

let done = assert.async();
let payload = { myParam: 'My first param' };

let model = this.subject();
model.set('id', 1);

model.clean(payload, { queryParams: { soap: true, include: 'sponge' } }).then((response) => {
assert.ok(response, true);
done();
});
});