独自機能の追加などで他の部分への影響が分かるようにCI周りの修正

[S-H-GAMELINKS]

kmy.blue独自の機能変更の他の部分に影響を与えたかどうかをチェックできるように、テストをパスするように修正しています。

[github-actions]

This pull request has merge conflicts that must be resolved before it can be merged.

[github-actions]

This pull request has resolved merge conflicts and is ready for review.

[kmycode]

@S-H-GAMELINKS
プルリクエストありがとうございます。先日はご迷惑おかけしましたこと、改めてお詫びします。

お恥ずかしながらお聞きしたいのですが、この変更の中には自動生成されたコードも含まれるように見えます。その部分はどのようにして生成されたのかご教授くださいますでしょうか？こちらも今後の参考にさせていただきたいです

[S-H-GAMELINKS]

お恥ずかしながらお聞きしたいのですが、この変更の中には自動生成されたコードも含まれるように見えます。その部分はどのようにして生成されたのかご教授くださいますでしょうか？こちらも今後の参考にさせていただきたいです

自動生成というのがどの部分のコードを指しているのかはちょっと意図が掴みかねているところがありますが、基本的にやったこととしては

• haml-lint
• eslint
• rubocop
  などのlinterでコードのスタイルをチェックし、修正している感じですね

一部bundle exec rubocop -Aやbrakemanなどで自動的にコードを修正しているものもあったと思いますが、それ以外は基本的にlinterを走らせてコードの修正箇所を洗い出しつつ、修正しています。

[kmycode]

@S-H-GAMELINKS
基本的にはマージさせていただきたいのですが、下記複数の点について確認させてください。
こちらもRubyを始めて日が浅いため、質問が４つも出てしまいました。Rubyに対する誤解などありましたら大変申し訳無いのですが、差し支えなければご教授いただきたく存じます。

なお先ほどの自動生成の話の意図としては、config/brakeman.ignoreファイルを生成する方法をお聞きしたかったです。
知識ゼロの私の想像で申し訳ないのですが、このファイルは拝見する限り「この部分に脆弱性がありますがテストのときに無視します」と書いているように見えました。
これらの脆弱性を無視しても構わない理由はさておき、まずはこのファイルをどのように生成したか確認したいです。
ファイル名の通りbrakemanというプログラムを使って生成されたことが分かれば、あとはこちらで調査いたしますので大丈夫です。

1. app/models/custom_emoji.rbでdependent: :destroyを削除した意図は何ですか？

# Old
has_many :emoji_reactions, inverse_of: :custom_emoji, dependent: :destroy
# New
has_many :emoji_reactions, inverse_of: :custom_emoji

このような更新がありますが、dependent: :destroyは、カスタム絵文字を削除したときに、そのカスタム絵文字を使用した絵文字リアクション（スタンプ）を削除することを意図して記述しました。この処理はすべきではないということでしょうか？

2. app/models/user_settings.rbでsetting :warning, default: trueを追加した意図は何ですか？

setting :warning, default: true

notification_emails.warningという設定が追加されています。おそらく警告を受け取った時のメール受信設定ですね。kmyblueに現在そのような設定項目は存在しないとの認識でしたが、この行を追加した意味を差し支えなければ教えてくださりますでしょうか？

3. app/services/update_status_service.rbで@status.reloadを追加した意図は何ですか？

@status.reload

投稿を編集した後に、この処理が入っています。reloadメソッドについて、Ruby素人ながら調べましたが、投稿のプロパティをDBから取得し直す（あるいはキャッシュから取り直す）メソッドだと認識しました。
https://k-koh.hatenablog.com/entry/2020/12/28/214557
投稿の編集が完了して、編集後の最新情報をいろんなところに配る処理も終わった後で、reloadメソッドの呼び出しが必要な理由を教えて下さい。

なおこの記述は本家Mastodonnにはありません。（２０２３年８月７日現在のMainブランチ）
https://github.com/mastodon/mastodon/blob/main/app/services/update_status_service.rb
この記述が必要になった理由として心当たりがあるかといえば最近追加した参照処理ですが、ここで私が至らぬミスでもしてしまったのでしょうか？

4. app/views/settings/profiles/show.html.hamlでコメントを外した意図は何ですか？

-# Old
  -#.fields-group
-# New
  .fields-group

gitを使っているのに古いコードをコメントアウトしてしまった私にも落ち度はあるのですが、この変更はコメントアウトしてしまった行を復活させたようにお見受けします。理由を差し支えなければ教えて下さいますでしょうか？

