konchanxxx / menta

MENTAのタスク管理用リポジトリ
0 stars 0 forks source link

render後、URLが変わってしまうことで問題が起きています #59

Closed machamp0714 closed 5 years ago

machamp0714 commented 5 years ago

概要

render後、URLが変わってしまうことで、意図しない動作が起きてしまうので それを修正したい。

実現したいこと

記事を投稿する際、validationに引っかかると、 render :new されるのですが、 URLが localhost:3000/articles/new から localhost:3000/articles へ変わってしまいます。 その状態でページを更新すると、 can't find record with friendly id: "articles" という エラーが生じてしまうので、これを回避したい。

困っていること

render後、URLを変化させない方法が分からない。 can't find record with friendly id: "articles" というエラーを出さないための 有効な案が思い浮かびません。

解決するために行ったこと

<script type="text/javascript">
$(document).ready(function() {
    history.pushState('', '', location.href + '/new')
});
</script>

を追加すると、URLは変更されなくなったのですが、 「戻る」ボタンを押すと、やはり can't find record with friendly id: "articles" の エラーが生じてしまう。 どうもこのやり方だと /articles.new → '/articles' → '/articles/new' と、URLが変わってるだけで問題の根本的な解決には繋がらないみたいですね。

問題となっているアプリケーションのGitHub URL

https://github.com/machamp0714/love_kitchen/blob/master/app/controllers/articles_controller.rb

お忙しいとは、思いますがよろしくお願い致します 🙇

machamp0714 commented 5 years ago
render :new
↓
redirect_to request.referrer

とすることで解決しました 🙇

konchanxxx commented 5 years ago

うーん、エラーになったときにリダイレクトするとエラーメッセージとかインスタンスが書き換わってしまうので消えてしまわないです? 原因としてはfriendly_idが設定されていないことなのでURLがわかることが原因というよりそちらを解消する方が筋が良さそうと思いました。 まずはvalidation errorになっている状態でエラーになっているmodelのインスタンスのデータを確認してみると良いと思います:bow:

machamp0714 commented 5 years ago

返信遅れて申し訳ありません 🙇 redirect_to だとダメですね。ご指摘の点、完全に忘れてました;;

friendly_idの設定ですとここら辺を参考にすれば いけそうな感じがするので試してみようと思います。

https://github.com/norman/friendly_id/wiki/Root-level-slugs-for-multiple-resources

konchanxxx commented 5 years ago
その状態でページを更新すると

これってページをリロードするってことでしょうか? ターミナルのhttpリクエストの出力ってどうなってますか? GET /articlesみたいなリクエスト投げてるのかな?

machamp0714 commented 5 years ago

リロードするってことですね。

ちなみにリクエストは

Started GET "/articles" for 127.0.0.1 at 2019-04-03 00:13:45 +0900
Processing by UsersController#show as HTML
  Parameters: {"id"=>"articles"}
  User Load (0.4ms)  SELECT  `users`.* FROM `users` WHERE `users`.`slug` = 'articles' LIMIT 1
  ↳ app/controllers/users_controller.rb:5
Completed 404 Not Found in 3ms (ActiveRecord: 0.4ms)

ActiveRecord::RecordNotFound - can't find record with friendly id: "articles":
  app/controllers/users_controller.rb:5:in `show'

Started POST "/__better_errors/e6d380c1dcc0e990/variables" for 127.0.0.1 at 2019-04-03 00:13:45 +0900
  Article Load (0.3ms)  SELECT  `articles`.* FROM `articles` LIMIT 11
  ↳ /Users/rakkusann/.rbenv/versions/2.5.3/lib/ruby/gems/2.5.0/gems/activerecord-5.2.2/lib/active_record/log_subscriber.rb:98
  User Load (0.3ms)  SELECT `users`.* FROM `users` WHERE `users`.`id` IN (2, 3, 4)
  ↳ /Users/rakkusann/.rbenv/versions/2.5.3/lib/ruby/gems/2.5.0/gems/activerecord-5.2.2/lib/active_record/log_subscriber.rb:98

こんな感じです。 後、上の記事はこの件には関係なさそうです。

konchanxxx commented 5 years ago

あーこのルーティング通ってるんですね https://github.com/machamp0714/love_kitchen/blob/master/config/routes.rb#L26

machamp0714 commented 5 years ago

そうなんですよねー 情けない話、この問題に対する有効な案が出て来ません 😢 urlを変えないようにするしかないように思えるのですが。。。

konchanxxx commented 5 years ago

というか / ルートでusersのリソースにアクセスしている??これはこれでルーティングの設計がおかしいですね。

konchanxxx commented 5 years ago

普通

/users/{friendly_id}のような形でルーティングするが /にルーティングしているせいで /articlesがfriendly_idとして認識されているという

/usersにルーティングするようにすればエラーにならないのではと思いました

machamp0714 commented 5 years ago

すいません、自分でも何故このように書いたのか思い出せません 🙇 多分、 localhost:3000/ユーザー名 と言うurl設計にしたかったんだと思います。

konchanxxx commented 5 years ago

多分、 localhost:3000/ユーザー名 と言うurl設計にしたかったんだと思います。

なるほど。これはREST APIだとアンチパターンなのでやめたほうが良いですね https://qiita.com/kamihork/items/f9d31c03505028d8a9e9

machamp0714 commented 5 years ago
get '/:id', to: 'users#show'

はどうなんでしょうか。

machamp0714 commented 5 years ago

なるほどー ルーティングとかREST APIとかちゃんと勉強しないとダメですね。。。 基礎が全く出来てませんね。

konchanxxx commented 5 years ago

deviceのルーティングと競合しないようなら下記のような感じで良いかもと思いました

resources :users, only: :show do
  resources :favorites, only: %i[index create destroy]

  member do
    get :following, :followers
  end
end

get '/:id', to: 'users#show' だと今のルーティングと同じかなと思います

/users/:id

みたいなルーティングを定義したいので。

なるほどー ルーティングとかREST APIとかちゃんと勉強しないとダメですね。。。 基礎が全く出来てませんね。

悲観することはないと思います:bow: おすすめ本のWebを支える技術にREST APIについて詳しく書かれているので見てみると良いかもです:bow:

machamp0714 commented 5 years ago

はい、ありがとうございます 🙇 上記の内容を参考にルーティングの方、設計し直そうと思います。

夜遅くまでお付き合い頂いてほんと申し訳ないです 🙇 とりあえず今日は寝て、明日また報告したいと思います。

ありがとうございました!

machamp0714 commented 5 years ago

こんばんは 🙇 昨日は深夜までありがとうございました。

続きなのですが、改善案を試してみたところ render 後、やはり urlが /articles/new から /articles が変わるのでその状態でページを更新すると routing errorが出てしまい、根本的な解決になっていない気がします。

deviseを使って作成した登録ページやログインページではこのような 不具合は起きないのでdeviseはどうやってやってるのかコードを確認してみたところ respond_with を使っていました。でもこのメソッドも結局 render することになるので urlが変わって上記のようなエラーが出るだけだと思います。

実際のプロジェクトでは、 'create' アクションや、 'update' アクションで validationエラーが起きた時、どんな処理を実行しているのか気になります。

konchanxxx commented 5 years ago

urlが /articles/new から /articles が変わるのでその状態でページを更新すると routing errorが出てしまい、根本的な解決になっていない気がします。

ルーティングエラーはどのようなエラーになるのでしょうか? /usersと競合しているのでfriendly_idの処理に該当してしまいエラーになるという事象は解消した感じでしょうか?

URLが変わること自体は不具合ではなくRails本来の挙動なので問題ないのかなと思います:bow:試しに新規でRailsアプリを作成してscaffoldで新規作成ページでエラーを起こせばURLが変更されていることが確認できます:bow:

machamp0714 commented 5 years ago

こんばんは 🙇 friendly_idの問題は解消しました。 エラーなのですが、 No route matches [GET] "/articles" というRouting Errorが表示されます。 https://i.gyazo.com/766ae27a8851a2f3a32253fbe49a1d43.png

machamp0714 commented 5 years ago

今scaffoldで動作確認して来ましたが、確かにurlは変わりますね。 validationエラーが出た状態でページを更新するとindexアクションにリクエストが送られるようです。 でも、これは僕がやりたいことじゃないというか。。。 更新しても新規投稿画面が出力されて欲しいんです。

konchanxxx commented 5 years ago

エラーなのですが、 No route matches [GET] "/articles" というRouting Errorが表示されます。

こちらのエラーが表示される原因はわかっている感じでしょうか?

konchanxxx commented 5 years ago

今scaffoldで動作確認して来ましたが、確かにurlは変わりますね。 validationエラーが出た状態でページを更新するとindexアクションにリクエストが送られるようです。 でも、これは僕がやりたいことじゃないというか。。。 更新しても新規投稿画面が出力されて欲しいんです。

なるほどです。

でも、これは僕がやりたいことじゃないというか。。。

を実現しようとするとRailsのデフォルトの挙動からずれてくるのであまりオススメはしません。 Railsというフレームワークというレールの上にのることで開発を効率的に行えるので。

同じURLを実現したいならdef createで実行されるリクエストURLを def newで実行するURLと同じものにすれば良いかなと思いました。

GET /articles/new => articles#new なら POST /articles/new => articles#create となるようにすればリロードした際に同一のURLになるかなと思います:bow: ただこちらもRESTの考え方からずれるのでオススメしないです:bow:

machamp0714 commented 5 years ago

回答ありがとうございます 🙇 Routing Errorが出る原因としては、getメソッドに対応したarticles_pathがないからですね。

resources :articles

とすればこのエラーは消えます。動作も確認しました。

machamp0714 commented 5 years ago

「動けばいいや」と言うのは嫌いな考え方なので RESTの考え方から外れるのならその案は採用しない方がいいですかね。 その辺の知識がないのでRESTから外れるデメリットが思い浮かばないのですが 😭

deviseのコードとか参考にしてちょっと試してみようかなーと今は思ってます。

konchanxxx commented 5 years ago

Routing Errorが出る原因としては、getメソッドに対応したarticles_pathがないからですね。

こちらおkです!:smile:

その辺の知識がないのでRESTから外れるデメリットが思い浮かばないのですが 😭

こちらはRESTやRailsという決まりごとからそれる実装になることが一番のデメリットかなと思いましたmm 本当に必要なら仕方ないですが、レール(もしくはフレームワーク)の実装からずれると開発者での共通認識がとりづらいオレオレ実装になったりするのでそこはデメリットですね。本当に要件で必要なら実装する必要があります。

例えば通常RESTだと POSTメソッド はリソース(Userとかデータのこと)を生成する際に利用してURLはリソースを表現するために使ったりします。def new,def create のURLを合わせてしまうと GET /articles/new POST /articles/new となってしまいcreateアクションのPOSTというメソッドとnewが共にリソースを生成する意味合いになり重複してしまったりします。

また今回の実装についてはエラー画面が表示された際にリロードするようなユーザーのユースケースが余り想定できないのでアプリケーション要件としても優先度の低いものなのかなと思ったりしました:bow:

machamp0714 commented 5 years ago

こんばんは 🙇 わかりやすい解説ありがとうございます!

決まりごとから逸れるとチーム開発に支障が出るというのは確かにその通りですね。 やはりRESTから外れた実装はやめておいた方が良いですね^^;

また今回の実装についてはエラー画面が表示された際にリロードするようなユーザーのユースケースが余り想定できないのでアプリケーション要件としても優先度の低いものなのかなと思ったりしました

この問題の対処についてはこちらを念頭に置いてコードの方を修正していこうかなと思いました。

長々とお付き合いいただきありがとうございました 🙇 RESTやルーティングの設計など未熟な点が浮かんできて、 これからの学習の課題も見つかりました 🙇

konchanxxx commented 5 years ago

RESTやルーティングの設計など未熟な点が浮かんできて、 これからの学習の課題も見つかりました

ここら辺がしっかり理解できて綺麗なルーティングが設計できるようになると、データ構造も複雑にならず疎結合な実装ができるようになるかなと思います:bow: 頑張ってください!:smile: