Skip to content
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

Enhancement/1492 dao identifier query param #1494

Merged
merged 10 commits into from
Mar 27, 2024
29 changes: 12 additions & 17 deletions src/constants/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed that this wasn't being used anywhere in the codebase, so removed it.

newSubDao: RouteInfo;
modifyGovernance: RouteInfo;
hierarchy: RouteInfo;
Expand All @@ -22,58 +21,54 @@ export interface DAORoutes extends RouteIndex {
}

export const DAO_ROUTES: DAORoutes = {
daos: {
relative: () => '/daos',
path: 'daos/:address/*',
},
Comment on lines -25 to -28
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed that this wasn't being used anywhere in the codebase, so removed it.

dao: {
relative: (daoAddress: string) => `/daos/${daoAddress}`,
relative: (daoAddress: string) => `/home?dao=${daoAddress}`,
path: '*',
},
newSubDao: {
relative: (daoAddress: string) => `/daos/${daoAddress}/new`,
relative: (daoAddress: string) => `/new?dao=${daoAddress}`,
path: 'new',
},
modifyGovernance: {
relative: (daoAddress: string) => `/daos/${daoAddress}/edit/governance`,
relative: (daoAddress: string) => `/edit/governance?dao=${daoAddress}`,
path: 'edit/governance',
},
hierarchy: {
relative: (daoAddress: string) => `/daos/${daoAddress}/hierarchy`,
relative: (daoAddress: string) => `/hierarchy?dao=${daoAddress}`,
path: 'hierarchy',
},
treasury: {
relative: (daoAddress: string) => `/daos/${daoAddress}/treasury`,
relative: (daoAddress: string) => `/treasury?dao=${daoAddress}`,
path: 'treasury',
},
proposals: {
relative: (daoAddress: string) => `/daos/${daoAddress}/proposals`,
relative: (daoAddress: string) => `/proposals?dao=${daoAddress}`,
path: 'proposals',
},
proposal: {
relative: (daoAddress: string, proposalId: string) =>
`/daos/${daoAddress}/proposals/${proposalId}`,
`/proposals/${proposalId}?dao=${daoAddress}`,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposal IDs are still living within the path structure, but that's gonna have to be ok for now.

path: 'proposals/:proposalId',
},
proposalNew: {
relative: (daoAddress: string) => `/daos/${daoAddress}/proposals/new`,
relative: (daoAddress: string) => `/proposals/new?dao=${daoAddress}`,
path: 'proposals/new',
},
settings: {
relative: (daoAddress: string) => `/daos/${daoAddress}/settings`,
relative: (daoAddress: string) => `/settings?dao=${daoAddress}`,
path: 'settings',
},
proposalTemplates: {
relative: (daoAddress: string) => `/daos/${daoAddress}/proposal-templates`,
relative: (daoAddress: string) => `/proposal-templates?dao=${daoAddress}`,
path: 'proposal-templates',
},
proposalTemplate: {
relative: (daoAddress: string, proposalTemplateKey: string) =>
`/daos/${daoAddress}/proposal-templates/${proposalTemplateKey}`,
`/proposal-templates/${proposalTemplateKey}?dao=${daoAddress}`,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same with these "proposal template keys"

path: 'proposal-templates/:proposalTemplateKey',
},
proposalTemplateNew: {
relative: (daoAddress: string) => `/daos/${daoAddress}/proposal-templates/new`,
relative: (daoAddress: string) => `/proposal-templates/new?dao=${daoAddress}`,
path: 'proposal-templates/new',
},
};
8 changes: 5 additions & 3 deletions src/hooks/DAO/useDAOController.ts
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useEffect } from 'react';
import { useParams } from 'react-router-dom';
import { useSearchParams } from 'react-router-dom';
import { useFractal } from '../../providers/App/AppProvider';
import { useERC20Claim } from './loaders/governance/useERC20Claim';
import { useSnapshotProposals } from './loaders/snapshot/useSnapshotProposals';
Expand All @@ -11,7 +11,9 @@ import { useFractalTreasury } from './loaders/useFractalTreasury';
import { useGovernanceContracts } from './loaders/useGovernanceContracts';

export default function useDAOController() {
const { daoAddress } = useParams();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type of this daoAddress is string | undefined.

const [searchParams] = useSearchParams();
const daoAddress = searchParams.get('dao');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type of this daoAddress is string | null.


const {
node: {
nodeHierarchy: { parentAddress },
Expand All @@ -24,7 +26,7 @@ export default function useDAOController() {
}
}, [action, daoAddress]);

const { nodeLoading, errorLoading } = useFractalNode({ daoAddress });
const { nodeLoading, errorLoading } = useFractalNode({ daoAddress: daoAddress || undefined });
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, to conform with useFractalNode expecting string | undefined, just did this to coerce the null to undefined.

useGovernanceContracts();
useFractalGuardContracts({});
useFractalFreeze({ parentSafeAddress: parentAddress });
Expand Down
14 changes: 10 additions & 4 deletions src/router.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createBrowserRouter } from 'react-router-dom';
import { createBrowserRouter, redirect } from 'react-router-dom';
import { ModalProvider } from './components/ui/modals/ModalProvider';
import Layout from './components/ui/page/Layout';
import FourOhFourPage from './pages/404';
Expand Down Expand Up @@ -34,15 +34,15 @@ export const router = createBrowserRouter([
element: <HomePage />,
},
{
path: '/create',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't need that slash

path: 'create',
element: <DaoCreatePage />,
},
{
path: 'daos/:daoAddress',
path: '/',
element: <DAOController />,
children: [
{
index: true,
path: 'home',
element: <DaoDashboardPage />,
},
{
Expand Down Expand Up @@ -97,6 +97,12 @@ export const router = createBrowserRouter([
},
],
},
{
// this exists to keep old links working
// /daos/0x0123/* will redirect to /home?dao=0x0123
path: 'daos/:daoAddress/*',
loader: ({ params: { daoAddress } }) => redirect(`/home?dao=${daoAddress}`),
},
{
path: '*', // 404
element: <FourOhFourPage />,
Expand Down
2 changes: 1 addition & 1 deletion src/utils/azorius.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh this again!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Such a annoying p3 issue. Like worth investigating? idk plenty of other things to do...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idk, I'm not super concerned about it at the moment.

});
};

Expand Down