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

Feat/bonus points payments #785

Merged
merged 8 commits into from
Dec 12, 2024
Merged

Feat/bonus points payments #785

merged 8 commits into from
Dec 12, 2024

Conversation

fonstack
Copy link
Member

@fonstack fonstack commented Dec 5, 2024

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced handling of GitHub pull requests alongside issues in various components.
    • New severity option "Complementary" added for split payouts based on project metadata.
    • Improved user feedback with new submission status messages.
    • Added support for linked pull requests in submission components.
  • Bug Fixes

    • Adjusted logic for filtering submissions to include both GitHub issues and pull requests.
  • Documentation

    • Updated language files to include new keys for submission statuses.
  • Chores

    • Refactored hooks to support fetching both GitHub issues and pull requests.

Copy link
Contributor

coderabbitai bot commented Dec 5, 2024

Walkthrough

The changes in this pull request involve enhancements to the handling of GitHub issues and pull requests across multiple components and files. Key updates include the introduction of the GithubPR type, modifications to existing types to accommodate both issues and pull requests, and adjustments to the logic in various components to improve data handling and user feedback. The updates also extend the functionality of the payout calculation logic and enhance user interaction with submission statuses.

Changes

File Change Summary
packages/shared/src/types/payout.ts Added GithubPR type with multiple properties; enhanced GithubIssue type with new optional fields; updated ghIssue property in interfaces to accept both GithubIssue and GithubPR.
packages/web/src/languages/en.json Added new keys for user feedback regarding submission statuses.
packages/web/src/pages/CommitteeTools/PayoutsTool/PayoutFormPage/PayoutFormPage.tsx Added "Complementary" severity option based on project metadata; adjusted logic for populating severities for split payouts.
packages/web/src/pages/CommitteeTools/PayoutsTool/components/PayoutAllocation/SplitPayoutBeneficiaryForm.tsx Enhanced component to handle GithubPR type; added state for GitHub pull requests and modified loading logic.
packages/web/src/pages/CommitteeTools/PayoutsTool/utils/autocalculateMultiPayout.ts Introduced BONUS_POINTS_CONSTRAINTS constant; updated payout calculation logic to handle "complementary" severity.
packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionsListPage/SubmissionCard.tsx Updated ghIssue prop to accept GithubPR; modified rendering logic based on ghIssue type.
packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionsListPage/SubmissionsListPage.tsx Integrated GitHub pull requests into submission filtering and loading logic; updated payout creation logic.
packages/web/src/pages/CommitteeTools/SubmissionsTool/submissionsService.api.ts Added functions for retrieving GitHub pull requests and modified existing submission data extraction logic.
packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultSubmissionsSection/PublicSubmissionCard/PublicSubmissionCard.tsx Introduced linkedPRs prop to handle linked pull requests in rendering logic.
packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultSubmissionsSection/PublicSubmissionCard/components/SplitPointsActions.tsx Added linkedPRs prop to manage actions based on pull request status.
packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultSubmissionsSection/VaultSubmissionsSection.tsx Replaced useSavedSubmissions with useGHIssues and useGHPRs for data handling.
packages/web/src/pages/Honeypots/VaultDetailsPage/VaultDetailsPage.tsx Updated to use useGHIssues for fetching submissions.
packages/web/src/pages/Honeypots/VaultDetailsPage/hooks.ts Renamed useSavedSubmissions to useGHIssues; added useGHPRs for fetching pull requests.

Possibly related PRs

Suggested reviewers

  • shayzluf

🐰 In the meadow, changes bloom,
With GitHub links, we clear the gloom.
Issues and PRs, hand in hand,
Enhancing payouts across the land.
Hop along, let’s celebrate,
For every change, we elevate! 🌼


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>, please review it.
    • 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @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 generate docstrings to generate docstrings for this PR. (Experiment)
  • @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
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: 10

🧹 Outside diff range and nitpick comments (6)
packages/web/src/pages/Honeypots/VaultDetailsPage/VaultDetailsPage.tsx (1)

The submissions section visibility should consider both issues and PRs

The current implementation in VaultDetailsPage.tsx only checks for savedSubmissions?.length (GitHub issues) while the VaultSubmissionsSection component actually fetches and handles both issues and PRs through useGHIssues and useGHPRs. The visibility condition should be updated to consider both types of submissions.

  • packages/web/src/pages/Honeypots/VaultDetailsPage/VaultDetailsPage.tsx: Update the submissions section visibility check to include both savedSubmissions?.length and pullRequests?.length
🔗 Analysis chain

Line range hint 95-102: Review submissions section visibility logic

The current filtering logic only checks for the presence of issues (savedSubmissions.length). If PRs are also relevant for showing the submissions section, consider updating the condition to account for both.


From the search results, I can see that the VaultSubmissionsSection component uses both GitHub issues and PRs through the hooks useGHIssues and useGHPRs. Let me verify the PR-related logic:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check how submissions section visibility is handled in VaultSubmissionsSection
rg -A 5 "VaultSubmissionsSection" --type ts --type tsx

Length of output: 82


Script:

#!/bin/bash
# Search for submissions-related components and logic
rg -g '*.{ts,tsx}' -A 5 "VaultSubmissionsSection|submissions.*section"

Length of output: 5502


Script:

#!/bin/bash
# Look for submission types or interfaces
ast-grep --pattern 'type $_ = { $$$submissions$$$}'
ast-grep --pattern 'interface $_ { $$$submissions$$$}'

Length of output: 110


Script:

#!/bin/bash
# Search for submission-related state or props
rg -g '*.{ts,tsx}' "savedSubmissions|pullRequests" -A 3

Length of output: 5239


Script:

#!/bin/bash
# Check the implementation of useGHPRs hook
rg -g '*.{ts,tsx}' "useGHPRs" -B 5 -A 10

Length of output: 6025


Script:

#!/bin/bash
# Look for PR-related filtering logic
ast-grep --pattern 'if ($$$PR$$$) { $$$ }'

Length of output: 44

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

254-258: Rename variable 'issues' to 'prs' for clarity

In the getGithubPRsFromVault function, the variable issues refers to an array of pull requests. Renaming it to prs will improve code readability and reduce confusion.

Apply this diff to update the variable name:

- const issues = res.data.githubPRs.map(mapGithubPRs) as GithubPR[];
+ const prs = res.data.githubPRs.map(mapGithubPRs) as GithubPR[];

- return issues.filter((issue) => issue.createdBy === HATS_GITHUB_BOT_ID) ?? [];
+ return prs.filter((pr) => pr.createdBy === HATS_GITHUB_BOT_ID) ?? [];
packages/shared/src/types/payout.ts (1)

71-71: Consider using an enum for submission status.

The string literal type for bonusSubmissionStatus could be replaced with an enum to ensure type safety and maintainability.

+export enum BonusSubmissionStatus {
+  COMPLETE = "COMPLETE",
+  INCOMPLETE = "INCOMPLETE",
+  PENDING = "PENDING"
+}

 export type GithubPR = {
   // ... other fields
-  bonusSubmissionStatus: "COMPLETE" | "INCOMPLETE" | "PENDING";
+  bonusSubmissionStatus: BonusSubmissionStatus;
   // ... other fields
 };
packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultSubmissionsSection/PublicSubmissionCard/components/SplitPointsActions.tsx (1)

77-80: Improve readability of status check logic.

The nested conditions could be simplified for better maintainability.

-    if (!isOpenToSubmissions) {
-      const isCompleted = linkedPRs.some((pr) => pr.bonusSubmissionStatus === "COMPLETE");
-      return { can: false, reason: isCompleted ? t("issueAlreadyHaveValidSubmission") : t("oneSubmissionIsBeingReviewed") };
-    }
+    if (isOpenToSubmissions) return { can: true };
+    
+    const hasCompletedSubmission = linkedPRs.some((pr) => pr.bonusSubmissionStatus === "COMPLETE");
+    return { 
+      can: false, 
+      reason: hasCompletedSubmission 
+        ? t("issueAlreadyHaveValidSubmission") 
+        : t("oneSubmissionIsBeingReviewed") 
+    };
packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionsListPage/SubmissionCard.tsx (2)

