Add Operators and Nominator Roles Verification

[marc-aurele-besner]

User description

Add Operators and Nominator Roles Verification

This add role verification logic for Operators and Nominators

---

Type

Enhancement, Bug fix

---

Description

• Updated GraphQL queries and types to include checks for multiple roles (farmer, operator, nominator).
• Refactored Discord role components to handle multiple roles and improved the logic for role verification and assignment.
• Enhanced authentication providers to support verification of multiple roles.
• Added utility functions for managing Discord roles for operators and nominators.

---

Changes walkthrough

Relevant files

Enhancement

gql.ts Update GraphQL Queries and Functions for Role Checks          explorer/gql/gql.ts Updated the CheckRole GraphQL query to include checks for operator and nominator roles. Modified the graphql function to use the updated CheckRole query structure. +2/-2      graphql.ts Update GraphQL Types for Role Verification                              explorer/gql/graphql.ts Updated the CheckRoleQuery type to include operator and nominator fields. +2/-2      GetDiscordRoles.tsx Refactor Discord Role Components and Logic                              explorer/src/components/WalletSideKick/GetDiscordRoles.tsx Refactored GetDiscordRoles component to handle multiple roles and renamed it to SubspaceWalletFlow. Added new DiscordFlow component for Discord connection logic. Updated component to use new GraphQL query for role checks. +83/-70  query.ts Update GraphQL Query for Role Checks                                          explorer/src/components/WalletSideKick/query.ts Renamed QUERY_CHECK_ROLE to QUERY_CHECK_ROLES and updated it to check for multiple roles. +16/-2    discord.ts Enhance Discord Authentication for Multiple Roles                explorer/src/utils/auth/providers/discord.ts Enhanced Discord authentication to handle multiple roles (farmer, operator, nominator). Implemented role verification and assignment based on session data. +27/-5    subspace.ts Update Subspace Provider for Role Verification                      explorer/src/utils/auth/providers/subspace.ts Updated Subspace provider to verify multiple roles (farmer, operator, nominator). +4/-5      index.ts Add Utility Functions for Discord Role Management                explorer/src/utils/auth/vcs/discord/index.ts Added utility functions for operator and nominator role management in Discord. +13/-1    index.ts Update Subspace VCS for Multiple Role Checks                          explorer/src/utils/auth/vcs/subspace/index.ts Updated to check multiple roles using the new GraphQL query. +20/-2

---

✨ PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[netlify]

✅ Deploy Preview for dev-astral ready!

Name	Link
🔨 Latest commit	0470571
🔍 Latest deploy log	https://app.netlify.com/sites/dev-astral/deploys/66435675df5ade00088c9814
😎 Deploy Preview	https://deploy-preview-577--dev-astral.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[github-actions]

PR Description updated to latest commit (d649cfc)

[github-actions]

PR Review

⏱️ Estimated effort to review [1-5]	3, because the PR involves multiple files and complex changes related to role management and GraphQL queries. Understanding the context and ensuring the logic aligns with the intended functionality requires a moderate level of effort.
🧪 Relevant tests	No
🔍 Possible issues	Possible Bug: The graphql function in gql.ts is exported multiple times with the same function signature but different query documents. This could lead to unexpected behavior or errors during runtime if the wrong document is used for a query.
🔒 Security concerns	No

Code feedback:

---

relevant file	explorer/gql/gql.ts
suggestion	Consider consolidating the multiple exports of the graphql function into a single export that can handle different queries based on parameters or different functions for each query to avoid potential conflicts and improve maintainability. [important]
relevant line	export function graphql(source: "\n query CheckRole($subspaceAccount: String!) {\n farmer: rewardEvents(\n where: { name_eq: \"Rewards.VoteReward\", account: { id_eq: $subspaceAccount } }\n limit: 1\n ) {\n account {\n id\n }\n }\n operator: operatorsConnection(\n first: 1\n where: { operatorOwner_eq: $subspaceAccount }\n orderBy: id_ASC\n ) {\n totalCount\n }\n nominator: nominatorsConnection(\n first: 1\n where: { account: { id_eq: $subspaceAccount } }\n orderBy: id_ASC\n ) {\n totalCount\n }\n }\n"): (typeof documents)["\n query CheckRole($subspaceAccount: String!) {\n farmer: rewardEvents(\n where: { name_eq: \"Rewards.VoteReward\", account: { id_eq: $subspaceAccount } }\n limit: 1\n ) {\n account {\n id\n }\n }\n operator: operatorsConnection(\n first: 1\n where: { operatorOwner_eq: $subspaceAccount }\n orderBy: id_ASC\n ) {\n totalCount\n }\n nominator: nominatorsConnection(\n first: 1\n where: { account: { id_eq: $subspaceAccount } }\n orderBy: id_ASC\n ) {\n totalCount\n }\n }\n"];

---

---

✨ Review tool usage guide:

---

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[github-actions]

PR Code Suggestions

Category	Suggestions
Possible issue	Adjust the limit in the GraphQL query to ensure all relevant records are checked. The GraphQL query CheckRole uses a limit: 1 for the rewardEvents, operatorsConnection, and nominatorsConnection queries. This might not be sufficient if the intention is to check for any valid records. Consider removing the limit or adjusting it based on the expected data size to ensure the query checks all relevant records. explorer/gql/gql.ts [51] -"\n query CheckRole($subspaceAccount: String!) {\n farmer: rewardEvents(\n where: { name_eq: \"Rewards.VoteReward\", account: { id_eq: $subspaceAccount } }\n limit: 1\n ) {\n account {\n id\n }\n }\n operator: operatorsConnection(\n first: 1\n where: { operatorOwner_eq: $subspaceAccount }\n orderBy: id_ASC\n ) {\n totalCount\n }\n nominator: nominatorsConnection(\n first: 1\n where: { account: { id_eq: $subspaceAccount } }\n orderBy: id_ASC\n ) {\n totalCount\n }\n }\n" +"\n query CheckRole($subspaceAccount: String!) {\n farmer: rewardEvents(\n where: { name_eq: \"Rewards.VoteReward\", account: { id_eq: $subspaceAccount } }\n ) {\n account {\n id\n }\n }\n operator: operatorsConnection(\n where: { operatorOwner_eq: $subspaceAccount }\n orderBy: id_ASC\n ) {\n totalCount\n }\n nominator: nominatorsConnection(\n where: { account: { id_eq: $subspaceAccount } }\n orderBy: id_ASC\n ) {\n totalCount\n }\n }\n"
	Add error handling for undefined or null account values. Consider handling the case where account might be undefined or null before using it to construct did and verify roles, to prevent runtime errors. explorer/src/utils/auth/providers/subspace.ts [34] +if (!account) throw new Error('Account is required'); const did = `did:subspace:${account}`
Best practice	Add dependencies to useCallback to avoid outdated closures. The handleConnectDiscord function is using an empty dependency array for useCallback. This could lead to outdated closures if any dependencies are added in the future. Include signIn in the dependency array to ensure the callback is updated when signIn changes. explorer/src/components/WalletSideKick/GetDiscordRoles.tsx [81-83] const handleConnectDiscord = useCallback( async () => await signIn('discord', { redirect: false }), - [], + [signIn], )
	Validate the environment variable format for the Discord operator role ID. To avoid potential runtime errors, ensure that DISCORD_GUILD_ROLE_ID_OPERATOR is not only present but also a valid format before proceeding to use it. explorer/src/utils/auth/vcs/discord/role-operator.ts [6] const { DISCORD_GUILD_ROLE_ID_OPERATOR } = process.env +if (!DISCORD_GUILD_ROLE_ID_OPERATOR || !/^\d+$/.test(DISCORD_GUILD_ROLE_ID_OPERATOR)) { + throw new Error('Invalid or missing Discord guild role ID for operator'); +}
Performance	Use React.memo for functional components to prevent unnecessary re-renders. The SubspaceWalletFlow and DiscordFlow components are exported but their usage within the same file does not leverage the React.memo for potential performance optimization. Consider wrapping these components with React.memo to avoid unnecessary re-renders if their props do not change. explorer/src/components/WalletSideKick/GetDiscordRoles.tsx [29-102] -export const SubspaceWalletFlow: FC<StakingSummaryProps> = ({ subspaceAccount }) => {...} -export const DiscordFlow: FC = () => {...} +export const SubspaceWalletFlow: FC<StakingSummaryProps> = React.memo(({ subspaceAccount }) => {...}) +export const DiscordFlow: FC = React.memo(() => {...})
Enhancement	Add error handling for Discord API calls to manage failures gracefully. The function Discord in discord.ts checks for roles and assigns them if they are missing. However, it does not handle the case where the Discord API might fail. Implement try-catch blocks around the API calls to handle potential errors gracefully. explorer/src/utils/auth/providers/discord.ts [51-68] if (session.subspace?.vcs.farmer && !farmer) { - await giveDiscordFarmerRole(profile.id) - newRolesAdded = true + try { + await giveDiscordFarmerRole(profile.id) + newRolesAdded = true + } catch (error) { + console.error('Failed to assign farmer role:', error); + } }
	Improve error handling by including the original error message. Instead of throwing a generic error message in the catch block, consider rethrowing the original error or using it to provide more context in the new error message. explorer/src/utils/auth/vcs/subspace/index.ts [18-19] console.error('Failed to fetch if user has any related events:', error) -throw new Error('Failed to fetch if user has any related events') +throw new Error(`Failed to fetch if user has any related events: ${error.message}`)
Maintainability	Refactor hardcoded roles into a configuration object for better maintainability. In the GetDiscordRoles component, the roles are hardcoded in the JSX. To improve maintainability and scalability, consider defining the roles in a configuration object or file and map over them to render the list items. explorer/src/components/WalletSideKick/GetDiscordRoles.tsx [120-127] -<StyledListItem title='You are a Farmer on Discord'>🌾</StyledListItem> -<StyledListItem title='You are a Nominator on Discord'>🌐</StyledListItem> -<StyledListItem title='You are a Operator on Discord'>🤝</StyledListItem> +const roles = [ + { title: 'You are a Farmer on Discord', icon: '🌾' }, + { title: 'You are a Nominator on Discord', icon: '🌐' }, + { title: 'You are a Operator on Discord', icon: '🤝' }, +]; +{roles.map(role => ( + <StyledListItem title={role.title}>{role.icon}</StyledListItem> +))}
	Use explicit exports for better code clarity and maintenance. Consider using explicit exports instead of a bulk export statement to improve code readability and maintainability. explorer/src/utils/auth/vcs/discord/index.ts [8-15] -export { - getUserRoles, - giveDiscordFarmerRole, - giveDiscordNominatorRole, - giveDiscordOperatorRole, - verifyDiscordFarmerRole, - verifyDiscordGuildMember, - verifyDiscordNominatorRole, - verifyDiscordOperatorRole, -} +export { getUserRoles }; +export { giveDiscordFarmerRole }; +export { giveDiscordNominatorRole }; +export { giveDiscordOperatorRole }; +export { verifyDiscordFarmerRole }; +export { verifyDiscordGuildMember }; +export { verifyDiscordNominatorRole }; +export { verifyDiscordOperatorRole };
	Refactor repeated environment variable checks into a single function. Refactor the repeated checks for DISCORD_GUILD_ROLE_ID_NOMINATOR into a single function to reduce code duplication and improve maintainability. explorer/src/utils/auth/vcs/discord/role-nominator.ts [4-6] -if (!process.env.DISCORD_GUILD_ROLE_ID_NOMINATOR) - throw new Error('No Discord guild role ID for nominator') -const { DISCORD_GUILD_ROLE_ID_NOMINATOR } = process.env +function checkNominatorRoleId() { + if (!process.env.DISCORD_GUILD_ROLE_ID_NOMINATOR) + throw new Error('No Discord guild role ID for nominator'); + return process.env.DISCORD_GUILD_ROLE_ID_NOMINATOR; +} +const DISCORD_GUILD_ROLE_ID_NOMINATOR = checkNominatorRoleId();

---

✨ Improve tool usage guide:

---

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

• When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

• With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.

[jim-counter]

We just need to get you the role IDs as everything else should already be in place here @marc-aurele-besner?

[EmilFattakhov]

Looks good, just a few small questions / comments left

[marc-aurele-besner]

@EmilFattakhov for your comment in .env.sample

These values should not be on this file, since this PR does not have any GitHub OpenID logic, the GitHub implementation is #578.

To answer your question, atm I only implemented the OpenID connect flow in the subsequent PR, but I did not implement any specific logic, waiting to know what will make sense.

[marc-aurele-besner]

We just need to get you the role IDs as everything else should already be in place here @marc-aurele-besner?

I already set up the roles in the dev discord server, so as soon as this PR is merged on staging, it should be testable with the dev server.

For production, indeed, we will need to create the "Verified X" role and communicate with me the role ID 👍