Skip to content

Feat/user timezone#908

Merged
Klakurka merged 11 commits intomasterfrom
feat/user-timezone
Jan 7, 2025
Merged

Feat/user timezone#908
Klakurka merged 11 commits intomasterfrom
feat/user-timezone

Conversation

@lissavxo
Copy link
Collaborator

@lissavxo lissavxo commented Dec 30, 2024

Related to #893

Description

Implement timezone option in user Account settings.
This PR will enable timezone option, the selector is initialized with the timezone of the user by location, once user changes the selector value it will set the timezone and it will make the dashboard data, the txs listing of each button and payments listing to use the timezone selected.

Test plan

Start the server with docker compose up

  • Go to account page, check if selector is initialized with timezone of your location
  • Change selector to another timezone
  • Check payments list see if the date of each payment is in the preferred timezone
  • Check the transaction list in button page see if the date of each tx is in the preferred timezone
  • Check dashboard data, it should display the data according to the new timezone set

@lissavxo lissavxo requested a review from chedieck December 30, 2024 15:47
@lissavxo lissavxo force-pushed the feat/user-timezone branch 2 times, most recently from 5e658f1 to 55e2934 Compare December 30, 2024 18:09
@Klakurka Klakurka self-requested a review December 31, 2024 02:24
Copy link
Member

@Klakurka Klakurka left a comment

Choose a reason for hiding this comment

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

If I change my account's time zone, the dashboard doesn't update to reflect the fact that txs made on a certain date in one time zone are actually a day ahead/behind in another.

@lissavxo
Copy link
Collaborator Author

lissavxo commented Jan 2, 2025

If I change my account's time zone, the dashboard doesn't update to reflect the fact that txs made on a certain date in one time zone are actually a day ahead/behind in another.

I was not clearing cache after setting the timezone, now It should work

@lissavxo lissavxo requested a review from Klakurka January 2, 2025 17:29
@lissavxo lissavxo force-pushed the feat/user-timezone branch from b2f446f to 05289ae Compare January 3, 2025 15:39
Copy link
Member

@Klakurka Klakurka left a comment

Choose a reason for hiding this comment

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

I got this when trying to update my time zone:

image

Copy link
Member

@Klakurka Klakurka left a comment

Choose a reason for hiding this comment

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

I changed the time zone a few times and eventually got this on /payments:

image

@lissavxo lissavxo requested a review from Klakurka January 3, 2025 20:33
Klakurka
Klakurka previously approved these changes Jan 3, 2025
Copy link
Collaborator

@chedieck chedieck left a comment

Choose a reason for hiding this comment

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

Requested some changes in the code, also:

The other settings (Preferred currency & organization) give us feedback when they're updated. This could do the same as preferred currency, showing "Updated timezone successfully" after the update, or the error in case there is some.

user,
userId
userId,
userProfile: user.userProfile
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems really redundant. You're sending the samething twice, first user then user.userProfile as userProfile


export const getCachedPaymentsCountForUser = async (userId: string): Promise<number> => {
const dashboardData = await getUserDashboardData(userId)
const dashboardData = await getUserDashboardData(userId, moment.tz.guess())
Copy link
Collaborator

Choose a reason for hiding this comment

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

this doesn't seem right. It will be run in the server. I tested adding

  console.log('WIP tz', moment.tz.guess(), typeof window, 'is window type')

right above this line and got

2|next  | 2025-01-07T00:45:35: WIP tz Africa/Abidjan undefined is window type

... as a result, which shows:

  • guess is wrong
  • is being run in the server, since this was in the server logs (not the browser console) and even shows that typeof window === undefined.

@lissavxo lissavxo requested review from Klakurka and chedieck January 7, 2025 18:38
setError('')
setSuccess('Timezone updated successfully.')
}
} catch (err: any) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should actually be at the else of the if above. If the api runs into an error, it will return a status code 500 response message, not throw here.

@Klakurka Klakurka merged commit e9d3103 into master Jan 7, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants