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

fix: i/notificationsで古い通知タイプを許容するなど、古い通知タイプとの互換性保持 #10042

Merged
merged 18 commits into from
Feb 23, 2023

Conversation

tamaina
Copy link
Member

@tamaina tamaina commented Feb 23, 2023

Fix #10040

What

  • クライアントのnotificationTypesはconst.tsに記載 (misskey-jsに依存するのをやめる)
  • NotificationSettingWindowをちょっと変更
  • i/notificationsで古い通知タイプを許容する(新しい通知タイプだけでクエリするようにフィルターをする処理は追加)
  • pollVoteを削除するマイグレーション

@github-actions github-actions bot added packages/backend Server side specific issue/PR packages/frontend Client side specific issue/PR labels Feb 23, 2023
@tamaina tamaina marked this pull request as draft February 23, 2023 09:50
@codecov
Copy link

codecov bot commented Feb 23, 2023

Codecov Report

❗ No coverage uploaded for pull request base (develop@3dd363a). Click here to learn what that means.
The diff coverage is 42.85%.

@@            Coverage Diff             @@
##             develop   #10042   +/-   ##
==========================================
  Coverage           ?   24.60%           
==========================================
  Files              ?      705           
  Lines              ?    65213           
  Branches           ?     2297           
==========================================
  Hits               ?    16048           
  Misses             ?    49165           
  Partials           ?        0           
Impacted Files Coverage Δ
...end/src/core/entities/NotificationEntityService.ts 34.96% <ø> (ø)
...ackend/src/server/api/endpoints/i/notifications.ts 0.00% <0.00%> (ø)
...ckages/backend/src/models/entities/Notification.ts 100.00% <100.00%> (ø)
...ackages/backend/src/models/entities/UserProfile.ts 100.00% <100.00%> (ø)
packages/backend/src/types.ts 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@tamaina tamaina marked this pull request as ready for review February 23, 2023 10:23
@syuilo syuilo changed the title wip fix: i/notificationsで古い通知タイプを許容するなど fix: i/notificationsで古い通知タイプを許容するなど Feb 23, 2023
@tamaina tamaina changed the title fix: i/notificationsで古い通知タイプを許容するなど fix: i/notificationsで古い通知タイプを許容するなど、古い通知タイプの清算 Feb 23, 2023
@tamaina
Copy link
Member Author

tamaina commented Feb 23, 2023

うお〜〜、p1.a9z.devで通知マイグレーション失敗した

@tamaina
Copy link
Member Author

tamaina commented Feb 23, 2023

てか通常のenumも減らすのダメじゃん

@tamaina
Copy link
Member Author

tamaina commented Feb 23, 2023

やっぱ通知は消したくないかも(もう遅いか)

@syuilo
Copy link
Member

syuilo commented Feb 23, 2023

単にvarcharで良い気がしてきた

@EbiseLutica
Copy link
Member

varcharであってくれるとひじょーにたすかるます

@tamaina
Copy link
Member Author

tamaina commented Feb 23, 2023

enum arrayからjsonbって変換できるのかしら

@tamaina
Copy link
Member Author

tamaina commented Feb 23, 2023

enum arrayからjsonbって変換

無理だった(DROPされてしまった)

@tamaina
Copy link
Member Author

tamaina commented Feb 23, 2023

てかenumからvarcharもTypeORMはやってくれない

@syuilo
Copy link
Member

syuilo commented Feb 23, 2023

TypeORMは頑張ってくれないがち

@EbiseLutica
Copy link
Member

手書きするしかなさげ?

@syuilo
Copy link
Member

syuilo commented Feb 23, 2023

一旦このPRでは

pollVoteを削除するマイグレーション

をオミットするのはどう

@tamaina
Copy link
Member Author

tamaina commented Feb 23, 2023

面倒なのでenumは削除しない方針が良さそう

This reverts commit 6cdb360.
@tamaina
Copy link
Member Author

tamaina commented Feb 23, 2023

pollVoteを削除するマイグレーション

をオミット

二度と削除できないと思う

@tamaina
Copy link
Member Author

tamaina commented Feb 23, 2023

でもこれやるなら ce5c78d が惜しまれる

@syuilo
Copy link
Member

syuilo commented Feb 23, 2023

groupInvitedなnotificationだけ消せば良さそうだけどレコード数によってはかなり時間かかるかも

@tamaina
Copy link
Member Author

tamaina commented Feb 23, 2023

(今enumを消さないのであればgroupInvitedを消さないでもよくて、それならTRUNCATE TABLE "notification"しなくて良かったよねの意

@syuilo
Copy link
Member

syuilo commented Feb 23, 2023

TRUNCATE TABLE "notification"を今からでも消すか

@tamaina
Copy link
Member Author

tamaina commented Feb 23, 2023

はい

@tamaina tamaina requested a review from syuilo February 23, 2023 11:36
@tamaina tamaina changed the title fix: i/notificationsで古い通知タイプを許容するなど、古い通知タイプの清算 fix: i/notificationsで古い通知タイプを許容するなど、古い通知タイプとの互換性保持 Feb 23, 2023
@tamaina
Copy link
Member Author

tamaina commented Feb 23, 2023

マージしてリリースしようぜ

@syuilo syuilo merged commit becc4d2 into misskey-dev:develop Feb 23, 2023
@syuilo
Copy link
Member

syuilo commented Feb 23, 2023

🙏🏻🙏🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR packages/frontend Client side specific issue/PR
Projects
None yet
3 participants