Update form layouts after project policy reset#116
Conversation
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 53 minutes and 26 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, 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 trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR enhances form policy reset handling by integrating ChangesPolicy Reset with Form Layout Updates
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
engine/src/test/java/pl/edu/icm/unity/engine/project/TestGroupDelegationConfigGenerator.java (1)
266-272: ⚡ Quick winAssert DB update invocation in the regression test.
At Line 266-272, the test only checks the mutated in-memory object. If
enqFormDB.update(...)is removed, this can still pass. Add an explicitverifyon the DAO update call.Proposed test hardening
import static org.mockito.Mockito.when; +import static org.mockito.Mockito.verify; @@ generator.resetFormsPolicies("aenSuffix", FormType.ENQUIRY, List.of(2L)); + verify(mockEnqFormDB).update(form); assertThat(form.getPolicyAgreements()).hasSize(1);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@engine/src/test/java/pl/edu/icm/unity/engine/project/TestGroupDelegationConfigGenerator.java` around lines 266 - 272, The test in TestGroupDelegationConfigGenerator only asserts in-memory changes after calling generator.resetFormsPolicies(FormType.ENQUIRY, ...), so removal of the DAO update would not fail the test; add an explicit Mockito verify that the DAO update method was invoked (e.g., verify(enqFormDB).update(..) or verify(enqFormDB).update(form)) after the assertions to ensure the persistence call occurs; locate the call site around generator.resetFormsPolicies and add the verify(enqFormDB) statement referencing the actual form variable used in the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@engine/src/test/java/pl/edu/icm/unity/engine/project/TestGroupDelegationConfigGenerator.java`:
- Around line 266-272: The test in TestGroupDelegationConfigGenerator only
asserts in-memory changes after calling
generator.resetFormsPolicies(FormType.ENQUIRY, ...), so removal of the DAO
update would not fail the test; add an explicit Mockito verify that the DAO
update method was invoked (e.g., verify(enqFormDB).update(..) or
verify(enqFormDB).update(form)) after the assertions to ensure the persistence
call occurs; locate the call site around generator.resetFormsPolicies and add
the verify(enqFormDB) statement referencing the actual form variable used in the
test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dcd0c6ed-2b62-487a-9973-31ea6d58f4cb
📒 Files selected for processing (2)
engine/src/main/java/pl/edu/icm/unity/engine/utils/GroupDelegationConfigGeneratorImpl.javaengine/src/test/java/pl/edu/icm/unity/engine/project/TestGroupDelegationConfigGenerator.java
|
Addressed the CodeRabbit test-hardening comment. Change:
Validation:
Pushed follow-up commit: |
Summary
Root cause
policyAgreementslist but did not update stored custom form layouts.sys:policy-agreement-state.Fix
FormLayoutUtils.updateRegistrationFormLayout(...)andFormLayoutUtils.updateEnquiryLayout(...)after resetting policy agreements.Tests
mvn -pl engine -Dtest=TestGroupDelegationConfigGenerator#shouldUpdateCustomEnquiryLayoutWhenProjectPoliciesAreReset test -Dgpg.skip=true -qfailed because the custom layout contained onlyGROUP_0.mvn -pl engine -Dtest=TestGroupDelegationConfigGenerator,TestEnquiryPolicyAgreements test -Dgpg.skip=true -qpassed.git diff --checkpassed.upstream/dev: no findings.Manual verification
POLICY_AGREEMENT_nelements.Summary by CodeRabbit
Bug Fixes
Tests