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: DB CONSTRAINT of RenoteMuting #11724

Merged
merged 2 commits into from
Aug 20, 2023

Conversation

atsu1125
Copy link
Contributor

@atsu1125 atsu1125 commented Aug 15, 2023

What

Fix: DB CONSTRAINT of RenoteMuting
リノートミュートしているユーザーが削除されてもリノートミュートが壊れないようにします。
現在リノートミュートが導入されていますが、ユーザーIDに対する外部キー制約を設定されていなかったため、リノートミュートされているユーザーが消された場合に、リノートミュートがうまく動かなくなります。
#11398 にて外部キー制約が設定されましたが、以前古いレコードは残ったままであり、このままリリースしてしまうとマイグレーションがエラーとなり通らなくなります。
そのためこのPRにて、外部キー制約を導入するために、事前に該当レコードを削除します。
サブクエリは重くなる可能性がありますが、該当PRのマイグレーションを通すためにはやむを得ません。
またこれは新しいマイグレーションファイルとして用意される必要があります。
なぜならすでにマイグレーションを完了しているインスタンスには同じマイグレーションを適用することはできないからです。

今回と直接関係はないですがリノートミュートのdownメソッドが定義されませんでしたので、これは元のマイグレーションファイルに追記します。

Why

Fix: #11343

Additional info (optional)

Misskey v13.14.2 以降の開発ブランチの開発環境で問題なし
Screenshot 2023-08-15 at 10 31 49

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Add story of storybook
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

@github-actions github-actions bot added the packages/backend Server side specific issue/PR label Aug 15, 2023
@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Merging #11724 (a48531a) into develop (c3fd848) will decrease coverage by 0.03%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop   #11724      +/-   ##
===========================================
- Coverage    78.75%   78.73%   -0.03%     
===========================================
  Files          923      922       -1     
  Lines        97814    97709     -105     
  Branches      7742     7742              
===========================================
- Hits         77035    76930     -105     
  Misses       20779    20779              

see 1 file with indirect coverage changes

@atsu1125
Copy link
Contributor Author

あまり望ましくはないかもしれないですが #11398 より先にこのマイグレーションが通らないと、意味がないのでファイル名・順番を変更しました。

@tamaina
Copy link
Contributor

tamaina commented Aug 15, 2023

このままリリースしてしまうとマイグレーションがエラーとなり通らなくなります。

特に問題ないけれど

@tamaina
Copy link
Contributor

tamaina commented Aug 15, 2023

それはリノートミュートがないからか

@atsu1125
Copy link
Contributor Author

atsu1125 commented Aug 15, 2023

インスタンスのリノートミュートテーブルに削除されたユーザーIDがある場合のみマイグレーションエラー、制約違反が発生します。

@atsu1125
Copy link
Contributor Author

今回はmuteeIDとmuterIDが同一であるユニークインデックスのマイグレーションエラーは考慮していません。通常APIエラーmuteeIsYourselfで登録できないからです。

@syuilo syuilo merged commit 750085f into misskey-dev:develop Aug 20, 2023
@syuilo
Copy link
Member

syuilo commented Aug 20, 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
Projects
None yet
3 participants