-
Notifications
You must be signed in to change notification settings - Fork 1
Feature/EN-10178-show-historical-detours #134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
7b6b921
to
2d508eb
Compare
.getPatternForSyncRouteEditor(1) | ||
.then(getPatternResponse => { | ||
getPatternResponse.should.be.an('object'); | ||
// todo: flesh this out after payload shape is confirmed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be updated before pushed into the main branch.
Also would be nice if the route ID was defined as a variable for readability
const patternId = 1;
[...].getPatternForSyncRouteEditor(patternId);
beforeEach(() => fetchMock.catch(503)); | ||
afterEach(fetchMock.restore); | ||
|
||
it('should get all historical detours without date parameters', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a page limit by default? Should it get all historical detours?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementing paging is a pain so in this first iteration we are retrieving all historical detours by default OR using the "count" parameter for the first x detours by startDate desc. We will likely have to add paging this year.
src/resources/Detour.js
Outdated
|
||
if (params.length > 0) { | ||
endpoint += `?${params.join('&')}`; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works. 👍
Browsers also have wide support for UrlSearchParams
that can build parameters, too.
const paramsObj = { foo: "bar", baz: "bar" };
const searchParams = new URLSearchParams(paramsObj);
console.log(searchParams.toString()); // "foo=bar&baz=bar"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah you're right this is a little old fashioned. Updating
@glenwperry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Uh oh!
There was an error while loading. Please reload this page.