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

TODO #1

Open
24 tasks done
hiroro-work opened this issue Aug 23, 2018 · 23 comments
Open
24 tasks done

TODO #1

hiroro-work opened this issue Aug 23, 2018 · 23 comments

Comments

@hiroro-work
Copy link
Owner

hiroro-work commented Aug 23, 2018

リソース生成

  • Home
  • User
  • Tweet
  • Reply
  • Retweet

機能実装

  • アカウント作成
  • アカウント削除
  • サインイン
  • サインアウト
  • フォロー
  • アンフォロー
  • ツイート(テキスト)
  • ツイートへのリプライ(テキスト)
  • リツイート
  • リプライへのリプライ(テキスト)
  • リツイートへのリプライ(テキスト)
  • リツイートにコメントを付加

テストコード

rspec/system

  • header
  • footer
  • home_index
  • user_show
  • tweet_show
  • reply_show
  • retweet_show
@hiroro-work
Copy link
Owner Author

  • DB作成
    • PostgreSQLでcreate database tubtter_development
  • Home controller
    • rails generate
    • rootにhome#indexを設定 (for devise)
  • User scaffold
    • rails generate
    • rails generate devise User

@hiroro-work
Copy link
Owner Author

deviseのviewはdevise-bootstrap-viewsで作成
$ rails generate devise:views:bootstrap_templates

@hiroro-work
Copy link
Owner Author

hiroro-work commented Aug 25, 2018

ログイン後にuser_pathに飛ぶようにApplicationControllerでafter_sign_in_path_forをオーバライド。
routesでuser_root指定する方法もあったけど、こっちだとuser_pathにidが指定されず
UsersControllerで対処が必要だったので上記の方法を採用。

ちなみにroutes.rbでの対応は以下のような感じ

authenticated :user do
  root to: 'users#show', as: :user_root
end

@hiroro-work
Copy link
Owner Author

Tweetのshow.html.haml作成中

@hiroro-work
Copy link
Owner Author

ReplyはUserの下に置いたほうがいいかな。User消えたらReplyも消える感じで。
んでTweetとも紐づける。

@hiroro-work
Copy link
Owner Author

acts_as_followerはmigrateファイルのActiveRecord::Migrationのおしりに[5.2]をつけないとdb:migrateで失敗した。

@hiroro-work
Copy link
Owner Author

deviseのregistrations/edit画面で一回更新エラーした後にbackしてbackしたら、存在しないusersに
遷移しようとするので、とりあえずedit.html.hamlの最後のリンクをコメントアウト

@hiroro-work
Copy link
Owner Author

hiroro-work commented Sep 1, 2018

Herokuにmaster以外のブランチをpushする場合は
$ git push heroku {branch name}:master
$ heroku run rails db:migrate

@hiroro-work
Copy link
Owner Author

hiroro-work commented Sep 7, 2018

質問

TweetとReplyは同じモデルにしちゃってもいいかな?
Replyに対するReplyを考えるとそっちのほうがよい?
(何に対するReplyかをカラムとして持つときにreplyかtweetのどっちかが入ってるみたいな
実装はコードが複雑になりそうでよくなさそう)

Reply機能をTweetモデルで実現。
TweetモデルにreferencesでTweetモデルをparentみたいなカラムとして持たせる感じ?
できるのかな?
 t.references :parent, foreign_key: { to_table: :tweets }
こんな感じでいけそうといううわさも。

それだとTweetコントローラが大きくなっちゃうからコントローラとしてはReplyControllerみたいの
作ったほうがいいのかな?
それとも1つのテーブルを複数のモデルで共有してそれぞれにコントローラ作るようにすればいいのかな?(できるかはわからないが)

TweetとReplyのidを共通化して、TweetとReplyに一発でreferenceをはるようなことはできたりするのかな?親モデル作って継承すれば行けそうだけど、Railsモデルで継承は微妙といううわさも。

回答

ReplyはTweetモデルで実現したほうが良い。

@hiroro-work
Copy link
Owner Author

hiroro-work commented Sep 7, 2018

「userのreplies」と「tweetに対するreplies」を表示したい場合、
ReplyControllerのindex的なアクションを複数つくるのがよいか、それとも、
UserControllerとTweetControllerにrepliesを表示するアクションを作るのがよいか?

とりあえず今とのころはtweetに対するrepliesはtweetのshowで表示してるから、
replyのindexとしてuserのrepliesを表示する機能を実装しておこう(暫定対処)。

user配下とtweet配下にコントローラを分けてやれば両方のindexを作れる?
(本当にできるかは試してないから不明)

@hiroro-work
Copy link
Owner Author

replyは全アクションをtweetの下に持っていこうと思ったけど、
そうすると削除されたtweetに対するreplyを残すことができなかったので、
作成後のアクションはuserの下に持っていく方向で。

@hiroro-work
Copy link
Owner Author

Retweetもtweetモデルを更新して実現できそうだけど、
とりあえずはreplyと同じ要領でretweetモデル作って実装する方向で。

@hiroro-work
Copy link
Owner Author

hiroro-work commented Sep 9, 2018

質問

Viewにて
メインで操作しているモデルのインスタンスからたどれるモデルの場合、
メインモデルからたどる形で参照したほうがよい?
参照したいモデルのインスタンスが存在するのであればそちらを参照したほうがよい?
@user@tweetがあって、userモデルは@userからも@tweet.userからも参照できるような場合)

回答

@userが明らかに存在しているところでは@userを使用する。

@hiroro-work
Copy link
Owner Author

ReplyとRetweetのedit/updateはUserのしたに持ってきて、
new/createと同じパスでform統一できるようにしたほうがよいかも。

@hiroro-work
Copy link
Owner Author

hiroro-work commented Sep 15, 2018

質問

rspecのfeatures/systemはどの単位でファイル分割するのがよい?
# 画面単位?: 画面上にある機能をテスト
# モデル単位?: モデルの機能をテスト

backgroundのまとめやすさ考えたら画面単位のほうがよさそう?

回答

関連する機能の単位でファイル分割することが多い。
画面単位でも問題ないが人によってはレビューで質問してくるかも。

@hiroro-work
Copy link
Owner Author

hiroro-work commented Sep 15, 2018

acts_as_followerを使ってるとrspec実行時に以下のエラーメッセージが出力される。

DEPRECATION WARNING: Setting custom parent classes is deprecated and will be removed in future versions.

プルリクがマージされていない模様...

tcocca/acts_as_follower#89
tcocca/acts_as_follower#93

プルリクを参考にモンキーパッチを作って対処。

@hiroro-work
Copy link
Owner Author

hiroro-work commented Sep 17, 2018

Chrome headlessだとCapybaraのpage.accept_confirmがうまく動かないっぽい。
とりあえず、page.accept_confirm do endを削除してclick_onを直で実行するようにしたら
テストがパスするようになったので暫定的にこの形で。

click_on '削除'
# page.accept_confirm do
#   click_on '削除'
# end

capybaraとchromedriver-helperを最新バージョンにしてみたけどダメだった。

capybara (3.7.2)
chromedriver-helper (2.0.0)

ちなみにdata-confirm-modal入れてない状態だと下記でテストパスしてた。

page.driver.browser.switch_to.alert.accept

data-confirm-modal入れるとポップアップがふわっとした感じになるから
タイミング的な問題なのかな?
⇒ 直前にsleepを10秒まで入れてみたけどダメだったのでタイミングでもないのかな。。
⇒ data-confirm-modal入れるとモーダル画面がhtmlになっちゃうからaccept_confirmで
  引っかからなくなってしまうらしい。
  なので暫定対処のとおりいきなりclick_on '削除'でOK
⇒ と思ったらgem: headless削除したらタイミングでclick_on '削除'が失敗するように。。。
⇒ click_onの前にモーダル画面をfindすることで対処。

@hiroro-work
Copy link
Owner Author

acts_as_followerは古いのであまり使わないほうがよいということで、
フォロー機能は自力で実装。

以下の3パターンで迷ったが1の実装方法で行く方向で。
# ちなみにRailsチュートリアルは2で実装。
# モデルのメソッドは受動的/能動的どちらがよいか迷いどころ。
# 迷うくらいならということでUserの下にFollowersリソースを置いて、
# create/destroyをそのユーザに対するfollow/unfollowって感じで実装する方向で。

前提: followee/follower(実体はUser)のリファレンスを持つRelationshipモデルを作成

1. FollowersController作ってcreate/destroyでfollow/unfollow機能を実現。
2. Userモデルのメソッドでfollow/unfollow(インスタンスユーザが他のユーザをfollow/unfollow)を実装。
3. Userモデルのメソッドでfollow/unfollow(他のユーザがインスタンスユーザをfollow/unfollow)を実装。

destroyのurl/path指定のときにrelationshipインスタンスが必要なのがいやだったので
create/destroyはfollowersじゃなくてfollowerの下に移動。

結局create/destroy内で呼び出すfollow/unfollowメソッドをUserモデルに実装。

@hiroro-work
Copy link
Owner Author

中の実装変えてもrspecのsystemテスト普通に動くの感動だ~。
やっぱsystemテストで自動テストがよさそうだな。

@hiroro-work
Copy link
Owner Author

referencesがデフォルトでrequired: trueとは。。。

@hiroro-work
Copy link
Owner Author

hiroro-work commented Sep 24, 2018

tweet/replyの画面周りをもうちょいちゃんとしよう。
(親がいるのにtweetとして表示されちゃってるところとか)
いっそ表示は統一するか?

あとReplyモデルは削除しよう。DBも。

@hiroro-work
Copy link
Owner Author

replyのparent_tweetを削除したらtweetになっちゃうのは
とりあえずよしとしておこうかな。
replyだった証みたいなカラムをTweetモデルに追加すればparent_tweet削除されても
replyのままにできるけど、とりあえずは保留で。

@hiroro-work
Copy link
Owner Author

画像アップデートできるようになったけど、入力フィールドがカッコ悪い。。
bootstrap-fileinputあたり使えばいい感じにできそう?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant