# Keiポートフォリオ
### サイトのURL
https://listing-app.net/
### ダミーユーザー
|権限|email|password|ログインページ|
|:-|:-|:-|:-|
|一般|guest@example.com|password|https://listing-app.net/login|
|ビジネス|business@example.com|password|https://listing-app.net/business/login|
|アドミン|admin@example.com|password|https://listing-app.net/pvsuwimvsuoitmucvyku/login|
### GitHubリポジトリ
https://github.com/keit1722/listing_app
### DB設計・エンドポイント
https://github.com/keit1722/listing_app_blueprint
### アプリの説明
私の地元の情報のみを扱ったトリップアドバイザーのようなアプリです(口コミ機能無し)。地域貢献・地元の活性化を目的としています。
私の地元は観光地ですが、地域の情報をまとめたサービスが少なかったので作成しました。
地元のお店などに対して営業活動おこない、情報を載せて行く予定です。
「listing app」というアプリ名はとりあえず付けたものです。サービス開始時期が決まっていないため、開始時に正式なものに変更します。
### 主な機能
- CRUD機能
- User
- Organization
- Restaurant
- Hotel
- Activity
- HotSpring
- SkiArea
- PhotoSpot
- Shop
- Post
- Announcement
- 会社や個人事業などの「組織」を登録するの管理者による承認機能
- 既存の組織からユーザーに対しての招待機能
- お気に入り登録機能
- メール通知設定機能
- Google OAuthによるユーザ登録・ログイン
- ユーザーアカウントのアクティベーション機能（Sorcery）
- ユーザーアカウントのパスワードリセット機能（Sorcery）
### 技術的に苦労したところ
- フォームオブジェクトを用いた複数モデル同時登録・編集
- 招待機能
- 承認申請機能
- 共通で使うモデルが多かったのでポリモーフィック関連付け多用した
- ビューの数が多かったためパーシャルを多用した
# レビュー
## だいそん
- READMEに環境構築手順が書いてあるのがGood
- RSpec, rubocop, CIが入ってるのがGood
- ユーザー承認 -> ユーザー認証 が正しい
- `database.for.heroku.ridgepole.yml`ってなんのためにあるんだっけか
- 本番のsidekiqはベーシック認証をかけた方が良い
- ここgetで問題ない？DBの更新系はPOSTかPATCHの方がベター
```rb=
resources :users, only: [] do
member { get :activate }
end
```
- 単一resourceの場合は単数系の方がしっくりくる
```rb=
resource :bookmarks, only: %i[create destroy], module: :restaurants
```
- コントローラのディレクトリもしっかり切られていてGood
- 単なるidではなくslugを使ってるのもユーザーフレンドリーでGood
```rb=
resources :hotels, param: :slug, only: %i[index show] do
```
- 飲食店ごとの投稿ページのURLがちょっとネストが深い気もする。
```
https://listing-app.net/restaurants/ward/posts/5
```
シンプルにこれでも良い気がしてきた。
```
https://listing-app.net/posts/5
```
- リソースごとに営業時間を設定するの大変そうですね。どのように管理してるのか教えて欲しい。
- 飲食店、宿泊施設、アクティビティなど、それぞれ持ってるカラムが同じ（またはほぼ同じ）なのであればSTI（単一テーブル継承）を使うのもありだったかもしれない。そうすればテーブル自体は一つで済む。そうするとポリモーフィック
```rb=
class Listing < ApplicationRecord
# record_type 'Hotel' | 'Activity'
end
class Hotel < Listing
has_many :categories
end
class Activity < Listing
end
```
- しっかり意味のあるまとまりとしてロジックをmoduleに切り出していてGood
```rb=
class Hotel < ApplicationRecord
include ActiveModel::Validations
belongs_to :organization
include Districtable
include Bookmarkable
include Postable
include ReservationLinkable
include OpeningHourable
include PageShowable
```
- ビジネスユーザーと一般ユーザーは完全に分けちゃった方が色々と実装はしやすいかもしれない
- カテゴリ設定のinputは何かライブラリを使ってるんですか？
```rb=
Bookmark.create(bookmarkable: bookmarkable, user_id: current_user.id)
```
```rb=
# app/controllers/concerns/bookmarkable.rb
module Bookmarkable
extend ActiveSupport::Concern # おまじない
included do
before_action :set_bookmarkable, only: [~~~]
end
private
def set_bookmarkable
raise NotImplementError, ~~~
end
end
# app/controllers/activities/bookmarks_controller.rb
class Activities::BookmarksController < ApplicationController
include Bookmarkable
private
def set_bookmarkable
~~~ = Activity.~~~~~
end
end
class Hotels::BookmarksController < ApplicationController
include Bookmarkable
private
def set_bookmarkable
~~~ = Hotel.~~~~~
end
end
```
## みけた
すごい。弊社に来てくれ。
schema.rbが400行は仕事やん。
ただ、これだけじゃレビューとはいえないので、とりあえずcustomer.rbだけ見てみました。
- association, callback, validationあたりの設定が整理されていない部分があるので、整えてあげると見やすくなりそうです
- sendメソッドを駆使していてすげえとなりました。ただ、セキュリティ的に脆弱性を生みやすいので、注意を特に払うようにしてください！
- destroyの引数は不要なのでは？（ちなみに、Article.destroy(1)といった書き方はできるようです: https://pikawaka.com/rails/destroy#destroyメソッドの引数
```rb=
def unbookmark(bookmarkable)
send(bookmark_object(bookmarkable)).destroy(bookmarkable)
end
```
```rb=
def resign(organization)
organizations.destroy(organization)
end
```
あと、BookmarksControllerとActivities::BookmarksControllerの関係なんかを見て、
継承を使って面白い実装をしているなと思いました。あくまで個人的な意見ですけど、継承していることが分かりづらいので、
こういう書き方をしてみるのも良いのかもしれないなと思いました。
変更前
```rb=
class BookmarksController < ApplicationController
before_action :require_login
def create
current_user.bookmark(@bookmarkable)
render 'listings/bookmarks/create'
end
def destroy
current_user.unbookmark(@bookmarkable)
render 'listings/bookmarks/destroy'
end
end
class Activities::BookmarksController < BookmarksController
before_action :set_bookmarkable, only: [:create, :destroy]
private
def set_bookmarkable
@bookmarkable = Activity.find_by(slug: params[:activity_slug])
end
end
```
変更後
```rb=
class BookmarksController < ApplicationController
before_action :require_login
before_action :set_bookmarkable, only: [:create, :destroy]
def create
current_user.bookmark(@bookmarkable)
render 'listings/bookmarks/create'
end
def destroy
current_user.unbookmark(@bookmarkable)
render 'listings/bookmarks/destroy'
end
private
def set_bookmarkable
# 必ず継承先でオーバーライドすること
raise NotImplementedError '@bookmarkableがsetされていません'
end
end
class Activities::BookmarksController < BookmarksController
private
def set_bookmarkable
@bookmarkable = Activity.find_by(slug: params[:activity_slug])
end
end
```
インスピレーションは、ここで受けています。
https://qiita.com/joker1007/items/9da1e279424554df7bb8
ただ、継承っていうのは、共通化させる作業なので、継承先の一部においてのみで特殊な分岐をしたいって場面が増えてくると、
ロジックがとにかく汚くなって破滅の道へと突き進むイメージです。
（奇特な事例しか思いつかないですが、例えば、activityのときだけrenderするtemplateを変えたい場合など。）
https://twitter.com/minodriven/status/1353251239237095430
このケースは、もしかしたらBookmarksControllerを削除して、Activities::BookmarksControllerでモジュールをincludeする形がよいかもしれないです。
私だったら、モジュールをincludeすることもせず、DRYには反しますが、同じコードをペタペタ貼るほうを選択するかなと思いました。
私が間違っていることを言っている可能性は十分にあるので、だいそんさんに相談してみてください。
## maki(wip)
PF作成、お疲れ様です！
自分も全然レビューできる立場の人間ではないので、参考程度にしてもえると！
- 全体的な感想
- すごい！サイト全体が見やすくいい感じで、素直にすごいなと思いました！
- TBL設計
- 営業時間のstart_time_hourやstart_time_minute などがstringで定義しているのって何か、こだわりがあったりしますか？ 個人的にはtimeにした方が良いと思いました。理由は、バリデーションをかけるのも辛いし、今後、「現時刻で営業中の場所を絞り込む」みたいな機能を作るのも結構辛そうな気がします。
```start_time_hour, [*0..23]```みたいに書くのも、dateにしてrubyでロジックを考えた方が良さそうな記載しました。
- 施設情報は持ってるカラムは同じなのですかね?まとめられそうな。。。
- その他・質問等
- 1つの温泉宿で宿泊してない人もレストランを利用できるみたいな、複数カテゴリにまたぐ施設があった場合データはどう持ちますかね？
- schema.rbが400行はすごいですが、table毎にファイル分割されてた方が好みだったりします。個人で作る場合は関係ないかもですが、複数人開発だとコンフリクトが起こりそうです。
（だいそん） Time.zone.parse("2022-01-01"), Time.zone.local(2022, 1, 1)
rails console ->
eigyojikan.update(start_time_hour: '12:00')
eigyojikan.start_time_hour # 2001 1 1 12:00
gem month
https://github.com/readysteady/month
## Shun
- 感想
- 最初ページを開いた時、すでに運営されてるサイトかなと思ったくらいの完成度です。
- このポートフォリオだと企業の引く手あまたな気がします！
- 気になった点
- メールアドレスでユーザー登録したのですが、認証のメールが送られてこず...迷惑メールBOXも見たのですが見当たらずでした。再度試して、「メールアドレスはすでに存在します」のフラッシュが表示されているので、メール送信で不具合が発生しているのかなと...(自分の確認ミスでしたら申し訳ないです)
- ユーザー登録や編集ページにおいて、フォーム部分が冗長な気がするので部分テンプレート(`_form.html.erb`)で書き出したほうがいいかなと思いました。
- `app/views/shared/_header_navigation.html.erb`や`app/views/shared/users/_show.html.erb`のアバター画像表示のロジックですが、 user登録の際にデフォルト画像を登録する方法もあります。そうすると、以下のロジックだけで済みます。
```ruby
<div class="avatar-preview margin-bottom-30">
<% if user.avatar.attached? %>
<%= image_tag user.avatar %>
<% else %>
<%= image_tag 'blank-profile-picture.png' %>
<% end %>
</div>
↓修正後
<div class="avatar-preview margin-bottom-30">
<%= image_tag user.avatar %>
</div>
```
```ruby
# app/models/user.rb
class User < ApplicationRecord
before_create :default_avatar
def default_avatar
return avatar if avatar.attached?
avatar.attach(io: File.open(Rails.root.join('app/assets/images/blank-profile-picture.png')),
filename: 'blank-profile-picture.png')
end
end
```
参考:[Active Storageでデフォルト画像を設定する - qiita](https://qiita.com/mashihiko_397/items/569384b8f8f333b3168e)
- 細かいですが、レイアウト崩れしておりました。
[](https://gyazo.com/c4f558bf8158d10c95d57e0736ffc693)
[](https://gyazo.com/e89d4f130bf1957c81d1532ca070644f)
- 良かった点
- `to_params`いいですね。`/users/1`よりずっと親切設計です。
- インデントや改行がきちんとされているのでコードが見やすいです。
## Todaka
### 感想
- ポートフォリオ作成、お疲れ様でした！この規模で作り切るのは本当に凄いと思います。
- 他の方も言われてますけど、凄すぎて個人開発感はゼロですね笑
- ダミーデータも気合が入っているので、実際に使われているような 印象を受けました！
### その他コメント
- お店やスキー場などのリスト表示について、リストの件数をコントローラでカウントしてビューに渡す必要があるか気になりました。ビュー側で`listings.count`と書くだけでは表示されないですかね？
- 各コントローラにある`render layout: 'listings_index'`や`render layout: 'listings_single'`の挙動を教えていただきたいです。
- プロフィール画面について、見出しの説明がちょっと冗長な気がしました（特にスマホ表示）。なくても意味がわかる項目は見出しを省いても良いかもしれません。
- [](https://gyazo.com/bb8230d4ca94239a880b8e054e456783)
- プロフィール画面について、特定の状況（15.6型ディスプレイを縦置き）で見ると、メニューバーの表示が少し変な感じです。ただ、横置きのディスプレイでデベロッパーツールを使って確認しても再現できなかったので、スルーしてもらって大丈夫です。
- [](https://gyazo.com/ffcef0bf0484c4831e1bef5c3a15ea47)
- Shunさんがおっしゃってたメール配信について、todakaが試したところ問題なく受信できました（一応ご報告）。
```rb=
create_table "page_shows", force: :cascade do |t|
t.bigint "page_showable_id"
t.string "page_showable_type"
t.json "setting"
t.datetime "updated_at", precision: 6, null: false
t.datetime "created_at", precision: 6, null: false
t.index ["page_showable_type", "page_showable_id"], name: "index_page_shows_on_page_showable_type_and_page_showable_id"
end
setting = {
"reservation_link": true,
"opening_hours": false,
}
JSON型。半構造化データ。
```
Google Drive
Gist
Clipboard
Sign in with Wallet
