Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

607 implementierung wahltage und wahlen #827

Open
wants to merge 32 commits into
base: dev
Choose a base branch
from

Conversation

dragonfly28
Copy link
Contributor

@dragonfly28 dragonfly28 commented Feb 19, 2025

Beschreibung:

  • Wahltage können vom BasisdatenService gelesen werden
  • Die Wahlen eines Wahltages können vom BasisdatenService gelesen werden
  • Die Wahlen eines Wahltages können dem BasisdatenService übermittelt werden

Backend

  • Naming Conventions beachtet
  • Doku aktualisiert
  • Swagger-API vollständig
  • Unit-Tests gepflegt
  • Integrationstests gepflegt
  • Beispiel-Requests gepflegt

Closes #607

Summary by CodeRabbit

  • New Features
    • Added new API endpoints for retrieving and updating election data and viewing voting day information.
    • Introduced enhanced data mapping and validation for structured election details, including color settings.
    • Implemented additional security controls with new permission requirements to ensure only authorized users can access these management features.

@dragonfly28 dragonfly28 linked an issue Feb 19, 2025 that may be closed by this pull request
3 tasks
Copy link
Contributor

coderabbitai bot commented Feb 19, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This pull request extends the administration service with comprehensive election (Wahlen) and election day (Wahltage) management capabilities. New client implementations and service classes have been added to support the retrieval and updating of election data via REST endpoints. Additionally, new DTOs, records, enumerations, and mapper interfaces have been introduced for structured data exchange and object conversion between various layers of the application. The package structure has been reorganized, with import statements adjusted accordingly. Input validation has been reinforced through dedicated validator classes for both election and election day identifiers. Security enhancements include updated access control checks and the addition of new admin permissions, as reflected in the SQL migration scripts. Furthermore, a wide range of unit, integration, and security tests have been implemented to ensure the correct functionality and access restrictions of the newly added features.


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary or `` to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or `` anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@dragonfly28
Copy link
Contributor Author

@coderabbitai review

Copy link
Contributor

coderabbitai bot commented Feb 19, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@dragonfly28 dragonfly28 marked this pull request as ready for review February 19, 2025 09:29
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 13

🧹 Nitpick comments (31)
wls-admin-service/src/test/resources/wahltage.http (1)

6-8: LGTM! Consider enhancing test coverage.

The Wahltage request is well-structured. Consider adding:

  • Example responses for documentation
  • Content-Type headers if needed
  • Additional test cases for error scenarios (e.g., unauthorized access, invalid profile)
wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/utils/Authorities.java (1)

10-12: Consider adding documentation for the authority constants.

Adding Javadoc comments to explain the purpose and usage of each authority constant would improve maintainability.

+    /** Authority to retrieve elections for a specific election day. */
     public static final String ADMIN_GETWAHLEN = "Admin_BUSINESSACTION_GetWahlen";
+    /** Authority to update elections for a specific election day. */
     public static final String ADMIN_UPDATEWAHLEN = "Admin_BUSINESSACTION_UpdateWahlen";
+    /** Authority to retrieve election days. */
     public static final String ADMIN_GETWAHLTAGE = "Admin_BUSINESSACTION_GetWahltage";
wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahlen/WahlenServiceSecurityTest.java (1)

72-74: Use a valid wahlen list for negative test case.

Using null for the wahlen list in the negative test case might mask potential NullPointerExceptions. Consider using a valid list to ensure the test specifically validates the security aspect.

-            .isThrownBy(() -> unitUnderTest.updateWahlen(null, wahltagID))
+            .isThrownBy(() -> unitUnderTest.updateWahlen(List.of(
+                    new WahlModel("wahlID", "name" + "wahl1", 1L,
+                            1L, LocalDate.now().plusMonths(1),
+                            WahlartModel.BAW, new FarbeModel(1, 1, 1))), wahltagID))
wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahlen/WahlDTOMapper.java (2)

7-8: Consider enhancing MapStruct configuration.

While the basic setup is correct, consider adding:

  1. Component model configuration for Spring integration
  2. Strategy for handling unmapped attributes
-@Mapper
+@Mapper(
+    componentModel = "spring",
+    unmappedTargetPolicy = ReportingPolicy.ERROR
+)

10-16: Remove redundant list mapping methods.

MapStruct automatically handles collection mappings. The toDtoList and toModelList methods are unnecessary when you have the single-element mapping methods toDto and toModel.

     WahlDTO toDto(WahlModel wahlModel);

     WahlModel toModel(WahlDTO wahlDTO);
-
-    List<WahlDTO> toDtoList(List<WahlModel> wahlen);
-
-    List<WahlModel> toModelList(List<WahlDTO> wahlDTOs);
wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahlen/WahlModel.java (1)

8-15: Consider adding more validation constraints and documentation.

The field types and null constraints are well-chosen. However, consider these improvements:

  1. Add validation constraints for:

    • name: String length limits
    • reihenfolge and waehlerverzeichnisnummer: Positive values only
    • wahltag: Future dates only
  2. Add Javadoc to explain the purpose of each field

Example implementation:

 @Builder
+/**
+ * Record representing an election model.
+ */
 public record WahlModel(
+        /** Unique identifier for the election */
         @NotNull
+        String wahlID,
+
+        /** Name of the election */
         @NotNull
+        @Size(min = 1, max = 255)
         String name,
+
+        /** Order/sequence number of the election */
         @NotNull
+        @Positive
         Long reihenfolge,
+
+        /** Voter registry number */
         @NotNull
+        @Positive
         Long waehlerverzeichnisnummer,
+
+        /** Date of the election */
         @NotNull
+        @Future
         LocalDate wahltag,
+
+        /** Type of the election */
         @NotNull
         WahlartModel wahlart,
+
+        /** Optional color associated with the election */
         FarbeModel farbe) {
 }
wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahltage/WahltageServiceTest.java (2)

14-22: LGTM! Well-structured test class with proper Mockito setup.

The test class follows JUnit 5 and Mockito best practices. Consider adding @DisplayName annotations to improve test reporting readability.

 @ExtendWith(MockitoExtension.class)
+@DisplayName("WahltageService Test")
 public class WahltageServiceTest {

23-33: Enhance test coverage with additional test cases.

While the current test verifies the API call, consider adding:

  1. Verification of the return value from getWahltage()
  2. Test cases for error scenarios (e.g., when the client throws exceptions)
  3. @DisplayName annotations for better test reporting

Example enhancement:

 @Nested
+@DisplayName("getWahltage method tests")
 class GetWahltage {
     @Test
+    @DisplayName("should delegate to client and return result when called")
     void should_verifyApiCall_when_serviceIsCalled() {
+        // Given
+        List<WahltagModel> expected = List.of(new WahltagModel());
+        when(wahltageClient.getWahltage()).thenReturn(expected);
+
+        // When
+        List<WahltagModel> actual = unitUnderTest.getWahltage();
+
+        // Then
         Mockito.verify(wahltageClient, times(1)).getWahltage();
+        assertThat(actual).isEqualTo(expected);
     }

+    @Test
+    @DisplayName("should propagate exception when client fails")
+    void should_propagateException_when_clientFails() {
+        // Given
+        when(wahltageClient.getWahltage()).thenThrow(new RuntimeException("API Error"));
+
+        // When/Then
+        assertThrows(RuntimeException.class, () -> unitUnderTest.getWahltage());
+    }
 }
wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahlen/FarbeModel.java (1)

5-6: Consider adding utility methods for color manipulation and validation.

To improve the usability of this color model, consider adding these utility methods:

 public record FarbeModel(@NotNull long r, @NotNull long g, @NotNull long b) {
+    /**
+     * Converts the RGB values to a hexadecimal color string.
+     * @return The color in hex format (e.g., "#FF0000" for red)
+     */
+    public String toHexString() {
+        return String.format("#%02X%02X%02X", r, g, b);
+    }
+
+    /**
+     * Creates a FarbeModel from a hex color string.
+     * @param hexColor The color in hex format (e.g., "#FF0000" for red)
+     * @return A new FarbeModel instance
+     * @throws IllegalArgumentException if the hex color string is invalid
+     */
+    public static FarbeModel fromHexString(String hexColor) {
+        if (!hexColor.matches("^#[0-9A-Fa-f]{6}$")) {
+            throw new IllegalArgumentException("Invalid hex color format");
+        }
+        return new FarbeModel(
+            Integer.parseInt(hexColor.substring(1, 3), 16),
+            Integer.parseInt(hexColor.substring(3, 5), 16),
+            Integer.parseInt(hexColor.substring(5, 7), 16)
+        );
+    }
 }

These methods would be particularly useful for:

  • Converting colors to/from hex format for web display
  • Validating color input from external sources
  • Maintaining consistency in color representation across the application
wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahltage/WahltagValidatorTest.java (2)

33-38: Consider adding more test cases for valid inputs.

While the current test case is good, consider parameterizing it to test different valid formats of wahltagID (e.g., different lengths, alphanumeric combinations) to ensure robust validation.

Example implementation:

-    @Test
-    void should_notThrowException_when_wahltagIDIsValid() {
-        val wahltagID = "wahltagID";
-
-        Assertions.assertThatNoException().isThrownBy(() -> unitUnderTest.validWahltagIDParamOrThrow(wahltagID));
-    }
+    @ParameterizedTest(name = "should accept valid wahltagID: {0}")
+    @MethodSource("validWahltagIDArguments")
+    void should_notThrowException_when_wahltagIDIsValid(String wahltagID) {
+        Assertions.assertThatNoException().isThrownBy(() -> unitUnderTest.validWahltagIDParamOrThrow(wahltagID));
+    }
+
+    static Stream<Arguments> validWahltagIDArguments() {
+        return Stream.of(
+            Arguments.of("wahltagID"),
+            Arguments.of("WAHLTAG_2025"),
+            Arguments.of("wahl-123"),
+            Arguments.of("a1b2c3")
+        );
+    }

53-58: Consider adding more edge cases to the test data.

While the current test cases cover the basic invalid scenarios, consider adding more edge cases to ensure comprehensive validation.

Example implementation:

     public static Stream<Arguments> invalidWahltagIDArgumentsWithTestcaseNameAppendix() {
         return Stream.of(
                 Arguments.of(null, "wahltagID is null"),
                 Arguments.of((""), "wahltagID is empty"),
-                Arguments.of(("   "), "wahltagID is blank"));
+                Arguments.of(("   "), "wahltagID is blank"),
+                Arguments.of(("\t\n"), "wahltagID contains only whitespace characters"),
+                Arguments.of(("\u200B"), "wahltagID contains only zero-width space"));
     }
wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahlen/WahlenControllerTest.java (1)

30-74: Consider adding more test cases for error scenarios.

While the current test coverage is good for happy paths and null/empty cases, consider adding tests for:

  • Invalid wahltagID handling
  • Service exception scenarios

Example test case structure:

@Test
void should_returnHttpStatusBadRequest_when_serviceThrowsIllegalArgumentException() {
    val wahltagID = "invalidID";
    
    Mockito.when(wahlenService.getWahlen(wahltagID))
           .thenThrow(new IllegalArgumentException("Invalid ID"));
    
    val result = unitUnderTest.getWahlen(wahltagID);
    
    Assertions.assertThat(result.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
}
wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahltage/WahltageControllerIntegrationTest.java (3)

32-47: Add class documentation and extract constants.

Consider the following improvements:

  1. Add Javadoc to describe the test class purpose and testing strategy.
  2. Extract the API endpoint path /businessActions/wahltage into a constant since it's used multiple times.
+/** Integration tests for the WahltageController, verifying the retrieval of election days
+ * through the REST API with proper authorization and response handling.
+ */
 @SpringBootTest(classes = MicroServiceApplication.class, webEnvironment = SpringBootTest.WebEnvironment.MOCK)
 @AutoConfigureMockMvc
 @AutoConfigureWireMock
 @ActiveProfiles(profiles = { SPRING_TEST_PROFILE })
 class WahltageControllerIntegrationTest {
+    private static final String WAHLTAGE_API_PATH = "/businessActions/wahltage";

49-97: Enhance test coverage with additional scenarios.

The current tests cover basic success and empty list scenarios well. Consider adding:

  1. Error scenarios (e.g., server errors, timeouts)
  2. Response header validations
  3. More descriptive test data instead of generic values

Example test to add:

@Test
@WithMockUser(authorities = { Authorities.ADMIN_GETWAHLTAGE })
void should_returnInternalServerError_when_wahltageClientFails() throws Exception {
    val request = MockMvcRequestBuilders.get(WAHLTAGE_API_PATH);
    
    stubFor(WireMock.get(WAHLTAGE_API_PATH)
        .willReturn(aResponse()
            .withStatus(HttpStatus.INTERNAL_SERVER_ERROR.value())));
    
    api.perform(request)
        .andExpect(status().isInternalServerError())
        .andReturn();
}

99-104: Enhance helper method with null safety and flexibility.

The helper method works well but could be improved with:

  1. Null safety checks for parameters
  2. Support for different content types
-    private ResponseDefinitionBuilder createWireMockResponse(final Object responseBody, final HttpStatus responseStatus) throws Exception {
+    private ResponseDefinitionBuilder createWireMockResponse(final Object responseBody, final HttpStatus responseStatus, final String contentType) throws Exception {
+        if (responseStatus == null) {
+            throw new IllegalArgumentException("Response status cannot be null");
+        }
         return aResponse()
-                .withBody(objectMapper.writeValueAsString(responseBody))
-                .withHeader("Content-Type", "application/json")
+                .withBody(responseBody != null ? objectMapper.writeValueAsString(responseBody) : "")
+                .withHeader("Content-Type", contentType != null ? contentType : "application/json")
                 .withStatus(responseStatus.value());
     }
wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahlen/WahlDTOMapperTest.java (4)

14-17: Consider adding test coverage for individual object mapping.

While the list mapping tests are comprehensive, consider adding test methods for individual object mapping (toDto and toModel) to ensure complete coverage of the mapper's functionality.

 class WahlDTOMapperTest {
     private final WahlDTOMapper unitUnderTest = Mappers.getMapper(WahlDTOMapper.class);
 
+    @Test
+    void should_returnDto_when_givenModel() {
+        val model = createWahlModelList().get(0);
+        val expectedDto = createWahlDTOList().get(0);
+        
+        val result = unitUnderTest.toDto(model);
+        Assertions.assertThat(result).isEqualTo(expectedDto);
+    }
+
+    @Test
+    void should_returnModel_when_givenDto() {
+        val dto = createWahlDTOList().get(0);
+        val expectedModel = createWahlModelList().get(0);
+        
+        val result = unitUnderTest.toModel(dto);
+        Assertions.assertThat(result).isEqualTo(expectedModel);
+    }

18-34: Add edge case tests for list conversion.

Consider adding tests for empty lists and lists with null elements to ensure robust error handling.

 @Nested
 class ToDtoList {
     @Test
     void should_returnNull_when_givenNull() {
         Assertions.assertThat(unitUnderTest.toDtoList(null)).isNull();
     }

     @Test
     void should_returnDtoList_when_givenModelList() {
         val modelsInput = createWahlModelList();
         val dtosExpected = createWahlDTOList();

         val result = unitUnderTest.toDtoList(modelsInput);
         Assertions.assertThat(result).isEqualTo(dtosExpected);
     }
+
+    @Test
+    void should_returnEmptyList_when_givenEmptyList() {
+        val result = unitUnderTest.toDtoList(List.of());
+        Assertions.assertThat(result).isEmpty();
+    }
+
+    @Test
+    void should_handleNullElements_when_givenListWithNulls() {
+        val modelsInput = List.of(createWahlModelList().get(0), null);
+        val result = unitUnderTest.toDtoList(modelsInput);
+        Assertions.assertThat(result)
+            .hasSize(2)
+            .containsExactly(createWahlDTOList().get(0), null);
+    }
 }

36-51: Add edge case tests for model list conversion.

Mirror the edge case tests from ToDtoList for consistency and complete coverage.

 @Nested
 class ToModelList {
     @Test
     void should_returnNull_when_givenNull() {
         Assertions.assertThat(unitUnderTest.toModelList(null)).isNull();
     }

     @Test
     void should_returnModelList_when_givenDtoList() {
         val modelsExpected = createWahlModelList();
         val dtosInput = createWahlDTOList();

         val result = unitUnderTest.toModelList(dtosInput);
         Assertions.assertThat(result).isEqualTo(modelsExpected);
     }
+
+    @Test
+    void should_returnEmptyList_when_givenEmptyList() {
+        val result = unitUnderTest.toModelList(List.of());
+        Assertions.assertThat(result).isEmpty();
+    }
+
+    @Test
+    void should_handleNullElements_when_givenListWithNulls() {
+        val dtosInput = List.of(createWahlDTOList().get(0), null);
+        val result = unitUnderTest.toModelList(dtosInput);
+        Assertions.assertThat(result)
+            .hasSize(2)
+            .containsExactly(createWahlModelList().get(0), null);
+    }
 }

53-67: Enhance test data variety and add documentation.

The test data could be more diverse to cover different scenarios. Consider:

  1. Adding documentation to explain the test data setup
  2. Including more varied test cases (e.g., different colors, election types)
  3. Using meaningful test data that represents real-world scenarios
+    /**
+     * Creates a list of WahlDTO objects for testing.
+     * Each DTO represents an election with different dates and identifiers.
+     * @return List of test DTOs
+     */
     private List<WahlDTO> createWahlDTOList() {
         val wahl1 = new WahlDTO("wahlID1", "name1", 3L, 1L, LocalDate.now(), WahlartDTO.BAW, new FarbeDTO(1, 1, 1));
-        val wahl2 = new WahlDTO("wahlID2", "name2", 3L, 1L, LocalDate.now(), WahlartDTO.BAW, new FarbeDTO(1, 1, 1));
-        val wahl3 = new WahlDTO("wahlID3", "name3", 3L, 1L, LocalDate.now().plusMonths(2), WahlartDTO.BAW, new FarbeDTO(1, 1, 1));
+        val wahl2 = new WahlDTO("wahlID2", "name2", 4L, 2L, LocalDate.now(), WahlartDTO.BRW, new FarbeDTO(255, 0, 0));
+        val wahl3 = new WahlDTO("wahlID3", "name3", 5L, 3L, LocalDate.now().plusMonths(2), WahlartDTO.LTW, new FarbeDTO(0, 255, 0));

         return List.of(wahl1, wahl2, wahl3);
     }

+    /**
+     * Creates a list of WahlModel objects for testing.
+     * Each model corresponds to a DTO in createWahlDTOList().
+     * @return List of test models
+     */
     private List<WahlModel> createWahlModelList() {
         val wahl1 = new WahlModel("wahlID1", "name1", 3L, 1L, LocalDate.now(), WahlartModel.BAW, new FarbeModel(1, 1, 1));
-        val wahl2 = new WahlModel("wahlID2", "name2", 3L, 1L, LocalDate.now(), WahlartModel.BAW, new FarbeModel(1, 1, 1));
-        val wahl3 = new WahlModel("wahlID3", "name3", 3L, 1L, LocalDate.now().plusMonths(2), WahlartModel.BAW, new FarbeModel(1, 1, 1));
+        val wahl2 = new WahlModel("wahlID2", "name2", 4L, 2L, LocalDate.now(), WahlartModel.BRW, new FarbeModel(255, 0, 0));
+        val wahl3 = new WahlModel("wahlID3", "name3", 5L, 3L, LocalDate.now().plusMonths(2), WahlartModel.LTW, new FarbeModel(0, 255, 0));

         return List.of(wahl1, wahl2, wahl3);
     }
wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahlen/WahlartDTO.java (1)

3-5: Add documentation for election type constants.

Please add Javadoc comments to document what each election type constant represents (e.g., BAW, BEB, BTW, etc.). This will improve code maintainability and help other developers understand the purpose of each constant.

Example:

 public enum WahlartDTO {
+    /** Bundestagswahl */
     BAW,
+    /** Bürgerentscheid Bayern */
     BEB,
+    /** Bundestagswahl */
     BTW,
     // Add documentation for remaining constants
wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahltage/WahltagDTO.java (1)

7-12: Add date validation and field documentation.

Consider the following improvements:

  1. Add validation to ensure wahltag is not in the past.
  2. Add Javadoc comments to document the purpose of each field.
+/**
+ * Data Transfer Object for election day information.
+ */
 @Builder
 public record WahltagDTO(
+    /** Unique identifier for the election day */
     @NotNull String wahltagID,
+    /** Date of the election */
+    @NotNull
+    @FutureOrPresent
     LocalDate wahltag,
+    /** Optional description of the election day */
     String beschreibung,
+    /** Optional reference number */
     String nummer) {
wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahlen/WahlenValidator.java (1)

15-19: Consider enhancing ID validation.

While the current validation checks for blank IDs, consider adding:

  1. Maximum length validation
  2. Format validation (e.g., using a regex pattern)
  3. Sanitization of input

Example enhancement:

 public void validWahlIDParamOrThrow(final String wahlID) {
     if (StringUtils.isBlank(wahlID)) {
         throw exceptionFactory.createFachlicheWlsException(ExceptionConstants.MISSING_ARGUMENT);
     }
+    if (wahlID.length() > MAX_ID_LENGTH) {
+        throw exceptionFactory.createFachlicheWlsException(ExceptionConstants.INVALID_ARGUMENT);
+    }
+    if (!VALID_ID_PATTERN.matcher(wahlID).matches()) {
+        throw exceptionFactory.createFachlicheWlsException(ExceptionConstants.INVALID_ARGUMENT);
+    }
 }
wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahltage/WahltageController.java (1)

44-51: Add logging for better observability.

While the endpoint is well-implemented, adding logging would improve observability and debugging capabilities.

Apply this diff to add logging:

     public ResponseEntity<List<WahltagDTO>> getWahltage() {
+        log.debug("Retrieving election days");
         final List<WahltagDTO> result = wahltagDTOMapper.toDtoList(wahltageService.getWahltage());
         if (result == null || result.isEmpty()) {
+            log.info("No election days found");
             return ResponseEntity.noContent().build();
         } else {
+            log.debug("Retrieved {} election days", result.size());
             return ResponseEntity.ok(result);
         }
     }
wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahlen/WahlenValidatorTest.java (1)

40-42: Consider adding more test cases for edge scenarios.

The parameterized test covers basic invalid cases (null, empty, blank). Consider adding test cases for:

  • Very long wahlIDs (to test length limits)
  • Special characters in wahlIDs (to test character validation)
  • Leading/trailing whitespace (to test trimming behavior)
 public static Stream<Arguments> invalidwahlIDArgumentsWithTestcaseNameAppendix() {
     return Stream.of(
             Arguments.of(null, "wahlID is null"),
             Arguments.of((""), "wahlID is empty"),
-            Arguments.of(("   "), "wahlID is blank"));
+            Arguments.of(("   "), "wahlID is blank"),
+            Arguments.of("a".repeat(256), "wahlID is too long"),
+            Arguments.of("wahl@#$%", "wahlID contains special characters"),
+            Arguments.of("  valid  ", "wahlID has whitespace"));
 }

Also applies to: 53-58

wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahltage/WahltagDTOMapperTest.java (1)

52-58: Consider adding validation tests for date boundaries and invalid data.

The test data includes a good mix of past and future dates, but consider adding tests for:

  • Edge case dates (e.g., minimum/maximum allowed dates)
  • Invalid date formats
  • Null or invalid fields within the DTO/Model objects
 private List<WahltagDTO> createWahltagDTOList() {
+    // Add edge cases and invalid data scenarios
+    val wahltag0 = new WahltagDTO("invalidId#$%", LocalDate.MAX, null, "");
     val wahltag1 = new WahltagDTO("identifikatorWahltag1", LocalDate.now().minusMonths(2), "beschreibungWahltag1", "nummerWahltag1");
     val wahltag2 = new WahltagDTO("identifikatorWahltag2", LocalDate.now().minusMonths(1), "beschreibungWahltag2", "nummerWahltag2");
     val wahltag3 = new WahltagDTO("identifikatorWahltag3", LocalDate.now().plusMonths(1), "beschreibungWahltag3", "nummerWahltag3");

-    return List.of(wahltag1, wahltag2, wahltag3);
+    return List.of(wahltag0, wahltag1, wahltag2, wahltag3);
 }
wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahlen/WahlenServiceTest.java (1)

38-46: Add test case for validation failure.

The test coverage for getWahlen should include a case where validWahlIDParamOrThrow throws an exception.

Add this test case:

@Test
void should_throwException_when_validationFails() {
    val wahlID = "invalidWahlID";
    val mockedException = new FachlicheWlsException("validation failed");
    
    Mockito.doThrow(mockedException)
        .when(wahlenValidator)
        .validWahlIDParamOrThrow(wahlID);
    
    Assertions.assertThatThrownBy(() -> unitUnderTest.getWahlen(wahlID))
        .isSameAs(mockedException);
}
wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/client/basisdaten/WahlenClientMapperTest.java (1)

28-53: Add test case for empty list mapping.

The toModelList test coverage should include a case for mapping an empty list.

Add this test case:

@Test
void should_returnEmptyList_when_givenEmptyList() {
    val result = unitUnderTest.toModelList(List.of());
    Assertions.assertThat(result).isEmpty();
}
wls-auth-service/src/main/resources/db/migrations/h2/V11_0__addAdminPermissionsWahlen.sql (2)

1-2: Specify Column Names in INSERT Statement

Consider explicitly specifying the column names in the INSERT statement. This enhances clarity and safeguards against potential schema changes where the column ordering might differ unexpectedly.

Example diff:

-INSERT INTO Permission
-VALUES ('00000000-0000-0011-0000-000000000001', 'Admin_BUSINESSACTION_GetWahlen');
+INSERT INTO Permission (id, action)
+VALUES ('00000000-0000-0011-0000-000000000001', 'Admin_BUSINESSACTION_GetWahlen');

3-4: Document Authority-Permission Association

For the association insert into the Secauthorities_Secpermissions table, explicitly specifying the column names can improve the script's maintainability and readability.

Example diff:

-INSERT INTO Secauthorities_Secpermissions
-VALUES ('00000000-0000-0000-0001-000000000001', '00000000-0000-0011-0000-000000000001');
+INSERT INTO Secauthorities_Secpermissions (secauthority_id, permission_id)
+VALUES ('00000000-0000-0000-0001-000000000001', '00000000-0000-0011-0000-000000000001');
wls-admin-service/src/test/resources/wahlen.http (2)

5-8: Add example wahltagID and document expected response.

Consider the following improvements:

  1. Replace the hardcoded "wahltagID" with an actual example ID or variable
  2. Document the expected response structure in comments
 ### GET Wahlen - works only with profile "dummy.client"
+### Expected response:
+### [
+###   {
+###     "wahlID": "string",
+###     "name": "string",
+###     ...
+###   }
+### ]
-GET {{ WLS_ADMIN_SERVICE_URL }}/businessActions/wahlen/wahltagID
+GET {{ WLS_ADMIN_SERVICE_URL }}/businessActions/wahlen/{{wahltagID}}
 Authorization: Bearer {{$auth.token("auth-service")}}

9-15: Enhance date handling robustness.

The current date handling could be improved:

  1. Add validation for the generated date
  2. Consider adding a more descriptive variable name than "dateOfWahltagID1"
  3. Add error handling for the date operations
 < {%
-    const wahltag = new Date();
-    wahltag.setMonth(wahltag.getMonth() - 2);
-    const wahltagAsDateString = wahltag.toJSON().substring(0, 10);
-    request.variables.set("dateOfWahltagID1", wahltagAsDateString);
+    try {
+        const wahltag = new Date();
+        wahltag.setMonth(wahltag.getMonth() - 2);
+        if (isNaN(wahltag.getTime())) {
+            throw new Error('Invalid date generated');
+        }
+        const wahltagAsDateString = wahltag.toJSON().substring(0, 10);
+        request.variables.set("electionDate", wahltagAsDateString);
+    } catch (error) {
+        console.error('Failed to generate election date:', error);
+    }
 %}
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 30fa9d3 and ed9246d.

📒 Files selected for processing (48)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/client/DummyClientImpl.java (3 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/client/basisdaten/WahlenClientImpl.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/client/basisdaten/WahlenClientMapper.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/client/basisdaten/WahltagClientMapper.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/client/basisdaten/WahltageClientImpl.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahlen/FarbeDTO.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahlen/WahlDTO.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahlen/WahlDTOMapper.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahlen/WahlartDTO.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahlen/WahlenController.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahltage/WahltagDTO.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahltage/WahltagDTOMapper.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahltage/WahltageController.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/common/WahltagModel.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/common/WahltageClient.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahlen/FarbeModel.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahlen/WahlModel.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahlen/WahlartModel.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahlen/WahlenClient.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahlen/WahlenService.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahlen/WahlenValidator.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahltage/WahltagValidator.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahltage/WahltageService.java (1 hunks)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahltermindaten/WahltermindatenService.java (1 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/client/basisdaten/WahlenClientImplTest.java (1 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/client/basisdaten/WahlenClientMapperTest.java (1 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/client/basisdaten/WahltagClientMapperTest.java (1 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/configuration/SecurityConfigurationTest.java (3 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahlen/WahlDTOMapperTest.java (1 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahlen/WahlenControllerIntegrationTest.java (1 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahlen/WahlenControllerTest.java (1 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahltage/WahltagDTOMapperTest.java (1 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahltage/WahltageControllerIntegrationTest.java (1 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahltage/WahltageControllerTest.java (1 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahltermindaten/WahltermindatenControllerIntegrationTest.java (1 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahlen/WahlenServiceSecurityTest.java (1 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahlen/WahlenServiceTest.java (1 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahlen/WahlenValidatorTest.java (1 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahltage/WahltagValidatorTest.java (1 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahltage/WahltageServiceSecurityTest.java (1 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahltage/WahltageServiceTest.java (1 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahltermindaten/WahltermindatenServiceTest.java (1 hunks)
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/utils/Authorities.java (1 hunks)
  • wls-admin-service/src/test/resources/wahlen.http (1 hunks)
  • wls-admin-service/src/test/resources/wahltage.http (1 hunks)
  • wls-admin-service/src/test/resources/wahltermindaten.http (1 hunks)
  • wls-auth-service/src/main/resources/db/migrations/h2/V10_0__addAdminPermissionsWahltage.sql (1 hunks)
  • wls-auth-service/src/main/resources/db/migrations/h2/V11_0__addAdminPermissionsWahlen.sql (1 hunks)
✅ Files skipped from review due to trivial changes (7)
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahlen/WahlartModel.java
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/common/WahltagModel.java
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/common/WahltageClient.java
  • wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/client/basisdaten/WahltagClientMapperTest.java
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/client/basisdaten/WahltageClientImpl.java
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahltermindaten/WahltermindatenService.java
  • wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/client/basisdaten/WahltagClientMapper.java
🧰 Additional context used
🧠 Learnings (3)
wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/client/DummyClientImpl.java (1)
Learnt from: MrSebastian
PR: it-at-m/Wahllokalsystem#621
File: wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/client/DummyClientImpl.java:27-46
Timestamp: 2024-12-06T14:40:02.820Z
Learning: In the `DummyClientImpl` class (`wls-ergebnismeldung-service/src/main/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/client/DummyClientImpl.java`), which is a dummy class used for testing purposes, input parameter validation is not required.
wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/configuration/SecurityConfigurationTest.java (1)
Learnt from: MrSebastian
PR: it-at-m/Wahllokalsystem#621
File: wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/configuration/SecurityConfigurationTest.java:116-143
Timestamp: 2024-12-06T14:46:34.146Z
Learning: In `SecurityConfigurationTest.java`, the tests are designed to validate access with authorized and unauthorized users only. Other scenarios, such as invalid input data, are tested in other classes.
wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahlen/WahlenControllerIntegrationTest.java (1)
Learnt from: MrSebastian
PR: it-at-m/Wahllokalsystem#621
File: wls-ergebnismeldung-service/src/test/java/de/muenchen/oss/wahllokalsystem/ergebnismeldungservice/rest/status/StatusControllerIntegrationTest.java:38-44
Timestamp: 2024-12-06T14:52:05.607Z
Learning: In `StatusControllerIntegrationTest`, integration tests are intended to verify the integration of all components and do not include security tests.
🔇 Additional comments (37)
wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahltermindaten/WahltermindatenServiceTest.java (1)

4-5: LGTM! Package reorganization looks good.

The imports reflect the move of WahltagModel and WahltageClient to the common package, which improves the overall organization of the codebase. The test class effectively uses these components to validate the service behavior.

wls-admin-service/src/test/resources/wahltage.http (1)

1-4: LGTM! Well-structured user info request.

The request is properly configured with authentication and follows standard practices.

wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/utils/Authorities.java (1)

10-12: Well-structured authority constants for election management.

The new authority constants follow good security practices:

  • Clear separation between read (GET) and write (UPDATE) operations
  • Consistent naming pattern with existing constants
  • Granular permissions aligned with the principle of least privilege
wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahltage/WahltageServiceSecurityTest.java (2)

1-22: LGTM! Well-structured test class setup.

The test class is properly configured with:

  • Correct Spring Boot test annotations
  • Appropriate test profiles
  • Clean dependency injection

23-41: LGTM! Comprehensive security test coverage.

The test cases effectively validate:

  • Access granted with correct authority
  • Access denied with incorrect authority
wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahlen/WahlenServiceSecurityTest.java (2)

1-25: LGTM! Well-structured test class setup.

The test class is properly configured with:

  • Correct Spring Boot test annotations
  • Appropriate test profiles
  • Clean dependency injection

26-48: LGTM! Comprehensive security test coverage for getWahlen.

The test cases effectively validate:

  • Access granted with correct authority
  • Access denied with incorrect authority
wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahlen/WahlDTOMapper.java (1)

1-6: LGTM! Clean package structure and imports.

The package structure follows Java conventions, and the imports are minimal and necessary. Good choice using MapStruct for type-safe bean mappings.

wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahlen/WahlModel.java (2)

1-6: LGTM!

The package name follows Java naming conventions, and the imports are appropriate for the required functionality.


7-8: LGTM!

Good choice using a record with @Builder for this immutable data class. The builder pattern will make it easier to create instances with multiple fields.

wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahltage/WahltageServiceTest.java (1)

1-13: LGTM! Well-organized package structure and imports.

The package structure follows Java conventions, and imports are clean and properly organized.

wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahltage/WahltagValidatorTest.java (2)

1-29: LGTM! Well-structured test class setup.

The class follows testing best practices with proper use of Mockito for mocking, clear dependency injection, and good organization using nested test classes.


40-51: LGTM! Well-implemented parameterized test for invalid inputs.

The test effectively validates error cases using parameterized tests and proper exception verification. Good use of mocking and assertion of the exact exception instance.

wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahlen/WahlenControllerTest.java (1)

1-29: LGTM! Well-structured test class setup.

The test class follows best practices with proper use of Mockito annotations and clear dependency management.

wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahltage/WahltageControllerIntegrationTest.java (1)

1-106: Well-structured integration tests with good coverage!

The implementation demonstrates good testing practices with:

  • Proper use of Spring Boot test annotations
  • Clean separation of test scenarios
  • Good use of WireMock for HTTP mocking
  • Comprehensive assertions
wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahltermindaten/WahltermindatenControllerIntegrationTest.java (2)

16-16: LGTM! Package reorganization improves model reusability.

The move of WahltagModel to the common package aligns with good architectural practices by making the model more accessible across different components.


16-16:

✅ Verification successful

Verify the impact of the WahltagModel package move.

The model has been moved from wahltermindaten to common package. Let's verify this change doesn't break other tests.


🏁 Script executed:

#!/bin/bash
# Description: Check for any remaining references to the old package path and verify all tests pass with the new package path

# Search for any remaining references to the old package path
echo "Searching for old package references..."
rg "wahltermindaten\.WahltagModel"

# Search for references to the new package path to ensure consistent usage
echo "Checking new package usage..."
rg "common\.WahltagModel"

Length of output: 2241


WahltagModel Package Move Verified
The search results confirm that there are no remaining references to the old package path (wahltermindaten.WahltagModel). All instances in both production and test files now correctly import from de.muenchen.oss.wahllokalsystem.adminservice.service.common.WahltagModel. This indicates that the refactoring was successful. Please ensure the full test suite is executed to catch any indirect issues, but based on the current findings, the package move does not introduce any breaking changes.

wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahltage/WahltagDTOMapper.java (1)

1-17: LGTM! Well-structured mapper interface.

The interface follows MapStruct best practices with clear method signatures for bidirectional mapping between DTOs and models, supporting both single object and list conversions.

wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/client/basisdaten/WahlenClientMapper.java (1)

12-13: Verify asymmetric mapping of 'nummer' field.

The nummer field is ignored when mapping to DTO but not when mapping from DTO to model. This asymmetric mapping could lead to data inconsistency.

Consider one of these approaches:

  1. Ignore the field in both directions:
 @Mapping(target = "nummer", ignore = true)
 WahlDTO toDto(WahlModel wahl);
 
+@Mapping(target = "nummer", ignore = true)
 List<WahlModel> toModelList(List<WahlDTO> wahlenDTOs);
  1. Document why the asymmetric mapping is intentional.
wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/service/wahltage/WahltageService.java (1)

11-21: LGTM! Well-structured service class.

The service follows Spring best practices with proper dependency injection and security configuration.

wls-admin-service/src/main/java/de/muenchen/oss/wahllokalsystem/adminservice/client/DummyClientImpl.java (1)

67-85: LGTM!

The implementation of getWahlen and postWahlen methods follows the established pattern for dummy clients, providing mock data for testing purposes.

wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/client/basisdaten/WahlenClientImplTest.java (4)

38-54: LGTM! Well-structured test for successful retrieval of elections.

The test effectively verifies the mapping and successful retrieval of election data with clear assertions and proper mock setup.


56-76: LGTM! Comprehensive error handling tests.

The tests thoroughly cover both WlsException and RuntimeException scenarios, ensuring proper exception propagation.


82-97: LGTM! Well-implemented test for successful update of elections.

The test properly verifies the interaction with the API and the mapping of data.


99-120: LGTM! Thorough error handling tests for update operation.

The tests effectively cover both WlsException and RuntimeException scenarios for the update operation.

wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/rest/wahlen/WahlenControllerIntegrationTest.java (4)

61-85: LGTM! Well-structured test for successful retrieval of elections.

The test effectively verifies the response mapping and status codes using WireMock for API simulation.


87-99: LGTM! Proper handling of empty response scenario.

The test correctly verifies that an empty list results in a NO_CONTENT status.


106-119: LGTM! Comprehensive test for successful update operation.

The test properly handles CSRF, content type, and request body serialization.


123-133: LGTM! Well-implemented helper methods for WireMock responses.

The helper methods provide clean abstractions for creating WireMock responses with and without bodies.

wls-admin-service/src/test/java/de/muenchen/oss/wahllokalsystem/adminservice/configuration/SecurityConfigurationTest.java (3)

123-143: LGTM! Comprehensive security tests for GetWahltage endpoint.

The tests properly verify both anonymous and authenticated access scenarios, aligning with the learning that security tests should focus on authorization checks.


145-167: LGTM! Well-structured security tests for GetWahlen endpoint.

The tests effectively verify authorization requirements for both anonymous and authenticated users.


169-195: LGTM! Thorough security tests for UpdateWahlen endpoint.

The tests properly handle CSRF protection and verify authorization requirements for both anonymous and authenticated users.

wls-auth-service/src/main/resources/db/migrations/h2/V10_0__addAdminPermissionsWahltage.sql (1)

1-4: LGTM! Well-structured SQL migration for adding admin permissions.

The migration script properly adds the GetWahltage permission and correctly associates it with the security authority using appropriate UUIDs.

wls-auth-service/src/main/resources/db/migrations/h2/V11_0__addAdminPermissionsWahlen.sql (2)

6-7: Verify Permission Naming Consistency

Ensure that the permission identifier and action name for updating elections (Admin_BUSINESSACTION_UpdateWahlen) conform to your project’s established naming conventions and match the usage in related secured endpoints.


8-9: Validate Authority Association for Update Permission

Confirm that the security authority ID '00000000-0000-0000-0001-000000000001' is correctly associated with the new update permission. This verification is crucial to ensure the proper enforcement of access control in administration operations.

wls-admin-service/src/test/resources/wahlen.http (2)

1-4: LGTM! Authentication request is properly structured.

The user info endpoint is correctly configured with Bearer token authentication.


16-19: LGTM! Request headers are properly configured.

The Content-Type and Authorization headers are correctly set.


@SpringBootTest(classes = MicroServiceApplication.class)
@ActiveProfiles({ TestConstants.SPRING_TEST_PROFILE, Profiles.DUMMY_CLIENTS })
class WahlenServiceSecurityTest {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auch wenn nur 1 Recht für den Zugriff benötigt wird könntest du die parametrisierte Variante für den Negativ-Fall verwenden (für die Einheitlichkeit) - rein optional; trifft auch auf den anderen ServiceSecurityTest zu

Copy link
Contributor Author

@dragonfly28 dragonfly28 Feb 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved with d75cb65

Vielleicht noch ein zusätzliches Issue für die ebenfalls nicht parametrisierten Tests in WahltageServiceSecurityTest und WahltermindatenServiceSecurityTest?

dragonfly28 and others added 3 commits February 20, 2025 16:39
…m/adminservice/rest/wahltage/WahltagDTO.java

Co-authored-by: MrSebastian <[email protected]>
…m/adminservice/service/wahlen/WahlenService.java

Co-authored-by: MrSebastian <[email protected]>
…m/adminservice/service/wahlen/WahlenService.java

Co-authored-by: MrSebastian <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implementierung Wahltage und Wahlen Endpunkt
2 participants