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: エクスポート完了時に通知を発行するように #14484

Merged
merged 9 commits into from
Sep 26, 2024

Conversation

kakkokari-gtyih
Copy link
Contributor

@kakkokari-gtyih kakkokari-gtyih commented Sep 1, 2024

What

Why

#14456 が一部修正される

Additional info (optional)

#14484 (comment) をなんとかする必要がある いけた

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/sw labels Sep 1, 2024
Copy link

codecov bot commented Sep 1, 2024

Codecov Report

Attention: Patch coverage is 45.07042% with 78 lines in your changes missing coverage. Please review.

Project coverage is 41.28%. Comparing base (89841e4) to head (0f87d6d).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
...ackages/frontend/src/components/MkNotification.vue 0.00% 28 Missing ⚠️
packages/backend/src/models/Notification.ts 0.00% 8 Missing ⚠️
...e/processors/ExportCustomEmojisProcessorService.ts 25.00% 6 Missing ⚠️
...queue/processors/ExportAntennasProcessorService.ts 28.57% 5 Missing ⚠️
...queue/processors/ExportBlockingProcessorService.ts 28.57% 5 Missing ⚠️
...ueue/processors/ExportFavoritesProcessorService.ts 28.57% 5 Missing ⚠️
...ueue/processors/ExportFollowingProcessorService.ts 28.57% 5 Missing ⚠️
...c/queue/processors/ExportMutingProcessorService.ts 28.57% 5 Missing ⚠️
...rc/queue/processors/ExportNotesProcessorService.ts 28.57% 5 Missing ⚠️
...ueue/processors/ExportUserListsProcessorService.ts 28.57% 5 Missing ⚠️
... and 1 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #14484      +/-   ##
===========================================
+ Coverage    39.57%   41.28%   +1.71%     
===========================================
  Files         1544     1548       +4     
  Lines       193244   199120    +5876     
  Branches      3563     2604     -959     
===========================================
+ Hits         76470    82202    +5732     
- Misses      116209   116353     +144     
  Partials       565      565              

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

@kakkokari-gtyih
Copy link
Contributor Author

image

image

通知にペイロードが乗らないの謎

Copy link
Contributor

github-actions bot commented Sep 1, 2024

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

差分はこちら
--- base
+++ head
@@ -46020,6 +46020,7 @@
                         "followRequestAccepted",
                         "roleAssigned",
                         "achievementEarned",
+                        "exportCompleted",
                         "app",
                         "test",
                         "pollVote",
@@ -46044,6 +46045,7 @@
                         "followRequestAccepted",
                         "roleAssigned",
                         "achievementEarned",
+                        "exportCompleted",
                         "app",
                         "test",
                         "pollVote",
@@ -46259,6 +46261,7 @@
                         "followRequestAccepted",
                         "roleAssigned",
                         "achievementEarned",
+                        "exportCompleted",
                         "app",
                         "test",
                         "reaction:grouped",
@@ -46285,6 +46288,7 @@
                         "followRequestAccepted",
                         "roleAssigned",
                         "achievementEarned",
+                        "exportCompleted",
                         "app",
                         "test",
                         "reaction:grouped",
