sormas-ui/src/main/java/de/symeda/sormas/ui/dashboard/sample/components/SampleDashboardMapComponent.java (2)

241-245: Wrong initial value for the environment samples checkbox

The checkbox value is tied to shouldShowEventParticipantSamples(), which is unrelated. It should reflect the current environment visibility toggle.

-			showEnvironmentSamplesCheckBox.setValue(shouldShowEventParticipantSamples());
+			showEnvironmentSamplesCheckBox.setValue(showEnvironmentalSamples);

---

259-264: Environment-only legend never shown

Early return when displayedHumanSamples is empty prevents the environment legend from rendering, even if showEnvironmentalSamples is true.

-		if (displayedHumanSamples.size() == 0) {
-			return Collections.emptyList();
-		}
+		if (displayedHumanSamples.size() == 0 && !showEnvironmentalSamples) {
+			return Collections.emptyList();
+		}

sormas-ui/src/main/java/de/symeda/sormas/ui/dashboard/diagram/AbstractEpiCurveComponent.java (1)

25-27: Drop Component import after constructor cleanup

Component is only used for a temporary variable introduced in the new constructor; once simplified (see below), this import becomes unnecessary.

Apply after simplifying the constructor:

-import com.vaadin.ui.Component;

sormas-ui/src/main/java/de/symeda/sormas/ui/dashboard/surveillance/components/statistics/LaboratoryResultsStatisticsComponent.java (1)

51-55: Nit: Prefer setIcon over HTML content for icons

Using HTML for icons is unnecessary; Vaadin Label supports setIcon. This avoids ContentMode.HTML and is more accessible.

For example:

-Label infoIcon = new Label(VaadinIcons.INFO_CIRCLE.getHtml(), ContentMode.HTML);
+Label infoIcon = new Label();
+infoIcon.setIcon(VaadinIcons.INFO_CIRCLE);

sormas-api/src/main/java/de/symeda/sormas/api/dashboard/SampleDashboardCriteria.java (1)

61-70: Clarify/enforce mutual exclusivity between sampleMaterial and environmentSampleMaterial

The criteria now has two material fields. Most downstream code likely expects one or the other to be set. Consider enforcing mutual exclusivity in the setters or document precedence.

Option A: Enforce exclusivity in setters (recommended for consistency):

// in SampleDashboardCriteria
public SampleDashboardCriteria sampleMaterial(SampleMaterial sampleMaterial) {
    this.sampleMaterial = sampleMaterial;
    if (sampleMaterial != null) {
        this.environmentSampleMaterial = null;
    }
    return self;
}

public SampleDashboardCriteria environmentSampleMaterial(EnvironmentSampleMaterial environmentSampleMaterial) {
    this.environmentSampleMaterial = environmentSampleMaterial;
    if (environmentSampleMaterial != null) {
        this.sampleMaterial = null;
    }
    return self;
}

Option B: Add Javadoc stating that only one of the two should be set and which one takes precedence if both are non-null.

sormas-ui/src/main/java/de/symeda/sormas/ui/dashboard/sample/SampleDashboardFilterLayout.java (2)

116-123: Redundant sorting and potential de-dupe side effects with TreeSet comparator

You sort the stream and then collect into a TreeSet with the same comparator, effectively sorting twice. Also, de-duplication is based on toString, which may collapse identically captioned entries across enums (e.g., “Other”).

• Remove the pre-collect sorted(...) step and rely on the TreeSet’s comparator.
• Optionally, document that “Other” is intentionally de-duplicated and handled specially in the value-change listener.

-		Set<Enum<?>> combinedSampleMaterials = Stream
-				.concat(Stream.of(SampleMaterial.values()), Stream.of(EnvironmentSampleMaterial.values()))
-				.sorted(Comparator.comparing(Enum::toString))
-				.collect(Collectors.toCollection(() -> new TreeSet<>(Comparator.comparing(Enum::toString))));
+		Set<Enum<?>> combinedSampleMaterials = Stream
+				.concat(Stream.of(SampleMaterial.values()), Stream.of(EnvironmentSampleMaterial.values()))
+				.collect(Collectors.toCollection(() -> new TreeSet<>(Comparator.comparing(Enum::toString))));

---

127-140: Listener logic OK; minor robustness/readability improvements

• Cache the selected value to avoid repeated e.getProperty().getValue().
• The comment can be clarified.

