Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[FEAT] Github connection with payouts #756

Merged
merged 13 commits into from
Sep 10, 2024
Merged

[FEAT] Github connection with payouts #756

merged 13 commits into from
Sep 10, 2024

Conversation

fonstack
Copy link
Member

@fonstack fonstack commented Aug 30, 2024

Summary by CodeRabbit

  • New Features

    • Added support for fetching and displaying GitHub issues related to vault submissions.
    • Introduced a vault filter for submissions, allowing users to filter by specific vaults.
    • Enhanced form components with helper text for better user guidance.
  • Bug Fixes

    • Improved loading indicators for GitHub issues to enhance user feedback during data fetching.
  • Documentation

    • Expanded localization support with new strings for GitHub issue management and vault interactions.
  • Style

    • Updated styles for better layout and visual consistency across submission and vault components.

Copy link
Contributor

coderabbitai bot commented Aug 30, 2024

Walkthrough

The pull request introduces several enhancements to the codebase, including the addition of a new GithubIssue type for better integration with GitHub issues, updates to various components to display and manage these issues, and the introduction of new constants and utility functions. Changes span across multiple files, enhancing data handling, user interface elements, and overall functionality related to submissions and vault management.

Changes

Files Change Summary
packages/shared/src/types/payout.ts, packages/web/src/pages/CommitteeTools/PayoutsTool/PayoutFormPage/forms/SinglePayout/SinglePayoutForm.tsx, packages/web/src/pages/CommitteeTools/PayoutsTool/components/PayoutAllocation/SplitPayoutAllocation/components/SplitPayoutBeneficiaryForm.tsx, packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionDetailsPage/SubmissionDetailsPage.tsx, packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionsListPage/SubmissionCard.tsx Added GithubIssue type and integrated it into various components to fetch and display GitHub issues related to vaults and submissions.
packages/web/src/components/FormControls/FormSelectInput/FormSelectInput.tsx, packages/web/src/components/FormControls/FormSelectInput/styles.ts Introduced an optional helper property in the FormSelectInput component to display additional context. Updated styles for the helper text.
packages/web/src/constants/constants.ts Added a new entry to the LocalStorage enum for tracking user-selected vaults in submissions.
packages/web/src/hooks/vaults/useUserVaults.tsx Implemented a sorting mechanism for vaults based on starttime to enhance user experience.
packages/web/src/languages/en.json Expanded localization support with new string entries related to GitHub issues and vault management.
packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionsListPage/SubmissionsListPage.tsx Introduced a vault filter and improved filtering logic for submissions, enhancing user interaction and data retrieval.
packages/web/src/pages/VaultEditor/VaultEditorFormPage/SetupSteps/shared/VaultCuratorForm/VaultCuratorForm.tsx Added a helper property to the VaultCuratorForm component for contextual assistance.
packages/web/src/settings.ts, packages/web/src/utils/slug.utils.ts Added a constant for the GitHub bot ID and a utility function to generate formatted repository names based on vault metadata.
packages/web/src/pages/CommitteeTools/SubmissionsTool/submissionsService.api.ts Introduced new asynchronous functions to fetch GitHub issues related to vaults and link submissions to their corresponding issues.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Vault
    participant GitHubAPI
    participant SubmissionService
    participant SubmissionCard

    User->>Vault: Selects a vault
    Vault->>SubmissionService: Fetch GitHub issues
    SubmissionService->>GitHubAPI: Request issues for vault
    GitHubAPI-->>SubmissionService: Return issues
    SubmissionService-->>Vault: Send issues
    Vault-->>SubmissionCard: Pass issues
    SubmissionCard->>User: Display submission and GitHub issue info
Loading

🐇 In a world of code and light,
A rabbit hops with pure delight.
New features bloom, like flowers in spring,
With GitHub issues, we dance and sing!
So let’s celebrate, both big and small,
In this code garden, there’s joy for all! 🌼✨

Tip

Announcements
  • The review status is no longer posted as a separate comment when there are no actionable or nitpick comments. In such cases, the review status is included in the walkthrough comment.
  • We have updated our review workflow to use the Anthropic's Claude family of models. Please share any feedback in the discussion post on our Discord.
  • Possibly related PRs: Walkthrough comment now includes a list of potentially related PRs to help you recall past context. Please share any feedback in the discussion post on our Discord.
  • Suggested labels: CodeRabbit can now suggest labels by learning from your past PRs in the walkthrough comment. You can also provide custom labeling instructions in the UI or configuration file.
  • Possibly related PRs, automatic label suggestions based on past PRs, learnings, and possibly related issues require data opt-in (enabled by default).

Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 4248ae1 and d10c406.

