# 3/12 sariさんポートフォリオレビュー会 ## サービス ### URL https://karoyaka.site/ ### Github https://github.com/sarii0213/karoyaka ### 概要 本アプリの目的は、ものを手放す活動 ー 捨て活 の習慣化をサポートすること。アプリの詳細は[GitHubのREADME](https://github.com/sarii0213/karoyaka/blob/develop/README.md)に。 ## レビュー ### サービス面 #### todaka - 一度登録すると捨てなきゃと思えるので、断捨離のフローが用意されているのはありがたかったです!気になってたものをようやく捨てることができました。 - 手放すものの登録画面へのアクセスが気になりました。 - ボタンが画面のリストの下の方にあれば、スマホ片手持ちでも使いやすいのかなと思いました。 - 個人的にはボトムナビゲーションに+ボタンを置いてto_let_go_items#newに飛ばしても良いかなと思いました。 [![Image from Gyazo](https://i.gyazo.com/69777a2c3abd67dc8229653c757afe08.png)](https://gyazo.com/69777a2c3abd67dc8229653c757afe08) - コンテンツとして足すなら、捨てるものの探し方などまとめて載せても良いかなと思いました。書類は全捨てが基本、洋服は心がときめくものだけ残す、など (近藤麻理恵さんの書籍の内容ですが) - Topページのナビゲーションについて、サービスのロゴなどつけると、キャッチーになるかと思いました。(現状の「トップページ」だと味気ない気が) #### だいそん - デザインがシンプルで良いと思いました。ガチャガチャしてなくてスマートで良いですね。 - 確かテンプレートを使ってたと思うのですが、どれを使ったんでしたっけ? - https://themeforest.net/item/appkit-mobile/27679559 - 捨て活というコンセプトが新鮮。 - SNSログインがあった方が助かる - アイコン形式のボタンが所々何を意味するのかわかりづらい(画面右上のログインっぽいボタンとか) - 画像のアップロードに時間がかかるのでそこの改善にチャレンジしてみるとレベルアップできそう - ファイル選択時にバックグラウンドでアップロードしておく - フロント側でリサイズしてそれを送る - etc - 全体的にサブミットボタンを押した時、ボタンのスタイルが変わらないのでサブミット中なのかどうかがわかりづらいかも。たぶんデフォではサブミット中は`disabled`クラスが付与されるはず?? - 写真アルバムから選択できないようにしたのは何か理由がある? - カテゴリーを選択すると手放す方法のヒントが動的に変わる部分は実装が案外面倒だったんじゃないかなと思います。 #### Kei - デザインがきれいなので、ファーストインプレッションで本格的なアプリを作っている、という印象を与えられると思います! - コンセプトや操作もシンプルで、「断捨離」的なことって今の時代にあっているのかと思います! - 利用規約やプライバシーポリシーなどを細かいところまで作り上げているのが良いですね! ### 実装面 #### だいそん - 個人開発なのにしっかり[PR](https://github.com/sarii0213/karoyaka/pulls?q=is%3Apr+is%3Aclosed)を作って開発をしてて素晴らしい - [コミットメッセージ](https://github.com/sarii0213/karoyaka/commits/develop)も丁寧 - CI入れてるのは好印象です - [エンドポイント](https://docs.google.com/spreadsheets/d/1zr-zw30afOB7XARiWX79aH-iVoj1HBa22X23bARWLJY/edit#gid=0)や[ER図](https://user-images.githubusercontent.com/105996822/216992444-b4d614cf-6cf2-47ec-8fd4-577a4780c27a.png)がこのようにまとめられてると助かります - `/to_let_go_items/show_hint` は `/to_let_go_items/hint`でもよかったかも?対応するアクションは`to_let_go_items/hints#show`とか? モバイルのsafari?だと入力しようとした時にズームされます。 ![](https://i.imgur.com/68btsOV.png) あるあるなんですが、これはfont-sizeを16px以上にすると解消されたはず。 https://kasumiblog.org/ios-input-zoom/ - 本番ではsidekiqが使われていない? https://github.com/sarii0213/karoyaka/blob/develop/config/environments/production.rb#L62 - で、もし本番でもsidekiqを使うのであればダッシュボードも見れるようにした方が良さそう。 - https://github.com/sarii0213/karoyaka/blob/develop/config/routes.rb#L88-L91 - deviseとの連携はこちらを参照 - https://github.com/sidekiq/sidekiq/wiki/Devise - `config/initializers/sidekiq.yml`には`active_storage_analysis`や`active_storage_purge`に関する記述は不要でしたっけ - https://qiita.com/west2538/items/f932978e79220b880693 ```rb= def show_hint if params[:category_id] && params[:reason_id] optimal_ways_with_category_reason(params[:category_id], params[:reason_id]) elsif params[:category_id] @way_ids = CategoryWayOptimality.where(category_id: params[:category_id]).order(score: :desc) .limit(3).map(&:letting_go_way_id) else @way_ids = ReasonWayOptimality.where(reason_id: params[:reason_id]).order(score: :desc) .limit(3).map(&:letting_go_way_id) end respond_to do |format| format.turbo_stream end end def optimal_ways_with_category_reason(category_id, reason_id) category_scores = CategoryWayOptimality.where(category_id:).map(&:score) reason_scores = ReasonWayOptimality.where(reason_id:).map(&:score) scores = [category_scores, reason_scores].transpose.map { |ary| ary.inject(:*) } @way_ids = [] 3.times do |_| @way_ids.push(scores.index(scores.max) + 1) scores[scores.index(scores.max)] = 0 end @way_ids end ``` ダイソン追記 ``` LettingGoWay.where(id: [1, 2, 3]) ``` ここはなんとかリファクタしたい。 まずコントローラではなくモデルに書いた方が良い。その方がテスト書きやすいので。人によってはサービスクラスに切り出すかもしれないが、安易にrailsの標準以外のディレクトリを作るのは避けた方が良い。きちんとルールを決めて運用しないとカオスになりがちなので。 現場Railsの最終章(10-12-2 共通処理を担当するオブジェクトを別に作って連携させる)にモデルに切り出す手法が書かれてる。ただ今回の場合はPOROではなくシンプルに`letting_go_way`モデルにメソッドを生やしてあげても良さそう。インターフェース的には ``` @ways = LettingGoWay.optimal(category_id:, reason_id:) ``` とか。 下にも書いたが`id`の配列を返す必要性があるのか。単純に`ActiveRecord::Relation`じゃいけない? ```rb= <ol> <% @way_ids.each do |way_id| %> <li><%= LettingGoWay.find(way_id).name %></li> <% end %> </ol> ``` https://github.com/sarii0213/karoyaka/blob/develop/app/views/to_let_go_items/show.html.erb#L19-L23 ここって`@way_ids`の数分だけSELECTが発行されるがもっといい方法ありそう。`@ways`にしといて`way.name`にするとか。 ```erb= <div class="card card-style"> <div class="content text-center"> <h3 class="font-18">捨て活 <span class="font-24"><%= @day_what %></span>日目</h3> <p class="font-10 line-height-xs color-green-dark my-3"> <%= render 'chart', days: @days %> </p> <p class="font-12 mb-1">捨て活 とは、<br>手放したいものを見つけたり 実際に手放すこと。<br>コツコツ手放して 軽やかな生活を。</p> </div> </div> ``` この部分もパーシャル化しちゃってよいかも?2か所で使われてるので。 #### Kei - コミットメッセージの絵文字可愛いですね! - quotesを作成、更新、削除あたりをどうするのか気になりました!seedファイルを編集する形ですかね? - ここの例外はどういう状況を想定していますか?(単に気になったので質問いたしました!) ```ruby= # app/models/user.rb def favorite(quote) favorite_quotes << quote rescue ActiveRecord::RecordInvalid false end ``` - itemモデルでSTI(単一テーブル継承)使っているの良いですね! - scopeにまとめられそうな箇所がいくつかありそうです! ```ruby= # 例: app/controllers/to_let_go_items_controller.rb ~.order(score: :desc).limit(3).map(&:letting_go_way_id) ``` - アクション内の処理が複雑なものが比較的多いので、モデルにロジックを寄せたり、フォームオブジェクトなどを利用してスッキリ書けそうな気がしました!(具体的な提案ができずすいません…) #### Tomoya - 他の方とも重なりますがPR・コミット数も多く、コミットメッセージも凝っていて丁寧な印象を受けました。このコミットメッセージってそれぞれ意味を持たせてたりしますか?(私の会社とかだと、破壊的な変更を含むものや修正や機能追加っていうコミットの内容に合わせてコミットメッセージの絵文字を選択してます。) - メニューの表示位置が下に固定になっていることが気になりました。基本的にスマホからの使用を想定している感じですかね?もしPCからの利用も想定しているのであればPCの場合は上にあった方が直感的かなあとは思いました。(デザインは、[Z,Fの法則](https://world-mart.jp/blog/research/skill/skill-eyes-rule/#:~:text=Z%E3%81%AE%E6%B3%95%E5%89%87%E3%81%AF%E3%80%81%E3%83%81%E3%83%A9%E3%82%B7,%E3%81%A8%E8%A8%80%E3%82%8F%E3%82%8C%E3%81%A6%E3%81%84%E3%81%BE%E3%81%99%E3%80%82)みたいなのがあったきがします) - 手放し登録、手放したものに対してやっぱいつ捨てたんだっけみたいな部分って結構思い出というか気になるところだと思っていて、なので日時情報あるといいかもなと思いました。 - 私がはじめ利用しようとした際に感じたのはなぜものを捨てるために入力しなきゃいけないんだっけ・・・?ってところだったのでその辺りをイメージできるといいなと思いました。 #### naoki __感想__ - 名言好き - ぱっと何がやれるのかわかるのは良いなと思う - これ > + 個人開発なのにしっかりPRを作って開発をしてて素晴らしい > + コミットメッセージの絵文字可愛いですね! - chatGPTとかで捨て方の提案とかしてくれないかなぁ(chatGPT使いたいだけ) __UI__(web向けの場合を含みます) - buttonをhover時に要素の大きさが変わるのが気になりました。 [![Image from Gyazo](https://i.gyazo.com/76d24ea8a94a92d7f6c47303838b4015.gif)](https://gyazo.com/76d24ea8a94a92d7f6c47303838b4015) - 利用規約などがスクロールせずに表示されていると嬉しいと思う。 - 手放すもの選択時のプレビューと登録後のサムネイルのobject-fitが違うような気がする - 手放す画面 -> 登録 -> 登録したものの手放すボタン -> 手放す画面 -> 登録 <- また登録?っと感じた - アカウント設定画面で画像のアップロードをしたのちにパスワードを間違えるなどのエラーが起こると選択した画像が消えてしまう。 __code__ - 不要なcontrollerなどは削除したいですね - `to_let_go_items`が何の機能を示してるのか想像つかないですね - `/app/views/done_letting_go_items/_form.html.erb` ```= <% if @to_let_go_item %> <%= f.collection_select :reason_id, Reason.selectable, :id, :name, { selected: @to_let_go_item.reason_id, prompt: '手放す理由を選択する'}, { class: 'form-control' } %> <% else %> <%= f.collection_select :reason_id, Reas on.selectable, :id, :name, { prompt: '手放す理由を選択する'}, { class: 'form-control' } %> <% end %> ``` こんな感じに簡潔に書けないかな?っと思った ```= <%= f.collection_select :reason_id, Reason.selectable, :id, :name, { selected: @to_let_go_item&.reason_id || 0, prompt: '手放す理由を選択する'}, { class: 'form-control' } %> ``` - もっとmodelに処理を寄せても良い気がする。modelに関連とvalildation以外ほぼ書かれていない気がする(機能的にそうなっているだけかも?) #### todaka - (既出ですが)コミットとPRが丁寧だなと思いました。アイコンはどうやって設定・表示しているか教えてほしいです。 - IssueとPRを紐付けてプロジェクトを管理している点も良いなと思いました。 - アイテムの管理にどうしてSTIを使ったか教えてほしいです(指摘ではなく、自分があまり詳しくないので質問です) ``` def create current_user.favorite(@quote) redirect_to xxx rescue ActiveRecord::RecordInvalid redirect_to xxx end ```