Repository navigation
[Feat/#110] 습관 재도전 기능 구현 - #113
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 30 minutes and 49 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds failed-habit tracking and retry functionality: new endpoints to list and retry failed habits, DTOs for failed-habit responses, a persisted Changes
Sequence DiagramsequenceDiagram
participant Client
participant Controller as HabitController
participant Service as HabitService
participant Repository as HabitRepository
participant DB as Database
rect rgba(100,200,150,0.5)
Note over Client,DB: List Failed Habits
Client->>Controller: GET /api/v1/habits/failed
Controller->>Service: getFailedHabits(userId)
Service->>Repository: findAllFailedHabitsByUserId(userId)
Repository->>DB: SELECT habits WHERE status='FAIL'
DB-->>Repository: List<Habit>
Repository-->>Service: List<Habit>
Service->>Service: map to FailedHabitListResponse
Service-->>Controller: ApiResponse<FailedHabitListResponse>
Controller-->>Client: 200 OK
end
rect rgba(150,150,200,0.5)
Note over Client,DB: Retry Failed Habit
Client->>Controller: PATCH /api/v1/habits/failed/{habitId}/status/in-progress
Controller->>Service: retryFailedHabit(userId, habitId)
Service->>Repository: findByIdAndUserIdAndStatus(habitId,userId,FAIL)
Repository->>DB: SELECT habit WHERE id,user,status=FAIL
DB-->>Repository: Habit
Repository-->>Service: Habit
Service->>Service: load User -> habit.retry(user) (reset failCount, set status)
Service->>Repository: save updated Habit
Repository->>DB: UPDATE habit
DB-->>Repository: success
Service-->>Controller: ApiResponse<Void>
Controller-->>Client: 200 OK
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/swyp/server/domain/habit/entity/Habit.java`:
- Around line 49-50: The new non-nullable field Habit.failCount (annotated
`@Column`(nullable = false)) will fail on existing databases when Hibernate
ddl-auto=update tries to add the column without backfilling; confirm your
deployment target is a fresh DB or else create a migration/backfill: either run
a manual ALTER TABLE to add fail_count with a DEFAULT 0 and update existing
rows, or add a Flyway/Liquibase migration that alters the habits table and sets
fail_count = 0 for existing records (or temporarily make the column
nullable/defaulted then flip to NOT NULL), and ensure the Builder that
initializes failCount to 0 remains for new entities.
In `@src/main/java/com/swyp/server/domain/habit/entity/RewardStatus.java`:
- Around line 13-14: Remove the FAIL enum constant from RewardStatus so the
reward-tab filter cannot bind to a "FAIL" value; update any code that relied on
RewardStatus.FAIL (notably HabitService.determineSearchStatuses()) to use a
different status or handle failed-habit logic separately (e.g., use a separate
enum or explicit check) so failed habits are never returned for the reward tab.
Ensure RewardStatus only contains values intended for the rewards filter (e.g.,
ALL and any other allowed states) and adjust
HabitService.determineSearchStatuses() to map incoming filter params to the
appropriate allowed RewardStatus values or alternative handling.
In `@src/main/java/com/swyp/server/domain/habit/repository/HabitRepository.java`:
- Around line 71-77: The JPQL in updateCumulativeFailureHabits() currently sets
status='FAIL' for habits with failCount>1 without checking today's completion;
modify the query to include a check that h.isCompleted = false so only habits
not completed in the current cycle are failed (keep the existing conditions on
h.failCount, h.status, and h.duration). Locate the method
updateCumulativeFailureHabits() in HabitRepository and add the isCompleted
condition to the WHERE clause to mirror updateHabitFailCount() behavior.
- Around line 27-29: The query method findAllFailedHabitsByUserId currently
returns all habits with status 'FAIL' regardless of expiration; update its JPQL
to add an "AND h.expiredAt >= :now" predicate (matching the pattern used in
other repository queries) and add a new `@Param`("now") parameter, then update the
service method HabitService.getFailedHabits() to pass the current
Instant/LocalDateTime as the now parameter when calling
findAllFailedHabitsByUserId so only still-retryable failures within the
expiration window are returned.
In `@src/main/java/com/swyp/server/domain/habit/service/HabitScheduler.java`:
- Around line 33-39: The current order applies failure escalations before the
10-day reset, causing habits to be marked FAIL prematurely; in HabitScheduler
change the sequence so habitRepository.resetFailCount(now) runs before
habitRepository.updateHabitFailCount(), updateImmediateFailureHabits(), and
updateCumulativeFailureHabits() (or alternatively incorporate the 10-day reset
condition into the failure update queries), ensuring updateHabitFailCount(),
updateImmediateFailureHabits(), and updateCumulativeFailureHabits() operate on
already-reset fail counts.
In `@src/main/java/com/swyp/server/domain/habit/service/HabitService.java`:
- Around line 229-236: The retryFailedHabit method must only operate on habits
that are actually in FAIL and within the configured retry window, and it must
perform a full state reset (clear failCount and relevant timestamps) and set the
correct post-retry status depending on parent/child semantics. Update
retryFailedHabit to: 1) verify habit.getRewardStatus() == RewardStatus.FAIL and
check a retry-window predicate (e.g., habit.isWithinRetryPeriod() or compare
habit.getFailedAt()/habit.getConfiguredPeriod()), throwing
CustomException(ErrorCode.INVALID_HABIT_STATE) if not; 2) reset state fields
like habit.setFailCount(0) and clear/refresh any failure timestamps; and 3) set
reward status appropriately — for child habits use RewardStatus.REWARD_CHECKING
(so child shows 진행중 while parent shows 보상 확인중), otherwise set
RewardStatus.IN_PROGRESS for owning/parent habits; keep these updates inside the
existing `@Transactional` retryFailedHabit method and use the existing Habit
methods (or add small helpers on Habit) to perform the resets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 527137d1-031e-4f7b-b816-7cca613f8215
📒 Files selected for processing (9)
src/main/java/com/swyp/server/domain/habit/controller/HabitController.javasrc/main/java/com/swyp/server/domain/habit/dto/FailedHabitListResponse.javasrc/main/java/com/swyp/server/domain/habit/dto/FailedHabitResponse.javasrc/main/java/com/swyp/server/domain/habit/entity/Habit.javasrc/main/java/com/swyp/server/domain/habit/entity/RewardStatus.javasrc/main/java/com/swyp/server/domain/habit/repository/HabitRepository.javasrc/main/java/com/swyp/server/domain/habit/service/HabitScheduler.javasrc/main/java/com/swyp/server/domain/habit/service/HabitService.javasrc/test/java/com/swyp/server/domain/habit/HabitRepositoryTest.java
| @Column(nullable = false) | ||
| private int failCount; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect migration/changelog files for the new habits.fail_count column and a backfill/default.
fd -HI 'V*__*.sql' .
fd -HI '*changelog*' .
fd -HI '*.sql' .
fd -HI '*.xml' .
fd -HI '*.yaml' .
fd -HI '*.yml' .
rg -n -C2 'fail_count|ALTER TABLE habits|CREATE TABLE habits|UPDATE habits SET .*fail_count' .Repository: swyp-4th-team7/server
Length of output: 355
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for migration/changelog files with corrected syntax
echo "=== Searching for migration files ==="
fd --glob "**/V*.sql" . 2>/dev/null || true
fd --glob "**/migrations/**" . 2>/dev/null || true
fd --glob "**/*changelog*" . 2>/dev/null || true
echo ""
echo "=== Searching for SQL files ==="
find . -name "*.sql" -type f 2>/dev/null | head -20
echo ""
echo "=== Searching for XML changelog files ==="
find . -name "*.xml" -type f 2>/dev/null | head -20
echo ""
echo "=== Searching for fail_count references ==="
rg -i 'fail_count' --type sql --type xml --type yaml . 2>/dev/null || true
echo ""
echo "=== Searching for habits table modifications ==="
rg -i 'ALTER TABLE.*habits|CREATE TABLE.*habits|UPDATE.*habits' --type sql . 2>/dev/null || trueRepository: swyp-4th-team7/server
Length of output: 273
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Project structure (top-level directories) ==="
ls -la | grep -E '^d'
echo ""
echo "=== Checking for db/migration directories ==="
find . -type d -name "db" -o -type d -name "migration*" -o -type d -name "sql" 2>/dev/null
echo ""
echo "=== Checking pom.xml for migration tool configuration ==="
if [ -f pom.xml ]; then
rg -A5 -B5 'flyway|liquibase|migration' pom.xml
fi
echo ""
echo "=== Looking for resources directory ==="
find . -type d -name "resources" | head -10
echo ""
echo "=== List all files in src/main/resources if it exists ==="
if [ -d "src/main/resources" ]; then
find src/main/resources -type f | head -20
fi
echo ""
echo "=== Check if fail_count exists in any migration-related files ==="
find . -type f \( -name "*.sql" -o -name "*.xml" -o -name "*.yaml" -o -name "*.yml" \) 2>/dev/null | xargs grep -l "fail_count" 2>/dev/null || echo "No fail_count found in any files"Repository: swyp-4th-team7/server
Length of output: 953
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking build.gradle for migration tool ==="
if [ -f build.gradle ]; then
cat build.gradle | grep -A10 -B2 -i "flyway\|liquibase\|migration" || echo "No migration tool found in build.gradle"
fi
echo ""
echo "=== Git diff summary to see what changed ==="
git diff --stat HEAD~1 HEAD 2>/dev/null || git show --stat 2>/dev/null || echo "Cannot determine git diff"
echo ""
echo "=== Check if Habit.java changed ==="
git log -1 --oneline src/main/java/com/swyp/server/domain/habit/entity/Habit.java 2>/dev/null || true
echo ""
echo "=== Looking for any database schema files ==="
find . -type f \( -name "*.sql" -o -name "schema*" -o -name "*init*" \) 2>/dev/null | grep -v ".git" | head -20
echo ""
echo "=== Check if there's any database initialization in application.yml ==="
cat src/main/resources/application.yml | grep -A10 -B2 "jpa\|hibernate\|sql" || trueRepository: swyp-4th-team7/server
Length of output: 9484
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Full application.yml content ==="
cat src/main/resources/application.yml
echo ""
echo "=== Full build.gradle to check for DB setup ==="
cat build.gradle | grep -A20 -B5 "jpa\|hibernate\|database\|h2\|mysql" || true
echo ""
echo "=== Checking Habit.java for `@GeneratedValue` and other schema hints ==="
head -60 src/main/java/com/swyp/server/domain/habit/entity/Habit.javaRepository: swyp-4th-team7/server
Length of output: 4234
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Complete Habit.java to see full builder ==="
cat src/main/java/com/swyp/server/domain/habit/entity/Habit.javaRepository: swyp-4th-team7/server
Length of output: 2571
Verify deployment strategy for databases with existing habit rows.
The builder correctly initializes failCount to 0 for new entities. However, Hibernate's ddl-auto: update cannot safely add the NOT NULL fail_count column to an existing habits table with rows—it requires an explicit backfill strategy. For fresh database instances or new project deployments, this works fine. If deploying to a database that already contains habit records, you must either:
- Use manual SQL to add the column with a default value and backfill existing rows, or
- Configure a migration tool (Flyway/Liquibase) to handle schema changes safely.
Confirm your deployment target supports this constraint.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/swyp/server/domain/habit/entity/Habit.java` around lines 49
- 50, The new non-nullable field Habit.failCount (annotated `@Column`(nullable =
false)) will fail on existing databases when Hibernate ddl-auto=update tries to
add the column without backfilling; confirm your deployment target is a fresh DB
or else create a migration/backfill: either run a manual ALTER TABLE to add
fail_count with a DEFAULT 0 and update existing rows, or add a Flyway/Liquibase
migration that alters the habits table and sets fail_count = 0 for existing
records (or temporarily make the column nullable/defaulted then flip to NOT
NULL), and ensure the Builder that initializes failCount to 0 remains for new
entities.
| ALL("전체"), | ||
| FAIL("실패"); |
There was a problem hiding this comment.
Don't expose FAIL through the reward-tab filter enum.
RewardStatus is bound directly from GET /api/v1/habits/rewards?status=..., and HabitService.determineSearchStatuses() passes the selected status through. Adding FAIL here makes failed habits requestable from the reward tab, which conflicts with the requirement that failed habits must not be shown there.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/swyp/server/domain/habit/entity/RewardStatus.java` around
lines 13 - 14, Remove the FAIL enum constant from RewardStatus so the reward-tab
filter cannot bind to a "FAIL" value; update any code that relied on
RewardStatus.FAIL (notably HabitService.determineSearchStatuses()) to use a
different status or handle failed-habit logic separately (e.g., use a separate
enum or explicit check) so failed habits are never returned for the reward tab.
Ensure RewardStatus only contains values intended for the rewards filter (e.g.,
ALL and any other allowed states) and adjust
HabitService.determineSearchStatuses() to map incoming filter params to the
appropriate allowed RewardStatus values or alternative handling.
| @EntityGraph(attributePaths = "user") | ||
| @Query("SELECT h FROM Habit h " + "WHERE h.user.id = :userId " + "AND h.status = 'FAIL'") | ||
| List<Habit> findAllFailedHabitsByUserId(@Param("userId") Long userId); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -i 'HabitService.java' src/main/java -x rg -n -C3 'getFailedHabits|findAllFailedHabitsByUserId|expiredAt|retry' {}Repository: swyp-4th-team7/server
Length of output: 580
🏁 Script executed:
fd -i 'FailedHabitListResponse.java' src/main/java -x cat {} | head -100Repository: swyp-4th-team7/server
Length of output: 439
🏁 Script executed:
fd -i 'Habit.java' src/main/java -type f -path '*/domain/habit/entity/*' -x cat {} | head -150Repository: swyp-4th-team7/server
Length of output: 235
🏁 Script executed:
rg -n 'expiredAt|retry.*period|RETRY_PERIOD' src/main/java --max-count=20Repository: swyp-4th-team7/server
Length of output: 760
🏁 Script executed:
rg -n -A 10 'public void retryFailedHabit' src/main/javaRepository: swyp-4th-team7/server
Length of output: 1349
Limit failed habit retrieval to still-retryable items within the expiration window.
Currently, findAllFailedHabitsByUserId() returns all FAIL status habits without an expiredAt boundary. The service layer applies no downstream filter, so expired failures leak into the API response. Since the feature targets retryable habits within the configured period, add an expiredAt >= :now predicate to align with the existing pattern used elsewhere in HabitRepository (lines 47, 60):
Suggested fix
- `@Query`("SELECT h FROM Habit h " + "WHERE h.user.id = :userId " + "AND h.status = 'FAIL'")
- List<Habit> findAllFailedHabitsByUserId(`@Param`("userId") Long userId);
+ `@Query`(
+ "SELECT h FROM Habit h "
+ + "WHERE h.user.id = :userId "
+ + "AND h.status = 'FAIL' "
+ + "AND h.expiredAt >= :now "
+ + "ORDER BY h.id DESC")
+ List<Habit> findAllFailedHabitsByUserId(
+ `@Param`("userId") Long userId, `@Param`("now") LocalDateTime now);Update the service call in HabitService.getFailedHabits() to pass the current time parameter.
📝 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.
| @EntityGraph(attributePaths = "user") | |
| @Query("SELECT h FROM Habit h " + "WHERE h.user.id = :userId " + "AND h.status = 'FAIL'") | |
| List<Habit> findAllFailedHabitsByUserId(@Param("userId") Long userId); | |
| `@EntityGraph`(attributePaths = "user") | |
| `@Query`( | |
| "SELECT h FROM Habit h " | |
| "WHERE h.user.id = :userId " | |
| "AND h.status = 'FAIL' " | |
| "AND h.expiredAt >= :now " | |
| "ORDER BY h.id DESC") | |
| List<Habit> findAllFailedHabitsByUserId( | |
| `@Param`("userId") Long userId, `@Param`("now") LocalDateTime now); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/swyp/server/domain/habit/repository/HabitRepository.java`
around lines 27 - 29, The query method findAllFailedHabitsByUserId currently
returns all habits with status 'FAIL' regardless of expiration; update its JPQL
to add an "AND h.expiredAt >= :now" predicate (matching the pattern used in
other repository queries) and add a new `@Param`("now") parameter, then update the
service method HabitService.getFailedHabits() to pass the current
Instant/LocalDateTime as the now parameter when calling
findAllFailedHabitsByUserId so only still-retryable failures within the
expiration window are returned.
| @Modifying(clearAutomatically = true) | ||
| @Query( | ||
| "UPDATE Habit h SET h.status = 'FAIL' " | ||
| + " WHERE h.failCount > 1 " | ||
| + " AND h.status IN ('REWARD_CHECKING', 'IN_PROGRESS')" | ||
| + " AND h.duration NOT IN ('THREE_DAYS', 'SEVEN_DAYS')") | ||
| void updateCumulativeFailureHabits(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -i 'HabitScheduler.java' src/main/java -x rg -n -C4 'checkExpiredHabits|updateImmediateFailureHabits|updateCumulativeFailureHabits|updateHabitFailCount|resetFailCount' {}Repository: swyp-4th-team7/server
Length of output: 775
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, find and examine the Habit entity
fd -i 'Habit.java' src/main/java -type f | head -5Repository: swyp-4th-team7/server
Length of output: 235
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for isCompleted field usage in the codebase
rg 'isCompleted' src/main/java --type java -B2 -A2 | head -80Repository: swyp-4th-team7/server
Length of output: 6883
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look at the updateHabitFailCount method implementation
rg -n 'updateHabitFailCount' src/main/java --type java -A10Repository: swyp-4th-team7/server
Length of output: 2655
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the Habit entity file and get line count
fd -i 'Habit.java' src/main/java -path '*/entity/*' -type fRepository: swyp-4th-team7/server
Length of output: 233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get the full updateHabitFailCount method
rg -n 'updateHabitFailCount' src/main/java -B2 -A15Repository: swyp-4th-team7/server
Length of output: 4334
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get the complete updateCumulativeFailureHabits query
rg -n 'updateCumulativeFailureHabits' src/main/java -B2 -A8Repository: swyp-4th-team7/server
Length of output: 2691
Add isCompleted = false check to prevent failing habits that were completed today.
The query marks a habit FAIL whenever failCount > 1, but doesn't verify whether the habit was actually incomplete today. This allows a habit with accumulated failures from previous days to be marked FAIL even if isCompleted = true in the current cycle. Since updateHabitFailCount() only increments the counter when isCompleted = false, the cumulative check should do the same.
Required fix
`@Query`(
"UPDATE Habit h SET h.status = 'FAIL' "
- + " WHERE h.failCount > 1 "
+ + " WHERE h.isCompleted = false "
+ + " AND h.failCount > 1 "
+ " AND h.status IN ('REWARD_CHECKING', 'IN_PROGRESS')"
+ " AND h.duration NOT IN ('THREE_DAYS', 'SEVEN_DAYS')")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/swyp/server/domain/habit/repository/HabitRepository.java`
around lines 71 - 77, The JPQL in updateCumulativeFailureHabits() currently sets
status='FAIL' for habits with failCount>1 without checking today's completion;
modify the query to include a check that h.isCompleted = false so only habits
not completed in the current cycle are failed (keep the existing conditions on
h.failCount, h.status, and h.duration). Locate the method
updateCumulativeFailureHabits() in HabitRepository and add the isCompleted
condition to the WHERE clause to mirror updateHabitFailCount() behavior.
| log.info("실패 습관 확인 스케줄러 시작"); | ||
| habitRepository.updateHabitFailCount(); | ||
| habitRepository.updateImmediateFailureHabits(); | ||
| habitRepository.updateCumulativeFailureHabits(); | ||
|
|
||
| log.info("습관 실패 횟수 초기화 스케줄러 시작"); | ||
| habitRepository.resetFailCount(now); |
There was a problem hiding this comment.
Apply the 10-day reset before escalating to FAIL.
For 14–99 day habits, the new policy says the failure count resets every 10 days. In the current order, a habit with failCount = 1 that misses on day 10 is incremented to 2 and marked FAIL before resetFailCount(now) runs, and then the reset no longer applies because the status is already FAIL. Reorder these calls or fold the reset condition into the failure queries.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/swyp/server/domain/habit/service/HabitScheduler.java`
around lines 33 - 39, The current order applies failure escalations before the
10-day reset, causing habits to be marked FAIL prematurely; in HabitScheduler
change the sequence so habitRepository.resetFailCount(now) runs before
habitRepository.updateHabitFailCount(), updateImmediateFailureHabits(), and
updateCumulativeFailureHabits() (or alternatively incorporate the 10-day reset
condition into the failure update queries), ensuring updateHabitFailCount(),
updateImmediateFailureHabits(), and updateCumulativeFailureHabits() operate on
already-reset fail counts.
| @Transactional | ||
| public void retryFailedHabit(Long userId, Long habitId) { | ||
| Habit habit = | ||
| habitRepository | ||
| .findByIdAndUserId(habitId, userId) | ||
| .orElseThrow(() -> new CustomException(ErrorCode.HABIT_NOT_FOUND)); | ||
| habit.updateRewardStatus(RewardStatus.IN_PROGRESS); | ||
| } |
There was a problem hiding this comment.
Retry needs state validation and a full state reset.
This currently rewrites any owned habit to IN_PROGRESS. That lets non-FAIL habits be retried, ignores the "within configured period" constraint, and leaves 14–99 day habits with their previous failCount, so they can fall back to FAIL again on the next scheduler run. It also breaks the child flow from the issue: child habits should go back to REWARD_CHECKING so the child sees 진행중 while the parent sees 보상 확인중.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/swyp/server/domain/habit/service/HabitService.java` around
lines 229 - 236, The retryFailedHabit method must only operate on habits that
are actually in FAIL and within the configured retry window, and it must perform
a full state reset (clear failCount and relevant timestamps) and set the correct
post-retry status depending on parent/child semantics. Update retryFailedHabit
to: 1) verify habit.getRewardStatus() == RewardStatus.FAIL and check a
retry-window predicate (e.g., habit.isWithinRetryPeriod() or compare
habit.getFailedAt()/habit.getConfiguredPeriod()), throwing
CustomException(ErrorCode.INVALID_HABIT_STATE) if not; 2) reset state fields
like habit.setFailCount(0) and clear/refresh any failure timestamps; and 3) set
reward status appropriately — for child habits use RewardStatus.REWARD_CHECKING
(so child shows 진행중 while parent shows 보상 확인중), otherwise set
RewardStatus.IN_PROGRESS for owning/parent habits; keep these updates inside the
existing `@Transactional` retryFailedHabit method and use the existing Habit
methods (or add small helpers on Habit) to perform the resets.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/main/java/com/swyp/server/domain/habit/repository/HabitRepository.java (2)
29-31:⚠️ Potential issue | 🟠 MajorFilter failed habits to the active retry window.
Line 30 still returns every
FAILhabit for the user, so expired failures leak into the failed-habits API as if they were still retryable. Please add theexpiredAtboundary here and passnowfromHabitService.getFailedHabits(...).Suggested fix
`@EntityGraph`(attributePaths = "user") -@Query("SELECT h FROM Habit h " + "WHERE h.user.id = :userId " + "AND h.status = 'FAIL'") -List<Habit> findAllFailedHabitsByUserId(`@Param`("userId") Long userId); +@Query( + "SELECT h FROM Habit h " + + "WHERE h.user.id = :userId " + + "AND h.status = 'FAIL' " + + "AND h.expiredAt >= :now " + + "ORDER BY h.id DESC") +List<Habit> findAllFailedHabitsByUserId( + `@Param`("userId") Long userId, `@Param`("now") LocalDateTime now);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/swyp/server/domain/habit/repository/HabitRepository.java` around lines 29 - 31, The repository query findAllFailedHabitsByUserId currently returns all FAIL habits; restrict it to the active retry window by adding an expiredAt boundary and a now parameter: update the JPQL in findAllFailedHabitsByUserId (method name) to include "AND h.expiredAt > :now" (or use >= if your semantics require inclusivity) and add a corresponding `@Param`("now") Instant now parameter so HabitService.getFailedHabits(...) can pass the current time.
73-79:⚠️ Potential issue | 🟠 MajorDon't fail habits that were completed in the current cycle.
Line 76 only checks
failCount > 1. A habit that already accumulated failures can still be markedFAILhere even whenisCompleted = truetoday. This should mirror the incomplete-only guard already used inupdateHabitFailCount().Suggested fix
`@Query`( "UPDATE Habit h SET h.status = 'FAIL' " - + " WHERE h.failCount > 1 " + + " WHERE h.isCompleted = false " + + " AND h.failCount > 1 " + " AND h.status IN ('REWARD_CHECKING', 'IN_PROGRESS')" + " AND h.duration NOT IN ('THREE_DAYS', 'SEVEN_DAYS')")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/swyp/server/domain/habit/repository/HabitRepository.java` around lines 73 - 79, The query in updateCumulativeFailureHabits() is incorrectly marking habits as FAIL even if they are completed today; add the same incomplete guard used in updateHabitFailCount() so the UPDATE only targets habits where isCompleted = false (or the equivalent boolean field), i.e., include "AND h.isCompleted = false" (or the matching property name) alongside the existing predicates to prevent failing habits completed in the current cycle.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/main/java/com/swyp/server/domain/habit/repository/HabitRepository.java`:
- Around line 29-31: The repository query findAllFailedHabitsByUserId currently
returns all FAIL habits; restrict it to the active retry window by adding an
expiredAt boundary and a now parameter: update the JPQL in
findAllFailedHabitsByUserId (method name) to include "AND h.expiredAt > :now"
(or use >= if your semantics require inclusivity) and add a corresponding
`@Param`("now") Instant now parameter so HabitService.getFailedHabits(...) can
pass the current time.
- Around line 73-79: The query in updateCumulativeFailureHabits() is incorrectly
marking habits as FAIL even if they are completed today; add the same incomplete
guard used in updateHabitFailCount() so the UPDATE only targets habits where
isCompleted = false (or the equivalent boolean field), i.e., include "AND
h.isCompleted = false" (or the matching property name) alongside the existing
predicates to prevent failing habits completed in the current cycle.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a840063d-1b21-4f4e-b355-8a11e8814309
📒 Files selected for processing (3)
src/main/java/com/swyp/server/domain/habit/entity/Habit.javasrc/main/java/com/swyp/server/domain/habit/repository/HabitRepository.javasrc/main/java/com/swyp/server/domain/habit/service/HabitService.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/swyp/server/domain/habit/entity/Habit.java
| + " WHERE h.failCount > 1 " | ||
| + " AND h.status IN ('REWARD_CHECKING', 'IN_PROGRESS')" | ||
| + " AND h.duration NOT IN ('THREE_DAYS', 'SEVEN_DAYS')") | ||
| void updateCumulativeFailureHabits(); |
There was a problem hiding this comment.
P1: updateCumulativeFailureHabits 쿼리에 isCompleted = false 조건 추가가 필요할 것 같습니다. 오늘 완료한 습관도 failCount > 1이면 FAIL이 될 수 있습니다!
There was a problem hiding this comment.
윤미님 좋은 의견 감사합니다!
하지만 해당 로직은 금일 완료 여부와 상관 없이 누적된 습관 실패 횟수를 기반으로 동작하는 것을 의도로 설계를 했기 때문에 수정하지 않아도 괜찮을 거라 생각합니다.
혹시 제가 오해한 부분이 있으면 코멘트 남겨주세요..!!
| habitRepository.updateCumulativeFailureHabits(); | ||
|
|
||
| log.info("습관 실패 횟수 초기화 스케줄러 시작"); | ||
| habitRepository.resetFailCount(now); |
There was a problem hiding this comment.
P1: resetFailCount가 updateCumulativeFailureHabits 이후에 실행되고 있어서 10일째 되는 날 failCount = 1인 습관이 2로 증가 후 FAIL 처리되고 나서 초기화가 되지 않는 문제가 있을 것 같습니다. resetFailCount를 먼저 실행하는 게 맞지 않을까 싶어서 의견 남깁니다!!
There was a problem hiding this comment.
윤미님 저도 해당 부분이 헷갈려서 저희 서비스 정책을 확인해보니, 10일에 2번 이상 실패하면 FAIL 처리되도록 정책이 설정되어 있어서 failCount를 먼저 증가시키고 초기화 시키는 순서가 맞다고 생각이 들어 위와 같이 로직을 작성하게 되었습니다.
혹시 다른 의도로 의견 남겨주신거면 코멘트 부탁드립니다! 의견 주셔서 감사해요 :)
| COMPLETE("완료"), | ||
| ALL("전체"); | ||
| ALL("전체"), | ||
| FAIL("실패"); |
There was a problem hiding this comment.
P2: FAIL이 RewardStatus에 추가되면 보상 탭에서 ?status=FAIL로 조회가 가능해집니다. determineSearchStatuses에서 FAIL을 필터링해주는 처리가 필요할 것 같습니다!
| private LocalDateTime expiredAt; | ||
|
|
||
| @Column(nullable = false) | ||
| private int failCount; |
There was a problem hiding this comment.
P2: 기존 DB에 habits 데이터가 있는 경우 NOT NULL 컬럼 추가 시 오류가 날 수 있습니다. 배포 전 RDS에서 ALTER TABLE habits ADD COLUMN fail_count INT NOT NULL DEFAULT 0; 실행이 필요합니다!
There was a problem hiding this comment.
해당 쿼리 반영하겠습니다! 감사합니다 :)
2 participants
📌 관련 이슈
✨ 변경 사항
📸 테스트 증명 (필수)
📚 리뷰어 참고 사항
✅ 체크리스트
Summary by CodeRabbit