Files selected for processing (2)
  • packages/shared/package.json (1 hunks)
  • packages/shared/src/utils/vaultEditor.utils.ts (1 hunks)
Files skipped from review due to trivial changes (1)
  • packages/shared/package.json
Additional comments not posted (1)
packages/shared/src/utils/vaultEditor.utils.ts (1)

450-452: LGTM, but verify the impact of restricting arbitrator permissions.

The code changes that set arbitratorCanChangeBeneficiary, arbitratorCanChangeBounty, and arbitratorCanSubmitClaims to false are approved.

These changes restrict the arbitrator's capabilities by not allowing them to change the beneficiary, change the bounty, or submit claims. This indicates a shift towards a more restrictive approach in managing vault operations.

Please verify that these changes align with the intended behavior and assess their impact on the overall system. Ensure that restricting the arbitrator's permissions does not introduce any unintended consequences or break any existing functionality.

Run the following script to verify the usage of the editedFormToCreateVaultOnChainCall function and the impact of the changes:


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

cloudflare-workers-and-pages bot commented Aug 30, 2024

Deploying dapp with  Cloudflare Pages  Cloudflare Pages

Latest commit: d10c406
Status: ✅  Deploy successful!
Preview URL: https://d1b0bf44.dapp-a9y.pages.dev
Branch Preview URL: https://feat-better-payouts.dapp-a9y.pages.dev

View logs

@fonstack fonstack marked this pull request as ready for review September 9, 2024 10:38
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Outside diff range, codebase verification and nitpick comments (2)
packages/web/src/hooks/vaults/useUserVaults.tsx (1)

76-78: Sorting function implemented correctly.

The sorting function correctly organizes the foundVaults array in descending order based on the starttime. To improve code readability and maintainability, consider adding a comment explaining the defaulting to 0 for undefined starttime.

+ // Sort vaults by `starttime` in descending order, defaulting to 0 if undefined
packages/shared/src/types/payout.ts (1)

45-55: New type GithubIssue defined correctly.

The GithubIssue type is well-defined with all necessary properties to represent GitHub issues in the payout system. Consider adding documentation comments to each property for better clarity and maintainability.

+ /**
+  * Represents a GitHub issue in the payout system.
+  * @property {number} id - The unique identifier for the GitHub issue.
+  * @property {number} number - The issue number on GitHub.
+  * @property {string} title - The title of the issue.
+  * @property {number} createdBy - The user ID of the issue creator.
+  * @property {string[]} labels - Labels associated with the issue.
+  * @property {string[]} validLabels - Labels that are considered valid for processing.
+  * @property {string} createdAt - The creation date of the issue.
+  * @property {string} body - The body text of the issue.
+  * @property {string} [txHash] - Optional transaction hash linking the issue to a blockchain transaction.
+  */
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 1b97ec7 and 4248ae1.

Files selected for processing (17)
  • packages/shared/src/types/payout.ts (3 hunks)
  • packages/web/src/components/FormControls/FormSelectInput/FormSelectInput.tsx (3 hunks)
  • packages/web/src/components/FormControls/FormSelectInput/styles.ts (1 hunks)
  • packages/web/src/constants/constants.ts (1 hunks)
  • packages/web/src/hooks/vaults/useUserVaults.tsx (1 hunks)
  • packages/web/src/languages/en.json (2 hunks)
  • packages/web/src/pages/CommitteeTools/PayoutsTool/PayoutFormPage/forms/SinglePayout/SinglePayoutForm.tsx (4 hunks)
  • packages/web/src/pages/CommitteeTools/PayoutsTool/components/PayoutAllocation/SplitPayoutAllocation/components/SplitPayoutBeneficiaryForm.tsx (4 hunks)
  • packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionDetailsPage/SubmissionDetailsPage.tsx (4 hunks)
  • packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionsListPage/SubmissionCard.tsx (3 hunks)
  • packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionsListPage/SubmissionsListPage.tsx (15 hunks)
  • packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionsListPage/styles.ts (5 hunks)
  • packages/web/src/pages/CommitteeTools/SubmissionsTool/submissionsService.api.ts (2 hunks)
  • packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultScopeSection/InScopeSection/InScopeSection.tsx (2 hunks)
  • packages/web/src/pages/VaultEditor/VaultEditorFormPage/SetupSteps/shared/VaultCuratorForm/VaultCuratorForm.tsx (1 hunks)
  • packages/web/src/settings.ts (1 hunks)
  • packages/web/src/utils/slug.utils.ts (1 hunks)
