Skip to content
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

Refactor: replace deprecated timezone_offset parameter with timezone #4159

Merged
merged 3 commits into from
Sep 12, 2024

Conversation

PooyaRaki
Copy link
Contributor

Removes deprecated timezone_offset, Adds timezone as a replacement.

What it solves

Resolves
safe-global/safe-client-gateway#1024

How this PR fixes it

A new query parameter, timezone, has been introduced. This parameter accepts only valid timezone strings (e.g., Europe/Berlin) and supersedes the deprecated timezone_offset, which previously accepted a numeric timezone difference.

How to test it

  • Review the transaction history list.
  • Make sure the transactions are listed with the correct time and date, and verify that they are grouped by day properly.
  • Ensure that transactions occurring around midnight are not mistakenly assigned to the following day.
    For more info please visit Add support for client timezone information safe-client-gateway#1024

Screenshots

Checklist

  • I've tested the branch on mobile 📱
  • I've documented how it affects the analytics (if at all) 📊
  • I've written a unit/e2e test for it (if applicable) 🧑‍💻

Copy link

github-actions bot commented Sep 11, 2024

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

Copy link

github-actions bot commented Sep 11, 2024

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@PooyaRaki
Copy link
Contributor Author

Currently, there are some errors present due to the SDK version not being updated. Please ensure to update the SDK version accordingly.

@PooyaRaki
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

Safe-infra added a commit to safe-global/cla-signatures that referenced this pull request Sep 11, 2024
Copy link

github-actions bot commented Sep 11, 2024

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
78.5% (+0.05% 🔼)
11882/15136
🔴 Branches
59.1% (+0.1% 🔼)
3053/5166
🟡 Functions
65.95% (+0.13% 🔼)
1894/2872
🟡 Lines
79.95% (+0.05% 🔼)
10720/13409
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / useRedefine.ts
98.28% 95.24% 100% 100%
🟢
... / index.ts
91.67% 70% 100% 91.67%
🔴
... / index.tsx
50.82% 2.63% 8.33% 55.56%
🟡
... / RedefineHint.tsx
54.55% 0% 0% 60%
🟡
... / RedefineBalanceChange.tsx
71.79% 7.14% 11.11% 71.05%

Test suite run success

1483 tests passing in 203 suites.

Report generated by 🧪jest coverage report action from cd8cbbf

@PooyaRaki PooyaRaki self-assigned this Sep 11, 2024
Copy link
Member

@katspaugh katspaugh left a comment

Choose a reason for hiding this comment

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

You can now install @safe-global/safe-gateway-typescript-sdk@3.22.4-beta.1 and commit that as well so that we can test this PR.

@katspaugh katspaugh changed the title add timezone parameter to the transaction history endpoint, removes d… Refactor: replace deprecated timezone_offset parameter with timezone Sep 11, 2024
@PooyaRaki
Copy link
Contributor Author

You can now install @safe-global/safe-gateway-typescript-sdk@3.22.4-beta.1 and commit that as well so that we can test this PR.

@katspaugh Done ✅

Copy link

github-actions bot commented Sep 12, 2024

📦 Next.js Bundle Analysis for safe-wallet-web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

⚠️ Global Bundle Size Increased

Page Size (compressed)
global 949.6 KB (🟡 +2 B)
Details

The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!

package.json Outdated
@@ -158,5 +158,6 @@
"budgetPercentIncreaseRed": 20,
"minimumChangeThreshold": 0,
"showDetails": true
}
},
"packageManager": "yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e"
Copy link
Member

Choose a reason for hiding this comment

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

🤔

package.json Outdated Show resolved Hide resolved
Copy link
Contributor

@compojoom compojoom left a comment

Choose a reason for hiding this comment

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

👍

@katspaugh
Copy link
Member

Tested, the timezone parameter is added and the date labels are correct. ✅

@katspaugh katspaugh merged commit 221973d into dev Sep 12, 2024
15 of 16 checks passed
@katspaugh katspaugh deleted the add-timezone-transaction-history branch September 12, 2024 09:52
@github-actions github-actions bot locked and limited conversation to collaborators Sep 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants