-
Notifications
You must be signed in to change notification settings - Fork 3
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
cors header #32
base: master
Are you sure you want to change the base?
cors header #32
Conversation
const signedProcessedText = await text.process(signed) | ||
|
||
const translation = await retrieveData(language, 'translations', { signed: signedProcessedText.hash }) | ||
|
||
if (!translation) throw new NotFoundError() |
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.
It's important to keep this validation error.
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.
In this case, shouldn't we just return no translation and let the backend handle this and show an alert to the user that no translation was found?
As our database is still small, most phrases will return no translation.
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 is the backend repo and NotFoundError
is equal to no translation was found.
src/routes/translate/get.js
Outdated
@@ -47,6 +47,7 @@ module.exports = (req, res, next) => { | |||
translations.retrieve(value.params.language, sanitize(value.query.text)) | |||
.then(response => { | |||
res.locals = response | |||
res.set('Access-Control-Allow-Origin', '*') |
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.
Have you tried using localhost
instead of *
?
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.
We should revert the changes on src/controllers/translations/retrieve.js
, there's no need for those changes in this pull request.
ALLOWED_ORIGINS=http://localhost:3001 | ||
``` | ||
|
||
|
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.
Instead of setting ALLOWED_ORIGINS
as mandatory, can we set it to http://localhost:3001
by default and move the update instruction to the Customize section bellow?
Fixes #36
Overview
Changes for running the backend outside the cloud.