-		// In Other as the selected sample, all samples should select.
-		if (e.getProperty().getValue() == SampleMaterial.OTHER || e.getProperty().getValue() == EnvironmentSampleMaterial.OTHER) {
+		// Selecting "Other" should select both human and environment "Other"
+		Object value = e.getProperty().getValue();
+		if (value == SampleMaterial.OTHER || value == EnvironmentSampleMaterial.OTHER) {
 			dashboardDataProvider.setEnvironmentSampleMaterial(EnvironmentSampleMaterial.OTHER);
 			dashboardDataProvider.setSampleMaterial(SampleMaterial.OTHER);
-		} else if (e.getProperty().getValue() instanceof EnvironmentSampleMaterial) {
-			dashboardDataProvider.setEnvironmentSampleMaterial((EnvironmentSampleMaterial) e.getProperty().getValue());
+		} else if (value instanceof EnvironmentSampleMaterial) {
+			dashboardDataProvider.setEnvironmentSampleMaterial((EnvironmentSampleMaterial) value);
 			dashboardDataProvider.setSampleMaterial(null);
-		} else if (e.getProperty().getValue() instanceof SampleMaterial) {
+		} else if (value instanceof SampleMaterial) {
 			dashboardDataProvider.setEnvironmentSampleMaterial(null);
-			dashboardDataProvider.setSampleMaterial((SampleMaterial) e.getProperty().getValue());
+			dashboardDataProvider.setSampleMaterial((SampleMaterial) value);
 		} else {
 			dashboardDataProvider.setEnvironmentSampleMaterial(null);
 			dashboardDataProvider.setSampleMaterial(null);
 		}

sormas-ui/src/main/java/de/symeda/sormas/ui/dashboard/sample/SampleEpiCurveComponent.java (2)

70-99: Epi curve correctly gated to human samples; consider UX for environment mode

The guard prevents computing the curve when environmentSampleMaterial is set. Currently this silently renders all-zero series. Consider hiding the chart or showing a short description that the EPI curve is only available for human samples to avoid confusing users with flat zero lines.

---

75-83: Potential performance concern: N remote calls inside loop

Fetching counts per date inside the loop can be chatty over EJB. If this becomes a bottleneck, consider a backend endpoint that aggregates by date bucket (day/week/month) in one call and returns a time series.

sormas-ui/src/main/java/de/symeda/sormas/ui/dashboard/sample/components/SampleDashboardMapComponent.java (1)

83-101: Helper naming/docs OK; logic aligns with usage

The empty criteria helpers are straightforward and used correctly in the branching. Minor nit: the Javadoc for “WithoutDisease” vs “WithDisease” could be crisper, but not blocking.

If you want, I can tighten the Javadoc wording for clarity.

sormas-backend/src/main/java/de/symeda/sormas/backend/dashboard/sample/SampleDashboardService.java (2)

551-552: Nit: unnecessary cb.or with single predicate

Wrapping a single cb.between(...) inside cb.or(...) is redundant. Simplify for readability.

-			case ASSOCIATED_ENTITY_REPORT_DATE:
-				dateFilter = cb.or(cb.between(joins.getEnvironment().get(Environment.REPORT_DATE), dateFrom, dateTo));
+			case ASSOCIATED_ENTITY_REPORT_DATE:
+				dateFilter = cb.between(joins.getEnvironment().get(Environment.REPORT_DATE), dateFrom, dateTo);

---

576-581: Comment refers to “cases” in environment filter

The comment “Exclude deleted cases” should say “Exclude deleted samples” to match the context.

-			// Exclude deleted cases. Archived cases should stay included
+			// Exclude deleted samples. Archived samples should stay included

sormas-ui/src/main/java/de/symeda/sormas/ui/dashboard/sample/SampleDashboardDataProvider.java (2)

210-212: Naming nit: pluralize to reflect Map return type

getEnvironmentSampleCount returns a Map; consider renaming to getEnvironmentSampleCounts for clarity and consistency with other getters.

If you agree, I can generate a follow-up diff (including call sites).

---

38-56: Reduce duplication by extracting clear/reset helpers

There’s repeated boilerplate to clear either human or environment maps. Consider small private helpers:

• clearHumanCounts()
• clearEnvironmentCounts()

This improves readability and reduces mistakes when adding/removing metrics.

Example (outside changed lines):

private void clearHumanCounts() {
  sampleCountsByResultType = Maps.newHashMap();
  sampleCountsByPurpose = Maps.newHashMap();
  sampleCountsBySpecimenCondition = Maps.newHashMap();
  sampleCountsByShipmentStatus = Maps.newHashMap();
  testResultCountsByResultType = Maps.newHashMap();
}

private void clearEnvironmentCounts() {
  envSampleCountsBySpecimenCondition = Maps.newHashMap();
  envSampleCountsByShipmentStatus = Maps.newHashMap();
  envTestResultCountsByResultType = Maps.newHashMap();
  envSampleCount = Maps.newHashMap();
}

Then use these in refreshData branches and on the early return.

Also applies to: 60-111

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

sormas-ui/src/test/java/de/symeda/sormas/ui/dashboard/sample/SampleDashboardDataProviderTest.java (1)

82-82: Ensure Maven modules compile with Java 11 for var support

I searched all POMs and found no <java.version>, <maven.compiler.source>, <maven.compiler.target>, or <release> settings—Maven defaults to Java 1.5/1.6. Meanwhile, the Gradle modules target Java 11 (source/targetCompatibility). To avoid CI failures on the new var in SampleDashboardDataProviderTest.java, please add or update your Maven compiler configuration (in sormas-ui/pom.xml or the parent POM):

• Location to update:

• sormas-ui/pom.xml (or its parent POM)

• Example snippet:

<properties>
  <java.version>11</java.version>
</properties>

<build>
  <plugins>
    <plugin>
      <groupId>org.apache.maven.plugins</groupId>
      <artifactId>maven-compiler-plugin</artifactId>
      <version>3.8.1</version>
      <configuration>
        <release>${java.version}</release>
      </configuration>
    </plugin>
  </plugins>
</build>

If you already manage Java toolchains in an external parent POM, confirm it sets the release/source/target to at least 11 for all modules (including tests).

sormas-ui/src/main/java/de/symeda/sormas/ui/dashboard/sample/SampleDashboardView.java (1)

163-171: Fix compile-time type mismatch and NPE risk when summing totals

Stream.concat over entry sets with different key types won’t compile, and getEnvironmentSampleCount() may be null. Compute human and environment totals independently and guard the environment map.

Apply this diff:

-        // combining sample counts of humans and environment samples
-        long totalSamples = Stream.concat(dataProvider.getSampleCountsByResultType().entrySet().stream(), dataProvider.getEnvironmentSampleCount().entrySet()
-                        .stream())
-                .collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue(), Long::sum))
-                .values()
-                .stream()
-                .mapToLong(Long::longValue).sum();
+        // Combine totals from human and environment samples (different key types)
+        long humanTotal = dataProvider.getSampleCountsByResultType()
+            .values()
+            .stream()
+            .mapToLong(Long::longValue)
+            .sum();
+        long environmentTotal = Optional.ofNullable(dataProvider.getEnvironmentSampleCount())
+            .map(m -> m.values().stream().mapToLong(Long::longValue).sum())
+            .orElse(0L);
+        long totalSamples = humanTotal + environmentTotal;
 
         heading.updateTotalLabel(String.valueOf(totalSamples));
         //Final laboratory results only for human samples.

sormas-ui/src/main/java/de/symeda/sormas/ui/dashboard/sample/components/SampleDashboardMapComponent.java (2)

154-156: Handle mixed criteria: load both human and environment samples

With both human and environment materials selected, the current else branch loads nothing. Fetch both sets to mirror DataProvider behavior; gate environment by showEnvironmentalSamples.

Apply this diff:

-        } else{
-            // In the case of mixed sample criteria, we want to show both human and environment samples; for now this is not supported
-        }
+        } else {
+            // Mixed selection: show both human and environment samples (environment gated by permission/checkbox)
+            humanSamples = FacadeProvider.getSampleDashboardFacade().getSamplesForMap(criteria, displayedHumanSamples);
+            environmentSamples = showEnvironmentalSamples
+                ? FacadeProvider.getSampleDashboardFacade().getEnvironmentalSamplesForMap(criteria)
+                : Collections.emptyList();
+        }

---

158-158: Fix initial capacity: sum human + environment sizes

You add environmentSamples.size() twice. Use humanSamples.size() + environmentSamples.size() to prevent unnecessary resizes.

Apply this diff:

-        List<LeafletMarker> markers = new ArrayList<>(environmentSamples.size() + environmentSamples.size());
+        List<LeafletMarker> markers = new ArrayList<>(humanSamples.size() + environmentSamples.size());

sormas-backend/src/main/java/de/symeda/sormas/backend/dashboard/sample/SampleDashboardService.java (1)

287-291: Thanks for addressing prior NPE risk with null test results

Filtering out tuples with a null result ensures toMap doesn’t NPE and aligns with the earlier review feedback.

sormas-ui/src/main/java/de/symeda/sormas/ui/dashboard/sample/SampleDashboardView.java (1)

176-182: Optional: Deduplicate map-merging with a small helper

You repeat the same null-safe merge pattern twice. Consider extracting a generic helper to reduce duplication and future mistakes.

Example helper (place it as a private static method in this class):

private static <K> Map<K, Long> mergedCounts(Map<K, Long> human, Map<K, Long> environment) {
    Map<K, Long> result = new HashMap<>(human);
    Optional.ofNullable(environment).ifPresent(m -> m.forEach((k, v) -> result.merge(k, v, Long::sum)));
    return result;
}

Then replace the current blocks with:

-        Map<SpecimenCondition, Long> resultSpecimenConditionMap = new HashMap<>(dataProvider.getSampleCountsBySpecimenCondition());
-        Optional.ofNullable(dataProvider.getEnvSampleCountsBySpecimenCondition()).ifPresent(m -> m.forEach((condition, count) -> {
-            resultSpecimenConditionMap.merge(condition, count, Long::sum);
-        }));
-        countsBySpecimenCondition.update(resultSpecimenConditionMap);
+        countsBySpecimenCondition.update(
+            mergedCounts(dataProvider.getSampleCountsBySpecimenCondition(), dataProvider.getEnvSampleCountsBySpecimenCondition())
+        );

-        Map<SampleShipmentStatus, Long> resultShipmentStatusMap = new HashMap<>(dataProvider.getSampleCountsByShipmentStatus());
-        Optional.ofNullable(dataProvider.getEnvSampleCountsByShipmentStatus()).ifPresent(m -> m.forEach((status, count) -> {
-            resultShipmentStatusMap.merge(status, count, Long::sum);
-        }));
-        countsByShipmentStatus.update(resultShipmentStatusMap);
+        countsByShipmentStatus.update(
+            mergedCounts(dataProvider.getSampleCountsByShipmentStatus(), dataProvider.getEnvSampleCountsByShipmentStatus())
+        );

Also applies to: 183-189

sormas-ui/src/main/java/de/symeda/sormas/ui/dashboard/sample/components/SampleDashboardMapComponent.java (2)

83-101: Nit: Javadoc wording is inaccurate

The criteria object isn’t “null”; you’re checking that no sample materials are selected. Consider tightening the doc to avoid confusion.

Apply this diff to both comments:

-    /**
-     * Checks if the sample criteria is null, meaning that no human sample material or environment sample material is selected.
-     * @param criteria
-     * @return boolean
-     */
+    /**
+     * Checks whether no human or environment sample material is selected and no disease is set.
+     */

And for the “with disease” variant:

-    /**
-     * Checks if the sample criteria is null, meaning that no human sample material or environment sample material is selected.
-     *
-     * @param criteria
-     * @return boolean
-     */
+    /**
+     * Checks whether no human or environment sample material is selected and a disease is set.
+     */

---

120-129: Nit: Incomplete Javadoc sentence

The sentence ends abruptly and doesn’t describe the rule clearly.

Apply this diff:

-    /**
-     * Checks if the sample criteria is for other sample materials, meaning that all other(Human+Environment) sample materials are selected except,
-     *
-     * @param criteria
-     * @return
-     */
+    /**
+     * Checks whether the criteria targets “OTHER” in either human or environment sample materials.
+     */

sormas-ui/src/test/java/de/symeda/sormas/ui/dashboard/sample/SampleDashboardDataProviderTest.java (2)

55-58: Rename variable for clarity (typo: toDateDate).

Minor readability nit: Rename toDateDate to toDate.

-        Date toDateDate = DateHelper.getDateZero(2023, 2, 25);
+        Date toDate = DateHelper.getDateZero(2023, 2, 25);
@@
-        dataProvider.setToDate(toDateDate);
+        dataProvider.setToDate(toDate);

---

72-78: Add a targeted test for mutually exclusive materials (human vs environment).

Good check that environmentSampleMaterial is passed and sampleMaterial remains null. To prevent regressions, consider adding a dedicated test to assert that both materials cannot be set simultaneously (i.e., provider or criteria enforces mutual exclusivity).

Example test to add:

@Test
public void testBuildDashboardCriteria_materialsAreMutuallyExclusive() {
    dataProvider.setSampleMaterial(SampleMaterial.CRUST);
    dataProvider.setEnvironmentSampleMaterial(EnvironmentSampleMaterial.AIR);

    // Depending on intended behavior, assert one of the following:
    // 1) Environment material wins and sample material is cleared:
    SampleDashboardCriteria criteria = dataProvider.buildDashboardCriteriaWithDates();
    Assertions.assertNull(criteria.getSampleMaterial());
    Assertions.assertEquals(EnvironmentSampleMaterial.AIR, criteria.getEnvironmentSampleMaterial());

    // OR 2) Provider rejects the invalid combo:
    // Assertions.assertThrows(IllegalArgumentException.class, () -> dataProvider.buildDashboardCriteriaWithDates());
}

sormas-backend/src/main/java/de/symeda/sormas/backend/dashboard/sample/SampleDashboardService.java (1)

267-272: Javadoc mismatch: method counts tests, not samples

The query groups by PathogenTest.TEST_RESULT and counts PathogenTest rows, i.e., tests-by-result-type, not samples.

Update the Javadoc:

-	/**
-	 * Returns a map of counts of environment samples by result type.
-	 *
-	 * @param dashboardCriteria
-	 * @return
-	 */
+	/**
+	 * Returns a map of counts of pathogen tests by result type for environment samples.
+	 * Groups by PathogenTest.TEST_RESULT and counts PathogenTest records.
+	 *
+	 * @param dashboardCriteria criteria to filter environment samples/tests
+	 * @return map keyed by PathogenTestResultType with the number of tests
+	 */

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

sormas-backend/src/main/java/de/symeda/sormas/backend/dashboard/sample/SampleDashboardService.java (1)

268-292: Nice: null-key filter avoids toMap NPE; align the human counterpart for consistency

The null-key filter here is correct and prevents NPEs when PathogenTest.TEST_RESULT is null (LEFT JOIN). Please apply the same safeguard to the human method getTestResultCountsByResultType for consistency and robustness.

• Option A (stream-level): add .filter(t -> t.get(0) != null) before collecting.
• Option B (DB-level): add cb.isNotNull(pathogenTestResult) to the WHERE clause and omit the stream filter.

Example for the human method (illustrative only; outside this range):

return QueryHelper.getResultList(em, cq, null, null, Function.identity())
    .stream()
    .filter(t -> t.get(0) != null)
    .collect(Collectors.toMap(t -> (PathogenTestResultType) t.get(0), t -> (Long) t.get(1)));

Option B is preferable to avoid transferring null-key rows from the DB.

Run this script to list all toMap usages in this file that may still collect null keys without filtering:

#!/bin/bash
set -euo pipefail
file="sormas-backend/src/main/java/de/symeda/sormas/backend/dashboard/sample/SampleDashboardService.java"

echo "Potential risky toMap usages (no preceding null-key filter on t.get(0)):"
# Find lines with toMap and include a bit of context
rg -nA2 -e 'Collectors\.toMap\(' "$file"

echo
echo "Occurrences of stream null-key filters:"
rg -n -e '\.filter\s*\(\s*t\s*->\s*t\.get\(\s*0\s*\)\s*!=\s*null\s*\)' "$file" || true

sormas-backend/src/main/java/de/symeda/sormas/backend/dashboard/sample/SampleDashboardService.java (2)

159-179: Filter null grouping keys at the DB level to avoid unnecessary rows

Good call adding a stream-level null-key filter to avoid Collectors.toMap NPEs. You can push this check into the WHERE clause to avoid transferring null-key groups from the DB and drop the stream filter.

Suggested change:

-        cq.where(CriteriaBuilderHelper.and(cb, criteriaFilter, additionalFilters != null ? additionalFilters.apply(cb, sample) : null));
+        cq.where(CriteriaBuilderHelper.and(
+            cb,
+            criteriaFilter,
+            additionalFilters != null ? additionalFilters.apply(cb, sample) : null,
+            cb.isNotNull(groupingProperty)));

With that in place, the stream .filter(t -> t.get(0) != null) becomes redundant and can be removed.

---

300-314: Push null-material filtering into WHERE and simplify the stream

Current stream-level filter is correct; you can offload it to the database and remove the stream filter.

Apply this diff:

-        cq.where(criteriaFilter);
+        cq.where(CriteriaBuilderHelper.and(cb, criteriaFilter, cb.isNotNull(groupingProperty)));
@@
-        return QueryHelper.getResultList(em, cq, null, null, Function.identity())
-                .stream()
-                .filter(t -> t.get(0) != null)
-                .collect(Collectors.toMap(t -> (EnvironmentSampleMaterial) t.get(0), t -> (Long) t.get(1)));
+        return QueryHelper.getResultList(em, cq, null, null, Function.identity())
+                .stream()
+                .collect(Collectors.toMap(t -> (EnvironmentSampleMaterial) t.get(0), t -> (Long) t.get(1)));

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro