-
Notifications
You must be signed in to change notification settings - Fork 141
Fixed user profile page error #2618
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
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
A couple questions for you @v-lerie. Thanks for this!
} else { | ||
dispUserSince = dateFormatter.format(accessedUserProfile.user_since); | ||
dispUserSince = dateFormatter.format( | ||
new Date(parseInt(accessedUserProfile.user_since) * 1000), |
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.
question, blocking: Why multiply by 1000?
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.
Without multiplying by 1000, the date kept showing up as January 1st, 1970. This happened because accessedUserProfile.user_since
is a Unix timestamp in seconds, but JavaScript’s Date
expects timestamps in milliseconds. So we multiply by 1000 to format the date correctly.
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.
question, blocking: But when using this code with working profiles, I'm able to properly display the date without multiplying by 1000
When we actually write the record to the table, I believe we use JS Date
, not something native to the database, for this exact purpose.
|
||
const POLICYENGINE_API = "https://api.policyengine.org"; | ||
const POLICYENGINE_API = | ||
process.env.REACT_APP_API_URL || "https://api.policyengine.org"; |
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.
question: Why add this environment variable?
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.
I added REACT_APP_API_URL
for local development and testing. I wasn’t able to get the frontend and backend to connect properly without it. It defaults to the live API if not set, so production behavior isn’t affected. I left it in since it might be helpful for others running things locally, but I can remove it if it’s not needed.
Description
Fixes #2589
Changes