[S-H-GAMELINKS]

brakemanの設定の自動生成に関して

brakemanのREADMEを参考に以下のコマンドを経由してIgnoreする設定を自動的に追加しています。

brakeman -I

なので、今後はこのコマンドを実行していただければOKだと思います。

修正箇所に関して

基本的にすべてCIがパスしないものだったため修正している感じです。
いくつかのものに関してはCI側またはテスト側を直すほうが良いかと思っていますが、一旦CIがパスしない事には全体の修正箇所が把握できなかったため一時的な対応としてコミットしています。

1. app/models/custom_emoji.rbでdependent: :destroyを削除した意図は何ですか？

こちらはMasotodon本体側でチェックしているマイグレーションのテストに失敗しているため削除しています。
通常通りrails db:migrateでマイグレーションを実行する分には削除前のコードでもOKなんですが、CIがパスしないと全体の修正範囲などが把握できないため一時的な対応としてコミットしています。

またコードを追った感じではカスタム絵文字が削除された際に関連するテーブルを削除したいということだとは理解していますので、特に問題がなければCI側をrails db:migrateが通っているかを確認するだけにすればよいかなと考えてますね。

2. app/models/user_settings.rbでsetting :warning, default: trueを追加した意図は何ですか？

こちらのコミットで導入された警告関係の対応の影響で、ユーザー設定周りにもsetting :warning, default: trueを渡す必要がでていたからですね。

e7492e0#diff-f0b426af9a51133db8f6a51f2ab156946995f7386bd4ad1641b48f4b7521ed13

上記のコミットで追加されたwarningが別の箇所で参照されており、おそらくそれによりユーザー設定が意図していない形で書き換わっている可能性がありそうです。

そのため一旦テストがパスするように追加している次第ですね。

3. app/services/update_status_service.rbで@status.reloadを追加した意図は何ですか？

メンション先を投稿の編集で切り替えるテストで意図しない挙動になっていたため追加しています。

コメントされている通り参照機能に現状ではバグがあり、以下のような手順でメンション先を投稿の編集から変更すると書き換わらないという挙動があるようです。

• Aさんという人に誤ってメンションして投稿
• 投稿の編集機能を使い、Bさんにメンション先を変更
• が、Aさんにメンションしたままになっている

なお、こちらはテスト上で確認できているだけなので本番環境でも再現するかは確認がとれておりません。

デバッグツールなどを使い、確認した限りだと参照の更新部分の処理で編集前の投稿情報が保持されたままになっていました。
そこでreloadメソッドを使用し、DBから取得しなおすようにしています。

4. app/views/settings/profiles/show.html.hamlでコメントを外した意図は何ですか？

こちらはhaml-lintで引っかかったためコメントアウトを解除しています。
使われていないようなので削除でもよいかと思ったんですが、そのあたりの匙加減が良くわからなかったので一旦コメントアウトを解除して対応しています。

不要であれば削除しておきます。

[kmycode]

@S-H-GAMELINKS
丁寧なご説明・ご指摘ありがとうございます。いったんこれでマージさせていただき、細かいところはこちらで修正いたします。
お手数おかけしてしまい申し訳ありません。

3.の参照機能のバグについてはこちらももともと承知しております。
投稿の編集時に投稿の参照を書き換える処理はWorkerを使って呼び出していますので、別のプロセスになります。これは参照によってリモートの投稿を取得しに行くことがあるため、このようにしています。
そのため、編集が終わった時点で参照は書き換わらなくても仕方ない想定です。参照通知のほうはWorkerの中から発行しますので大丈夫です。
最初の投稿時にも参照処理をWorkerで呼び出しますので、投稿時にも同じ問題が発生する可能性はあると思います。こちらの場合は、投稿したはずなのに参照データがついていないというものです。
現在これに関する修正案を考えている途中です。ただ編集APIを叩いた結果と実際のデータが食い違うのはあまりよくない状態なので、直せたら直したほうが良さそうですね。

2.の通知の方はちょっと想定から漏れておりました。これはあなたに警告メールを送りましたよという通知で、警告メールが別途いくので当サーバーとしてこの設定項目は不要なのですが、変更を最低限にするためにトマソンみたいにするのはありかもしれませんね。

[S-H-GAMELINKS]

すみません、少なくともCIが通っているのを確認してからマージしたほうがいいと思います........

[kmycode]

あ、ごめんなさい、たしかにそのとおりでした。。
代わりになるか分かりませんがこちらでローカルでテストします