-
Notifications
You must be signed in to change notification settings - Fork 56
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
Allow short token URLs #1962
Allow short token URLs #1962
Conversation
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.
Code is looking good. I noticed that edit proposals is still redirecting to the full token URL. Can we use short token URLs there too?
@tiagoalvesdulce Intended mate, BE edit route need the full token so I kept it as is in that case - when edit is done I do use the short token when redirecting the user to proposal's details page |
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.
Code looking good! Left only one inline comment. It is just a doubt :D
@@ -27,19 +26,6 @@ export const getStatusBarData = (voteSummary) => { | |||
.sort((a) => (a.label === "yes" ? -1 : 1)); | |||
}; | |||
|
|||
export const getProposalUrl = (token, javascriptEnabled) => |
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 any specific reason for removing this piece of code?
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.
hehe sure, it's just a cleanup at some point we created useProposalURLs
hook and it uses the same functions which are defined in https://github.com/amassarwi/politeiagui/blob/tokenPrefix/src/containers/Proposal/helpers.js#L225-L236
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.
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.
oh good! Thanks! haha
This diff allows users to use short proposal's token in URLs - first 7 chars of original 64 chars hex.
Solution description
useProposal
hook to fetch voteSummary only when full token is fetched and stored in redux store.makeGetProposalByToken
selector to try find the stored proposal by using the 7 chars prefix instead of full length token.useProposalURLs
hook to use prefix instead of full token to struct proposal URLs.ProposalForm
submit handler to use short token when navigating to proposal details after successful edit/create.Dependencies
Closes #1939