feat(admin): require expiration for credit grants unless 'Never expires' is checked#15
Conversation
…es' is checked - Add 'Never expires' checkbox option to the credit grant form - Make expiration date OR expiration hours required by default - When 'Never expires' is checked, expiration fields become disabled - Add form validation to enforce the requirement - Clear expiration fields when 'Never expires' is checked This prevents accidentally creating credits without expiration dates, which creates unnecessary financial liabilities. Users who genuinely need non-expiring credits can explicitly check the 'Never expires' box.
![kiloconnect[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| <div> | ||
| <Label className="text-sm font-medium" htmlFor="expiry-hours"> | ||
| Expiry Hours | ||
| Expiry Hours{!neverExpires && !expirationDate ? ' (required)' : ''} |
There was a problem hiding this comment.
WARNING: Required indicator condition is incorrect for "Expiry Hours"
This shows "(required)" whenever expirationDate is empty, even if the user already entered expiryHours. Consider only showing required when neither expiration field is set.
| Expiry Hours{!neverExpires && !expirationDate ? ' (required)' : ''} | |
| Expiry Hours{!neverExpires && !expiryHours && !expirationDate ? ' (required)' : ''} |
| step="0.01" | ||
| id="expiry-hours" | ||
| disabled={!selectedCredit} | ||
| disabled={!selectedCredit || neverExpires} |
There was a problem hiding this comment.
WARNING: Required indicator condition is incorrect for "Expiration Date"
This shows "(required)" whenever expiryHours is empty, even if the user already selected an expirationDate. Consider only showing required when neither expiration field is set.
| disabled={!selectedCredit || neverExpires} | |
| Expiration Date{!neverExpires && !expiryHours && !expirationDate ? ' (required)' : ''} |
| <div> | ||
| <Label className="text-sm font-medium" htmlFor="date"> | ||
| Expiration Date | ||
| Expiration Date{!neverExpires && !expiryHours ? ' (required)' : ''} |
There was a problem hiding this comment.
SUGGESTION: Date input value fallback likely never triggers + format mismatch
expirationDate is a string state initialized to '', so expirationDate ?? ... will always pick expirationDate (empty string is not nullish) and never fall back to the category default. Also, <input type="date"> expects YYYY-MM-DD, while toISOString() produces a full timestamp (YYYY-MM-DDTHH:mm:ss.sssZ).
Consider using something like expirationDate || (selectedCreditCategory?.credit_expiry_date ? selectedCreditCategory.credit_expiry_date.toISOString().slice(0, 10) : '').
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
SUGGESTION
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (1 files)
|
Summary
This PR updates the admin user detail page to require an expiration date or hours when granting credits, unless the user explicitly checks a "Never expires" checkbox.
Changes
Motivation
This prevents accidentally creating credits without expiration dates, which creates unnecessary financial liabilities. Users who genuinely need non-expiring credits can explicitly check the "Never expires" box.
Testing
Built for Brendan by Kilo for Slack