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

enhance: 編集を連合させる(送信のみ) #11945

Closed
wants to merge 3 commits into from

Conversation

tai-cha
Copy link
Contributor

@tai-cha tai-cha commented Oct 1, 2023

What

編集を連合に送信可能にする仕組み
ざっくりUpdateでNoteのObjectの更新を通知する、Noteにupdate属性を足す(updatedはObjectの属性にある)

https://www.w3.org/TR/activitystreams-vocabulary/#dfn-updated:~:text=True-,updated,-URI%3A

Why

編集を残すのであれば連合しないことは混乱を招く
-> すでに連合されているノートで編集ができるので一応書いてみた

Resolve #11929 (comment)

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 the packages/backend Server side specific issue/PR label Oct 1, 2023
@codecov
Copy link

codecov bot commented Oct 1, 2023

Codecov Report

Attention: 122 lines in your changes are missing coverage. Please review.

Comparison is base (6840434) 79.00% compared to head (a33b5ca) 78.89%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #11945      +/-   ##
===========================================
- Coverage    79.00%   78.89%   -0.11%     
===========================================
  Files          928      753     -175     
  Lines        99089    76216   -22873     
  Branches      7913     7416     -497     
===========================================
- Hits         78289    60134   -18155     
+ Misses       20800    16082    -4718     
Files Coverage Δ
packages/backend/src/core/CoreModule.ts 100.00% <100.00%> (ø)
packages/backend/src/core/GlobalEventService.ts 98.79% <100.00%> (ø)
packages/backend/src/core/NoteCreateService.ts 88.22% <100.00%> (+0.01%) ⬆️
.../backend/src/core/activitypub/ApRendererService.ts 81.85% <100.00%> (+0.02%) ⬆️
...ckend/src/core/activitypub/models/ApNoteService.ts 65.58% <100.00%> (+0.08%) ⬆️
packages/backend/src/core/activitypub/type.ts 95.13% <100.00%> (+0.01%) ⬆️
...s/backend/src/server/api/endpoints/notes/update.ts 77.27% <36.36%> (-0.26%) ⬇️
packages/backend/src/core/NoteUpdateService.ts 51.88% <51.88%> (ø)

... and 193 files with indirect coverage changes

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

@tai-cha
Copy link
Contributor Author

tai-cha commented Oct 1, 2023

まだ動くかどうかも試していない

一通り書いて編集可能になっていることを確認しただけ

@tai-cha
Copy link
Contributor Author

tai-cha commented Oct 1, 2023

多分余計な部分と足りない部分が多い(雑にノート作成と削除あたりからキメラを作ったため)

@16439s
Copy link

16439s commented Oct 2, 2023

まだ動くかどうかも試していない

一通り書いて編集可能になっていることを確認しただけ

検証したところ、それをコミットして、適応させた状態で編集しても、Mastodonでは編集したのが反映されていなかった。なので、連合できていないと思う...
※私の環境だけかもしれない。
※失礼いたしました。

@tai-cha
Copy link
Contributor Author

tai-cha commented Oct 2, 2023

動作報告助かります
(なるべく早く実装したい気持ちはあるのですが私の時間が確保しにくい問題もあるので巻き取っていただける方は募集しています)
(とはいえ早急に欲しいものなので最大限の努力はする)

@tai-cha
Copy link
Contributor Author

tai-cha commented Oct 3, 2023

普通にupdated足してなかった

@tai-cha
Copy link
Contributor Author

tai-cha commented Oct 3, 2023

もし参照のほう作るなら多分ApNoteServiceのresolveNoteでexistがtrueなときにupdatedAtとtext,cwの書き換えをしてあげる必要があるな

@tai-cha
Copy link
Contributor Author

tai-cha commented Oct 3, 2023

照会もcreate前提になってる…?

@tai-cha
Copy link
Contributor Author

tai-cha commented Oct 3, 2023

一緒に送信だけじゃなくて受信や照会も書けるかなと思ったけど結構見るべき点が多い

@syuilo
Copy link
Member

syuilo commented Oct 4, 2023

機能自体キャンセルする可能性が高まってきたので作業はストップしていただけると🙏

@tai-cha
Copy link
Contributor Author

tai-cha commented Oct 4, 2023

OKです、直近で追加の作業はしないです

更新の方は実はできている説がある(検証できてない)

あと受信の方は…Mastodonだけじゃなく他が対応し始めてきてるのですぐ出せなくても取り組む必要はあると思うし取り込みたいんですよね

@syuilo
Copy link
Member

syuilo commented Oct 4, 2023

んー受信対応するとなると実質的に編集機能の実装と同じと思われるから、編集の実装を諦めるのであれば受信も対応したくない感

@syuilo
Copy link
Member

syuilo commented Oct 4, 2023

考慮するべきことがいかんせん多すぎる

@syuilo
Copy link
Member

syuilo commented Oct 4, 2023

削除して編集を推していきたい

@tai-cha
Copy link
Contributor Author

tai-cha commented Oct 4, 2023

なるほどなあ 実装コストが高いという点では割と元々Updateする前提じゃなかったからという点があるのでメンテナンスのコスト的には爆増するほど複雑な実装にはならない気がしています

Misskeyとして削除して編集を推したいのもわかる(私は元々なんなら編集自体には消極的立場なため)

一方で受信に関してはMastodon, Firefish, Sharkeyなども対応しててMisskeyでだけ見える情報が違うとなるのでもう少し検討する余地はあると思っています

@syuilo
Copy link
Member

syuilo commented Oct 4, 2023

まあ対応するとしてもAP周りのユニットテスト/e2eテストが充実してからかしらね

@tai-cha
Copy link
Contributor Author

tai-cha commented Oct 4, 2023

(ただ受信に対応するほうが倍は大変なので受信やるなら送信を入れることはそんなに難しくないみたいところはある)

@tai-cha
Copy link
Contributor Author

tai-cha commented Oct 4, 2023

さすがにテストも書かないといけないなあとは思っていました(それはそう)

@syuilo
Copy link
Member

syuilo commented Oct 4, 2023

一旦ノート編集機能は撤回されました 🙏🏻

@syuilo syuilo closed this Oct 4, 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
Development

Successfully merging this pull request may close these issues.

連合する編集機能
3 participants