Files skipped from review due to trivial changes (1)
  • packages/web/src/settings.ts
Additional comments not posted (32)
packages/web/src/components/FormControls/FormSelectInput/styles.ts (1)

28-34: Approve the addition of span.helper style.

The new CSS rules for span.helper are clear and well-defined, enhancing the visual presentation of helper text within the StyledFormSelectInput component. The use of CSS variables for colors and spacing ensures consistency with the rest of the application's styling.

packages/web/src/pages/VaultEditor/VaultEditorFormPage/SetupSteps/shared/VaultCuratorForm/VaultCuratorForm.tsx (1)

59-59: Approve the addition of the helper property with a suggestion for verification.

The implementation of the helper property in the FormSelectInput component is well-done, enhancing the user experience by providing contextual information based on the selected curator. However, ensure that the addresses array is always populated for all curators to avoid displaying undefined or incorrect information.

Consider adding a check or handling cases where the addresses array might be empty:

helper={curators?.find((c) => c.username === selectedCurator)?.addresses?.[0] || 'No address available'}
packages/web/src/constants/constants.ts (1)

12-12: New constant added correctly.

The addition of SelectedVaultInSubmissions to the LocalStorage enum is correctly implemented and follows the established naming conventions.

packages/shared/src/types/payout.ts (2)

79-79: Integration of ghIssue into ISinglePayoutData is appropriate.

The optional property ghIssue enhances the data structure by allowing the inclusion of GitHub issue details, which is crucial for linking payouts to specific GitHub activities.


105-105: Integration of ghIssue into ISplitPayoutBeneficiary is appropriate.

This integration allows for detailed data representation in split payouts, linking specific GitHub issues to payout beneficiaries.

packages/web/src/components/FormControls/FormSelectInput/FormSelectInput.tsx (2)

21-21: Approved addition of helper property.

The addition of the helper property to the FormSelectInputProps interface is correctly implemented and aligns with the PR objectives to enhance user experience by providing additional context.


56-56: Approved modification in function signature.

The inclusion of helper in the FormSelectInputComponent function signature is correctly implemented, enabling the use of this property within the component.

packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionsListPage/SubmissionCard.tsx (3)

21-21: Approved addition of ghIssue property.

The addition of the ghIssue property to the SubmissionCardProps type is correctly implemented, enabling the component to display GitHub issue information alongside submission details.


27-27: Approved modification in function signature.

The inclusion of ghIssue in the SubmissionCard function signature is correctly implemented, enabling the use of this property within the component to display GitHub issue information.


70-90: Approved conditional rendering of GitHub issue information.

The conditional rendering of GitHub issue information in the SubmissionCard is correctly implemented. It enhances the component's functionality by allowing it to display relevant GitHub issue data alongside submission details.

packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionDetailsPage/SubmissionDetailsPage.tsx (5)

25-32: Approved addition of state variables and use of useVaults hook.

The introduction of the vaultGithubIssues and isLoadingGH state variables, along with the use of the useVaults hook, is correctly implemented. These additions are essential for managing the fetching and display of GitHub issues related to vault submissions.


38-53: Approved new useEffect logic for fetching GitHub issues.

The new useEffect logic implemented for fetching GitHub issues is correctly set up. It ensures that GitHub issues are fetched and stored based on the submission's linked vault, aligning with the PR objectives to enhance functionality related to GitHub issue tracking.


108-112: Approved modification in SubmissionCard component usage.

The update to the SubmissionCard component to include the ghIssue prop is correctly implemented. This modification enhances the component's functionality by linking the submission to its corresponding GitHub issue.


140-141: Approved conditional rendering of loading indicators.

The conditional rendering of loading indicators based on the states of vault readiness and GitHub issue loading is correctly implemented. This enhancement improves user feedback during data fetching.


