added reject status #173
added reject status #173
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
src/app/[pohid]/[chain]/[request]/ActionBar.tsxOops! Something went wrong! :( ESLint: 9.18.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by WalkthroughThe pull request introduces a new Changes
Sequence DiagramsequenceDiagram
participant Page as Request Page
participant Card as Request Card
participant ActionBar as Action Bar
participant Util as Case Utility
Page->>Card: Pass request details
Card->>Util: Call camelToTitle with rejected status
Util-->>Card: Return formatted status
Card->>ActionBar: Pass rejected status
ActionBar->>ActionBar: Render status based on rejected flag
Poem
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for proof-of-humanity-v2 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/utils/case.ts (1)
30-36: Consider refactoring nested ternaries for readability.While the logic is functionally correct, the layered conditional can be harder to maintain. Consider an if-else chain or a small helper function to improve clarity and maintainability.
src/components/Request/Card.tsx (1)
113-114: Remove or replace direct console log for production.The
console.log(rejected)statement is useful for debugging but might cause noise in production logs. Consider either removing it or wrapping it in a debug-logging mechanism.src/app/[pohid]/[chain]/[request]/ActionBar.tsx (1)
469-469: Passrejectedparameter explicitly.Including the
rejectedargument in disputed status messages is consistent with the newly introduced condition. If no logic changes for disputed statuses, consider whetherrejectedis relevant here or safe to omit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/[pohid]/[chain]/[request]/ActionBar.tsx(5 hunks)src/app/[pohid]/[chain]/[request]/page.tsx(2 hunks)src/components/Request/Card.tsx(2 hunks)src/utils/case.ts(2 hunks)
🔇 Additional comments (6)
src/utils/case.ts (1)
5-5: Ensure optional parameter aligns with naming consistency.
The new rejected parameter is clearly optional, which helps maintain backward compatibility. However, confirm that its naming is consistent with other optional parameters (e.g., revocation vs. rejected) so that future readers can easily understand the logical grouping.
src/components/Request/Card.tsx (1)
128-128: Confirm consistent usage of rejected across the app.
Adding rejected to camelToTitle is consistent with the new status logic. Ensure all calls to this function in the codebase correctly handle or pass the rejected argument.
src/app/[pohid]/[chain]/[request]/ActionBar.tsx (3)
63-63: Interface updated correctly to include rejected.
This aligns with the new logic introduced in other components. Good job maintaining a single source of truth for the props needed to display statuses.
307-307: Use the new parameter consistently.
The addition of rejected to the camelToTitle call ensures a consistent display across components. Confirm calls to camelToTitle match the same signature to avoid potential runtime errors.
506-508: Status text enhancements look correct.
Conditionally displaying “Request was accepted” or “Request was rejected” clarifies the resolved state to users. If you plan to localize or further customize these strings, consider a centralized solution for status messages.
src/app/[pohid]/[chain]/[request]/page.tsx (1)
85-86: Check for edge cases in rejected logic.
The condition request.status.id === "resolved" combined with a missing winning claim is valid. However, if winnerClaim is empty or undefined in edge scenarios, verify that no runtime errors occur.
madhurMongia
force-pushed
the
bug/add-reject-status
branch
from
1d9ceb8 to
f6ec16e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/config/misc.ts (1)
33-33: Use nested ternaries sparingly for better readability.While the logic is correct, consider refactoring to a clear if-else chain or a switch statement for better readability.
-case "resolved": - return revocation ? "removed" : expired ? "expired" : rejected ? "rejected" : "registered"; +case "resolved": { + if (revocation) return "removed"; + if (expired) return "expired"; + if (rejected) return "rejected"; + return "registered"; +}src/components/Request/Grid.tsx (1)
101-103: Simplify the resolved expiration logic.Multiple consecutive
return false;statements may be simplified. For instance, combining them into a single return statement or clarifying conditions can improve readability.- return false; - } - return false; ... - return false; + return false; // Provide a single consolidated exit path if conditions failAlso applies to: 111-111
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/app/[pohid]/[chain]/[request]/ActionBar.tsx(5 hunks)src/app/[pohid]/[chain]/[request]/page.tsx(3 hunks)src/components/Request/Card.tsx(2 hunks)src/components/Request/Grid.tsx(1 hunks)src/config/misc.ts(2 hunks)src/utils/case.ts(2 hunks)tailwind.config.cjs(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/utils/case.ts
- src/components/Request/Card.tsx
- src/app/[pohid]/[chain]/[request]/ActionBar.tsx
🔇 Additional comments (5)
src/config/misc.ts (1)
20-20: Ensure consistent function signature usage in calling code.
You've introduced a new boolean parameter rejected to colorForStatus. Make sure all call sites include and properly handle this parameter.
tailwind.config.cjs (2)
34-34: Good addition of “rejected” color codes.
Explicitly adding rejected status color tokens in both light and dark themes helps keep the design consistent.
Also applies to: 58-58
112-114: Successfully extending safelist classes for "rejected" status.
This ensures that Tailwind doesn’t purge these classes and that the rejected status is visually represented.
src/components/Request/Grid.tsx (1)
93-93: Validate logic for expired requests.
Line 93 extends the logic for "transferring" status. When transferring, the request is considered expired if (creationTime + humanityLifespan) < currentTime. Then line 100 returns false for non-handled cases. Ensure no status inadvertently skips the expiration check.
Also applies to: 100-100
src/app/[pohid]/[chain]/[request]/page.tsx (1)
85-86: Propagate the “rejected” status carefully.
This new logic identifies requests that are resolved but not the winner claim as “rejected.” Verify all usage sites to ensure the UI & business rules align with the intended behavior.
| return false; | ||
| } | ||
| return false; |
There was a problem hiding this comment.
Double-check default expiration outcomes.
Defaulting to false when not matching “transferring” or a recognized scenario may lead to never-expiring requests if unhandled statuses arise. Revisit your acceptance criteria to confirm this logic.
...
} else if (request.status.id === "transferring") {
return ...
}
- return false;
+ // Consider an explicit return for each known status,
+ // or handle an unknown status with a default branch if needed.
return false;Also applies to: 111-111
madhurMongia
force-pushed
the
bug/add-reject-status
branch
from
cc14d44 to
f832d71
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/app/[pohid]/page.tsx (1)
167-173: Simplify the past requests filtering logic.The filtering condition is complex and could be simplified for better readability.
Consider refactoring to:
- ) && - (!( - winnerClaimData.request && - req.index === winnerClaimData.request.index && - chain.id === winnerClaimData.chainId - ) || - !winnerClaimData.request), + ) && ( + !winnerClaimData.request || + req.index !== winnerClaimData.request.index || + chain.id !== winnerClaimData.chainId + ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/app/[pohid]/[chain]/[request]/ActionBar.tsx(6 hunks)src/app/[pohid]/[chain]/[request]/page.tsx(3 hunks)src/app/[pohid]/page.tsx(1 hunks)src/app/globals.css(2 hunks)src/components/Request/Card.tsx(2 hunks)src/components/Request/Grid.tsx(1 hunks)src/config/misc.ts(2 hunks)src/utils/case.ts(2 hunks)tailwind.config.cjs(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- src/components/Request/Grid.tsx
- src/app/[pohid]/[chain]/[request]/page.tsx
- src/config/misc.ts
- src/components/Request/Card.tsx
- tailwind.config.cjs
- src/utils/case.ts
- src/app/[pohid]/[chain]/[request]/ActionBar.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/app/globals.css
[error] 212-212: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 216-216: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 374-374: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 378-378: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
🔇 Additional comments (2)
src/app/globals.css (2)
211-217: LGTM! Text color utilities for rejected status are well implemented.The implementation follows the established pattern and correctly handles both light and dark themes.
🧰 Tools
🪛 Biome (1.9.4)
[error] 212-212: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.(lint/correctness/noUnknownFunction)
[error] 216-216: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.(lint/correctness/noUnknownFunction)
373-379: Verify theme color definitions for rejected status.The background color utilities are well-implemented and follow the established pattern. However, we should ensure that the corresponding color values are properly defined in the theme configuration.
✅ Verification successful
Theme colors for rejected status are properly configured
The rejected status colors are correctly defined in both themes:
- Light theme:
#D90429- Dark theme:
#F25F5F🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the rejected status colors are defined in the theme configuration # Look for theme color definitions in tailwind config rg -A 2 "colors.*status.*rejected" tailwind.config.*Length of output: 56
Script:
#!/bin/bash # First, find all potential theme configuration files echo "=== Theme Configuration Files ===" fd -e js -e cjs -e ts -e mjs "tailwind.config" # Then, examine the complete theme structure in these files echo -e "\n=== Theme Color Definitions ===" rg -A 10 "colors.*=.*{" # Specifically look for status-related color definitions echo -e "\n=== Status Color Definitions ===" rg -A 5 "status.*[:=]"Length of output: 36006
Script:
#!/bin/bash # Get the complete theme configuration section from tailwind.config.cjs echo "=== Complete Theme Configuration ===" rg -A 30 "theme.*=.*{" tailwind.config.cjs # Also search for any "rejected" related configurations echo -e "\n=== Rejected Related Configurations ===" rg "rejected" tailwind.config.cjsLength of output: 492
🧰 Tools
🪛 Biome (1.9.4)
[error] 374-374: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.(lint/correctness/noUnknownFunction)
[error] 378-378: Unexpected unknown function: theme
Use a known function instead.
See MDN web docs for more details.(lint/correctness/noUnknownFunction)
| expired: | ||
| req.status.id === "resolved" | ||
| ? (!req.revocation && | ||
| winnerClaimData?.request?.index === req.index && | ||
| (! humanity[chain.id]!.humanity!.registration || | ||
| Number(humanity[chain.id]!.humanity!.registration?.expirationTime) < Date.now() / 1000)) | ||
| : req.status.id === "transferring" | ||
| ? (Number(req.creationTime) + Number(contractData[chain.id].humanityLifespan) < Date.now() / 1000) | ||
| : false |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve readability of expiration status calculation.
The nested ternary operators and complex conditions make the code hard to read and maintain.
Consider refactoring to a more readable format:
- expired:
- req.status.id === "resolved"
- ? (!req.revocation &&
- winnerClaimData?.request?.index === req.index &&
- (! humanity[chain.id]!.humanity!.registration ||
- Number(humanity[chain.id]!.humanity!.registration?.expirationTime) < Date.now() / 1000))
- : req.status.id === "transferring"
- ? (Number(req.creationTime) + Number(contractData[chain.id].humanityLifespan) < Date.now() / 1000)
- : false
+ expired: calculateExpiration(req, {
+ status: req.status.id,
+ chainId: chain.id,
+ humanity: humanity[chain.id]!.humanity!,
+ contractData: contractData[chain.id],
+ winnerClaimData
+ })And create a helper function:
function calculateExpiration(
req: Request,
{
status,
chainId,
humanity,
contractData,
winnerClaimData
}: CalculateExpirationParams
): boolean {
if (status === "resolved") {
const isWinnerClaim = winnerClaimData?.request?.index === req.index;
const hasNoRevocation = !req.revocation;
const isRegistrationExpired = !humanity.registration ||
Number(humanity.registration.expirationTime) < Date.now() / 1000;
return hasNoRevocation && isWinnerClaim && isRegistrationExpired;
}
if (status === "transferring") {
const expirationTime = Number(req.creationTime) +
Number(contractData.humanityLifespan);
return expirationTime < Date.now() / 1000;
}
return false;
}| })) | ||
| : []), | ||
| ], | ||
| [] as PoHRequest[], | ||
| ), | ||
| ); | ||
|
|
||
| console.log(pastRequests.map(req => req.status.id)); |
There was a problem hiding this comment.
Remove debug console.log statement.
Debug logging should not be committed to production code. If logging is needed for debugging purposes, consider using a proper logging framework with appropriate log levels.
- console.log(pastRequests.map(req => req.status.id));
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style