-
Notifications
You must be signed in to change notification settings - Fork 71
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
Twitter IDが不正でも退会できるように修正 #4321
Twitter IDが不正でも退会できるように修正 #4321
Conversation
383cfc8
to
1d71e60
Compare
1d71e60
to
a8982cc
Compare
@eatplaynap さん |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@clio209
レビュー依頼ありがとうございます!私の立てたIssueへのご対応もありがとうございます😄 db/fixtures/users.yml
への当該するユーザー情報の登録がされていて、rails db:seed
だけでtwitterのIDが不正なユーザーが作れるのがありがたかったです〜!
コード自体は問題なさそうでしたが、2点だけご対応お願いします。
- PRがまだドラフトなのでready for reviewに変更をお願いします💪
- descriptionに下記の手順がありますが、
db/fixtures/users.yml
に不正なtwitter IDユーザーがすでに存在しているのでこの部分は削ってよさそうです。-
2.
db/fixtures/users.yml
のユーザーのtwitter IDを1つ不正な値に変更します。
-
@eatplaynap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@clio209
修正ありがとうございます〜!LGTMです👍
@eatplaynap 再レビュー、有難うございます! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
確認しました、OKですー🙆♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
確認しました、OKですー🙆♂️
issue #4224
概要
変更前
このように、twitterコードが不正の場合、エラーが出て退会できません。
変更後
退会の際はこのエラーの元になるバリデーションを外すことで退会を可能にしています。
#4220
こちらと同様のエラーを解消するissueであり、大変参考にしています。
確認方法
bug/enable-retirement-regardless-validity-of-twitter-id
をローカルに取り込みます。bin/rails db:seed
でユーザー情報をデータに取り込み、bin/rails s
でサーバーを立ち上げます。twitterinvalid
でログインします。