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: フォローされた際のメッセージを設定できるようにする #14430

Merged
merged 11 commits into from
Sep 28, 2024

Conversation

syuilo
Copy link
Member

@syuilo syuilo commented Aug 18, 2024

What

#14425

Why

Resolve #14425

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 packages/backend:test labels Aug 18, 2024
Copy link

codecov bot commented Aug 18, 2024

Codecov Report

Attention: Patch coverage is 59.63303% with 44 lines in your changes missing coverage. Please review.

Project coverage is 41.46%. Comparing base (89841e4) to head (51b4c56).
Report is 5 commits behind head on develop.

Files with missing lines Patch % Lines
packages/frontend/src/pages/settings/profile.vue 0.00% 19 Missing ⚠️
...ackages/frontend/src/components/MkNotification.vue 0.00% 16 Missing ⚠️
...end/src/core/activitypub/models/ApPersonService.ts 57.14% 3 Missing ⚠️
.../backend/src/core/activitypub/ApRendererService.ts 0.00% 1 Missing ⚠️
packages/backend/src/models/Notification.ts 0.00% 1 Missing ⚠️
...ackend/src/server/api/endpoints/admin/show-user.ts 80.00% 1 Missing ⚠️
...kages/backend/src/server/api/endpoints/i/update.ts 66.66% 1 Missing ⚠️
...ackages/frontend/src/components/MkFollowButton.vue 0.00% 1 Missing ⚠️
...ackages/frontend/src/components/MkNoteDetailed.vue 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #14430      +/-   ##
===========================================
+ Coverage    39.57%   41.46%   +1.89%     
===========================================
  Files         1544     1548       +4     
  Lines       193244   199219    +5975     
  Branches      3563     2625     -938     
===========================================
+ Hits         76470    82616    +6146     
+ Misses      116209   116040     -169     
+ Partials       565      563       -2     

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

Copy link
Contributor

github-actions bot commented Aug 18, 2024

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

差分はこちら
--- base
+++ head
@@ -11965,6 +11965,12 @@
                     "emailVerified": {
                       "type": "boolean"
                     },
+                    "followedMessage": {
+                      "type": [
+                        "string",
+                        "null"
+                      ]
+                    },
                     "autoAcceptFollowed": {
                       "type": "boolean"
                     },
@@ -12685,6 +12691,7 @@
                   "required": [
                     "email",
                     "emailVerified",
+                    "followedMessage",
                     "autoAcceptFollowed",
                     "noCrawle",
                     "preventAiLearning",
@@ -49475,6 +49482,14 @@
                     "minLength": 1,
                     "maxLength": 1500
                   },
+                  "followedMessage": {
+                    "type": [
+                      "string",
+                      "null"
+                    ],
+                    "minLength": 1,
+                    "maxLength": 256
+                  },
                   "location": {
                     "type": [
                       "string",
@@ -76956,6 +76971,12 @@
               "$ref": "#/components/schemas/RoleLite"
             }
           },
+          "followedMessage": {
+            "type": [
+              "string",
+              "null"
+            ]
+          },
           "memo": {
             "type": [
               "string",
@@ -77053,6 +77074,12 @@
             ],
             "format": "id"
           },
