[Exercise 10.5] Add an integration test for the expired password reset and more #16

Merged
merged 4 commits into from Jun 25, 2015

Projects

None yet

3 participants

@ku00
Owner
ku00 commented Jun 22, 2015

Exercises

https://www.railstutorial.org/book/account_activation_password_reset#sec-activation_resets_exercises

TODO

  • 1. Add an integration test for the expired password reset to check that the response body includes the word expired
  • 2. Improve index and show action by filling in the template shown in Listing 10.58
    • Replace FILL_IN in Listing 10.58
    • Extra credit: Add integration tests for both /users and /users/:id
  • 3. Replace each pair of update_attribute calls with a single call to update_columns by filling in the template shown in Listing 10.59
    • Replace FILL_IN in Listing 10.59
    • Verify the test suite is covering the right thing by running the test

Completion Conditions

  • Passed by all green TravisCI test
  • Get OK or LGTM from two of rjk
added some commits Jun 22, 2015
@ku00 added an integration test for the expired passowrd reset
to solve exercise10.5-1
89d88bf
@ku00 improved user 'index' and 'show' page to disable unactivated users
to solve exercise10.5-2 and extra credit
24a851e
@ku00 improved attr update method because 'update_attributes' is deprecated
to solve exercise10.5-3
f9a876a
@alotofwe

ExercisesのURLがChapter 9だ...

@ku00
Owner
ku00 commented Jun 23, 2015

あれ本当だ...。修正しました!

@alotofwe

ありがとう! LGTM 🍺

@hanazuki

improved attr update method because 'update_attributes' is deprecated

deprecatedなんですか?

@ku00
Owner
ku00 commented Jun 23, 2015

@hanazuki
http://apidock.com/rails/ActiveRecord/Base/update_attributes
にはdeprecatedもしくはmovedって書いてある。
どちらにせよaliasとして update があるから使わないほうがいいかなと思ってるのだけどどうだろうか。

@hanazuki

@takuminnnn それはActiveRecord::Base#update_attributesが廃止されてActiveRecord::Persistence#update_attributesに移ったといういう意味の記述ですよね.

update_attributesを使うべきではないとはどこにも書かれていないと思いますが,仮にそうだったとしてupdateを使うべきであって,update_columnsにするのはおかしくないですか.

@ku00
Owner
ku00 commented Jun 23, 2015

@hanazuki なるほど確かに自分の早とちりでした。ただ、今回のExerciseではTODOにも示した通り、 update_attributeupdate_columns に変更する問題であるため update を使いませんでした。

@hanazuki

@takuminnnn なるほどそういう設問だったんですね(いま問題よみました,ごめんなさい).

ただ,設問の意図は,update_attributeを単に2回呼ぶと単一トランザクションにならない,という問題を解決することだと思うので,それは元々のコードのようにupdate_attributes(ないしupdate)を使うことで解決できていると思います.

今回はたまたま関係ないのですが,update_columnsがバリデーションを省略することなどを説明しないあたりあまり良くない課題と思いました.

@ku00
Owner
ku00 commented Jun 23, 2015

@hanazuki

ただ,設問の意図は,update_attributeを単に2回呼ぶと単一トランザクションにならない,という問題を解決することだと思うので,それは元々のコードのようにupdate_attributes(ないしupdate)を使うことで解決できていると思います.

確かに設問の意図的にはそちらでも問題ないですね。

validation, callbackをスキップする update_columns の挙動を考えたら、元のコードの方が後々ハマらなくて済みそうなので戻しておきます。ツッコミありがとう!

@ku00 Revert "improved attr update method because 'update_attributes' is de…
…precated"

This reverts commit f9a876a.
d8077b2
@hanazuki

よいとおもいます〜

@ku00
Owner
ku00 commented Jun 25, 2015

@hanazuki ありがとう!

@ku00 ku00 merged commit d8077b2 into master Jun 25, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@ku00 ku00 deleted the exercise-10 branch Jun 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment