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

suspend周りの改修 #14409

Merged
merged 9 commits into from
Aug 17, 2024
Merged

suspend周りの改修 #14409

merged 9 commits into from
Aug 17, 2024

Conversation

syuilo
Copy link
Member

@syuilo syuilo commented Aug 14, 2024

What

Why

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

@github-actions github-actions bot added packages/frontend Client side specific issue/PR packages/backend Server side specific issue/PR packages/misskey-js labels Aug 14, 2024
Copy link
Contributor

github-actions bot commented Aug 14, 2024

このPRによるapi.jsonの差分

差分はこちら

Get diff files from Workflow Page

Copy link

codecov bot commented Aug 14, 2024

Codecov Report

Attention: Patch coverage is 24.30556% with 109 lines in your changes missing coverage. Please review.

Project coverage is 41.65%. Comparing base (45d8857) to head (daeab84).
Report is 2 commits behind head on develop.

Files Patch % Lines
packages/backend/src/core/UserSuspendService.ts 21.05% 60 Missing ⚠️
packages/backend/src/core/DeleteAccountService.ts 21.15% 41 Missing ⚠️
...ackages/frontend/src/pages/admin/modlog.ModLog.vue 0.00% 4 Missing ⚠️
.../src/server/api/endpoints/admin/accounts/delete.ts 33.33% 2 Missing ⚠️
...end/src/server/api/endpoints/admin/suspend-user.ts 50.00% 1 Missing ⚠️
...d/src/server/api/endpoints/admin/unsuspend-user.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #14409      +/-   ##
===========================================
+ Coverage    39.91%   41.65%   +1.73%     
===========================================
  Files         1545     1549       +4     
  Lines       190506   196605    +6099     
  Branches      3506     3573      +67     
===========================================
+ Hits         76046    81892    +5846     
- Misses      113869   114150     +281     
+ Partials       591      563      -28     

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

@syuilo
Copy link
Member Author

syuilo commented Aug 15, 2024

pnpm build-misskey-js-with-types
すると

Oops! Something went wrong! :(

ESLint: 9.8.0

SyntaxError: Unexpected identifier 'assert'
    at compileSourceTextModule (node:internal/modules/esm/utils:337:16)
    at ModuleLoader.moduleStrategy (node:internal/modules/esm/translators:166:18)
    at callTranslator (node:internal/modules/esm/loader:436:14)
    at ModuleLoader.moduleProvider (node:internal/modules/esm/loader:442:30)

@syuilo
Copy link
Member Author

syuilo commented Aug 15, 2024

tasukete

@syuilo
Copy link
Member Author

syuilo commented Aug 15, 2024

依存関係更新したら出なくなったけど
Unsupported schema format, expected openapi: 3.x
と言われる

@syuilo
Copy link
Member Author

syuilo commented Aug 15, 2024

直った

@syuilo
Copy link
Member Author

syuilo commented Aug 15, 2024

今度は

✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.
 ✘  Every operation must have a unique `operationId`.

@syuilo
Copy link
Member Author

syuilo commented Aug 15, 2024

tasukete

@syuilo
Copy link
Member Author

syuilo commented Aug 15, 2024

// misskey-jsはPOST固定で送っているので、こちらも決め打ちする。別メソッドに対応することがあればこちらも直す必要あり

が関係している?

@kakkokari-gtyih
Copy link
Contributor

kakkokari-gtyih commented Aug 15, 2024

pnpm --filter misskey-js api (api extractor生成物の再生成)する必要がありそう(build-misskey-js-with-typesをすれば普通は自動で生成されるけど)

@samunohito
Copy link
Member

Every operation must have a unique operationId.

getとpostがあるエンドポイントに同一のoperationidがあてがわれていることが原因と推測します。
この辺と関連ありそう #13498

@samunohito
Copy link
Member

(というかなんの対応を行うprなんだろう…)

@KisaragiEffective
Copy link
Collaborator

(というかなんの対応を行うprなんだろう…)

物理削除に対してモデログを残す対応っぽい?

@syuilo
Copy link
Member Author

syuilo commented Aug 16, 2024

(というかなんの対応を行うprなんだろう…)

物理削除に対してモデログを残す対応っぽい?

なんかいろいろやる

@samunohito
Copy link
Member

コード以外にも何したかわかるような情報を残してくれるとちぇりぴする時とかに嬉しいかも

@syuilo
Copy link
Member Author

syuilo commented Aug 16, 2024

語彙力がないから説明できないわね

@syuilo
Copy link
Member Author

syuilo commented Aug 16, 2024

SyntaxError: Unexpected identifier 'assert' が再発した

@syuilo
Copy link
Member Author

syuilo commented Aug 16, 2024

eslint ./built/**/*.ts --fix って要る??

@syuilo
Copy link
Member Author

syuilo commented Aug 16, 2024

/home/runner/work/misskey/misskey/packages/misskey-js/generator/built/autogen/types.ts
4:8 error Parsing error: ',' expected

nani

@samunohito
Copy link
Member

misskey-jsまわりのupdate deps、必須でないのであれば後回しが丸いのでは…

@syuilo
Copy link
Member Author

syuilo commented Aug 16, 2024

必須っぽそう

@syuilo
Copy link
Member Author

syuilo commented Aug 16, 2024

自分のnodeを20に下げれば良いかもしれないけど

"@typescript-eslint/eslint-plugin": "6.11.0",
"@typescript-eslint/parser": "6.11.0",
"@readme/openapi-parser": "2.6.0",
"@types/node": "22.3.0",
Copy link
Member

Choose a reason for hiding this comment

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

ここをbumpすると新しいほうしかないAPIを誤って使って20で動かなくなるが有り得そうだから上げたくない気持ちが少しある (他パッケージにもtypes/nodeのbumpがあればそれも同じく)

@anatawa12
Copy link
Member

SyntaxError: Unexpected identifier 'assert'

eslint周りのどっかのパッケージが node v22 で廃止された import asserts を使ってそう。てもとでもnode v22だと再現した

@anatawa12
Copy link
Member

/home/runner/work/misskey/misskey/packages/misskey-js/generator/built/autogen/types.ts
4:8 error Parsing error: ',' expected

nani

types.tsがこうなってたのでジェネレータのどっかがおかしい。openapi-typescriptのmajor更新が原因そう

/* eslint @typescript-eslint/naming-convention: 0 */
/* eslint @typescript-eslint/no-explicit-any: 0 */

[object Object],[object Object],[object Object],[object Object],[object Object]

@anatawa12
Copy link
Member

misskey-dev/eslint-pluginがimport assert使ってるからそれじゃないかな

@syuilo
Copy link
Member Author

syuilo commented Aug 16, 2024

🙏🏿🙏🏿🙏🏿

@anatawa12
Copy link
Member

misskey-dev/eslint-pluginがimport assert使ってるからそれじゃないかな

とりあえず issueだけ misskey-dev/eslint-plugin-misskey-dev#4

@anatawa12
Copy link
Member

misskey-dev/eslint-plugin を直せば misskey js 周りの変更を全体的に戻しても node v22 でも問題なく動きそうとりあえず戻すPRと、eslint-plugin側の修正を準備します

@anatawa12
Copy link
Member

当面は node v20 で作業していただけると多分ありがたいです

@syuilo
Copy link
Member Author

syuilo commented Aug 16, 2024

近いうちにやらなきゃいけないし戻さなくても良さそう

@anatawa12
Copy link
Member

anatawa12 commented Aug 16, 2024

一つのPRにまとめるとわけがわからなくなるので分けてほしい気持ち

@syuilo
Copy link
Member Author

syuilo commented Aug 16, 2024

難しい

@syuilo
Copy link
Member Author

syuilo commented Aug 16, 2024

PR形式でやるとこうなっちゃうからdevelopに直接入れようと思ったけどテストの様子を見たいからPR形式にした

@syuilo
Copy link
Member Author

syuilo commented Aug 16, 2024

どかっとなんかいろいろな言葉にできないような変更を一括的に行いたいときの良い方法がなさそう

@anatawa12
Copy link
Member

どかっとなんかいろいろな言葉にできないような変更を一括的に行いたいとき

分割できない範囲はまぁまとまっててもいいと思いますが分割できる範囲は別にしてて欲しい...

あとはSquashせずにPRにして、Mergeまえにrebase iで適宜reward . fixupするとかがあとから遡るときにわかりやすくていいと思ってます

@syuilo syuilo marked this pull request as ready for review August 17, 2024 00:56
@syuilo syuilo merged commit ef950a3 into develop Aug 17, 2024
33 of 34 checks passed
@syuilo syuilo deleted the nanka-iroiro-2024-08-14 branch August 17, 2024 00:57
@syuilo
Copy link
Member Author

syuilo commented Aug 17, 2024

🙏🏻

LemonDouble pushed a commit to LemonDouble/misskey that referenced this pull request Aug 19, 2024
* enhance(backend): 凍結されたアカウントのフォローリクエストを表示しないように

* Update CHANGELOG.md

* wip

* Update gen-spec.ts

* Update packages/backend/src/server/api/endpoints/admin/suspend-user.ts

Co-authored-by: Kisaragi <[email protected]>

* owa-

* revert misskey-js related changes (misskey-dev#14414)

---------

Co-authored-by: Kisaragi <[email protected]>
Co-authored-by: anatawa12 <[email protected]>
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 packages/misskey-js
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants