-
Notifications
You must be signed in to change notification settings - Fork 5
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
MVJ-245_paikkatietovipunen-urls-broken-fix #487
MVJ-245_paikkatietovipunen-urls-broken-fix #487
Conversation
src/util/constants.ts
Outdated
* @readonly | ||
* @const {string} | ||
*/ | ||
export const PAIKKATIETOVIPUNEN_URL = 'http://paikkatietovipunen.hel.fi:10058'; | ||
export const PTP_URL = 'https://ptp.hel.fi'; |
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.
Here it would be really nice if we could figure out what PTP stands for and write it out in the variable to describe what this is
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.
Renamed PTP_URL to PAIKKATIETOPALVELU_URL
src/util/helpers.tsx
Outdated
* @returns {string} | ||
*/ | ||
export const createPaikkatietovipunenUrl = (url: string): string => `${PAIKKATIETOVIPUNEN_URL}/${url}`; | ||
export const createPTPPlanReportUrl = (reportId: string): string => `${PTP_URL}/DataForms/planreport/?id=${reportId}`; |
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.
Really like this approach, nice!
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.
Very good refactor!
It would be helpful to try to write out what this PTP stands for, e.g. by renaming the constant. I don't know what it is, we should maybe ask :)
@robertrytovuori This can be merged I suppose? |
createPaikkatietovipunenUrl to createPTPPlanReportUrl and createPTPPlotDivisionUrl