btsk-116: readme작성 및 삭제 관련 에러 해결 - #152
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
WalkthroughThe PR refactors deletion flows: QuestionSets are dissociated from Sources before removal, learning stats are adjusted when sets with correct answers are deleted, new repository count methods for wrong answers are added, tests updated, and README rewritten into a product/architecture-focused landing page. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant QSService as QuestionSetService
participant MarkingRepo as MarkingResultRepository
participant LearnStatsAPI as LearnStatsPublicApi
participant SourceSvc as SourceService
participant QSRepo as QuestionSetRepository
Client->>QSService: deleteQuestionSet(setId, memberId)
rect rgb(230,240,255)
Note over QSService,SourceSvc: Detach bidirectional links
QSService->>SourceSvc: dissociateSources(setId)
SourceSvc-->>QSService: sources dissociated
end
rect rgb(240,255,230)
Note over QSService,MarkingRepo: Count member's correct answers for questions in set
QSService->>MarkingRepo: countCorrectByQuestionSetIdAndMemberId(setId, memberId)
MarkingRepo-->>QSService: correctCount
end
rect rgb(255,245,230)
Note over QSService,LearnStatsAPI: Apply stats decrement if needed
alt correctCount > 0
QSService->>LearnStatsAPI: applyQuestionSetDeleted(memberId, correctCount)
LearnStatsAPI-->>QSService: ack
end
end
rect rgb(245,240,255)
Note over QSService,QSRepo: Soft-delete question set and questions
QSService->>QSRepo: softDelete(setId)
QSRepo-->>QSService: deleted
end
QSService-->>Client: deletion complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Hyeonjun0527
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/java/kr/it/pullit/modules/questionset/service/QuestionSetServiceTest.java (1)
491-500: Fix incomplete test verification for deletion behavior.The test only verifies that
deletedAtis set, but doesn't verify the deletion interactions with new dependencies. The productiondelete()method calculates correct answer counts viaWrongAnswerRepository.countByMemberIdAndQuestionIdIn()and updates learning statistics viaLearnStatsPublicApi.applyQuestionSetDeleted(). Both mocks are already available in the test class.Add verification for these interactions:
@Test @DisplayName("문제집을 삭제한다") void deleteQuestionSet() { QuestionSet questionSet = createQuestionSetWithId(1201L); when(questionSetRepository.findByIdWithQuestions(1201L)).thenReturn(Optional.of(questionSet)); + when(wrongAnswerRepository.countByMemberIdAndQuestionIdIn(eq(questionSet.getOwnerId()), anyList())).thenReturn(2L); questionSetService.delete(1201L, questionSet.getOwnerId()); assertThat(questionSet.getDeletedAt()).isNotNull(); + verify(wrongAnswerRepository).countByMemberIdAndQuestionIdIn(eq(questionSet.getOwnerId()), anyList()); + verify(learnStatsPublicApi).applyQuestionSetDeleted(eq(questionSet.getOwnerId()), anyLong()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
docs/architecture.pngis excluded by!**/*.pngdocs/erd.pngis excluded by!**/*.pngdocs/grafana.pngis excluded by!**/*.pngdocs/sentry.pngis excluded by!**/*.pngdocs/test-coverage.pngis excluded by!**/*.png
📒 Files selected for processing (10)
README.md(1 hunks)src/main/java/kr/it/pullit/modules/learningsource/source/service/SourceService.java(1 hunks)src/main/java/kr/it/pullit/modules/projection/learnstats/api/LearnStatsPublicApi.java(1 hunks)src/main/java/kr/it/pullit/modules/projection/learnstats/domain/LearnStats.java(1 hunks)src/main/java/kr/it/pullit/modules/projection/learnstats/service/LearnStatsService.java(1 hunks)src/main/java/kr/it/pullit/modules/questionset/service/QuestionSetService.java(5 hunks)src/main/java/kr/it/pullit/modules/wronganswer/repository/WrongAnswerRepository.java(1 hunks)src/main/java/kr/it/pullit/modules/wronganswer/repository/WrongAnswerRepositoryImpl.java(1 hunks)src/main/java/kr/it/pullit/modules/wronganswer/repository/adapter/jpa/WrongAnswerJpaRepository.java(1 hunks)src/test/java/kr/it/pullit/modules/questionset/service/QuestionSetServiceTest.java(3 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~113-~113: Using many exclamation marks might seem excessive (in this case: 17 exclamation marks for a text that’s 6335 characters long)
Context: ... | | :--- | :--- | | Core Backend | 
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.18.1)
README.md
37-37: Link fragments should be valid
(MD051, link-fragments)
39-39: Link fragments should be valid
(MD051, link-fragments)
248-248: Images should have alternate text (alt text)
(MD045, no-alt-text)
249-249: Images should have alternate text (alt text)
(MD045, no-alt-text)
250-250: Images should have alternate text (alt text)
(MD045, no-alt-text)
275-275: Images should have alternate text (alt text)
(MD045, no-alt-text)
276-276: Images should have alternate text (alt text)
(MD045, no-alt-text)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: code_check
🔇 Additional comments (9)
README.md (2)
17-28: All external URLs are correct and accessible; no action required.The verification confirms all six URLs in the README are reachable and point to the correct endpoints: deployment site, documentation, API Swagger UI, and monitoring dashboards (Prometheus, Grafana, RabbitMQ). While Grafana's frontend shows a load error, the endpoint itself is accessible, indicating a potential temporary infrastructure issue rather than a documentation problem. The URLs are accurate as documented.
121-139: The referenced imagedocs/architecture.pngexists in the repository (verified: 284,834 bytes). All other images referenced in the README (docs/grafana.png,docs/sentry.png,docs/erd.png,docs/test-coverage.png) are also present. No broken image placeholders will occur, and the architecture documentation is functional as written.Likely an incorrect or invalid review comment.
src/main/java/kr/it/pullit/modules/projection/learnstats/domain/LearnStats.java (1)
84-87: Clamping keeps totals non-negativeAppreciate the guard that clamps
totalCorrectQuestionCountso deletions can’t drive the projection below zero. Looks solid.src/main/java/kr/it/pullit/modules/projection/learnstats/api/LearnStatsPublicApi.java (1)
12-13: Public API surface aligns with new domain methodThe added contract cleanly mirrors the new projection hook. No concerns here.
src/main/java/kr/it/pullit/modules/wronganswer/repository/adapter/jpa/WrongAnswerJpaRepository.java (1)
17-18: Derived count query looks good
countByMemberIdAndQuestionIdInfits Spring Data’s naming rules and should translate correctly.src/main/java/kr/it/pullit/modules/projection/learnstats/service/LearnStatsService.java (1)
35-43: Service delegate matches the new API contractFetching, mutating, and saving in one place keeps the projection consistent. Looks good.
src/main/java/kr/it/pullit/modules/wronganswer/repository/WrongAnswerRepositoryImpl.java (1)
32-35: Implementation simply delegates to JPA repositoryThe new method mirrors the interface addition without extra logic. All good.
src/main/java/kr/it/pullit/modules/wronganswer/repository/WrongAnswerRepository.java (1)
17-18: Repository surface expanded appropriatelyInterface addition lines up with the implementation; no issues spotted.
src/test/java/kr/it/pullit/modules/questionset/service/QuestionSetServiceTest.java (1)
73-85: LGTM!The
setUpmethod correctly initializesQuestionSetServicewith the updated constructor signature, including the newwrongAnswerRepositoryandlearnStatsPublicApidependencies in the correct parameter positions.
| - [**소개 (Why Pullit?)**](#-why-pullit--왜-pullit을-만들었나요) | ||
| - [**주요 기능 (Key Features)**](#-key-features--pullit의-핵심-기능) | ||
| - [**기술 스택 (Tech Stack)**](#️-tech-stack--pullit을-움직이는-기술들) | ||
| - [**시스템 아키텍처 (Architecture)**](#-architecture--시스템-아키텍처) | ||
| - [**시스템 모니터링 (Monitoring)**](#-monitoring--시스템-모니터링) | ||
| - [**데이터베이스 ERD (Database ERD)**](#-database-erd--데이터-구조) | ||
| - [**팀원 소개 (Team)**](#-team--pullit을-만든-사람들) |
There was a problem hiding this comment.
Fix invalid markdown link fragments.
Link fragments containing emojis and special characters may not resolve correctly in all markdown renderers. Lines 34 and 40 use emojis in their fragment identifiers.
Apply this diff to use simple, ASCII-only identifiers in link fragments:
- [**소개 (Why Pullit?)**](#-why-pullit--왜-pullit을-만들었나요)
+ [**소개 (Why Pullit?)**](#why-pullit)- [**데이터베이스 ERD (Database ERD)**](#-database-erd--데이터-구조)
+ [**데이터베이스 ERD (Database ERD)**](#database-erd)Then update the corresponding heading anchors to match (e.g., ## Why Pullit instead of ## 🧐 Why Pullit?), or use explicit HTML anchors: <a id="why-pullit"></a>.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - [**소개 (Why Pullit?)**](#-why-pullit--왜-pullit을-만들었나요) | |
| - [**주요 기능 (Key Features)**](#-key-features--pullit의-핵심-기능) | |
| - [**기술 스택 (Tech Stack)**](#️-tech-stack--pullit을-움직이는-기술들) | |
| - [**시스템 아키텍처 (Architecture)**](#-architecture--시스템-아키텍처) | |
| - [**시스템 모니터링 (Monitoring)**](#-monitoring--시스템-모니터링) | |
| - [**데이터베이스 ERD (Database ERD)**](#-database-erd--데이터-구조) | |
| - [**팀원 소개 (Team)**](#-team--pullit을-만든-사람들) | |
| - [**소개 (Why Pullit?)**](#why-pullit) | |
| - [**주요 기능 (Key Features)**](#-key-features--pullit의-핵심-기능) | |
| - [**기술 스택 (Tech Stack)**](#️-tech-stack--pullit을-움직이는-기술들) | |
| - [**시스템 아키텍처 (Architecture)**](#-architecture--시스템-아키텍처) | |
| - [**시스템 모니터링 (Monitoring)**](#-monitoring--시스템-모니터링) | |
| - [**데이터베이스 ERD (Database ERD)**](#database-erd) | |
| - [**팀원 소개 (Team)**](#-team--pullit을-만든-사람들) |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
37-37: Link fragments should be valid
(MD051, link-fragments)
39-39: Link fragments should be valid
(MD051, link-fragments)
🤖 Prompt for AI Agents
In README.md around lines 34 to 40 the link fragments include emojis/special
characters which can break Markdown anchors; replace those fragment identifiers
with simple ASCII-only ids (e.g., #why-pullit, #key-features, #tech-stack,
#architecture, #monitoring, #database-erd, #team) and update the corresponding
headings to match by removing emojis/special characters (or add explicit HTML
anchors like <a id="why-pullit"></a> above each heading) so links resolve
correctly across renderers.
| <img src="docs/grafana.png" width="800" alt="Grafana Dashboard"> | ||
| <p><em>Grafana 대시보드를 통한 실시간 서버 지표 모니터링</em></p> | ||
| <img src="docs/sentry.png" width="800" alt="Sentry Error Tracking"> |
There was a problem hiding this comment.
Add alt text to all images for accessibility and linting compliance.
Images are missing descriptive alt text attributes, which violates accessibility standards and markdown linting rules (MD045). This affects screen reader users and markdown validation.
Apply these diffs to add alt text:
- <img src="docs/grafana.png" width="800" alt="Grafana Dashboard">
+ <img src="docs/grafana.png" width="800" alt="Grafana dashboard displaying real-time server metrics including CPU, memory, request rates, and response times">- <img src="docs/sentry.png" width="800" alt="Sentry Error Tracking">
+ <img src="docs/sentry.png" width="800" alt="Sentry error tracking dashboard showing application errors, stack traces, and error frequency analytics">- <img src="docs/erd.png" width="800" alt="ERD Diagram">
+ <img src="docs/erd.png" width="800" alt="Database Entity Relationship Diagram showing tables and relationships between Member, Source, Question, MarkingResult, WrongAnswer, LearnStats, and CommonFolder">- <img src="docs/test-coverage.png" alt="Test Coverage Report">
+ <img src="docs/test-coverage.png" alt="Jacoco test coverage report showing coverage percentages by module and method">Also applies to: 171-171, 194-194
🤖 Prompt for AI Agents
In README.md around lines 149-151 (and also apply same change at lines 171 and
194), the image tags lack descriptive alt text which fails accessibility and
MD045 linting; update each <img> tag to include concise, meaningful alt
attributes that describe the image content (e.g., "Grafana dashboard showing
server metrics", "Sentry error tracking dashboard", etc.), ensure alt text is
not empty, and run the markdown linter to confirm MD045 is resolved.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/kr/it/pullit/modules/learningsource/source/service/SourceService.java(2 hunks)src/main/java/kr/it/pullit/modules/questionset/service/QuestionSetService.java(5 hunks)src/test/java/kr/it/pullit/modules/questionset/service/QuestionSetServiceTest.java(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/kr/it/pullit/modules/learningsource/source/service/SourceService.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: code_check
🔇 Additional comments (7)
src/main/java/kr/it/pullit/modules/questionset/service/QuestionSetService.java (2)
4-4: LGTM: Dependencies properly added.The new imports and field declarations for
MarkingResultRepositoryandLearnStatsPublicApiare correctly structured and follow the existing pattern of constructor injection via@RequiredArgsConstructor.Also applies to: 14-14, 26-26, 50-51
317-318: LGTM: Defensive copy prevents ConcurrentModificationException.The
HashSetcopy correctly prevents modification exceptions when removing sources during iteration. This is the proper pattern for breaking bidirectional relationships.src/test/java/kr/it/pullit/modules/questionset/service/QuestionSetServiceTest.java (5)
24-24: LGTM: Test dependencies properly mocked.The new mock declarations for
MarkingResultRepositoryandLearnStatsPublicApiare correctly added and follow the existing test pattern.Also applies to: 36-36, 70-71
75-84: LGTM: Test setup correctly wired.The constructor call properly includes the new dependencies
markingResultRepositoryandlearnStatsPublicApi, matching the production service constructor.
491-509: LGTM: Test properly verifies LearnStats interaction.This test correctly addresses the past review comment by:
- Mocking
markingResultRepository.countCorrectByQuestionSetIdAndMemberIdto return a non-zero count- Verifying that
learnStatsPublicApi.applyQuestionSetDeletedis called with the expected parametersThe test validates the happy path where learning statistics need to be updated upon deletion.
511-530: LGTM: Test validates zero-count edge case.This test properly covers the scenario where a question set is deleted without any correct answer history. The verification that
learnStatsPublicApi.applyQuestionSetDeletedis never called ensures the service correctly skips the LearnStats update whencorrectCountis 0.
621-629: LGTM: Helper method refactored cleanly.The overloaded
createQuestionSetWithIdmethods provide flexibility for tests that need to specify a customownerIdwhile maintaining backward compatibility with existing tests that use the default.
1 participant
PR 설명
readme작성 및 삭제 관련 에러 해결
Summary by CodeRabbit
Bug Fixes
Documentation
Tests