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

Discord IDが不正でも退会できるようにした #4220

Merged

Conversation

eatplaynap
Copy link
Contributor

@eatplaynap eatplaynap commented Feb 14, 2022

issue概要

DiscordのIDが「ユーザー名#4桁の数字」以外だと不正な値と見なされ、退会処理が行えませんでした。
こちらをDiscordのIDが「ユーザー名#4桁の数字」以外のユーザーでも退会処理が行えるよう修正し、テストを追加しました。
app/models/user.rbは、本Issueとは直接関係ない箇所もlinterをかけて自動修正された結果変更されています。

Discord IDが不正なユーザー

kimura___FJORD_BOOT_CAMP(フィヨルドブートキャンプ)

修正前

退会・休会手続き___FJORD_BOOT_CAMP(フィヨルドブートキャンプ)

修正後

Image from Gyazo

kimura#12345678という不正なDiscord IDのユーザーが退会処理を行った↑後、↓のページに飛ぶようになりました

image

確認方法

  1. ブランチbug/enable-retirement-regardless-validity-of-discord-id をローカルに取り込む
  2. db/fixtures/users.yml内のユーザーのDiscord IDを不正な値に変更する(手元ではkimuradiscord_accountkimura#12345678にして確認しました)
  3. bin/rails db:seedでユーザー情報を取り込む
  4. bin/rails sでサーバーを立ち上げる
  5. 不正なDiscord IDを持つユーザーkimuraでログインする
  6. プロフィールよりログインユーザーのDiscord IDが不正な値であることを確認し、退会・休会手続きに進む
  7. 必要項目をチェックし退会するボタンで無事退会できることを確認する

手順が分かりづらかったら直接お声かけください。

@eatplaynap eatplaynap marked this pull request as ready for review February 15, 2022 01:58
@eatplaynap eatplaynap self-assigned this Feb 15, 2022
@eatplaynap eatplaynap requested a review from tksmasaki February 15, 2022 02:01
@eatplaynap
Copy link
Contributor Author

@tksmasaki
お疲れさまです!お手すきのときにレビューをお願いいたします:pray:

Copy link
Contributor

@tksmasaki tksmasaki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eatplaynap
お疲れさまです。
レビューさせていただきました! 1点コメントしたので、確認よろしくお願いします🙏
validation_context を使った覚えが無かったので、勉強になりました!

visit_with_auth new_retirement_path, 'discordinvalid'
choose 'とても悪い', visible: false
click_on '退会する'
page.driver.browser.switch_to.alert.accept
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capybara の accept_comfirm を用いたほうが適切そうです。

一個の下のテストでも使われていました。

page.accept_confirm '本当によろしいですか?' do

参考:Capybara でデータ削除時の confirm ボタンを押したい - Just do IT

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

なるほど!知らなかったです、ありがとうございます:pray:
ご指摘の通りに修正してみました!

@eatplaynap eatplaynap force-pushed the bug/enable-retirement-regardless-validity-of-discord-id branch from 2bd3911 to 2e39563 Compare February 22, 2022 06:30
@eatplaynap eatplaynap requested a review from tksmasaki February 22, 2022 06:51
@eatplaynap
Copy link
Contributor Author

@tksmasaki
レビューありがとうございました!お手すきのときに再確認をお願いします〜!:pray:
(コンフリクト解消しようとしたら履歴が消えてしまったみたいで変更箇所が分かりづらくてすみません:sob: page.driver.browser.switch_to.alert.acceptpage.accept_comfirmに書き換えました!)

Copy link
Contributor

@tksmasaki tksmasaki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確認しました! LGTMです🙆‍♂️

@eatplaynap
Copy link
Contributor Author

@tksmasaki
ご確認ありがとうございました!:pray:

@komagata
チームメンバーにレビューOKいただきましたので、レビューをお願いいたします:pray:

@eatplaynap eatplaynap requested a review from komagata February 22, 2022 07:48
Copy link
Member

@komagata komagata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確認しました、OKですー🙆‍♂️

@komagata komagata merged commit 9be0a6a into main Feb 26, 2022
@komagata komagata deleted the bug/enable-retirement-regardless-validity-of-discord-id branch February 26, 2022 16:22
@github-actions github-actions bot mentioned this pull request Feb 26, 2022
34 tasks
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

Successfully merging this pull request may close these issues.

3 participants