-
Notifications
You must be signed in to change notification settings - Fork 47
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(web): email-updateable-at-info #1729
Conversation
WalkthroughThe changes in this pull request enhance the email update functionality within the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 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 kleros-v2-neo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-university ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
web/src/utils/date.ts (1)
39-48
: Enhance function documentation for completeness.
The JSDoc documentation could be more comprehensive by including all possible return values.
Consider updating the documentation:
/**
* Calculates the time left until a specified date and formats it.
*
* @param {string} isoString - An ISO 8601 formatted date string (e.g., "2024-10-29T09:52:08.580Z").
* @returns {string} A human-readable string indicating the time left until the specified date.
* @example
* console.log(timeLeftUntil("2024-10-29T09:52:08.580Z"));
- * // Outputs: "in x secs", "in x mins", "in x hrs", or "after October 29, 2024"
+ * // Possible outputs:
+ * // - "The date has already passed."
+ * // - "in x secs"
+ * // - "in x mins"
+ * // - "in x hrs"
+ * // - "in x days"
+ * // - "after October 29, 2024"
*/
web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx (2)
41-45
: Consider using consistent spacing values
The margin-bottom: 8px
in StyledInfoCard is inconsistent with the gap: 16px
used in FormContainer. Consider using 16px for consistency.
const StyledInfoCard = styled(InfoCard)`
width: fit-content;
font-size: 14px;
- margin-bottom: 8px;
+ margin-bottom: 16px;
word-wrap: break-word;
`;
Line range hint 47-138
: Consider splitting component responsibilities
The FormContactDetails component handles multiple responsibilities:
- User creation
- Email updates
- Update restriction management
- Form validation
Consider splitting these into smaller, more focused components for better maintainability and testing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx (5 hunks)
- web/src/layout/Header/navbar/Menu/Settings/index.tsx (1 hunks)
- web/src/utils/atlas/fetchUser.ts (1 hunks)
- web/src/utils/date.ts (1 hunks)
🔇 Additional comments (5)
web/src/utils/atlas/fetchUser.ts (2)
9-9
: LGTM! Well-structured type definitions.
The type definitions are well-organized and follow TypeScript best practices:
emailUpdateableAt
is properly typed asstring | null
GetUserResponse
type improves maintainability by referencing theUser
type
Also applies to: 12-14
20-20
: Verify GraphQL schema compatibility.
The addition of emailUpdateableAt
field looks correct, but we should verify it matches the backend schema to prevent runtime errors.
web/src/layout/Header/navbar/Menu/Settings/index.tsx (1)
55-55
: LGTM! Clean styling improvement.
The addition of align-self: center
to StyledTabs
improves UI consistency by properly centering the tabs, matching the centered settings text above.
web/src/utils/date.ts (1)
49-74
:
Several improvements needed for robustness and consistency.
The implementation has a few areas that could be enhanced:
- Missing input validation for invalid date strings
- Time calculations could leverage existing utility functions
- Singular/plural handling needs improvement
- Inconsistent time formatting with other functions in the file
Consider applying these improvements:
export function timeLeftUntil(isoString: string): string {
+ // Validate input
+ const parsedDate = new Date(isoString);
+ if (isNaN(parsedDate.getTime())) {
+ throw new Error('Invalid date string provided');
+ }
+
const targetDate = new Date(isoString);
const now = new Date();
- const timeDifference = targetDate.getTime() - now.getTime();
+ const secondsLeft = Math.floor((targetDate.getTime() - now.getTime()) / 1000);
- if (timeDifference <= 0) {
+ if (secondsLeft <= 0) {
return "The date has already passed.";
}
- const secondsLeft = Math.floor(timeDifference / 1000);
const minutesLeft = Math.floor(secondsLeft / 60);
const hoursLeft = Math.floor(minutesLeft / 60);
const daysLeft = Math.floor(hoursLeft / 24);
if (secondsLeft < 60) {
- return `in ${secondsLeft} secs`;
+ return `in ${secondsLeft} ${secondsLeft === 1 ? 'sec' : 'secs'}`;
} else if (minutesLeft < 60) {
- return `in ${minutesLeft} mins`;
+ return `in ${minutesLeft} ${minutesLeft === 1 ? 'min' : 'mins'}`;
} else if (hoursLeft < 24) {
- return `in ${hoursLeft} hrs`;
+ return `in ${hoursLeft} ${hoursLeft === 1 ? 'hr' : 'hrs'}`;
} else if (daysLeft < 2) {
- return `in ${daysLeft} days`;
+ return `in ${daysLeft} day`;
} else {
- const options: Intl.DateTimeFormatOptions = { year: "numeric", month: "long", day: "numeric" };
- return `after ${targetDate.toLocaleDateString("en-US", options)}`;
+ return `after ${formatDate(Math.floor(targetDate.getTime() / 1000))}`;
}
}
This implementation:
- Validates the input date string
- Uses proper singular/plural forms
- Leverages the existing
formatDate
function for consistency - Improves the readability of time calculations
Let's verify the usage context:
web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx (1)
56-58
: 🛠️ Refactor suggestion
Verify type safety and date handling
While the logic is functional, consider these improvements:
- Add type safety for emailUpdateableAt
- Use consistent date handling to avoid timezone issues
Consider this more robust implementation:
const isEmailUpdateable = user?.email
- ? !isUndefined(user?.emailUpdateableAt) && new Date(user.emailUpdateableAt).getTime() < new Date().getTime()
+ ? !isUndefined(user?.emailUpdateableAt) &&
+ Date.now() > new Date(user.emailUpdateableAt).getTime()
: true;
web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx
Show resolved
Hide resolved
web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx
Show resolved
Hide resolved
web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
web/src/utils/date.ts (1)
39-48
: LGTM! Well-documented function signature.
The JSDoc documentation is thorough and clear. The TypeScript types are properly defined.
Consider adding more examples to cover edge cases (e.g., past dates, different time ranges).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- web/src/utils/date.ts (1 hunks)
🔇 Additional comments (1)
web/src/utils/date.ts (1)
49-61
: 🛠️ Refactor suggestion
Consider adding input validation and time zone handling.
While the core logic is sound, there are a few potential improvements:
Consider these improvements:
- Add input validation to handle invalid ISO strings:
export function timeLeftUntil(isoString: string): string {
+ if (!isoString || isNaN(Date.parse(isoString))) {
+ throw new Error('Invalid date format. Expected ISO 8601 string.');
+ }
const targetDate = new Date(isoString);
- Consider using UTC methods to ensure consistent time zone handling:
- const targetDate = new Date(isoString);
- const now = new Date();
+ const targetDate = new Date(isoString).getTime();
+ const now = Date.now();
- const timeDifference = targetDate.getTime() - now.getTime();
+ const timeDifference = targetDate - now;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Code Climate has analyzed commit 3869072 and detected 5 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
|
PR-Codex overview
This PR introduces enhancements to the user settings functionality, including the ability to determine when a user's email can be updated and a new utility function to calculate the time left until a specified date.
Detailed summary
align-self: center
to theSettings
component.User
type to includeemailUpdateableAt
.timeLeftUntil
function for calculating remaining time until a date.timeLeftUntil
inFormContactDetails
to show when email can be updated.InfoCard
to display email update information.Summary by CodeRabbit
New Features
Bug Fixes
Style