Enhancement/1492 dao identifier query param #1494
Conversation
✅ Deploy Preview for fractal-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -7,7 +7,6 @@ type RouteInfo = { relative: (...args: any) => string; path: string }; | |||
| type RouteIndex = { [key: string]: RouteInfo }; | |||
| export interface DAORoutes extends RouteIndex { | |||
| dao: RouteInfo; | |||
| daos: RouteInfo; | |||
There was a problem hiding this comment.
Noticed that this wasn't being used anywhere in the codebase, so removed it.
| daos: { | ||
| relative: () => '/daos', | ||
| path: 'daos/:address/*', | ||
| }, |
There was a problem hiding this comment.
Noticed that this wasn't being used anywhere in the codebase, so removed it.
| path: 'proposals', | ||
| }, | ||
| proposal: { | ||
| relative: (daoAddress: string, proposalId: string) => | ||
| `/daos/${daoAddress}/proposals/${proposalId}`, | ||
| `/proposals/${proposalId}?dao=${daoAddress}`, |
There was a problem hiding this comment.
Proposal IDs are still living within the path structure, but that's gonna have to be ok for now.
| path: 'proposal-templates', | ||
| }, | ||
| proposalTemplate: { | ||
| relative: (daoAddress: string, proposalTemplateKey: string) => | ||
| `/daos/${daoAddress}/proposal-templates/${proposalTemplateKey}`, | ||
| `/proposal-templates/${proposalTemplateKey}?dao=${daoAddress}`, |
There was a problem hiding this comment.
Same with these "proposal template keys"
| @@ -11,7 +11,9 @@ import { useFractalTreasury } from './loaders/useFractalTreasury'; | |||
| import { useGovernanceContracts } from './loaders/useGovernanceContracts'; | |||
|
|
|||
| export default function useDAOController() { | |||
| const { daoAddress } = useParams(); | |||
There was a problem hiding this comment.
The type of this daoAddress is string | undefined.
| @@ -24,7 +26,7 @@ export default function useDAOController() { | |||
| } | |||
| }, [action, daoAddress]); | |||
|
|
|||
| const { nodeLoading, errorLoading } = useFractalNode({ daoAddress }); | |||
| const { nodeLoading, errorLoading } = useFractalNode({ daoAddress: daoAddress || undefined }); | |||
There was a problem hiding this comment.
So, to conform with useFractalNode expecting string | undefined, just did this to coerce the null to undefined.
There was a problem hiding this comment.
This file is the entry point to it all -- it's where we used to grab the DAO address from the path, but now grab the DAO address from a query param.
| @@ -34,69 +36,69 @@ export const router = createBrowserRouter([ | |||
| element: <HomePage />, | |||
| }, | |||
| { | |||
| path: '/create', | |||
| @@ -103,7 +103,7 @@ export const getProposalVotes = async ( | |||
| ...rest, | |||
| voter, | |||
| choice: VOTE_CHOICES[voteType], | |||
| } as ProposalVote; // This bypasses the type check, but it's fine | |||
| }; | |||
There was a problem hiding this comment.
Such a annoying p3 issue. Like worth investigating? idk plenty of other things to do...
There was a problem hiding this comment.
idk, I'm not super concerned about it at the moment.
|
I'm seeing an issue with this code. If you attempt to load an invalid DAO, the "Invalid DAO" page is rendering without the context of the edit: ah, but I think I know how to fix it. Will push more code in a bit edit2: fixed, with a realization that I didn't need to remove the |
There was a problem hiding this comment.
@adamgall - approval given
I did lots of testing of this PR with Decent, shutter, and myosin by pasting old URLs into this deploy link
Good news is they were all handled successfully 🙂
I can also confirm your point above that linking to an old link to the treasury page, will now link back to the core homepage (note - this is absolutely fine)
Closes #1492
I decided to split the work from #1483 into two PRs, this being the first. To that effect, I created a new issue (#1492), which this PR closes.
This PR:
/daospath prefix from all DAO-specific routes.path, to in a newqueryParam(called "dao")./home".For example, a DAO's URL used to be
But as per this PR, it will be
Additionally, we make sure to preserve existing URLs that are out there on the internet. To do that, we still match the
/daos/:address/*path, and automatically redirect to/home?dao=:address.A side effect that reviewers of this PR should be aware of, is that if someone had bookmarked or previously linked to any page within a DAO (like the treasury or proposals page), the redirect will take a user to the home page in the new URL structure.