Skip to content
This repository has been archived by the owner on Mar 29, 2022. It is now read-only.

Add baseUrl Request Helper #8

Merged

Conversation

louiealmeda
Copy link
Contributor

Computed baseUrl

Parent issue

Closes #7

Technical implementation details

  • Added baseUrl to RequestHelpers
  • Added Environment as a required parameter for TemplateParser and RequestHelpers
  • Updated all unit tests for TemplateParser to set the Environment as null
  • Added unit tests for building the baseUrl

@louiealmeda louiealmeda requested a review from 255kb as a code owner May 6, 2021 15:03
src/libs/server.ts Outdated Show resolved Hide resolved
@louiealmeda
Copy link
Contributor Author

Hmmm.. lints are failing, I'll check tomorrow.

@255kb
Copy link
Member

255kb commented May 7, 2021

Yes, you need to use prettier for formatting, it's mandatory. Easiest is to use the VSCode/Webstorm extension, but you could also do a npx prettier --write.

Copy link
Member

@255kb 255kb left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, it looks great 👍
Maybe my idea of allowing null for the environment wasn't that good, we should use {} as any in the tests, so we don't have to allow Environment | null in the TemplateParser just for them. Could you please make the changes?
Sorry for the extra work.

src/libs/templating-helpers/request-helpers.ts Outdated Show resolved Hide resolved
test/template-parser.spec.ts Outdated Show resolved Hide resolved
test/template-parser.spec.ts Outdated Show resolved Hide resolved
test/template-parser.spec.ts Outdated Show resolved Hide resolved
@louiealmeda louiealmeda force-pushed the feature/7-add-baseUrl-request-helper branch from 518464e to e548eea Compare May 7, 2021 12:22
@louiealmeda
Copy link
Contributor Author

Thanks for the feedback and tips @255kb, ready for another review 👍

Copy link
Member

@255kb 255kb left a comment

Choose a reason for hiding this comment

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

Thanks it's perfect :)
One small change though and I think we can merge.

src/libs/templating-helpers/request-helpers.ts Outdated Show resolved Hide resolved
@255kb
Copy link
Member

255kb commented May 7, 2021

Thanks! I will take care of the documentation and integrate the lib in the next release.

@255kb 255kb merged commit c5d081a into mockoon:main May 7, 2021
255kb pushed a commit that referenced this pull request May 7, 2021
- add and split tests
- update faker.js to latest version, make setSeed null safe
Closes #8
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add baseUrl variable to templating RequestHelpers
2 participants