Line range hint 1-141: Approved overall integration of new functionality.

The integration of new functionality related to fetching and displaying GitHub issues in the SubmissionDetailsPage is well-implemented. This enhances the component's ability to manage and display relevant data effectively.

packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionsListPage/styles.ts (6)

117-117: Approved: Full width for controls and controls-row.

The changes to set the width of .controls and .controls-row to 100% are appropriate for ensuring that these elements occupy the full width of their container, enhancing the responsiveness of the layout.

Also applies to: 129-130


130-130: Approved: Full width for vaults-filter.

Setting the .vaults-filter class to have a width of 100% aligns with the design goal of uniform filter widths across the interface.


134-136: Approved: Right alignment for pagination controls.

The addition of margin-left: auto; to the .pagination class effectively aligns the pagination controls to the right, which is a standard practice for such UI elements.


151-153: Approved: Fixed width for numerical indicators.

The fixed width of 15px for the .number class helps standardize the appearance of numerical indicators, which is beneficial for maintaining a consistent UI.


186-197: Approved: Flexbox layout for better alignment and spacing.

The introduction of .flex and .flex-end classes utilizes the flexbox layout to improve alignment and spacing of elements, with .flex-end specifically ensuring right alignment of its contents.


280-289: Approved: Enhanced visual hierarchy for severity indicators.

The .severity class within the StyledSubmissionCard uses a flex container to ensure proper alignment and spacing. The specific styles for span elements enhance the visual hierarchy by adjusting font size and weight, which improves clarity and emphasis on severity indicators.

packages/web/src/languages/en.json (2)

733-737: Review of New Localization Keys for GitHub Issues

The new keys added for GitHub issue management are correctly formatted and seem appropriate for the described functionality. These keys will enhance the user interface by providing clear labels and messages related to GitHub issue handling within the application.

  • "loadingGithubIssues": "Loading GitHub issues" - Indicates that GitHub issues are being loaded, which is a necessary feedback for the user.
  • "submittedAs": "Submitted as" - Useful for labeling how issues were submitted.
  • "labeledAs": "Labeled as" - Useful for displaying labels associated with issues.
  • "onlyShowLabeledIssues": "Show only labeled issues" - Provides a filtering option to the user, enhancing usability.
  • "ghIssue": "GH issue" - A concise label for referring to GitHub issues.

1133-1136: Review of New Localization Keys for Vault Management

The newly added keys related to vault management are well-formulated and align with the intended enhancements for vault interaction capabilities:

  • "filterByVault": "Filter by vault" - Allows users to filter data or views by specific vaults, which is crucial for navigation and usability in applications dealing with multiple vaults.
  • "showAllVaults": "Show all vaults" - Provides a straightforward option for users wanting to view all vaults, enhancing the application's navigational aspects.
  • "goToVaultGithubIssues": "Go to vault GitHub issues" - This key directly supports the new feature integration, linking vault management with GitHub issues, which is likely part of the feature set described in the PR objectives.

These additions are consistent with the PR's goal of enhancing GitHub integration and vault management functionalities.

packages/web/src/pages/CommitteeTools/PayoutsTool/PayoutFormPage/forms/SinglePayout/SinglePayoutForm.tsx (2)

86-87: State variables correctly initialized.

The addition of vaultGithubIssues and isLoadingGH state variables is correctly implemented with appropriate initial values.


122-125: Correct integration of ghIssue prop in SubmissionCard.

The ghIssue prop is correctly passed to the SubmissionCard component, enhancing its functionality to display relevant GitHub issue information alongside the submission data.

packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultScopeSection/InScopeSection/InScopeSection.tsx (2)

24-24: Approved import statement for utility function.

The import statement correctly imports getForkedRepoName from utils/slug.utils, which is used to abstract the logic for generating repository names.


73-73: Refactor improves maintainability by using utility function.

The use of getForkedRepoName(vault) at line 73 correctly abstracts the logic for generating the forkedRepoName. This change enhances code readability and maintainability.

packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionsListPage/SubmissionsListPage.tsx (3)

81-104: State management for filtering is correctly implemented.

The new state variables vaultFilter, onlyShowLabeled, and vaultGithubIssues are correctly initialized and used to manage the filtering of submissions based on vaults and GitHub issues.


87-102: Asynchronous function handles user confirmation and navigation correctly.

The function goToFilteredVaultGithubIssues correctly prompts the user for confirmation before navigating to the GitHub issues page, ensuring a good user experience by preventing unwanted navigation.


130-143: Expanded filtering logic is effectively implemented.

The filtering logic for submissions, implemented using the useMemo hook, correctly evaluates conditions based on the selected vault and its associated GitHub issues, effectively refining the data presented to the user.

packages/web/src/pages/CommitteeTools/SubmissionsTool/submissionsService.api.ts (1)

1-11: Imports are correctly structured.

The import statements are well-organized and include all necessary components for the new functionality.

packages/web/src/pages/CommitteeTools/PayoutsTool/components/PayoutAllocation/SplitPayoutAllocation/components/SplitPayoutBeneficiaryForm.tsx (1)

101-102: State variables are appropriately used.

The state variables vaultGithubIssues and isLoadingGH are correctly declared and used to manage data and loading states effectively.

Comment on lines +11 to +19
export const getForkedRepoName = (vault: IVault) => {
const vaultName = vault.description?.["project-metadata"].name;
if (!vaultName) return "";

const vaultSlug = vaultName.replace(/[^\w\s]| /gi, "-");
const forkedRepoName = `${vaultSlug}-${vault.id}`;

return forkedRepoName;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Approve the addition of getForkedRepoName with a suggestion for improvement.

The getForkedRepoName function is well-implemented and follows good practices in handling potential null values and sanitizing input. However, consider enhancing the sanitization process to handle multiple consecutive non-word characters or spaces more gracefully by collapsing them into a single hyphen. This can improve the readability and consistency of the generated repository names.

Consider this improvement to the sanitization logic:

- const vaultSlug = vaultName.replace(/[^\w\s]| /gi, "-");
+ const vaultSlug = vaultName.replace(/[\W_]+/g, "-");
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const getForkedRepoName = (vault: IVault) => {
const vaultName = vault.description?.["project-metadata"].name;
if (!vaultName) return "";
const vaultSlug = vaultName.replace(/[^\w\s]| /gi, "-");
const forkedRepoName = `${vaultSlug}-${vault.id}`;
return forkedRepoName;
};
export const getForkedRepoName = (vault: IVault) => {
const vaultName = vault.description?.["project-metadata"].name;
if (!vaultName) return "";
const vaultSlug = vaultName.replace(/[\W_]+/g, "-");
const forkedRepoName = `${vaultSlug}-${vault.id}`;
return forkedRepoName;
};

@@ -145,6 +147,7 @@ export function FormSelectInputComponent(
})}
</SelectMenuOptions>
)}
{!error && helper && <span className="helper">{helper}</span>}
Copy link
Contributor

Choose a reason for hiding this comment

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

Approved conditional rendering of helper text.

The conditional rendering of the helper text in the FormSelectInputComponent is correctly implemented. It enhances the user experience by providing additional information without cluttering the interface when not needed.

Consider adding an aria-describedby attribute to the select element to improve accessibility by linking the helper text.

Comment on lines +89 to +101
// Get information from github
useEffect(() => {
if (!beneficiarySubmission || !vault) return;
if (vaultGithubIssues !== undefined) return;

const loadGhIssues = async () => {
setIsLoadingGH(true);
const ghIssues = await getGithubIssuesFromVault(vault);
setVaultGithubIssues(ghIssues);
setIsLoadingGH(false);
};
loadGhIssues();
}, [vault, vaultGithubIssues, beneficiarySubmission]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Well-implemented useEffect hook for fetching GitHub issues.

The useEffect hook is well-structured to fetch GitHub issues based on the necessary conditions. However, consider adding error handling within the loadGhIssues function to manage potential fetch failures gracefully.

Consider adding a try-catch block around the fetch operation to handle errors:

const loadGhIssues = async () => {
  setIsLoadingGH(true);
  try {
    const ghIssues = await getGithubIssuesFromVault(vault);
    setVaultGithubIssues(ghIssues);
  } catch (error) {
    console.error('Failed to fetch GitHub issues:', error);
    // Optionally set an error state here
  }
  setIsLoadingGH(false);
};

Comment on lines +243 to +250
export function getGhIssueFromSubmission(submission?: ISubmittedSubmission, ghIssues?: GithubIssue[]): GithubIssue | undefined {
if (!ghIssues || !submission) return undefined;

const sameTxHash = ghIssues.filter((issue) => issue.txHash === submission.txid);
const sameTitle = sameTxHash.filter((issue) => issue.title === submission.submissionDataStructure?.title);

return sameTitle[0];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Function logic is clear, consider enhancing readability.

The getGhIssueFromSubmission function is correctly implemented with efficient use of filtering. To enhance readability, consider using more descriptive variable names than sameTxHash and sameTitle.

Consider renaming variables for clarity:

- const sameTxHash = ghIssues.filter((issue) => issue.txHash === submission.txid);
- const sameTitle = sameTxHash.filter((issue) => issue.title === submission.submissionDataStructure?.title);
+ const issuesWithMatchingTxHash = ghIssues.filter((issue) => issue.txHash === submission.txid);
+ const matchingIssues = issuesWithMatchingTxHash.filter((issue) => issue.title === submission.submissionDataStructure?.title);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function getGhIssueFromSubmission(submission?: ISubmittedSubmission, ghIssues?: GithubIssue[]): GithubIssue | undefined {
if (!ghIssues || !submission) return undefined;
const sameTxHash = ghIssues.filter((issue) => issue.txHash === submission.txid);
const sameTitle = sameTxHash.filter((issue) => issue.title === submission.submissionDataStructure?.title);
return sameTitle[0];
}
export function getGhIssueFromSubmission(submission?: ISubmittedSubmission, ghIssues?: GithubIssue[]): GithubIssue | undefined {
if (!ghIssues || !submission) return undefined;
const issuesWithMatchingTxHash = ghIssues.filter((issue) => issue.txHash === submission.txid);
const matchingIssues = issuesWithMatchingTxHash.filter((issue) => issue.title === submission.submissionDataStructure?.title);
return matchingIssues[0];
}

Comment on lines +214 to +241
export async function getGithubIssuesFromVault(vault: IVault): Promise<GithubIssue[]> {
const extractTxHashFromBody = (issue: GithubIssue): any => {
// const txHash = issue.body.match(/(0x[a-fA-F0-9]{64})/)?.[0];
const txHash = issue.body.match(/(\*\*Submission hash \(on-chain\):\*\* (.*)\n)/)?.[2] ?? undefined;
return txHash;
};

const mapGithubIssue = (issue: any): GithubIssue => {
return {
id: issue.id,
number: issue.number,
title: issue.title,
createdBy: issue.user.id,
labels: issue.labels.map((label: any) => label.name),
validLabels: issue.labels
.filter((label: any) => severitiesOrder.includes((label.name as string).toLowerCase()))
.map((label: any) => (label.name as string).toLowerCase()),
createdAt: issue.created_at,
body: issue.body,
txHash: extractTxHashFromBody(issue),
};
};

const res = await axiosClient.get(`${BASE_SERVICE_URL}/github-repos/gh-issues/${vault.id}`);
const issues = res.data.githubIssues.map(mapGithubIssue) as GithubIssue[];

return issues.filter((issue) => issue.createdBy === HATS_GITHUB_BOT_ID) ?? [];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Well-implemented function, consider adding error handling.

The getGithubIssuesFromVault function is well-structured and effectively fetches and processes GitHub issues. However, consider adding error handling to manage potential issues with the network request or data processing.

Consider wrapping the axios call in a try-catch block to handle potential errors gracefully:

+ try {
  const res = await axiosClient.get(`${BASE_SERVICE_URL}/github-repos/gh-issues/${vault.id}`);
  const issues = res.data.githubIssues.map(mapGithubIssue) as GithubIssue[];
  return issues.filter((issue) => issue.createdBy === HATS_GITHUB_BOT_ID) ?? [];
+ } catch (error) {
+   console.error('Failed to fetch GitHub issues:', error);
+   return [];
+ }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function getGithubIssuesFromVault(vault: IVault): Promise<GithubIssue[]> {
const extractTxHashFromBody = (issue: GithubIssue): any => {
// const txHash = issue.body.match(/(0x[a-fA-F0-9]{64})/)?.[0];
const txHash = issue.body.match(/(\*\*Submission hash \(on-chain\):\*\* (.*)\n)/)?.[2] ?? undefined;
return txHash;
};
const mapGithubIssue = (issue: any): GithubIssue => {
return {
id: issue.id,
number: issue.number,
title: issue.title,
createdBy: issue.user.id,
labels: issue.labels.map((label: any) => label.name),
validLabels: issue.labels
.filter((label: any) => severitiesOrder.includes((label.name as string).toLowerCase()))
.map((label: any) => (label.name as string).toLowerCase()),
createdAt: issue.created_at,
body: issue.body,
txHash: extractTxHashFromBody(issue),
};
};
const res = await axiosClient.get(`${BASE_SERVICE_URL}/github-repos/gh-issues/${vault.id}`);
const issues = res.data.githubIssues.map(mapGithubIssue) as GithubIssue[];
return issues.filter((issue) => issue.createdBy === HATS_GITHUB_BOT_ID) ?? [];
}
export async function getGithubIssuesFromVault(vault: IVault): Promise<GithubIssue[]> {
const extractTxHashFromBody = (issue: GithubIssue): any => {
// const txHash = issue.body.match(/(0x[a-fA-F0-9]{64})/)?.[0];
const txHash = issue.body.match(/(\*\*Submission hash \(on-chain\):\*\* (.*)\n)/)?.[2] ?? undefined;
return txHash;
};
const mapGithubIssue = (issue: any): GithubIssue => {
return {
id: issue.id,
number: issue.number,
title: issue.title,
createdBy: issue.user.id,
labels: issue.labels.map((label: any) => label.name),
validLabels: issue.labels
.filter((label: any) => severitiesOrder.includes((label.name as string).toLowerCase()))
.map((label: any) => (label.name as string).toLowerCase()),
createdAt: issue.created_at,
body: issue.body,
txHash: extractTxHashFromBody(issue),
};
};
try {
const res = await axiosClient.get(`${BASE_SERVICE_URL}/github-repos/gh-issues/${vault.id}`);
const issues = res.data.githubIssues.map(mapGithubIssue) as GithubIssue[];
return issues.filter((issue) => issue.createdBy === HATS_GITHUB_BOT_ID) ?? [];
} catch (error) {
console.error('Failed to fetch GitHub issues:', error);
return [];
}
}

Comment on lines +104 to +116
// Get information from github
useEffect(() => {
if (!beneficiarySubmission || !vault) return;
if (vaultGithubIssues !== undefined) return;

const loadGhIssues = async () => {
setIsLoadingGH(true);
const ghIssues = await getGithubIssuesFromVault(vault);
setVaultGithubIssues(ghIssues);
setIsLoadingGH(false);
};
loadGhIssues();
}, [vault, vaultGithubIssues, beneficiarySubmission]);
Copy link
Contributor

Choose a reason for hiding this comment

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

useEffect is well-implemented, consider handling empty responses.

The useEffect hook correctly triggers the fetching of GitHub issues based on relevant dependencies. Consider handling cases where ghIssues might be empty to improve user experience.

Consider adding a condition to handle empty responses and provide feedback to the user:

  const ghIssues = await getGithubIssuesFromVault(vault);
+ if (ghIssues.length === 0) {
+   console.log('No GitHub issues found for this vault.');
+ }
  setVaultGithubIssues(ghIssues);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Get information from github
useEffect(() => {
if (!beneficiarySubmission || !vault) return;
if (vaultGithubIssues !== undefined) return;
const loadGhIssues = async () => {
setIsLoadingGH(true);
const ghIssues = await getGithubIssuesFromVault(vault);
setVaultGithubIssues(ghIssues);
setIsLoadingGH(false);
};
loadGhIssues();
}, [vault, vaultGithubIssues, beneficiarySubmission]);
// Get information from github
useEffect(() => {
if (!beneficiarySubmission || !vault) return;
if (vaultGithubIssues !== undefined) return;
const loadGhIssues = async () => {
setIsLoadingGH(true);
const ghIssues = await getGithubIssuesFromVault(vault);
if (ghIssues.length === 0) {
console.log('No GitHub issues found for this vault.');
}
setVaultGithubIssues(ghIssues);
setIsLoadingGH(false);
};
loadGhIssues();
}, [vault, vaultGithubIssues, beneficiarySubmission]);

@fonstack fonstack merged commit 4d28e87 into develop Sep 10, 2024
4 checks passed
@fonstack fonstack deleted the feat/better-payouts branch September 10, 2024 14:31
@coderabbitai coderabbitai bot mentioned this pull request Dec 5, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants