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.

Sign up for GitHub

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

Jump to bottom

learning.vueを非Vue化した #8131

Merged
merged 6 commits into from
Nov 10, 2024
Merged

learning.vueを非Vue化した #8131

merged 6 commits into from
Nov 10, 2024
+117 −98

Conversation

Shrimprin
Copy link
Contributor

@Shrimprin Shrimprin commented Oct 12, 2024 •
edited by reckyy
Loading

Issue

概要

プラクティスの詳細ページのlearning.vue(提出物ボタン)が今はVueで実装されているため、Railsのviewに移行する。
スクリーンショット 2024-10-13 211313

変更確認方法

1. 準備

  1. chore/learning-from-vue-to-htmlをローカルに取り込む
  2. ローカルサーバーを立ち上げる
  3. 適当なユーザでログインする

2. 提出物のあるプラクティス

  1. 提出物のあるプラクティスのページにアクセスする
  2. 以下を確認する
    • 提出物を作るボタンが表示されている
    • 修了ボタンが表示されている
  3. 提出物を作るボタンをクリックして以下を確認する
    • 提出物の新規作成画面(URLが/products/new?practice_id=363506445)に遷移する
    • 提出物を提出またはWIPして先ほどの提出物のあるプラクティスのページに戻ると、提出物を作るボタンのテキストが提出物へに変わっている
  4. 修了ボタンをクリックして下記を確認する
    • プラクティス「Linuxのファイル操作の基礎を覚える」を修了しました🎉というモーダルが表示される
    • 修了ボタンの色がグレーに変わり、テキストが修了していますに変わっている
    • 画面をリロードすると、プラクティスのステータスが修了に変わっている
    • 画面をリロードすると、画面上部にこのプラクティスは修了しました🎉と表示されている

3. 提出物のないプラクティス

  1. 提出物のないプラクティスのページにアクセスする
  2. 以下を確認する
    • 提出物を作るボタンが表示されていない
    • 修了ボタンが表示されている
  3. 修了ボタンをクリックして下記を確認する
    • プラクティス「Debianをインストールする」を修了しました🎉というモーダルが表示される
    • 修了ボタンの色がグレーに変わり、テキストが修了していますに変わっている
    • 画面をリロードすると、プラクティスのステータスが修了に変わっている
    • 画面をリロードすると、画面上部にこのプラクティスは修了しました🎉と表示されている

Screenshot

非Vue化のみで画面に変更はなし

@Shrimprin Shrimprin force-pushed the chore/learning-from-vue-to-html branch from e1c8946 to cb8ebd8 Compare October 13, 2024 09:30
@Shrimprin Shrimprin marked this pull request as ready for review October 13, 2024 12:25
@Shrimprin Shrimprin requested a review from reckyy October 13, 2024 12:28
@Shrimprin
Copy link
Contributor Author

@reckyy
お疲れ様です！
本PRのレビューをお願いしたいのですが、ご対応いただくことは可能でしょうか？ 🙏

@Shrimprin Shrimprin self-assigned this Oct 13, 2024
@reckyy
Copy link
Contributor

reckyy commented Oct 13, 2024

@Shrimprin
お疲れ様です！
少し立て込んでおり時間がかかるかもしれません。
来週末までを目処にレビューできればと思います。
もしお急ぎでしたら、他の方にご依頼をお願いいたします。 🙇
よろしくお願いいたします。

@Shrimprin
Copy link
Contributor Author

@reckyy
ありがとうございます! 🙏
急ぎでは全くありませんので、ご都合の良いタイミングで対応いただければ大丈夫です。
よろしくお願いいたします。

reckyy
reckyy reviewed Oct 26, 2024
View reviewed changes
Copy link
Contributor

@reckyy reckyy left a comment

Choose a reason for hiding this comment

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

@Shrimprin
大変お待たせしております。
大まかOKでしたが何点かコメントしていますので、お手隙の際にご確認お願いいたします〜！

app/components/learnings/learning_component.rb Outdated
@product ? '提出物へ' : '提出物を作る'
end

def complete?
Copy link
Contributor

Choose a reason for hiding this comment

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

[imo]Practice#completed?メソッドを使えば、このcomplete?メソッドはなくせると思いましたがいかがでしょうか👀(practice.rbの85行目)
completed?メソッドについてすでにご存知で、使わなかった理由があれば教えていただけますと幸いです。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ご指摘ありがとうございます！
モデルメソッドの存在に気付いておりませんでした 💦
コンポーネントメソッドはモデルメソッドと重複するため削除しました。

test/components/learning_component_test.rb Outdated
assert_link '提出物を作る', href: '/products/new?practice_id=315059988'
end

def test_not_render_product_link_when_practice_is_not_submission
Copy link
Contributor

Choose a reason for hiding this comment

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

[imo]冒頭はdoes notの方がいいかなと思います。

test/components/learning_component_test.rb Outdated
assert_no_link '提出物を作る'
end

def test_render_completed_when_practice_is_completed
Copy link
Contributor

Choose a reason for hiding this comment

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

[nits]重箱の隅を突くようなコメントで申し訳ないですが、completed_message(or text)だとよりテストの内容を表すかなと思いました。

@Shrimprin Shrimprin force-pushed the chore/learning-from-vue-to-html branch from cb8ebd8 to cbc1dcc Compare October 27, 2024 00:14
@Shrimprin
Copy link
Contributor Author

@reckyy
お疲れ様です！
確認いただきありがとうございます。
コメントいただいた3点を修正しましたので再度ご確認をお願いいたします～ 🙏

reckyy
reckyy approved these changes Nov 8, 2024
View reviewed changes
Copy link
Contributor

@reckyy reckyy left a comment

Choose a reason for hiding this comment

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

@Shrimprin
お疲れ様です。
立て込んでおり、確認に時間がかかってしまいました。申し訳ございません 🙇
修正ありがとうございます！
自分からはOKです🙆

@reckyy reckyy mentioned this pull request Nov 9, 2024
Open
@Shrimprin
Copy link
Contributor Author

@reckyy
ありがとうございます！

@komagata
メンバーからApproveいただきましたのでレビューをお願いできますでしょうか 🙏

komagata
komagata approved these changes Nov 10, 2024
View reviewed changes
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 ebd158e into main Nov 10, 2024
5 checks passed
@komagata komagata deleted the chore/learning-from-vue-to-html branch November 10, 2024 16:37
@github-actions github-actions bot mentioned this pull request Nov 10, 2024
Open
15 tasks
@Shrimprin
Copy link
Contributor Author

@komagata
ありがとうございます！

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reckyy reckyy reckyy approved these changes

@komagata komagata komagata approved these changes

Assignees

@Shrimprin Shrimprin

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Shrimprin @reckyy @komagata