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

[Import Multisig] Added import multisig data logic #35

Closed
wants to merge 2 commits into from

Conversation

matextrem
Copy link
Contributor

This PR has the following changes:

  • Added logic to query squid graphql API for getting multisig data.
  • Modify useFetchSignersAccount hook to query graphql multisigs data.
  • Modify handleOwners method to check for duplicated addresses.

@matextrem matextrem added the enhancement New feature or request label Aug 25, 2023
@matextrem matextrem self-assigned this Aug 25, 2023
@vercel
Copy link

vercel bot commented Aug 25, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ink-multisig-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 25, 2023 7:53pm
story-ink-multisig-ui ✅ Ready (Inspect) Visit Preview Aug 25, 2023 7:53pm

},
}));
export const StyledStack = styled(Stack, {
shouldForwardProp: (prop) => isPropValid(prop) && prop !== "logoSize",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, this is definitely the way to go, however when there are several properties it becomes very tedious. But for one or 2 properties let's go this way and lastly try with ownerState or lowercase 😅

@@ -26,11 +33,15 @@ function WalletImportStep({
handleThreshold: (threshold: number) => void;
handleAddress: (address: string, step: number, field?: number) => void;
errors: Array<ValidationError[]>;
setErrors: React.Dispatch<React.SetStateAction<ValidationError[][]>>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use SetState for better readability


export function useFetchSignersAccount({ address, walletName }: Props) {
const { loading, error, data } = useQuery<MyQueryResponse>(FETCH_MULTISIG, {
variables: { address },
Copy link
Collaborator

@henrypalacios henrypalacios Aug 28, 2023

Choose a reason for hiding this comment

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

I would to keep the third parties logic on a repository pattern like the others cases (Index DB, LocalStorage).

Something like that:

class XsignerOwnersRepository implements IXsignerOwnersRepository {
  constructor(private client: ApolloClient<MyQueryResponse, MyQueryVariables>) {}

  async getMultisigByAddress(address: string): Promise<MultisigData | null> {
    const { data } = await this.client.query<MyQueryResponse, MyQueryVariables>({
      query: FETCH_MULTISIG,
      variables: { address },
    });

    return data?.multisigs[0] || null;
  }
}

And then use it in this hook.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this logic focuses on getting the owners given an xsignerAccount, a name like useFindXsignerOwners is better.

And in case you imagine it will have more than one hook, its own xsignerOwners folder.

@matextrem matextrem closed this Aug 29, 2023
@matextrem
Copy link
Contributor Author

Closed in favor of #36

@matextrem matextrem deleted the feature/graphql-getMultisig branch August 29, 2023 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants