update workflow date formatting to use IST timezone and 24-hour format#1132
update workflow date formatting to use IST timezone and 24-hour format#1132abhishekkumar004-lgtm wants to merge 1 commit intomainfrom
Conversation
abhishekkumar004-lgtm
requested review from
devesh-juspay,
junaid-shirur,
kalpadhwaryu,
shivamashtikar and
zereraz
as code owners
Summary of ChangesHello @abhishekkumar004-lgtm, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request standardizes the date and time displays throughout the application's workflow features. The changes ensure that all timestamps, including those for workflow creation, execution, and completion, consistently reflect the Indian Standard Time (IST) and are presented in a clear 24-hour format. This improves clarity and consistency for users, particularly those operating in or concerned with the IST timezone. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates date formatting to use the IST timezone and a 24-hour format. The changes are applied consistently across several frontend and backend files. However, this introduces significant code duplication for the date formatting options and an inconsistency in the locale used (en-US vs. en-IN). My feedback focuses on refactoring this duplicated logic into a shared utility or constant to improve maintainability and ensure a consistent date format across the application.
| // Create form data matching the curl command format | ||
| const formData: Record<string, any> = { | ||
| name: `${currentTemplate?.name || "Workflow"} - ${new Date().toLocaleString()}`, | ||
| name: `${currentTemplate?.name || "Workflow"} - ${new Date().toLocaleString("en-US", { timeZone: "Asia/Kolkata", hour12: false, year: 'numeric', month: '2-digit', day: '2-digit', hour: '2-digit', minute: '2-digit', second: '2-digit' })}`, |
There was a problem hiding this comment.
To improve maintainability and ensure consistency, consider creating a shared utility for date formatting. The current implementation hardcodes the formatting options, which are duplicated in several other files. A shared utility would centralize this logic.
Additionally, this file uses the en-US locale, while others use en-IN. Standardizing on a single locale (e.g., en-IN for the 'Asia/Kolkata' timezone) would prevent inconsistencies in date representation (DD/MM/YYYY vs. MM/DD/YYYY).
| return date.toLocaleString("en-US", { | ||
| timeZone: "Asia/Kolkata", | ||
| hour12: false, | ||
| year: 'numeric', | ||
| month: '2-digit', | ||
| day: '2-digit', | ||
| hour: '2-digit', | ||
| minute: '2-digit', | ||
| second: '2-digit' | ||
| }) |
There was a problem hiding this comment.
While combining toLocaleDateString and toLocaleTimeString is a good refactor, it introduces a new issue of duplicated date formatting options across the codebase. It's best to define these options in a shared constant or utility function to avoid inconsistencies and make future changes easier.
Also, please note the locale inconsistency: en-US is used here, whereas other parts of the app use en-IN for the same timezone.
| // Create form data matching the curl command format | ||
| const formData: Record<string, any> = { | ||
| name: `${workflowName} - ${new Date().toLocaleString()}`, | ||
| name: `${workflowName} - ${new Date().toLocaleString("en-IN", { timeZone: "Asia/Kolkata", hour12: false, year: 'numeric', month: '2-digit', day: '2-digit', hour: '2-digit', minute: '2-digit', second: '2-digit' })}`, |
There was a problem hiding this comment.
The date formatting options are duplicated from other files. To promote code reuse and prevent inconsistencies, please extract this logic into a shared utility function or constant. This file correctly uses the en-IN locale, which seems more appropriate for the IST timezone, but this highlights an inconsistency with other files that use en-US.
| </span> | ||
| <span className="text-xs text-gray-900"> | ||
| {new Date(step.completedAt).toLocaleString()} | ||
| {new Date(step.completedAt).toLocaleString("en-US", { timeZone: "Asia/Kolkata", hour12: false, year: 'numeric', month: '2-digit', day: '2-digit', hour: '2-digit', minute: '2-digit', second: '2-digit' })} |
There was a problem hiding this comment.
The date formatting options are hardcoded here and duplicated across multiple files. This should be refactored into a shared utility to improve maintainability.
This change also introduces an inconsistency in locale usage (en-US here vs. en-IN elsewhere). A shared utility would help enforce a standard.
| name: | ||
| requestData.name || | ||
| `${template[0].name} - ${new Date().toLocaleDateString()}`, | ||
| `${template[0].name} - ${new Date().toLocaleString("en-IN", { timeZone: "Asia/Kolkata", hour12: false, year: 'numeric', month: '2-digit', day: '2-digit', hour: '2-digit', minute: '2-digit', second: '2-digit' })}`, |
There was a problem hiding this comment.
The date formatting options are hardcoded and duplicated in multiple places within this file (lines 743, 1916) and across the project. This is a maintenance concern. Please define these options in a single, shared constant or utility function.
For example:
const IST_DATE_FORMAT_OPTIONS: Intl.DateTimeFormatOptions = {
timeZone: 'Asia/Kolkata',
hour12: false,
year: 'numeric',
month: '2-digit',
day: '2-digit',
hour: '2-digit',
minute: '2-digit',
second: '2-digit',
};Using a shared constant will also help resolve the en-IN vs. en-US locale inconsistency between the frontend and backend.
abhishekkumar004-lgtm
force-pushed
the
workflow_timeformat_fix
branch
from
ddf37a5 to
f33c811
Compare
1 participant
update workflow date formatting to use IST timezone and 24-hour format