Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion client/src/components/ArchiveDelete/RolesArchive.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import UserContext from "../../contexts/UserContext";
import { MdMoreVert, MdOutlineSearch } from "react-icons/md";
import ContentContainerWithTables from "components/Layout/ContentContainerWithTables";
import DeleteArchivedAccountModal from "components/Modals/WarningArchivedAccountDelete";
import { formatDate } from "../../helpers/util";

const useStyles = createUseStyles(theme => ({
main: {
Expand Down Expand Up @@ -287,7 +288,7 @@ const RolesArchive = ({ contentContainerRef }) => {
{account?.numberOfSubmissions || "0"}
</td>
<td className={classes.td}>
{new Date(account.archivedAt).toLocaleDateString()}
{formatDate(account.archivedAt)}
</td>
<td className={classes.tdCenter}>
<Popup
Expand Down
37 changes: 30 additions & 7 deletions client/src/helpers/util.js

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for catching the problem with missing Time Zone conversions for formatDate. As you pointed out, the formatDate function need to do a time zone conversion. In addition, these functions were confusing to use because the formatDate function only worked property when you gave it a JavaScript date, and the formatDatetime function only worked when you gave an ISO formatted date string. We kind of need to use both date representation in the app in different places - ISO strings are ok for display, sorting and comparison, but there are places where we actually need to do arithmetic with Javascript dates. So I modified the utility functions to handle both representations, and added comments to explicitly explain the acceptable input formats, and added the commit to your PR.

Good work!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Great, thank you for the review!

Original file line number Diff line number Diff line change
@@ -1,17 +1,40 @@
import { DateTime } from "luxon";

// parse a string as a Javascript date.
// This is primarily used when you need to do arithmetic or comparison
// of dates.
export const toDate = s => (s ? new Date(s) : null);

// export const formatDate = date => {
// return date ? new Date(date).toISOString().split("T")[0] : null;
// };

// Format a date without time in the Pacific Time Zone.
// The input is expected to be either a Javascript Date
// OR a string in one of the formats
// parsable by luxon as listed here: https://moment.github.io/luxon/#/parsing
export const formatDate = date => {
return date ? new Date(date).toISOString().split("T")[0] : null;
if (!date) return null;
const isoDate =
date instanceof Date ? DateTime.fromJSDate(date) : DateTime.fromISO(date);
const formattedDate = isoDate
.setZone("America/Los_Angeles")
.toFormat("yyyy-MM-dd");
return formattedDate;
};

export const formatDatetime = datetime => {
return datetime
? DateTime.fromISO(datetime)
.setZone("America/Los_Angeles")
.toFormat("yyyy-MM-dd hh:mm a")
: null;
// Format a date with time in the Pacific Time Zone
// The input is expected to be either a Javascript Date
// OR a string in one of the formats
// parsable by luxon as listed here: https://moment.github.io/luxon/#/parsing
export const formatDatetime = date => {
if (!date) return null;
const isoDate =
date instanceof Date ? DateTime.fromJSDate(date) : DateTime.fromISO(date);
const formattedDate = isoDate
.setZone("America/Los_Angeles")
.toFormat("yyyy-MM-dd hh:mm a");
return formattedDate;
};

export const formatId = id => {
Expand Down