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

修了したプラクティスを重複して計算してしまう不具合を修正 #7809

Merged
merged 2 commits into from
Jun 24, 2024
Merged

修了したプラクティスを重複して計算してしまう不具合を修正 #7809

merged 2 commits into from
Jun 24, 2024

Conversation

kyokucho1989
Copy link
Contributor

@kyokucho1989 kyokucho1989 commented May 28, 2024 •
edited
Loading

Issue

概要

以前 #7635 のIssueを解消するため下記のPRを作成しました。
#7699
その後本番環境に反映されましたが、修了した数が正しい状態になりませんでした。

それを解消するPRです。

変更確認方法

  1. bug/mismatch_complete_practice_sizeをローカルに取り込む
  2. bootcamp を起動
  3. 任意のアカウントでログイン
  4. 複数のカテゴリに所属するプラクティスを修了しているユーザー（harikirio)のページへログイン
    (http://localhost:3000/users/527754862)
  5. 修了プラクティス が「4 （必須:2）」 修了したプラクティスが「3% 」→ 「修了: 4 （必須: 2/56）」となるのを確認

プラクティスの説明

  • Linuxのファイル操作の基礎を覚える: 必修
  • 企業研究　:必修ではない
  • 複数カテゴリに所属するプラクティス1 : 必修ではない
  • 複数カテゴリに所属するプラクティス2: 必修

Screenshot

変更前

変更後

スクリーンショット 2024-05-31 6 18 42

@kyokucho1989 kyokucho1989 force-pushed the bug/mismatch_complete_practice_size branch 4 times, most recently from 90c2ede to aa54e38 Compare May 30, 2024 07:06
@kyokucho1989 kyokucho1989 force-pushed the bug/mismatch_complete_practice_size branch 2 times, most recently from 4d499b2 to 380c78f Compare May 30, 2024 21:10
@kyokucho1989 kyokucho1989 marked this pull request as ready for review May 30, 2024 21:58
@kyokucho1989
Copy link
Contributor Author

@komagata
こちら、PRが完成しました。レビューの順序は通常通りメンバーの方にお願いしてからの方が良いでしょうか。

@kyokucho1989 kyokucho1989 mentioned this pull request Jun 2, 2024
Open
@kyokucho1989 kyokucho1989 force-pushed the bug/mismatch_complete_practice_size branch from 380c78f to 7e8ea47 Compare June 3, 2024 10:51
@kyokucho1989
Copy link
Contributor Author

kyokucho1989 commented Jun 4, 2024 •
edited
Loading

@komagata
bootcampアプリ側で「Ruby on Rails （Rails 6版）」のカテゴリとRailsコースの紐付けが削除されました。これにより、本PRを反映しなくても修了プラクティスの数が正しく表示されるようになっています。

こちらのPRはクローズしたほうが良いでしょうか。それとも、マージまで行った方がよいでしょうか。

@komagata
Copy link
Member

komagata commented Jun 5, 2024

@kyokucho1989 潜在する問題はなおっていないので、最後までおねがいします～

@kyokucho1989
Copy link
Contributor Author

@komagata
　了解しました！

@naokinaokiboo
　お手数ですが、こちら、レビューをお願いします。

kyokucho1989
kyokucho1989 commented Jun 5, 2024
View reviewed changes
app/models/user.rb
practices_include_progress.joins(:learnings)
.merge(Learning.complete.where(user_id: id))
.merge(Learning.complete.where(user_id: id)).pluck(:id).uniq.size
Copy link
Contributor Author

Choose a reason for hiding this comment

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

.pluck(:id).uniq.size　を追加することで、プラクティスの重複をなくしています。

@naokinaokiboo
Copy link
Contributor

@kyokucho1989
お疲れ様です。
レビューの件、現在なかなか時間がとれておらず、1週間以上かかってしまうかもしれませんが、よろしいでしょうか？💦
それでよろしければ、レビューさせて頂きます。

@kyokucho1989
Copy link
Contributor Author

@naokinaokiboo 問題ないですー
よろしくお願いします！

@kyokucho1989 kyokucho1989 force-pushed the bug/mismatch_complete_practice_size branch from 7e8ea47 to 8397ace Compare June 14, 2024 08:54
@kyokucho1989
Copy link
Contributor Author

@naokinaokiboo
こんばんは！
こちらのレビュー、いかがでしょうか。
難しそうであれば他の人にお願いしようと思います

@naokinaokiboo
Copy link
Contributor

@kyokucho1989
レビュー、遅くなっており申し訳ありません。🙏
状況としては、Issueのこれまでの経緯を把握し、変更点も一部確認しています。
残りは主にテスト周りの変更点だけなので、明日の夜に終えれるかと考えています。

naokinaokiboo
naokinaokiboo reviewed Jun 19, 2024
View reviewed changes
Copy link
Contributor

@naokinaokiboo naokinaokiboo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

レビュー遅くなり申し訳ありません🙏
質問とコメント入れさせて頂きました〜。ご確認お願い致します。

あと、誤記ですが、本PRのプラクティスの説明にある複数カテゴリに所属するプラクティスの必修or必修ではないの説明が逆ではないかと思います。

db/fixtures/learnings.yml Outdated

learning25:
user: harikirio
practice: practice62
Copy link
Contributor

Choose a reason for hiding this comment

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

こちら、practice111ではないでしょうか？

Copy link
Contributor Author

Choose a reason for hiding this comment

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

すみません、修正しました！

db/fixtures/learnings.yml Outdated

learning26:
user: harikirio
practice: practice63
Copy link
Contributor

Choose a reason for hiding this comment

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

こちら、practice112ではないでしょうか？

Copy link
Contributor Author

Choose a reason for hiding this comment

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

こちらも修正しました！

db/fixtures/courses_categories.yml
courses_category27:
course: course1
category: category23
position: 19
Copy link
Contributor

Choose a reason for hiding this comment

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

category23を追加した目的は何でしょうか？
（categories_practices.ymlからはcategory23は参照されておらず、現状、特に必要ではないように思っています。）

Copy link
Contributor Author

Choose a reason for hiding this comment

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

すみません。忘れていました！　categories_practices.ymlでcategory23 を参照するようにしています。

こちらは複数のカテゴリに所属するプラクティスを設定する上で必要なカテゴリです。

@kyokucho1989
Copy link
Contributor Author

@naokinaokiboo
ありがとうございます！！
家に帰ってから諸々確認しますー

@kyokucho1989
修了したプラクティスを重複して計算する不具合を修正
b274e45 
db/fixtureデータのconflictを解消
@kyokucho1989
db/fixtureの整合性をとった
4e46230 
conflictに対応する際に修正をしていなかった
@kyokucho1989 kyokucho1989 force-pushed the bug/mismatch_complete_practice_size branch from 8397ace to 4e46230 Compare June 21, 2024 13:54
@kyokucho1989
Copy link
Contributor Author

@naokinaokiboo
指摘事項を修正しました！

あと、誤記ですが、本PRのプラクティスの説明にある複数カテゴリに所属するプラクティスの必修or必修ではないの説明が逆ではないかと思います。

こちらも修正しています。
確認をお願いしますー

@naokinaokiboo
Copy link
Contributor

@kyokucho1989
確認しました！
Approveとさせて頂きます〜。

@kyokucho1989
Copy link
Contributor Author

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

@komagata
メンバーの方のレビューが終了しました。
レビューをお願いします

komagata
komagata approved these changes Jun 24, 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 6f436ce into main Jun 24, 2024
7 checks passed
@komagata komagata deleted the bug/mismatch_complete_practice_size branch June 24, 2024 15:07
@github-actions github-actions bot mentioned this pull request Jun 24, 2024
Merged
13 tasks
@kyokucho1989
Copy link
Contributor Author

@komagata
お疲れ様です。本番環境の動作確認について、お願いしたいことがあります。

bootcampアプリ側で「Ruby on Rails （Rails 6版）」のカテゴリとRailsコースの紐付けを一時的に復活していただけませんか。

このPRによって、複数カテゴリに所属するプラクティスについても修了数が正しく表示されてされるようになりました。

以前FBCのカリキュラムに載っていた上記のカテゴリがそれにあてはまります。

紐付けが復活してもプラクティスの修了数が問題なく表示されていれば、本Issueをクローズできます。

@komagata
Copy link
Member

@kyokucho1989

bootcampアプリ側で「Ruby on Rails （Rails 6版）」のカテゴリとRailsコースの紐付けを一時的に復活していただけませんか。

復活させました～

@kyokucho1989
Copy link
Contributor Author

@komagata 確認できました！問題ないことがわかりました。クローズします

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

@komagata komagata komagata approved these changes

@naokinaokiboo naokinaokiboo naokinaokiboo approved these changes

Assignees

@kyokucho1989 kyokucho1989

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@kyokucho1989 @komagata @naokinaokiboo