misskey-dev / misskey

🌎 A completely free and open interplanetary microblogging platform 🚀
https://misskey-hub.net/
GNU Affero General Public License v3.0
10.12k stars 1.38k forks source link

コントロールパネルの全般ページでサーバ情報を編集・保存するとnullな項目が空文字に置き換わる #15075

Open samunohito opened 2 hours ago

samunohito commented 2 hours ago

💡 Summary

Image 取得時は↑のような内容なのですが、shortName(画面上ではサーバ略称)だけを空欄にして保存ボタンを押すと

Image Image ↑空文字が送信され、nullだった項目が空文字となってしまいました。

🥰 Expected Behavior

ClientServerService.tsなど、フィールドがnullであることを期待する処理はいくつかあるので、空文字の場合はnullとして登録するのが望ましい挙動と考えます。

🤬 Actual Behavior

nullだったフィールドがフロントエンドを経由することで空文字として登録される

📝 Steps to Reproduce

  1. サーバ情報などがnullなサーバを用意する
  2. コントロールパネルの全般ページでサーバ情報を更新する

💻 Frontend Environment

* Model and OS of the device(s): any
* Browser: any
* Server URL: any
* Misskey: 2024.11.0

🛰 Backend Environment (for server admin)

* Installation Method or Hosting Service: any
* Misskey: 2024.11.0
* Node: any
* PostgreSQL: any
* Redis: any
* OS and Architecture: any

Do you want to address this bug yourself?

samunohito commented 2 hours ago

サーバ側の実装を見るとnullも受け付けることから、

のような仕組みをフロントエンド側に設けるのが良いと考えます

syuilo commented 1 hour ago

バックエンド側でnullと空文字が区別される状態になっているのが諸悪の根源だわね

samunohito commented 1 hour ago

(変えてないはずの値を別の値にして送り返してる…という所に対して違和感を感じるのだけど、僕だけかしら…)

syuilo commented 1 hour ago

別の値にしてしまうのはバックエンドがおかしな値を送ってきてるせいだからフロントエンドの実装にそこまで違和感はないわね

samunohito commented 1 hour ago

本来、meta.nameなどで「空の値」として想定されるのは空文字であるということです…? (だとすると、テーブル定義はなぜnullableに…?という話に)

syuilo commented 1 hour ago

本来空の場合は空文字にしたい nullableは歴史的経緯だわね

syuilo commented 1 hour ago

まあ空文字で持つとしてもAPIのレスポンス的にはnullにしたい可能性があるからその場合はフロントエンドで何かしらのケアは要るわね

syuilo commented 1 hour ago

今からカラム定義を変えるのは現実的ではないからバックエンド的には空文字を許さない方向にして、フロントエンドから空文字が飛んできたらnullとみなすようにするのが良さそう

syuilo commented 1 hour ago

本来空の場合は空文字にしたい nullableは歴史的経緯だわね

理由: 型が簡潔になり、空判定も簡潔になる

samunohito commented 1 hour ago

フロントエンドから空文字が飛んできたらnullとみなすようにするのが良さそう

undefinedがきた -> その値は変えない null -> nullにする 空文字 -> nullにする

なら、まあ…いいのかな…?

fruitriin commented 1 hour ago

明示的にnullになることに依存した実装がなければ、BEでnullを空文字にして保存、DBのnullを空文字変換、非nullableかにマイグレーションするほうが今度の展開的によい気がする


追記: カラム定義変更が難しい理由ってあるっけ(DBのマイグレーションに時間がかかるのはダウンタイムを予告しておけばいいのではないかなあ)