Bugfix 13843 merging case survey token issue#13873
Merged
KarnaiahPesula merged 8 commits intodevelopmentfrom Mar 11, 2026
Merged
Bugfix 13843 merging case survey token issue#13873KarnaiahPesula merged 8 commits intodevelopmentfrom
KarnaiahPesula merged 8 commits intodevelopmentfrom
Conversation
…//github.com/SORMAS-Foundation/SORMAS-Project into bugfix-13843-merging-case-survey-token-issue
📝 WalkthroughWalkthroughExtends case-merge logic to reassign SurveyToken entities to the lead case, adds a finder method to query SurveyToken by criteria, and introduces test helpers and a new test validating survey token creation and retrieval. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.41.0)sormas-backend/src/main/java/de/symeda/sormas/backend/caze/CaseFacadeEjb.javaThanks 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.
🧹 Nitpick comments (4)
sormas-backend/src/main/java/de/symeda/sormas/backend/survey/SurveyTokenService.java (1)
112-123: Extract the shared token-query builder.
findBynow duplicates the same filter/order assembly already present ingetToken. A small private helper would keep those two paths from drifting the next time the criteria or sort order changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-backend/src/main/java/de/symeda/sormas/backend/survey/SurveyTokenService.java` around lines 112 - 123, findBy duplicates the filter and ordering logic used in getToken; extract a private helper (e.g., buildTokenQuery or createTokenCriteriaQuery) that accepts SurveyTokenCriteria, CriteriaBuilder and returns a configured CriteriaQuery/TypedQuery or Predicate+Order, reusing buildCriteriaFilter and new SurveyTokenJoins(root) and applying cb.desc(root.get(SurveyToken.ASSIGNMENT_DATE)); replace the duplicated code in both findBy and getToken to call this helper so filter construction and order are centralized.sormas-backend/src/test/java/de/symeda/sormas/backend/survey/SurveyServiceTest.java (2)
18-21: Class name is misleading.The class is named
SurveyServiceTestbut it testsSurveyTokenService.findBy()functionality. Consider renaming toSurveyTokenServiceTestto accurately reflect what is being tested.♻️ Suggested rename
-public class SurveyServiceTest extends AbstractBeanTest { +public class SurveyTokenServiceTest extends AbstractBeanTest {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-backend/src/test/java/de/symeda/sormas/backend/survey/SurveyServiceTest.java` around lines 18 - 21, The test class is misnamed: currently class SurveyServiceTest but exercises SurveyTokenService.findBy(); rename the class to SurveyTokenServiceTest (update the class declaration SurveyServiceTest -> SurveyTokenServiceTest) and update the test file name and any references/imports or test-suite entries that reference SurveyServiceTest so they match the new name; ensure the test runner/annotations remain intact and run tests to verify no remaining broken references.
33-41: Strengthen test assertions to verify actual behavior.The current assertion only checks that the list is non-empty, which is a weak validation. The test should verify:
- The exact number of tokens returned (should be 1)
- The token is associated with the correct case
- The token value matches
This makes the test more reliable and catches regressions more effectively.
♻️ Proposed improvement
`@Test` public void testSurveyTokens() { CaseDataDto caze = creator.createCase(surveillanceOfficer.toReference(), rdcf, (c) -> { c.setDisease(Disease.MALARIA); }); - creator.createSurveyToken(caze.toReference()); - List<SurveyToken> tokenList = getSurveyTokenService().findBy(new SurveyTokenCriteria()); - assertThat(tokenList.size(), is(greaterThan(0))); + SurveyTokenDto createdToken = creator.createSurveyToken(caze.toReference()); + + SurveyTokenCriteria criteria = new SurveyTokenCriteria(); + List<SurveyToken> tokenList = getSurveyTokenService().findBy(criteria); + + assertThat(tokenList.size(), is(1)); + SurveyToken retrievedToken = tokenList.get(0); + assertThat(retrievedToken.getUuid(), is(createdToken.getUuid())); + assertThat(retrievedToken.getCaseAssignedTo().getUuid(), is(caze.getUuid())); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-backend/src/test/java/de/symeda/sormas/backend/survey/SurveyServiceTest.java` around lines 33 - 41, Update the testSurveyTokens test to assert concrete behavior: after creating a case (CaseDataDto caze) and calling creator.createSurveyToken(caze.toReference()), query SurveyToken list via getSurveyTokenService().findBy(new SurveyTokenCriteria()) and assert the list size equals 1, then fetch the single SurveyToken and assert its case/reference matches caze.toReference() and its token value is not null and equals the value returned/created by creator.createSurveyToken (or the stored token value), using the SurveyToken class fields to compare (e.g., getCaseReference()/getToken()) so the test verifies number, association, and token value rather than just non-empty list.sormas-backend/src/test/java/de/symeda/sormas/backend/TestDataCreator.java (1)
2480-2486: Consider adding flexibility to the test helper method.The current implementation has several limitations:
- Hardcoded survey name and disease (
"Malaria-Survey",Disease.MALARIA) regardless of the case's actual disease- Fixed token value
"MALA01"may cause conflicts if called multiple times- A new survey is created on every call, which may lead to unnecessary test data
Consider following the established pattern in this class by adding a
Consumer<SurveyTokenDto>parameter for customization:♻️ Proposed refactor for flexibility
- public SurveyTokenDto createSurveyToken(CaseReferenceDto caze) { - SurveyDto survey = this.createSurvey("Malaria-Survey", Disease.MALARIA); + public SurveyTokenDto createSurveyToken(CaseReferenceDto caze, SurveyDto survey, Consumer<SurveyTokenDto> customConfig) { SurveyTokenDto token = SurveyTokenDto.build(survey.toReference()); - token.setToken("MALA01"); + token.setToken(DataHelper.createUuid().substring(0, 8)); token.setCaseAssignedTo(caze); + if (customConfig != null) { + customConfig.accept(token); + } return beanTest.getSurveyTokenFacade().save(token); } + + public SurveyTokenDto createSurveyToken(CaseReferenceDto caze) { + SurveyDto survey = this.createSurvey("Test-Survey", Disease.MALARIA); + return createSurveyToken(caze, survey, null); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-backend/src/test/java/de/symeda/sormas/backend/TestDataCreator.java` around lines 2480 - 2486, The createSurveyToken(CaseReferenceDto caze) helper is too rigid—change it to accept a Consumer<SurveyTokenDto> (or add an overloaded method) so callers can customize the token; when creating the SurveyTokenDto (currently built via SurveyTokenDto.build(survey.toReference())), derive the survey parameters from the provided case (use the case's disease or reuse an existing survey instead of always calling createSurvey("Malaria-Survey", Disease.MALARIA)), generate a unique token value instead of the fixed "MALA01" (e.g., append a timestamp or counter), apply the Consumer to allow further tweaks, and then save via beanTest.getSurveyTokenFacade().save(token); keep the original method as a simple wrapper to preserve tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@sormas-backend/src/main/java/de/symeda/sormas/backend/survey/SurveyTokenService.java`:
- Around line 112-123: findBy duplicates the filter and ordering logic used in
getToken; extract a private helper (e.g., buildTokenQuery or
createTokenCriteriaQuery) that accepts SurveyTokenCriteria, CriteriaBuilder and
returns a configured CriteriaQuery/TypedQuery or Predicate+Order, reusing
buildCriteriaFilter and new SurveyTokenJoins(root) and applying
cb.desc(root.get(SurveyToken.ASSIGNMENT_DATE)); replace the duplicated code in
both findBy and getToken to call this helper so filter construction and order
are centralized.
In
`@sormas-backend/src/test/java/de/symeda/sormas/backend/survey/SurveyServiceTest.java`:
- Around line 18-21: The test class is misnamed: currently class
SurveyServiceTest but exercises SurveyTokenService.findBy(); rename the class to
SurveyTokenServiceTest (update the class declaration SurveyServiceTest ->
SurveyTokenServiceTest) and update the test file name and any references/imports
or test-suite entries that reference SurveyServiceTest so they match the new
name; ensure the test runner/annotations remain intact and run tests to verify
no remaining broken references.
- Around line 33-41: Update the testSurveyTokens test to assert concrete
behavior: after creating a case (CaseDataDto caze) and calling
creator.createSurveyToken(caze.toReference()), query SurveyToken list via
getSurveyTokenService().findBy(new SurveyTokenCriteria()) and assert the list
size equals 1, then fetch the single SurveyToken and assert its case/reference
matches caze.toReference() and its token value is not null and equals the value
returned/created by creator.createSurveyToken (or the stored token value), using
the SurveyToken class fields to compare (e.g., getCaseReference()/getToken()) so
the test verifies number, association, and token value rather than just
non-empty list.
In `@sormas-backend/src/test/java/de/symeda/sormas/backend/TestDataCreator.java`:
- Around line 2480-2486: The createSurveyToken(CaseReferenceDto caze) helper is
too rigid—change it to accept a Consumer<SurveyTokenDto> (or add an overloaded
method) so callers can customize the token; when creating the SurveyTokenDto
(currently built via SurveyTokenDto.build(survey.toReference())), derive the
survey parameters from the provided case (use the case's disease or reuse an
existing survey instead of always calling createSurvey("Malaria-Survey",
Disease.MALARIA)), generate a unique token value instead of the fixed "MALA01"
(e.g., append a timestamp or counter), apply the Consumer to allow further
tweaks, and then save via beanTest.getSurveyTokenFacade().save(token); keep the
original method as a simple wrapper to preserve tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 392dfe13-378b-414c-b200-d14a2f6e6761
📒 Files selected for processing (5)
sormas-api/src/main/java/de/symeda/sormas/api/docgeneneration/DocumentWorkflow.javasormas-backend/src/main/java/de/symeda/sormas/backend/caze/CaseFacadeEjb.javasormas-backend/src/main/java/de/symeda/sormas/backend/survey/SurveyTokenService.javasormas-backend/src/test/java/de/symeda/sormas/backend/TestDataCreator.javasormas-backend/src/test/java/de/symeda/sormas/backend/survey/SurveyServiceTest.java
Contributor
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13873 |
…n SORMAS Summary: Incorporate survey token while merging the cases. Changes: Introduced a new method to find the survey token for the case. For a normal merge scenario, assign the respective caseId. In case of cloning, it's not correct to assign the existing survey to the new case. Updated the TestDataCreator for returning the test survey token details. Created SurveyServiceTest to validate findSurveyToken of the case.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
sormas-backend/src/test/java/de/symeda/sormas/backend/TestDataCreator.java (1)
2480-2491: Incomplete Javadoc and hardcoded token value.The Javadoc has empty
@param cazeand@returndescriptions. Consider completing them for clarity.The hardcoded token value
"MALA01"works because thetokencolumn lacks a unique constraint (perSurveyToken.java), but this approach may cause test isolation issues if multiple tests call this method and later query tokens without filtering by the specific case or survey.📝 Suggested Javadoc improvement
/** - * - * `@param` caze - * `@return` + * Creates a survey token assigned to the given case for testing purposes. + * + * `@param` caze the case reference to associate with the token + * `@return` the persisted SurveyTokenDto */ public SurveyTokenDto createSurveyToken(CaseReferenceDto caze) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-backend/src/test/java/de/symeda/sormas/backend/TestDataCreator.java` around lines 2480 - 2491, The createSurveyToken method's Javadoc is incomplete and the token value is hardcoded which can cause cross-test collisions; update the Javadoc to describe the caze parameter and the returned SurveyTokenDto, and change the token generation in createSurveyToken (rather than token.setToken("MALA01")) to produce a unique value per call (e.g., append a UUID or use survey/case identifiers) before saving via beanTest.getSurveyTokenFacade().save(token); reference SurveyTokenDto.build, token.setToken, and SurveyToken.java when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@sormas-backend/src/test/java/de/symeda/sormas/backend/survey/SurveyServiceTest.java`:
- Around line 52-65: The test has two issues: the Javadoc incorrectly references
SurveyService#findBy (update it to SurveyTokenService#findBy) and the query uses
new SurveyTokenCriteria() which returns all tokens causing flaky ordering; fix
by populating the criteria with the created case reference (use the appropriate
setter on SurveyTokenCriteria, e.g., setCaseRef(caze.toReference()) or
equivalent) before calling getSurveyTokenService().findBy so the result is
limited to the token created by creator.createSurveyToken(caze.toReference())
and update assertions accordingly.
---
Nitpick comments:
In `@sormas-backend/src/test/java/de/symeda/sormas/backend/TestDataCreator.java`:
- Around line 2480-2491: The createSurveyToken method's Javadoc is incomplete
and the token value is hardcoded which can cause cross-test collisions; update
the Javadoc to describe the caze parameter and the returned SurveyTokenDto, and
change the token generation in createSurveyToken (rather than
token.setToken("MALA01")) to produce a unique value per call (e.g., append a
UUID or use survey/case identifiers) before saving via
beanTest.getSurveyTokenFacade().save(token); reference SurveyTokenDto.build,
token.setToken, and SurveyToken.java when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f9bea33e-ca4f-4ffd-9e50-e1dd134d35c8
📒 Files selected for processing (3)
sormas-backend/src/main/java/de/symeda/sormas/backend/caze/CaseFacadeEjb.javasormas-backend/src/test/java/de/symeda/sormas/backend/TestDataCreator.javasormas-backend/src/test/java/de/symeda/sormas/backend/survey/SurveyServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- sormas-backend/src/main/java/de/symeda/sormas/backend/caze/CaseFacadeEjb.java
Comment on lines
+52
to
+65
| /** | ||
| * Test method for {@link de.symeda.sormas.backend.survey.SurveyService#findBy(de.symeda.sormas.api.survey.SurveyTokenCriteria)}. | ||
| * | ||
| */ | ||
| @Test | ||
| public void testSurveyTokens() { | ||
| CaseDataDto caze = creator.createCase(surveillanceOfficer.toReference(), rdcf, (c) -> { | ||
| c.setDisease(Disease.MALARIA); | ||
| }); | ||
| creator.createSurveyToken(caze.toReference()); | ||
| List<SurveyToken> tokenList = getSurveyTokenService().findBy(new SurveyTokenCriteria()); | ||
| assertThat(tokenList.size(), is(greaterThan(0))); | ||
| assertThat(tokenList.get(0).getToken(), is(equalTo("MALA01"))); | ||
| } |
There was a problem hiding this comment.
Test may be flaky due to unfiltered query and incorrect Javadoc reference.
-
Incorrect Javadoc: Line 53 references
SurveyService#findBybut the actual method being tested isSurveyTokenService#findBy. -
Test isolation concern: Using
new SurveyTokenCriteria()returns ALL survey tokens in the database (perSurveyTokenService.buildCriteriaFilterwhich returns null when all criteria fields are null). If other tests in the suite create tokens, this test could:- Pass assertion at Line 63 (
size > 0) but for the wrong reason - Fail assertion at Line 64 if another token with a newer
ASSIGNMENT_DATEappears first
- Pass assertion at Line 63 (
Consider filtering by the created case or survey to ensure test isolation:
🛡️ Suggested fix for test isolation
/**
- * Test method for {`@link` de.symeda.sormas.backend.survey.SurveyService#findBy(de.symeda.sormas.api.survey.SurveyTokenCriteria)}.
+ * Test method for {`@link` de.symeda.sormas.backend.survey.SurveyTokenService#findBy(de.symeda.sormas.api.survey.SurveyTokenCriteria)}.
*
*/
`@Test`
public void testSurveyTokens() {
CaseDataDto caze = creator.createCase(surveillanceOfficer.toReference(), rdcf, (c) -> {
c.setDisease(Disease.MALARIA);
});
- creator.createSurveyToken(caze.toReference());
- List<SurveyToken> tokenList = getSurveyTokenService().findBy(new SurveyTokenCriteria());
+ SurveyTokenDto createdToken = creator.createSurveyToken(caze.toReference());
+ SurveyTokenCriteria criteria = new SurveyTokenCriteria();
+ criteria.setCaseAssignedTo(caze.toReference());
+ List<SurveyToken> tokenList = getSurveyTokenService().findBy(criteria);
assertThat(tokenList.size(), is(greaterThan(0)));
assertThat(tokenList.get(0).getToken(), is(equalTo("MALA01")));
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@sormas-backend/src/test/java/de/symeda/sormas/backend/survey/SurveyServiceTest.java`
around lines 52 - 65, The test has two issues: the Javadoc incorrectly
references SurveyService#findBy (update it to SurveyTokenService#findBy) and the
query uses new SurveyTokenCriteria() which returns all tokens causing flaky
ordering; fix by populating the criteria with the created case reference (use
the appropriate setter on SurveyTokenCriteria, e.g.,
setCaseRef(caze.toReference()) or equivalent) before calling
getSurveyTokenService().findBy so the result is limited to the token created by
creator.createSurveyToken(caze.toReference()) and update assertions accordingly.
Contributor
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13873 |
…survey-token-issue
Contributor
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13873 |
raulbob
approved these changes
Mar 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: Issue# 13873 Survey token is missing while merging two cases in SORMAS
Summary:
Incorporate survey token while merging the cases.
Changes:
Introduced a new method to find the survey token for the case.
For a normal merge scenario, assign the respective caseId.
In case of cloning, it's not correct to assign the existing survey to the new case.
Updated the TestDataCreator for returning the test survey token details.
Created SurveyServiceTest to validate findSurveyToken of the case.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Tests
Style