85-97: Simplify status text logic.

The nested conditional logic for status text could be simplified using a map or switch statement.

+                  const getStatusText = (labels: string[], linkedSeverity?: string) => {
+                    const statusMap = {
+                      'both': `COMPLETE (fix & test) -> ${linkedSeverity}`,
+                      'fix': `COMPLETE (fix) -> ${linkedSeverity}`,
+                      'test': `COMPLETE (test) -> ${linkedSeverity}`,
+                    };
+                    
+                    if (labels.includes('complete-fix') && labels.includes('complete-test')) return statusMap.both;
+                    if (labels.includes('complete-fix')) return statusMap.fix;
+                    if (labels.includes('complete-test')) return statusMap.test;
+                    
+                    return (ghIssue as GithubPR).bonusSubmissionStatus;
+                  };
+
-                  const getText = () => {
-                    const labels = (ghIssue as GithubPR).labels;
-                    if (labels.includes("complete-fix") && labels.includes("complete-test")) {
-                      console.log(ghIssue);
-                      return `COMPLETE (fix & test) -> ${(ghIssue as GithubPR).linkedIssue?.severity}`;
-                    } else if (labels.includes("complete-fix")) {
-                      return `COMPLETE (fix) -> ${(ghIssue as GithubPR).linkedIssue?.severity}`;
-                    } else if (labels.includes("complete-test")) {
-                      return `COMPLETE (test) -> ${(ghIssue as GithubPR).linkedIssue?.severity}`;
-                    }
-
-                    return (ghIssue as GithubPR).bonusSubmissionStatus;
-                  };

112-121: Optimize repeated type assertions.

Multiple type assertions to GithubIssue could be replaced with a single destructured assignment.

-                  {ghIssue && (ghIssue as GithubIssue)?.validLabels?.length > 0 && (
+                  {ghIssue && 'validLabels' in ghIssue && ghIssue.validLabels?.length > 0 && (
                     <>
                       <span>{t("labeledAs")}:</span>
                       <Pill
                         textColor={severityColors[severityIndex ?? 0]}
                         isSeverity
-                        text={parseSeverityName((ghIssue as GithubIssue).validLabels[0])}
+                        text={parseSeverityName(ghIssue.validLabels[0])}
                       />
                     </>
                   )}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between ea42172 and 04f88e2.

📒 Files selected for processing (13)
  • packages/shared/src/types/payout.ts (3 hunks)
  • packages/web/src/languages/en.json (1 hunks)
  • packages/web/src/pages/CommitteeTools/PayoutsTool/PayoutFormPage/PayoutFormPage.tsx (1 hunks)
  • packages/web/src/pages/CommitteeTools/PayoutsTool/components/PayoutAllocation/SplitPayoutAllocation/components/SplitPayoutBeneficiaryForm.tsx (6 hunks)
  • packages/web/src/pages/CommitteeTools/PayoutsTool/utils/autocalculateMultiPayout.ts (2 hunks)
  • packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionsListPage/SubmissionCard.tsx (4 hunks)
  • packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionsListPage/SubmissionsListPage.tsx (11 hunks)
  • packages/web/src/pages/CommitteeTools/SubmissionsTool/submissionsService.api.ts (4 hunks)
  • packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultSubmissionsSection/PublicSubmissionCard/PublicSubmissionCard.tsx (3 hunks)
  • packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultSubmissionsSection/PublicSubmissionCard/components/SplitPointsActions.tsx (3 hunks)
  • packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultSubmissionsSection/VaultSubmissionsSection.tsx (4 hunks)
  • packages/web/src/pages/Honeypots/VaultDetailsPage/VaultDetailsPage.tsx (2 hunks)
  • packages/web/src/pages/Honeypots/VaultDetailsPage/hooks.ts (2 hunks)
👮 Files not reviewed due to content moderation or server errors (3)
  • packages/web/src/pages/CommitteeTools/PayoutsTool/PayoutFormPage/PayoutFormPage.tsx
  • packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionsListPage/SubmissionsListPage.tsx
  • packages/web/src/languages/en.json
🔇 Additional comments (11)
packages/web/src/pages/CommitteeTools/PayoutsTool/components/PayoutAllocation/SplitPayoutAllocation/components/SplitPayoutBeneficiaryForm.tsx (1)

1-1: LGTM: Import changes are well-organized

The new imports for GitHub PR functionality are properly grouped and necessary for the feature implementation.

Also applies to: 11-16

packages/web/src/pages/Honeypots/VaultDetailsPage/VaultDetailsPage.tsx (1)

15-15: Verify if useGHPRs import is needed

The AI summary mentions that both useGHIssues and useGHPRs are part of this change, but only useGHIssues is imported. If PR functionality is needed, consider adding the useGHPRs import.

packages/web/src/pages/CommitteeTools/PayoutsTool/utils/autocalculateMultiPayout.ts (1)

4-7: Definition of BONUS_POINTS_CONSTRAINTS is correct.

The bonus point constraints are correctly defined with appropriate values for "fix" and "test" categories.

packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultSubmissionsSection/PublicSubmissionCard/PublicSubmissionCard.tsx (4)

1-1: Update import statement to include 'GithubPR'

The import statement correctly includes GithubPR, which is necessary for handling linked pull requests.


13-13: Add optional 'linkedPRs' prop to 'PublicSubmissionCardProps'

The optional linkedPRs prop enhances the component to handle linked pull requests.


16-16: Set default value for 'linkedPRs' parameter

Initializing linkedPRs with an empty array ensures that the component functions correctly even if no PRs are passed.


47-47: Pass 'linkedPRs' prop to 'SplitPointsActions' component

Passing linkedPRs allows SplitPointsActions to utilize linked PRs for its functionality.

packages/web/src/pages/Honeypots/VaultDetailsPage/hooks.ts (2)

Line range hint 12-19: Rename 'useSavedSubmissions' to 'useGHIssues' for clarity

Renaming the hook to useGHIssues more accurately reflects its purpose of fetching GitHub issues.


21-29: Add 'useGHPRs' hook to fetch GitHub pull requests

The new useGHPRs hook efficiently retrieves GitHub PRs associated with the vault.

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

52-55: LGTM: New fields enhance issue tracking capabilities.

The additional fields provide necessary metadata for tracking issue lifecycle and severity.


62-73: Consider adding validation for linked issue references.

While the type structure is good, there's a potential for inconsistency between linkedIssueNumber and linkedIssue.

Let's verify the relationship between these fields:

Copy link

cloudflare-workers-and-pages bot commented Dec 10, 2024

Deploying dapp with  Cloudflare Pages  Cloudflare Pages

Latest commit: c93f3bd
Status: ✅  Deploy successful!
Preview URL: https://44e09ae1.dapp-a9y.pages.dev
Branch Preview URL: https://feat-bonus-points-payments.dapp-a9y.pages.dev

View logs

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: 2

🧹 Outside diff range and nitpick comments (5)
packages/web/src/pages/CommitteeTools/PayoutsTool/components/PayoutAllocation/SplitPayoutAllocation/components/SplitPayoutBeneficiaryForm.tsx (1)

168-171: Improve type safety and readability of GitHub reference handling

While the current implementation works, it can be improved for better maintainability and type safety.

Consider this refactor:

 const selectedSubmission = isPayoutCreated
   ? beneficiaries[index]?.decryptedSubmission ?? beneficiarySubmission!
   : beneficiarySubmission!;

+const getGithubReference = (submission: typeof selectedSubmission) => {
+  if (!submission) return null;
+  return (
+    getGhIssueFromSubmission(submission, vaultGithubIssues) ||
+    getGhPRFromSubmission(submission, vaultGithubPRs, vaultGithubIssues)
+  );
+};

 <SubmissionCard
   inPayout
   submission={selectedSubmission}
-  ghIssue={
-    getGhIssueFromSubmission(selectedSubmission, vaultGithubIssues) ||
-    getGhPRFromSubmission(selectedSubmission, vaultGithubPRs, vaultGithubIssues)
-  }
+  ghIssue={getGithubReference(selectedSubmission)}
 />

Benefits:

  1. Encapsulates GitHub reference logic in a reusable function
  2. Improves readability by reducing inline complexity
  3. Adds null check for better type safety

Also applies to: 185-188

packages/web/src/pages/CommitteeTools/PayoutsTool/utils/autocalculateMultiPayout.ts (2)

4-7: Add JSDoc documentation for BONUS_POINTS_CONSTRAINTS

Consider adding documentation to explain the purpose and usage of these bonus point percentages.

+/**
+ * Defines bonus point multipliers for different contribution types
+ * @property {number} fix - 10% bonus for providing a fix
+ * @property {number} test - 5% bonus for providing tests
+ */
 const BONUS_POINTS_CONSTRAINTS = {
   fix: 0.1, // 10%
   test: 0.05, // 5%
 };

113-119: Extract bonus points calculation to a separate function

The bonus points calculation logic could be more maintainable if extracted to a dedicated function.

+function calculateBonusPoints(ghIssue: GithubPR, mainIssuePoints: string): number {
+  let totalMultiplier = 0;
+  
+  if (ghIssue.labels?.includes("complete-fix")) totalMultiplier += BONUS_POINTS_CONSTRAINTS.fix;
+  if (ghIssue.labels?.includes("complete-test")) totalMultiplier += BONUS_POINTS_CONSTRAINTS.test;
+  
+  return totalMultiplier * +mainIssuePoints;
+}

-      let totalMultiplier = 0;
-      if (beneficiary.ghIssue?.labels?.includes("complete-fix")) totalMultiplier += BONUS_POINTS_CONSTRAINTS.fix;
-      if (beneficiary.ghIssue?.labels?.includes("complete-test")) totalMultiplier += BONUS_POINTS_CONSTRAINTS.test;
-      const complementaryPoints = totalMultiplier * +mainIssuePoints;
+      const complementaryPoints = calculateBonusPoints(beneficiary.ghIssue as GithubPR, mainIssuePoints);
packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionsListPage/SubmissionCard.tsx (2)

40-42: Move type guard to shared utilities

The isGithubPR type guard could be useful in other components. Consider moving it to a shared utility file.

// Create new file: src/utils/github.utils.ts
+export const isGithubPR = (issue: GithubIssue | GithubPR): issue is GithubPR => {
+  return "linkedIssueNumber" in issue;
+};

// In SubmissionCard.tsx
+import { isGithubPR } from 'utils/github.utils';
-const isGithubPR = (issue: GithubIssue | GithubPR): issue is GithubPR => {
-  return "linkedIssueNumber" in issue;
-};

105-109: Extract status colors to constants

The color values should be extracted to named constants for better maintainability.

+const STATUS_COLORS = {
+  COMPLETE: '#3ee136',
+  IN_PROGRESS: '#bab441'
+} as const;

 <Pill
-  textColor={(ghIssue as GithubPR).bonusSubmissionStatus === "COMPLETE" ? "#3ee136" : "#bab441"}
+  textColor={isGithubPR(ghIssue) && ghIssue.bonusSubmissionStatus === "COMPLETE" 
+    ? STATUS_COLORS.COMPLETE 
+    : STATUS_COLORS.IN_PROGRESS}
   isSeverity
   text={getText()}
 />
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 04f88e2 and c93f3bd.

📒 Files selected for processing (3)
  • packages/web/src/pages/CommitteeTools/PayoutsTool/components/PayoutAllocation/SplitPayoutAllocation/components/SplitPayoutBeneficiaryForm.tsx (6 hunks)
  • packages/web/src/pages/CommitteeTools/PayoutsTool/utils/autocalculateMultiPayout.ts (2 hunks)
  • packages/web/src/pages/CommitteeTools/SubmissionsTool/SubmissionsListPage/SubmissionCard.tsx (4 hunks)
🔇 Additional comments (2)
packages/web/src/pages/CommitteeTools/PayoutsTool/components/PayoutAllocation/SplitPayoutAllocation/components/SplitPayoutBeneficiaryForm.tsx (2)

123-127: ⚠️ Potential issue

Fix PR loading state handling

The PR loading logic should follow the same pattern as issue loading to prevent race conditions and duplicate requests.

Apply this fix:

 const loadGhPRs = async () => {
+  if (vaultGithubPRs !== undefined || isLoadingGH) return;
+  setIsLoadingGH(true);
   const ghPRs = await getGithubPRsFromVault(vault);
   setVaultGithubPRs(ghPRs);
+  setIsLoadingGH(false);
 };
-loadGhPRs();
+if (!beneficiarySubmission || !vault) return;
+loadGhPRs();

185-188: Verify PR and Issue handling behavior

Let's verify the behavior when both PR and Issue references exist for a submission.

✅ Verification successful

Let me generate additional verification scripts to check the implementation details of the functions handling PR and Issue data:


Based on the code analysis, I can now provide a final response:

PR and Issue handling is correctly implemented with clear precedence

The code shows that:

  1. getGhIssueFromSubmission is checked first, followed by getGhPRFromSubmission in the OR condition
  2. PRs are linked to Issues through linkedIssueNumber and maintain their relationship
  3. When both exist, the Issue data is preserved while the PR adds complementary information (fix/test status)

The implementation ensures proper handling of both standalone Issues and PRs linked to Issues, with Issues taking precedence in the display hierarchy.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for submissions with both PR and Issue references

# Search for submissions with both PR and Issue references
ast-grep --pattern 'getGhIssueFromSubmission($submission, $_) || getGhPRFromSubmission($submission, $_, $_)'

# Check for any error handling or precedence documentation
rg -i "priority|precedence|order" -g "*.{md,ts,tsx}" -C 2

Length of output: 65916


Script:

#!/bin/bash
# Check implementation of getGhIssueFromSubmission and getGhPRFromSubmission functions
ast-grep --pattern 'function getGhIssueFromSubmission($submission, $_) {
  $$$
}'

ast-grep --pattern 'function getGhPRFromSubmission($submission, $_, $_) {
  $$$
}'

# Check for any comments or documentation about precedence
rg -i "issue.*pr|pr.*issue" -g "*.{ts,tsx}" -C 2

Length of output: 37204

Comment on lines +106 to +112
if (beneficiary.severity.toLowerCase() === "complementary") {
const mainIssueSev = (beneficiary.ghIssue as GithubPR).linkedIssue?.severity;
const mainIssueSevInfo = constraints.find(
(constraint) => constraint.severity.toLowerCase() === mainIssueSev?.toLowerCase()
);
const mainIssuePoints = mainIssueSevInfo?.points ? `${mainIssueSevInfo.points.value.first}` : "1";

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add type guard and null checks for mainIssueSev

The current implementation has potential type safety issues. Consider adding proper type guards and null checks.

-      const mainIssueSev = (beneficiary.ghIssue as GithubPR).linkedIssue?.severity;
+      if (!isGithubPR(beneficiary.ghIssue)) {
+        throw new Error('Expected GitHub PR for complementary submission');
+      }
+      const mainIssueSev = beneficiary.ghIssue.linkedIssue?.severity;
+      if (!mainIssueSev) {
+        throw new Error('Linked issue severity is required for complementary submission');
+      }

Where isGithubPR is defined as:

function isGithubPR(issue: any): issue is GithubPR {
  return issue && 'linkedIssue' in issue;
}

Comment on lines +89 to +100
const getText = () => {
const labels = (ghIssue as GithubPR).labels;
if (labels.includes("complete-fix") && labels.includes("complete-test")) {
return `COMPLETE (fix & test) -> ${(ghIssue as GithubPR).linkedIssue?.severity}`;
} else if (labels.includes("complete-fix")) {
return `COMPLETE (fix) -> ${(ghIssue as GithubPR).linkedIssue?.severity}`;
} else if (labels.includes("complete-test")) {
return `COMPLETE (test) -> ${(ghIssue as GithubPR).linkedIssue?.severity}`;
}

return (ghIssue as GithubPR).bonusSubmissionStatus;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Simplify status text generation and remove type assertions

The status text generation logic can be improved in several ways:

  1. Remove type assertions using the type guard
  2. Extract string literals to constants
  3. Simplify the logic using a map or switch statement
+const LABEL_TYPES = {
+  FIX: 'complete-fix',
+  TEST: 'complete-test'
+} as const;
+
+const STATUS_MESSAGES = {
+  BOTH: (severity: string) => `COMPLETE (fix & test) -> ${severity}`,
+  FIX: (severity: string) => `COMPLETE (fix) -> ${severity}`,
+  TEST: (severity: string) => `COMPLETE (test) -> ${severity}`
+} as const;
+
 const getText = () => {
-  const labels = (ghIssue as GithubPR).labels;
+  if (!isGithubPR(ghIssue)) return '';
+  const { labels, linkedIssue, bonusSubmissionStatus } = ghIssue;
+  const severity = linkedIssue?.severity;
+  
+  if (!severity) return bonusSubmissionStatus;
   
-  if (labels.includes("complete-fix") && labels.includes("complete-test")) {
-    return `COMPLETE (fix & test) -> ${(ghIssue as GithubPR).linkedIssue?.severity}`;
-  } else if (labels.includes("complete-fix")) {
-    return `COMPLETE (fix) -> ${(ghIssue as GithubPR).linkedIssue?.severity}`;
-  } else if (labels.includes("complete-test")) {
-    return `COMPLETE (test) -> ${(ghIssue as GithubPR).linkedIssue?.severity}`;
-  }
+  const hasFix = labels.includes(LABEL_TYPES.FIX);
+  const hasTest = labels.includes(LABEL_TYPES.TEST);
   
-  return (ghIssue as GithubPR).bonusSubmissionStatus;
+  if (hasFix && hasTest) return STATUS_MESSAGES.BOTH(severity);
+  if (hasFix) return STATUS_MESSAGES.FIX(severity);
+  if (hasTest) return STATUS_MESSAGES.TEST(severity);
+  
+  return bonusSubmissionStatus;
 };
📝 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
const getText = () => {
const labels = (ghIssue as GithubPR).labels;
if (labels.includes("complete-fix") && labels.includes("complete-test")) {
return `COMPLETE (fix & test) -> ${(ghIssue as GithubPR).linkedIssue?.severity}`;
} else if (labels.includes("complete-fix")) {
return `COMPLETE (fix) -> ${(ghIssue as GithubPR).linkedIssue?.severity}`;
} else if (labels.includes("complete-test")) {
return `COMPLETE (test) -> ${(ghIssue as GithubPR).linkedIssue?.severity}`;
}
return (ghIssue as GithubPR).bonusSubmissionStatus;
};
const LABEL_TYPES = {
FIX: 'complete-fix',
TEST: 'complete-test'
} as const;
const STATUS_MESSAGES = {
BOTH: (severity: string) => `COMPLETE (fix & test) -> ${severity}`,
FIX: (severity: string) => `COMPLETE (fix) -> ${severity}`,
TEST: (severity: string) => `COMPLETE (test) -> ${severity}`
} as const;
const getText = () => {
if (!isGithubPR(ghIssue)) return '';
const { labels, linkedIssue, bonusSubmissionStatus } = ghIssue;
const severity = linkedIssue?.severity;
if (!severity) return bonusSubmissionStatus;
const hasFix = labels.includes(LABEL_TYPES.FIX);
const hasTest = labels.includes(LABEL_TYPES.TEST);
if (hasFix && hasTest) return STATUS_MESSAGES.BOTH(severity);
if (hasFix) return STATUS_MESSAGES.FIX(severity);
if (hasTest) return STATUS_MESSAGES.TEST(severity);
return bonusSubmissionStatus;
};

@shayzluf shayzluf merged commit f098432 into develop Dec 12, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants