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

feat(backend/ApiCallService): allow limited access for suspended accounts #11715

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

saschanaz
Copy link
Member

What

prohibitMovedをprohibitDeactivatedに変えてisSuspendedにも対応するようにしました

Why

Fixes #11470

こうしたら管理者側ではデータアクセスの不満を考慮せず即座にアカウントの停止ができますし、停止された側では引っ越しとかデータエクスポートとかができます

Additional info (optional)

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

@saschanaz saschanaz changed the title feat(backend/ApiCallService): allow limited access for suspend accounts feat(backend/ApiCallService): allow limited access for suspended accounts Aug 13, 2023
@github-actions github-actions bot added packages/backend Server side specific issue/PR packages/backend:test labels Aug 13, 2023
@codecov
Copy link

codecov bot commented Aug 13, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (526b3ae) 78.86% compared to head (aba02a0) 78.86%.
Report is 1 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #11715   +/-   ##
========================================
  Coverage    78.86%   78.86%           
========================================
  Files          928      928           
  Lines        98178    98179    +1     
  Branches      7820     7820           
========================================
+ Hits         77428    77433    +5     
+ Misses       20750    20746    -4     
Files Changed Coverage Δ
packages/backend/src/server/api/ApiCallService.ts 79.00% <100.00%> (+1.80%) ⬆️
packages/backend/src/server/api/endpoints.ts 100.00% <100.00%> (ø)
...ackend/src/server/api/endpoints/antennas/create.ts 98.52% <100.00%> (ø)
...ackend/src/server/api/endpoints/antennas/update.ts 96.94% <100.00%> (ø)
...ackend/src/server/api/endpoints/channels/create.ts 90.62% <100.00%> (ø)
...kend/src/server/api/endpoints/channels/favorite.ts 77.61% <100.00%> (ø)
...ackend/src/server/api/endpoints/channels/follow.ts 77.61% <100.00%> (ø)
...nd/src/server/api/endpoints/channels/unfavorite.ts 79.03% <100.00%> (ø)
...kend/src/server/api/endpoints/channels/unfollow.ts 79.03% <100.00%> (ø)
...backend/src/server/api/endpoints/clips/add-note.ts 97.64% <100.00%> (ø)
... and 38 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tamaina
Copy link
Contributor

tamaina commented Aug 13, 2023

movedとsuspendedは分けたほうが良いかもしれない(deactivatedでまとめてしまうのは都合が悪い場面が出てきそうな予感がする)

@saschanaz
Copy link
Member Author

saschanaz commented Aug 13, 2023

movedとsuspendedは分けたほうが良いかもしれない(deactivatedでまとめてしまうのは都合が悪い場面が出てきそうな予感がする)

moved垢に欲しくないものは多分suspended垢にも欲しくなさそうですが、suspended垢にもうちょっと制限を置きたい状況もたしかにありそうです

その場合、prohibitDeactivatedとprohibitSuspended二つにしてdeactivatedは両方に対応、suspendedはsuspendedにだけ対応できるようにしてもいいかもです

(prohibitSuspendedがどこに必要なのかは今すぐには分かりませんが)

@tamaina
Copy link
Contributor

tamaina commented Aug 13, 2023

個人的にはprohibitMoved: trueのエンドポイントにprohibitSuspended: trueを書き足すのが良いと思った

@saschanaz
Copy link
Member Author

prohibitMoved: trueなのにprohibitSuspended: falseな異様なバグが発生しそうでして

@tamaina
Copy link
Contributor

tamaina commented Aug 13, 2023

そう…?
(VS Codeの検索機能でよーく確認すれば大丈夫では)

@saschanaz
Copy link
Member Author

よーく確認すれば

いつか必ず誰かが間違ってしまいそうな気がします(人間ってそういうものなのでは😆)

ほぼ全てのprohibitMovedにprohibitSuspendedを追加しなければならないので、無駄なduplicationではないかなとも思います

@saschanaz
Copy link
Member Author

saschanaz commented Aug 13, 2023

prohibitMovedの名前をそのままにしてprohibitMoved: trueならprohibitSuspendedのデフォルト値をtrueにする方法もありそうです

でも、まずprohibitSuspendedの使いどころが知りたいかも(notes/showとかもできないようにする?)

@tamaina
Copy link
Contributor

tamaina commented Aug 13, 2023

使いどころとかは関係なく、movedとsuspendedを一緒くたにするのがなんか嫌

@tamaina
Copy link
Contributor

tamaina commented Aug 13, 2023

(挙動としては似通ったものになるだろうけど)

@syuilo
Copy link
Member

syuilo commented Sep 22, 2023

凍結されてると使えないエンドポイントをtrueにする(ブラックリスト)のではなくて、凍結されてても使えるエンドポイントをtrueにする(ホワイトリスト)方が自然な感じはした

@syuilo
Copy link
Member

syuilo commented Sep 22, 2023

いやそうでもないか

@syuilo
Copy link
Member

syuilo commented Sep 22, 2023

個人的にはprohibitMoved: trueのエンドポイントにprohibitSuspended: trueを書き足すのが良いと思った

賛成

@syuilo
Copy link
Member

syuilo commented Sep 22, 2023

(コードを見たときにその方が分かりやすそう)

@syuilo
Copy link
Member

syuilo commented Sep 22, 2023

巻き取るか

@syuilo syuilo self-assigned this Sep 22, 2023
@syuilo
Copy link
Member

syuilo commented Sep 22, 2023

prohibitMoved と prohibitSuspended をまとめる場合は prohibitReadOnly といったプロパティ名にした方が良さそう

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend:test packages/backend Server side specific issue/PR
Projects
Development

Successfully merging this pull request may close these issues.

アカウントが停止されても完全削除の前にはログインはできるように
3 participants