@@ -78999,6 +79003,50 @@
             ]
           },
           {
+            "type": "object",
+            "properties": {
+              "id": {
+                "type": "string",
+                "format": "id"
+              },
+              "createdAt": {
+                "type": "string",
+                "format": "date-time"
+              },
+              "type": {
+                "type": "string",
+                "enum": [
+                  "exportCompleted"
+                ]
+              },
+              "exportedEntity": {
+                "type": "string",
+                "enum": [
+                  "antenna",
+                  "blocking",
+                  "clip",
+                  "customEmoji",
+                  "favorite",
+                  "following",
+                  "muting",
+                  "note",
+                  "userList"
+                ]
+              },
+              "fileId": {
+                "type": "string",
+                "format": "id"
+              }
+            },
+            "required": [
+              "id",
+              "createdAt",
+              "type",
+              "exportedEntity",
+              "fileId"
+            ]
+          },
+          {
             "type": "object",
             "properties": {
               "id": {

Get diff files from Workflow Page

@zyoshoka
Copy link
Contributor

zyoshoka commented Sep 1, 2024

通知にペイロードが乗らないの謎

ここに追記する必要がありそうです

return await awaitAll({
id: notification.id,
createdAt: new Date(notification.createdAt).toISOString(),
type: notification.type,
userId: 'notifierId' in notification ? notification.notifierId : undefined,
...(userIfNeed != null ? { user: userIfNeed } : {}),
...(noteIfNeed != null ? { note: noteIfNeed } : {}),
...(notification.type === 'reaction' ? {
reaction: notification.reaction,
} : {}),
...(notification.type === 'roleAssigned' ? {
role: role,
} : {}),
...(notification.type === 'achievementEarned' ? {
achievement: notification.achievement,
} : {}),
...(notification.type === 'app' ? {
body: notification.customBody,
header: notification.customHeader,
icon: notification.customIcon,
} : {}),
});

@kakkokari-gtyih kakkokari-gtyih marked this pull request as ready for review September 1, 2024 06:06
@kakkokari-gtyih
Copy link
Contributor Author

いけた🙏

@@ -2436,6 +2438,8 @@ _notification:
followRequestAccepted: "フォローが受理された"
roleAssigned: "ロールが付与された"
achievementEarned: "実績の獲得"
exportCompleted: "エクスポートが完了した"
test: "通知のテスト"
Copy link
Member

@syuilo syuilo Sep 1, 2024

Choose a reason for hiding this comment

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

これらどこで使われるかしら

Copy link
Contributor Author

@kakkokari-gtyih kakkokari-gtyih Sep 1, 2024

Choose a reason for hiding this comment

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

通知フィルタ(↓)

image

Copy link
Member

Choose a reason for hiding this comment

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

あー
通知設定の方って通知の何の設定かしら

Copy link
Contributor Author

@kakkokari-gtyih kakkokari-gtyih Sep 1, 2024

Choose a reason for hiding this comment

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

image

通知受信設定のやつ(ただしtestもexportCompletedも設定不可なので実際には表示されない)

@kakkokari-gtyih
Copy link
Contributor Author

コンフリクト解消

@kakkokari-gtyih
Copy link
Contributor Author

コンフリクト解消

Comment on lines 56 to 59
export const exportableEntities = ['antenna', 'blocking', 'clip', 'customEmoji', 'favorite', 'following', 'muting', 'note', 'userList'] as const;

export const importableEntities = ['antenna', 'blocking', 'customEmoji', 'following', 'muting', 'userList'] as const;

Copy link
Member

Choose a reason for hiding this comment

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

これが何の値なのかの情報量が少ないように感じる

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
export const exportableEntities = ['antenna', 'blocking', 'clip', 'customEmoji', 'favorite', 'following', 'muting', 'note', 'userList'] as const;
export const importableEntities = ['antenna', 'blocking', 'customEmoji', 'following', 'muting', 'userList'] as const;
export const exportableEntityKinds = ['antenna', 'blocking', 'clip', 'customEmoji', 'favorite', 'following', 'muting', 'note', 'userList'] as const;
export const importableEntitiyKinds = ['antenna', 'blocking', 'customEmoji', 'following', 'muting', 'userList'] as const;

とか…? (何なのかわからない、というのは元からどうしようもなさそう)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

userExportableEntities / userImportableEntites とか?

Copy link
Member

Choose a reason for hiding this comment

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

↑ + コメントとかで補足すれば良いんじゃないかしら

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
export const exportableEntities = ['antenna', 'blocking', 'clip', 'customEmoji', 'favorite', 'following', 'muting', 'note', 'userList'] as const;
export const importableEntities = ['antenna', 'blocking', 'customEmoji', 'following', 'muting', 'userList'] as const;
// ユーザーがエクスポートできるエンティティ
export const userExportableEntities = ['antenna', 'blocking', 'clip', 'customEmoji', 'favorite', 'following', 'muting', 'note', 'userList'] as const;
// ユーザーがインポートできるエンティティ
export const userImportableEntities = ['antenna', 'blocking', 'customEmoji', 'following', 'muting', 'userList'] as const;

Copy link
Member

Choose a reason for hiding this comment

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

エンティティというのが何を指しているのか明確にしたいかも

Copy link
Contributor Author

Choose a reason for hiding this comment

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

エンティティというのが何を指しているのか明確にしたいかも

良い感じの表現を募集中

Copy link
Member

Choose a reason for hiding this comment

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

  • この値は何に使われるのを想定しているのか?
  • 既存のデータベースのテーブル名などと一致させる必要があるのか?それとも自由に決めていいものなのか?

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

syuilo commented Sep 26, 2024

ping

@syuilo
Copy link
Member

syuilo commented Sep 26, 2024

マージするか

@syuilo syuilo merged commit d8a2eeb into misskey-dev:develop Sep 26, 2024
34 of 35 checks passed
@syuilo
Copy link
Member

syuilo commented Sep 26, 2024

👍🏻

@kakkokari-gtyih kakkokari-gtyih deleted the feat-14456 branch September 26, 2024 05:16
@tamaina
Copy link
Contributor

tamaina commented Sep 26, 2024

image

ちょっと気になった

@kakkokari-gtyih
Copy link
Contributor Author

image

ちょっと気になった

MkNotificationとかの問題そう

HotoRas pushed a commit to HotoRas/misskey-neko that referenced this pull request Sep 27, 2024
* feat: エクスポート完了時に通知を発行するように

* Update Changelog

* entitity -> entity

* fix: ペイロードを含むように

* fix icon

* exportableEntities -> userExportableEntities
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 packages/sw
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants