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

feat: progress on problems #36

Merged
merged 14 commits into from
Feb 14, 2024
Merged

feat: progress on problems #36

merged 14 commits into from
Feb 14, 2024

Conversation

ykit00
Copy link
Collaborator

@ykit00 ykit00 commented Feb 9, 2024

Close #33

image

Self Check

  • I've confirmed All checks have passed on PR page. (You may leave this box unchecked due to long workflows.)
    • PR title follows Angular's commit message format.
      • PR title doesn't have WIP:.
    • All tests are passed.
      • Test command (e.g., yarn test) is passed.
      • Lint command (e.g., yarn lint) is passed.
  • I've reviewed my changes on PR's diff view.

@ykit00 ykit00 self-assigned this Feb 13, 2024
@ykit00 ykit00 changed the title [WIP] Feature/progress on problems Feature/progress on problems Feb 13, 2024
@ykit00 ykit00 changed the title Feature/progress on problems feat: progress on problems Feb 13, 2024
@ykit00 ykit00 marked this pull request as ready for review February 13, 2024 11:51
@ykit00 ykit00 requested a review from Tatehito February 13, 2024 11:51
}
}

export async function getUserSolvedProblems(
Copy link
Collaborator

Choose a reason for hiding this comment

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

個人的な感覚なので変更するかはお任せしたいですが、dbアクセスするので get より fetchUserSolvedProblems のほうがしっくりくる気がしました

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed 7496165

@@ -36,6 +52,15 @@ const CoursePage: NextPage<{ params: { courseId: string } }> = ({ params }) => {
const inputValue = event.target.value;
setLanguageIdToSessionStorage(inputValue);
setSelectedLanguageId(inputValue);
getUserSolvedProblems(userId, courseId).then((data) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

この処理を handleSelectLanguage 内でやってるのて何か理由ありますか?
問題を正解してから科目一覧に戻ってきた時、言語を再選択しないと最新の進捗が表示されない気がします。useEffect内で動かすといいかもと思いました

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

最初 useEffect でやってたんですけど、連鎖で使うと renderしまくりでパフォーマンスがよくなくて。。

言語を再選択しないと最新の進捗が表示されない気がします。

これに対応しつつサーバーサイドでフェッチなどするようにして改善してみましたー
4b9d503

userId String
courseId String
programId String
languageId String
Copy link
Collaborator

Choose a reason for hiding this comment

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

programIdやcourseIdはハードコーディングしてるから、問題IDやコースIDの変更は基本できない(やる場合はUserSolvedProblemのレコードも更新する)のは気をつけないといけないですね・・・💡

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ですね! どこかでDBを使用するように改善する余地もあるかもしれません!

@ykit00 ykit00 requested a review from Tatehito February 14, 2024 07:58
Copy link
Collaborator

@Tatehito Tatehito left a comment

Choose a reason for hiding this comment

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

LGTM!

@ykit00 ykit00 merged commit 913072e into main Feb 14, 2024
5 checks passed
@ykit00 ykit00 deleted the feature/progress-on-problems branch February 14, 2024 08:06
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.

ユーザーは問題ごとの進捗を確認できる
2 participants