+          "followedMessage": {
+            "type": [
+              "string",
+              "null"
+            ]
+          },
           "isModerator": {
             "type": [
               "boolean",
@@ -77843,6 +77870,7 @@
         "required": [
           "avatarId",
           "bannerId",
+          "followedMessage",
           "isModerator",
           "isAdmin",
           "injectFeaturedNote",
@@ -78855,6 +78883,12 @@
               "userId": {
                 "type": "string",
                 "format": "id"
+              },
+              "message": {
+                "type": [
+                  "string",
+                  "null"
+                ]
               }
             },
             "required": [
@@ -78862,7 +78896,8 @@
               "createdAt",
               "type",
               "user",
-              "userId"
+              "userId",
+              "message"
             ]
           },
           {

Get diff files from Workflow Page

@syuilo syuilo marked this pull request as ready for review August 18, 2024 08:26
@kakkokari-gtyih kakkokari-gtyih added the 🌌Federation The Federation/ActivityPub feature label Aug 18, 2024
@tai-cha
Copy link
Contributor

tai-cha commented Aug 18, 2024

(後でみる:連合時の挙動を注意深く見たい)
(サーバーが観測済みのユーザーかつそのサーバーにフォローしたい相手をフォローしてる人[リモートフォロイーに対してローカルにフォロワー]がいない場合などで、初観測時と違うフォロー時メッセージが設定されている場合にきちんと現在設定されているメッセージが出るかなどを見たい)

@kakkokari-gtyih
Copy link
Contributor

kakkokari-gtyih commented Aug 20, 2024

フォローを承認制にしていない場合は、通知を発行せず吹き出しみたいなものを出してメッセージが出てくる仕様でも良いかもしれない(どっちが良いかはわからない)

image

@syuilo
Copy link
Member Author

syuilo commented Aug 20, 2024

フォローのたびに吹き出しが出るとちょっと邪魔かもしれん

@syuilo
Copy link
Member Author

syuilo commented Sep 23, 2024

マージするか

@tai-cha
Copy link
Contributor

tai-cha commented Sep 23, 2024

(後でみる:連合時の挙動を注意深く見たい) (サーバーが観測済みのユーザーかつそのサーバーにフォローしたい相手をフォローしてる人[リモートフォロイーに対してローカルにフォロワー]がいない場合などで、初観測時と違うフォロー時メッセージが設定されている場合にきちんと現在設定されているメッセージが出るかなどを見たい)

やはりこの時の懸念通りだったけど
自分のサーバーからのフォローがいない、かつRNや照会などによって一度サーバーで観測済みのリモートアカウントで(フォロー時にリモートに関して情報取得しなおすなどがなく)フォローメッセージが古いものが表示される可能性がけっこうある

再現手順:misskey-tgaで二つサーバー(A, B)を建ててAのアカウントを照会→Aでフォローメッセージを変更→Bでフォロー

@syuilo
Copy link
Member Author

syuilo commented Sep 23, 2024

メッセージに限らずリモートの内容は最新であることは保証されてないから良いんじゃないかしら

@tai-cha
Copy link
Contributor

tai-cha commented Sep 23, 2024

リモートの情報は最新であるとは限らないのはそうなのだけれど…

  • リモートのユーザー情報は能動的にfetchできるものではあるから、そういうケースでは情報の更新をしてあげてもいいのかもと思ったのと
  • ほかの部分(ユーザーページ)などに比べてリモートの情報は古いことがあるというのがこの機能においては伝わりづらい気がするという感触がある(特に大手サーバーを使っているユーザーに対しては)
    • もちろんユーザー間でのナレッジ共有などである程度は補えるけれど知識がないと扱いづらいものになりそう

あくまでこの機能自体がよくないという話ではないので改善の余地があるとすれば、程度の軽めのレビューの気持ちです(リモートの情報は不完全なのでそれでいいということであればそのままマージでもよいかと)

@syuilo
Copy link
Member Author

syuilo commented Sep 23, 2024

仮に古くてもあまり問題が発生しなさそう

@tai-cha
Copy link
Contributor

tai-cha commented Sep 23, 2024

モデレーション観点で観測当時に不適切なフォロー時メッセージが設定されていたが修正されたなんていうのとかも思いつかなくはなかったけれど、まあフォローメッセージで通報が来ても現在のユーザー情報ベースでモデレーションすればいいのか、管理者側は

@syuilo
Copy link
Member Author

syuilo commented Sep 25, 2024

とりあえずマージするか

@kakkokari-gtyih
Copy link
Contributor

kakkokari-gtyih commented Sep 25, 2024

フォローのたびに吹き出しが出るとちょっと邪魔かもしれん

立て続けにフォローしまくることがあまりないかもしれないけど(逆に通常の通知を発する方式だとフォローインポートとかでフォローされた際のメッセージが大量に飛んできてうざい等はあると思う)

@syuilo
Copy link
Member Author

syuilo commented Sep 25, 2024

ほむん

@syuilo
Copy link
Member Author

syuilo commented Sep 26, 2024

吹き出しにするか

@kakkokari-gtyih
Copy link
Contributor

吹き出しのコンポーネント前作ったことあるので参考までに↓
https://github.com/misskey-dev/misskey/blob/93f301742f61f452c9d5b87c90184e91890c4d3f/packages/frontend/src/components/MkBalloon.vue

@syuilo
Copy link
Member Author

syuilo commented Sep 26, 2024

とりあえずダイアログにするか

@syuilo
Copy link
Member Author

syuilo commented Sep 26, 2024

フォローインポートとかでフォローされた際のメッセージが大量に飛んできてうざい等

まあこれはこのPRに関わらず起き得るわね(リモートユーザーの場合フォローリクエストが作成される扱いになるため)

@syuilo
Copy link
Member Author

syuilo commented Sep 26, 2024

insert直後にpackするからレプリケーション環境と相性が悪い可能性がある

@syuilo
Copy link
Member Author

syuilo commented Sep 26, 2024

レプリケーションについては後で考えよう

@syuilo
Copy link
Member Author

syuilo commented Sep 26, 2024

ダイアログもそれはそれでめんどいわね

@syuilo
Copy link
Member Author

syuilo commented Sep 26, 2024

MPが0になった

@kakkokari-gtyih
Copy link
Contributor

吹き出しのUIだけあとでやってみるか・・・

@syuilo syuilo added this to the v2024.9.0 milestone Sep 26, 2024
@syuilo
Copy link
Member Author

syuilo commented Sep 27, 2024

とりあえずマージするか

@syuilo
Copy link
Member Author

syuilo commented Sep 28, 2024

テストが通らない :angry_ai: :angry_ai: :angry_ai:

@syuilo
Copy link
Member Author

syuilo commented Sep 28, 2024

通った

@syuilo syuilo merged commit 28e9d4e into develop Sep 28, 2024
35 checks passed
@syuilo syuilo deleted the followed-message branch September 28, 2024 00:55
@syuilo
Copy link
Member Author

syuilo commented Sep 28, 2024

🙏🏻

@kakkokari-gtyih
Copy link
Contributor

misskey hubのnsに追記する必要がありそう

@KisaragiEffective
Copy link
Collaborator

@kakkokari-gtyih

misskey hubのnsに追記する必要がありそう

misskey-dev/misskey-hub-next#241

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌌Federation The Federation/ActivityPub feature packages/backend:test 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.

フォローされた際のメッセージを設定できるようにする
4 participants