From b1d67c7fb36f981efb79fe0cb107eec5ddf5e598 Mon Sep 17 00:00:00 2001 From: Ibrahim Chehab Date: Fri, 28 Aug 2026 11:56:35 -0400 Subject: [PATCH 1/2] Prayer spaces: type, coordinates, and admin CRUD MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prayer spaces were readable and nothing else. Two GET endpoints returned the JPA entity directly, there was no service layer, and no way to create, edit or delete one — a space could only be changed by editing the database by hand, which the admin console's own page comment called out as the largest gap between what it can manage and what Minbar runs. Model - PrayerSpaceType {BROTHERS, SISTERS, MULTIFAITH, REFLECTION}. Audience was doing double duty as both "who is this listed for" and "what kind of room is it", which cannot describe the campus: a multifaith room and a reflection bay are both shown to everyone and are not the same place. Type carries a default audience so the common case still only states one of them. - latitude/longitude, nullable and written as a pair. Without them the app cannot pin a space, sort by distance, or offer a Maps link — the reason the map screen was still reading mock data. - imageUrl, notes, and a prose `directions` fallback for a space nobody has written step-by-step directions for yet. - `location` becomes `roomInfo`; it described where in the building the room is, which stops reading as "location" once the table also holds coordinates. Wire PrayerSpaceView replaces the entity on both endpoints. `steps` is a lazy @OneToMany, so whether a caller got the walking directions depended on whether a transaction happened to be open — which is why the console fetched each space again one at a time. The view is assembled inside the service, so the list carries full directions and that fan-out goes away. Directions stay stored as their parts (walkTimeMinutes + startingPoint, not "2 min walk from the CCT main entrance"). Clients compose the copy; the app and the board word it differently and the server should not pick a side. Admin /api/admin/minbar/prayer-spaces, GET/POST/PATCH/DELETE. Reads on board:content:read — where the musallahs are is not a secret, and /api/minbar serves the same data unauthenticated. Writes on a new board:prayerspace:write, bundled into BOARD_EDITOR and BOARD_ADMIN: everything else in board content is displayed on a wall, where wrong means stale; these are turn-by-turn instructions someone follows through a building to find somewhere to pray, where wrong means the wrong floor. Every write is audited. Migration V8, both dialects. Existing rows backfill space_type from audience. 613 tests pass, including 19 service tests and 18 covering the read/write permission split. Co-Authored-By: Claude Opus 5 --- openapi.yaml | 401 +++++++++++++++++- .../controller/MinbarController.java | 29 +- .../PrayerSpaceAdminController.java | 118 ++++++ .../request/CreatePrayerSpaceRequest.java | 89 ++++ .../minbar/request/DirectionStepRequest.java | 25 ++ .../request/UpdatePrayerSpaceRequest.java | 91 ++++ .../dto/minbar/response/PrayerSpaceView.java | 112 +++++ .../lensbridge/model/audit/AuditAction.java | 7 + .../lensbridge/model/auth/Permission.java | 8 + .../model/auth/RolePermissions.java | 1 + .../lensbridge/model/minbar/PrayerSpace.java | 71 +++- .../model/minbar/PrayerSpaceType.java | 71 ++++ .../repository/sql/PrayerSpaceRepository.java | 3 + .../service/PrayerSpaceService.java | 212 +++++++++ .../V8__prayer_space_type_and_coordinates.sql | 82 ++++ .../V8__prayer_space_type_and_coordinates.sql | 83 ++++ ...yerSpaceAdminControllerPermissionTest.java | 220 ++++++++++ .../service/PrayerSpaceServiceTest.java | 360 ++++++++++++++++ 18 files changed, 1961 insertions(+), 22 deletions(-) create mode 100644 src/main/java/com/ibrasoft/lensbridge/controller/PrayerSpaceAdminController.java create mode 100644 src/main/java/com/ibrasoft/lensbridge/dto/minbar/request/CreatePrayerSpaceRequest.java create mode 100644 src/main/java/com/ibrasoft/lensbridge/dto/minbar/request/DirectionStepRequest.java create mode 100644 src/main/java/com/ibrasoft/lensbridge/dto/minbar/request/UpdatePrayerSpaceRequest.java create mode 100644 src/main/java/com/ibrasoft/lensbridge/dto/minbar/response/PrayerSpaceView.java create mode 100644 src/main/java/com/ibrasoft/lensbridge/model/minbar/PrayerSpaceType.java create mode 100644 src/main/java/com/ibrasoft/lensbridge/service/PrayerSpaceService.java create mode 100644 src/main/resources/db/migration/postgresql/V8__prayer_space_type_and_coordinates.sql create mode 100644 src/main/resources/db/migration/sqlite/V8__prayer_space_type_and_coordinates.sql create mode 100644 src/test/java/com/ibrasoft/lensbridge/controller/PrayerSpaceAdminControllerPermissionTest.java create mode 100644 src/test/java/com/ibrasoft/lensbridge/service/PrayerSpaceServiceTest.java diff --git a/openapi.yaml b/openapi.yaml index 32e0121..d0f1bde 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -78,6 +78,9 @@ paths: - CREATE_POSTER - UPDATE_POSTER - DELETE_POSTER + - CREATE_PRAYER_SPACE + - UPDATE_PRAYER_SPACE + - DELETE_PRAYER_SPACE - CREATE_SOCIAL - UPDATE_SOCIAL - DELETE_SOCIAL @@ -175,6 +178,9 @@ paths: - CREATE_POSTER - UPDATE_POSTER - DELETE_POSTER + - CREATE_PRAYER_SPACE + - UPDATE_PRAYER_SPACE + - DELETE_PRAYER_SPACE - CREATE_SOCIAL - UPDATE_SOCIAL - DELETE_SOCIAL @@ -1706,6 +1712,133 @@ paths: description: Request failed; body carries a human-readable message tags: - board-admin-controller + /api/admin/minbar/prayer-spaces: + get: + operationId: listAdminPrayerSpaces + responses: + "200": + content: + application/json: + schema: + type: array + items: + $ref: "#/components/schemas/PrayerSpaceView" + description: OK + default: + content: + application/json: + schema: + $ref: "#/components/schemas/MessageResponse" + description: Request failed; body carries a human-readable message + summary: "List every prayer space, regardless of audience" + tags: + - prayer-space-admin-controller + post: + operationId: createPrayerSpace + requestBody: + content: + application/json: + schema: + $ref: "#/components/schemas/CreatePrayerSpaceRequest" + required: true + responses: + "200": + content: + application/json: + schema: + $ref: "#/components/schemas/PrayerSpaceView" + description: OK + default: + content: + application/json: + schema: + $ref: "#/components/schemas/MessageResponse" + description: Request failed; body carries a human-readable message + summary: Create a prayer space + tags: + - prayer-space-admin-controller + /api/admin/minbar/prayer-spaces/{id}: + delete: + operationId: deletePrayerSpace + parameters: + - in: path + name: id + required: true + schema: + type: string + format: uuid + responses: + "200": + content: + application/json: + schema: + $ref: "#/components/schemas/MessageResponse" + description: OK + default: + content: + application/json: + schema: + $ref: "#/components/schemas/MessageResponse" + description: Request failed; body carries a human-readable message + summary: Delete a prayer space and its directions + tags: + - prayer-space-admin-controller + get: + operationId: getAdminPrayerSpace + parameters: + - in: path + name: id + required: true + schema: + type: string + format: uuid + responses: + "200": + content: + application/json: + schema: + $ref: "#/components/schemas/PrayerSpaceView" + description: OK + default: + content: + application/json: + schema: + $ref: "#/components/schemas/MessageResponse" + description: Request failed; body carries a human-readable message + summary: Fetch one prayer space as an administrator + tags: + - prayer-space-admin-controller + patch: + operationId: updatePrayerSpace + parameters: + - in: path + name: id + required: true + schema: + type: string + format: uuid + requestBody: + content: + application/json: + schema: + $ref: "#/components/schemas/UpdatePrayerSpaceRequest" + required: true + responses: + "200": + content: + application/json: + schema: + $ref: "#/components/schemas/PrayerSpaceView" + description: OK + default: + content: + application/json: + schema: + $ref: "#/components/schemas/MessageResponse" + description: Request failed; body carries a human-readable message + summary: Update a prayer space; omitted fields are left unchanged + tags: + - prayer-space-admin-controller /api/admin/permissions: get: operationId: getAvailablePermissions @@ -1867,6 +2000,7 @@ paths: - BOARD_SOCIAL_WRITE - BOARD_WEEKLY_WRITE - BOARD_TICKER_WRITE + - BOARD_PRAYER_SPACE_WRITE - BOARD_CONFIG_READ - BOARD_CONFIG_WRITE - BOARD_REFRESH @@ -1966,6 +2100,7 @@ paths: - BOARD_SOCIAL_WRITE - BOARD_WEEKLY_WRITE - BOARD_TICKER_WRITE + - BOARD_PRAYER_SPACE_WRITE - BOARD_CONFIG_READ - BOARD_CONFIG_WRITE - BOARD_REFRESH @@ -2494,7 +2629,7 @@ paths: schema: type: array items: - $ref: "#/components/schemas/PrayerSpace" + $ref: "#/components/schemas/PrayerSpaceView" description: OK default: content: @@ -2502,7 +2637,7 @@ paths: schema: $ref: "#/components/schemas/MessageResponse" description: Request failed; body carries a human-readable message - summary: List prayer spaces for an audience + summary: "List prayer spaces for an audience, with directions" tags: - minbar-controller /api/minbar/prayer-spaces/{id}: @@ -2520,7 +2655,7 @@ paths: content: application/json: schema: - $ref: "#/components/schemas/PrayerSpace" + $ref: "#/components/schemas/PrayerSpaceView" description: OK default: content: @@ -4241,6 +4376,10 @@ components: properties: zoneName: type: string + maxLength: 20 + minLength: 0 + required: + - zoneName AdminUploadDto: type: object properties: @@ -4356,6 +4495,9 @@ components: - CREATE_POSTER - UPDATE_POSTER - DELETE_POSTER + - CREATE_PRAYER_SPACE + - UPDATE_PRAYER_SPACE + - DELETE_PRAYER_SPACE - CREATE_SOCIAL - UPDATE_SOCIAL - DELETE_SOCIAL @@ -4665,7 +4807,8 @@ components: type: array items: $ref: "#/components/schemas/OrderItemRequest" - minItems: 1 + maxItems: 100 + minItems: 0 providerId: type: string required: @@ -4706,6 +4849,103 @@ components: - imageFile - startTime - title + CreatePrayerSpaceRequest: + type: object + properties: + amenities: + type: array + items: + type: string + maxLength: 255 + minLength: 0 + audience: + type: string + enum: + - brothers + - sisters + - both + building: + type: string + maxLength: 255 + minLength: 0 + capacity: + type: integer + format: int32 + minimum: 0 + directions: + type: string + entranceDescription: + type: string + maxLength: 255 + minLength: 0 + entranceName: + type: string + maxLength: 255 + minLength: 0 + floor: + type: string + maxLength: 255 + minLength: 0 + imageUrl: + type: string + pattern: "" + latitude: + type: number + format: double + maximum: 90.0 + minimum: -90.0 + longitude: + type: number + format: double + maximum: 180.0 + minimum: -180.0 + mapsUrl: + type: string + pattern: "" + name: + type: string + maxLength: 255 + minLength: 0 + notes: + type: string + roomInfo: + type: string + maxLength: 255 + minLength: 0 + startingPoint: + type: string + maxLength: 255 + minLength: 0 + steps: + type: array + items: + $ref: "#/components/schemas/DirectionStepRequest" + tag: + type: string + maxLength: 255 + minLength: 0 + tips: + type: array + items: + type: string + maxLength: 255 + minLength: 0 + type: + type: string + enum: + - brothers + - sisters + - multifaith + - reflection + walkTimeMinutes: + type: integer + format: int32 + minimum: 0 + required: + - building + - name + - tag + - type CreatePromotableSocialMediaRequest: type: object properties: @@ -4758,18 +4998,31 @@ components: properties: email: type: string + format: email + maxLength: 255 + minLength: 0 eventId: type: string format: uuid firstName: type: string + maxLength: 50 + minLength: 0 lastName: type: string + maxLength: 50 + minLength: 0 sendEmail: type: boolean ticketTypeId: type: string format: uuid + required: + - email + - eventId + - firstName + - lastName + - ticketTypeId CreateTicketTypeRequest: type: object properties: @@ -4890,7 +5143,20 @@ components: revokedAt: type: string format: date-time - DirectionStep: + DirectionStepRequest: + type: object + properties: + instruction: + type: string + maxLength: 255 + minLength: 0 + subtext: + type: string + maxLength: 255 + minLength: 0 + required: + - instruction + DirectionStepView: type: object properties: id: @@ -4898,7 +5164,7 @@ components: format: uuid instruction: type: string - stepOrder: + order: type: integer format: int32 subtext: @@ -5852,7 +6118,7 @@ components: type: string title: type: string - PrayerSpace: + PrayerSpaceView: type: object properties: amenities: @@ -5870,6 +6136,8 @@ components: capacity: type: integer format: int32 + directions: + type: string entranceDescription: type: string entranceName: @@ -5879,24 +6147,41 @@ components: id: type: string format: uuid - location: + imageUrl: type: string + latitude: + type: number + format: double + longitude: + type: number + format: double mapsUrl: type: string name: type: string + notes: + type: string + roomInfo: + type: string startingPoint: type: string steps: type: array items: - $ref: "#/components/schemas/DirectionStep" + $ref: "#/components/schemas/DirectionStepView" tag: type: string tips: type: array items: type: string + type: + type: string + enum: + - brothers + - sisters + - multifaith + - reflection walkTimeMinutes: type: integer format: int32 @@ -6068,6 +6353,9 @@ components: zoneId: type: string format: uuid + required: + - ticketId + - zoneId ScanResult: type: object properties: @@ -6330,6 +6618,98 @@ components: format: date-time title: type: string + UpdatePrayerSpaceRequest: + type: object + properties: + amenities: + type: array + items: + type: string + maxLength: 255 + minLength: 0 + audience: + type: string + enum: + - brothers + - sisters + - both + building: + type: string + maxLength: 255 + minLength: 0 + capacity: + type: integer + format: int32 + minimum: 0 + directions: + type: string + entranceDescription: + type: string + maxLength: 255 + minLength: 0 + entranceName: + type: string + maxLength: 255 + minLength: 0 + floor: + type: string + maxLength: 255 + minLength: 0 + imageUrl: + type: string + pattern: "" + latitude: + type: number + format: double + maximum: 90.0 + minimum: -90.0 + longitude: + type: number + format: double + maximum: 180.0 + minimum: -180.0 + mapsUrl: + type: string + pattern: "" + name: + type: string + maxLength: 255 + minLength: 0 + notes: + type: string + roomInfo: + type: string + maxLength: 255 + minLength: 0 + startingPoint: + type: string + maxLength: 255 + minLength: 0 + steps: + type: array + items: + $ref: "#/components/schemas/DirectionStepRequest" + tag: + type: string + maxLength: 255 + minLength: 0 + tips: + type: array + items: + type: string + maxLength: 255 + minLength: 0 + type: + type: string + enum: + - brothers + - sisters + - multifaith + - reflection + walkTimeMinutes: + type: integer + format: int32 + minimum: 0 UpdateProfileRequest: type: object properties: @@ -6566,6 +6946,9 @@ components: zoneId: type: string format: uuid + required: + - ticketId + - zoneId ValidateResetTokenRequest: type: object properties: diff --git a/src/main/java/com/ibrasoft/lensbridge/controller/MinbarController.java b/src/main/java/com/ibrasoft/lensbridge/controller/MinbarController.java index 3cb8d4c..2c36932 100644 --- a/src/main/java/com/ibrasoft/lensbridge/controller/MinbarController.java +++ b/src/main/java/com/ibrasoft/lensbridge/controller/MinbarController.java @@ -3,9 +3,9 @@ import com.ibrasoft.lensbridge.dto.board.response.frames.EventView; import com.ibrasoft.lensbridge.model.minbar.Audience; import com.ibrasoft.lensbridge.model.minbar.BoardEvent; -import com.ibrasoft.lensbridge.model.minbar.PrayerSpace; -import com.ibrasoft.lensbridge.repository.sql.PrayerSpaceRepository; +import com.ibrasoft.lensbridge.dto.minbar.response.PrayerSpaceView; import com.ibrasoft.lensbridge.service.BoardService; +import com.ibrasoft.lensbridge.service.PrayerSpaceService; import com.ibrasoft.tcketmanagebackend.model.ticket.TicketType; import com.ibrasoft.tcketmanagebackend.repository.TicketTypeRepository; import io.swagger.v3.oas.annotations.Operation; @@ -27,14 +27,14 @@ public class MinbarController { private final BoardService boardService; - private final PrayerSpaceRepository prayerSpaceRepository; + private final PrayerSpaceService prayerSpaceService; private final TicketTypeRepository ticketTypeRepository; MinbarController(BoardService boardService, - PrayerSpaceRepository prayerSpaceRepository, + PrayerSpaceService prayerSpaceService, ObjectProvider ticketTypeRepoProvider) { this.boardService = boardService; - this.prayerSpaceRepository = prayerSpaceRepository; + this.prayerSpaceService = prayerSpaceService; this.ticketTypeRepository = ticketTypeRepoProvider.getIfAvailable(); } @@ -69,20 +69,25 @@ public ResponseEntity> getEvents( return ResponseEntity.ok(events); } + /** + * Every space carries its full walking directions. The list used to omit them -- the + * endpoint returned the entity and {@code steps} is lazy -- which forced clients into + * a follow-up request per space just to render a directions sheet. There are single + * digits of prayer spaces on a campus; serving them whole is cheaper than the fan-out + * was. + */ @GetMapping("/prayer-spaces") - @Operation(operationId = "getMinbarPrayerSpaces", summary = "List prayer spaces for an audience") - public ResponseEntity> getPrayerSpaces(@RequestParam Audience audience) { + @Operation(operationId = "getMinbarPrayerSpaces", summary = "List prayer spaces for an audience, with directions") + public ResponseEntity> getPrayerSpaces(@RequestParam Audience audience) { log.debug("Minbar fetching prayer spaces for {}", audience); - return ResponseEntity.ok(prayerSpaceRepository.findByAudienceOrBoth(audience)); + return ResponseEntity.ok(prayerSpaceService.getSpacesForAudience(audience)); } @GetMapping("/prayer-spaces/{id}") @Operation(operationId = "getMinbarPrayerSpaceById", summary = "Fetch a single prayer space with directions") - public ResponseEntity getPrayerSpaceById(@PathVariable UUID id) { + public ResponseEntity getPrayerSpaceById(@PathVariable UUID id) { log.debug("Minbar fetching prayer space {}", id); - return prayerSpaceRepository.findById(id) - .map(ResponseEntity::ok) - .orElse(ResponseEntity.notFound().build()); + return ResponseEntity.ok(prayerSpaceService.getSpace(id)); } private Map> loadTicketTypes(List boardEvents) { diff --git a/src/main/java/com/ibrasoft/lensbridge/controller/PrayerSpaceAdminController.java b/src/main/java/com/ibrasoft/lensbridge/controller/PrayerSpaceAdminController.java new file mode 100644 index 0000000..4185610 --- /dev/null +++ b/src/main/java/com/ibrasoft/lensbridge/controller/PrayerSpaceAdminController.java @@ -0,0 +1,118 @@ +package com.ibrasoft.lensbridge.controller; + +import com.ibrasoft.lensbridge.dto.auth.response.MessageResponse; +import com.ibrasoft.lensbridge.dto.minbar.request.CreatePrayerSpaceRequest; +import com.ibrasoft.lensbridge.dto.minbar.request.UpdatePrayerSpaceRequest; +import com.ibrasoft.lensbridge.dto.minbar.response.PrayerSpaceView; +import com.ibrasoft.lensbridge.model.audit.AuditAction; +import com.ibrasoft.lensbridge.model.auth.Permission; +import com.ibrasoft.lensbridge.service.AdminAuditService; +import com.ibrasoft.lensbridge.service.PrayerSpaceService; +import io.swagger.v3.oas.annotations.Operation; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.validation.Valid; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.web.bind.annotation.*; + +import java.util.List; +import java.util.UUID; + +/** + * Managing the prayer spaces the app navigates people to. + *

+ * Reads are gated on {@code board:content:read} — seeing where the musallahs are is not a + * privilege, and the public endpoint under {@code /api/minbar} serves the same data + * unauthenticated. What this adds over that one is the unfiltered list: the public + * endpoint answers "what should this student see", and a console editing a sisters' + * musallah has to be able to load it whoever is signed in. + *

+ * Writes are gated on {@code board:prayerspace:write}. As elsewhere in this codebase there + * is no class-level {@code @PreAuthorize}; see {@code docs/PERMISSIONS.md}. + */ +@RestController +@RequestMapping("/api/admin/minbar/prayer-spaces") +@RequiredArgsConstructor +@Slf4j +public class PrayerSpaceAdminController { + + private final PrayerSpaceService prayerSpaceService; + private final AdminAuditService auditService; + + @GetMapping + @PreAuthorize("hasAuthority('" + Permission.Authority.BOARD_CONTENT_READ + "')") + @Operation(operationId = "listAdminPrayerSpaces", + summary = "List every prayer space, regardless of audience") + public ResponseEntity> listPrayerSpaces() { + log.debug("Admin fetching all prayer spaces"); + return ResponseEntity.ok(prayerSpaceService.getAllSpaces()); + } + + @GetMapping("/{id}") + @PreAuthorize("hasAuthority('" + Permission.Authority.BOARD_CONTENT_READ + "')") + @Operation(operationId = "getAdminPrayerSpace", summary = "Fetch one prayer space as an administrator") + public ResponseEntity getPrayerSpace(@PathVariable UUID id) { + log.debug("Admin fetching prayer space {}", id); + return ResponseEntity.ok(prayerSpaceService.getSpace(id)); + } + + @PostMapping + @PreAuthorize("hasAuthority('" + Permission.Authority.BOARD_PRAYER_SPACE_WRITE + "')") + @Operation(operationId = "createPrayerSpace", summary = "Create a prayer space") + public ResponseEntity createPrayerSpace( + @Valid @RequestBody CreatePrayerSpaceRequest createRequest, + HttpServletRequest request) { + + log.info("Admin creating prayer space: {}", createRequest.getName()); + PrayerSpaceView created = prayerSpaceService.create(createRequest); + + auditService.logAuditEvent(currentUserEmail(), AuditAction.CREATE_PRAYER_SPACE, + "PrayerSpace", created.getId(), request.getRemoteAddr()); + + return ResponseEntity.status(HttpStatus.CREATED).body(created); + } + + @PatchMapping("/{id}") + @PreAuthorize("hasAuthority('" + Permission.Authority.BOARD_PRAYER_SPACE_WRITE + "')") + @Operation(operationId = "updatePrayerSpace", + summary = "Update a prayer space; omitted fields are left unchanged") + public ResponseEntity updatePrayerSpace( + @PathVariable UUID id, + @Valid @RequestBody UpdatePrayerSpaceRequest updateRequest, + HttpServletRequest request) { + + log.info("Admin updating prayer space: {}", id); + PrayerSpaceView updated = prayerSpaceService.update(id, updateRequest); + + auditService.logAuditEvent(currentUserEmail(), AuditAction.UPDATE_PRAYER_SPACE, + "PrayerSpace", id, request.getRemoteAddr()); + + return ResponseEntity.ok(updated); + } + + @DeleteMapping("/{id}") + @PreAuthorize("hasAuthority('" + Permission.Authority.BOARD_PRAYER_SPACE_WRITE + "')") + @Operation(operationId = "deletePrayerSpace", summary = "Delete a prayer space and its directions") + public ResponseEntity deletePrayerSpace( + @PathVariable UUID id, + HttpServletRequest request) { + + log.info("Admin deleting prayer space: {}", id); + prayerSpaceService.delete(id); + + auditService.logAuditEvent(currentUserEmail(), AuditAction.DELETE_PRAYER_SPACE, + "PrayerSpace", id, request.getRemoteAddr()); + + return ResponseEntity.ok(new MessageResponse("Prayer space deleted successfully")); + } + + private String currentUserEmail() { + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + return authentication != null ? authentication.getName() : "unknown"; + } +} diff --git a/src/main/java/com/ibrasoft/lensbridge/dto/minbar/request/CreatePrayerSpaceRequest.java b/src/main/java/com/ibrasoft/lensbridge/dto/minbar/request/CreatePrayerSpaceRequest.java new file mode 100644 index 0000000..9f488ea --- /dev/null +++ b/src/main/java/com/ibrasoft/lensbridge/dto/minbar/request/CreatePrayerSpaceRequest.java @@ -0,0 +1,89 @@ +package com.ibrasoft.lensbridge.dto.minbar.request; + +import com.ibrasoft.lensbridge.model.minbar.Audience; +import com.ibrasoft.lensbridge.model.minbar.PrayerSpaceType; +import jakarta.validation.Valid; +import jakarta.validation.constraints.*; +import lombok.*; +import org.hibernate.validator.constraints.URL; + +import java.util.ArrayList; +import java.util.List; + +@Data +@Builder +@NoArgsConstructor +@AllArgsConstructor +public class CreatePrayerSpaceRequest { + + @NotBlank(message = "Name is required") + @Size(max = 255, message = "Name must be 255 characters or fewer") + private String name; + + @NotBlank(message = "Tag is required") + @Size(max = 255, message = "Tag must be 255 characters or fewer") + private String tag; + + @NotBlank(message = "Building is required") + @Size(max = 255, message = "Building must be 255 characters or fewer") + private String building; + + @Size(max = 255) + private String floor; + + @Size(max = 255) + private String roomInfo; + + @Positive(message = "Capacity must be positive") + private Integer capacity; + + @NotNull(message = "Type is required") + private PrayerSpaceType type; + + /** + * Optional. Omitted, it follows from {@code type} — a brothers' musallah is listed + * to brothers, a reflection bay to everyone. Set it only to override that. + */ + private Audience audience; + + @DecimalMin(value = "-90.0", message = "Latitude must be between -90 and 90") + @DecimalMax(value = "90.0", message = "Latitude must be between -90 and 90") + private Double latitude; + + @DecimalMin(value = "-180.0", message = "Longitude must be between -180 and 180") + @DecimalMax(value = "180.0", message = "Longitude must be between -180 and 180") + private Double longitude; + + @URL(message = "Maps URL must be a valid URL") + private String mapsUrl; + + @URL(message = "Image URL must be a valid URL") + private String imageUrl; + + private String notes; + + /** Prose directions, shown when no {@code steps} have been written. */ + private String directions; + + @Size(max = 255) + private String startingPoint; + + @PositiveOrZero(message = "Walk time cannot be negative") + private Integer walkTimeMinutes; + + @Size(max = 255) + private String entranceName; + + @Size(max = 255) + private String entranceDescription; + + @Builder.Default + private List<@NotBlank @Size(max = 255) String> amenities = new ArrayList<>(); + + @Valid + @Builder.Default + private List steps = new ArrayList<>(); + + @Builder.Default + private List<@NotBlank @Size(max = 255) String> tips = new ArrayList<>(); +} diff --git a/src/main/java/com/ibrasoft/lensbridge/dto/minbar/request/DirectionStepRequest.java b/src/main/java/com/ibrasoft/lensbridge/dto/minbar/request/DirectionStepRequest.java new file mode 100644 index 0000000..87f0951 --- /dev/null +++ b/src/main/java/com/ibrasoft/lensbridge/dto/minbar/request/DirectionStepRequest.java @@ -0,0 +1,25 @@ +package com.ibrasoft.lensbridge.dto.minbar.request; + +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Size; +import lombok.*; + +/** + * One walking instruction. Position comes from the list order, not from a field — + * a client that sends steps out of order and numbers them separately can disagree + * with itself, and the server would have no way to tell which it meant. + */ +@Data +@Builder +@NoArgsConstructor +@AllArgsConstructor +public class DirectionStepRequest { + + @NotBlank(message = "Each step needs an instruction") + @Size(max = 255, message = "Instruction must be 255 characters or fewer") + private String instruction; + + /** Optional landmark confirming the step was followed correctly. */ + @Size(max = 255, message = "Landmark must be 255 characters or fewer") + private String subtext; +} diff --git a/src/main/java/com/ibrasoft/lensbridge/dto/minbar/request/UpdatePrayerSpaceRequest.java b/src/main/java/com/ibrasoft/lensbridge/dto/minbar/request/UpdatePrayerSpaceRequest.java new file mode 100644 index 0000000..c669023 --- /dev/null +++ b/src/main/java/com/ibrasoft/lensbridge/dto/minbar/request/UpdatePrayerSpaceRequest.java @@ -0,0 +1,91 @@ +package com.ibrasoft.lensbridge.dto.minbar.request; + +import com.ibrasoft.lensbridge.model.minbar.Audience; +import com.ibrasoft.lensbridge.model.minbar.PrayerSpaceType; +import jakarta.validation.Valid; +import jakarta.validation.constraints.*; +import lombok.*; +import org.hibernate.validator.constraints.URL; + +import java.util.List; + +/** + * A partial update. Null means "leave it alone" for every field, which is why nothing + * here is {@code @NotNull} — see {@code UpdatePosterRequest} for the same idiom. + *

+ * The list fields are the exception worth stating: sending {@code amenities} replaces the + * whole list, and sending an empty list clears it. There is no per-item patch, because a + * list of strings has no stable identity to patch against. + *

+ * {@code audience} does not re-derive from {@code type} here. Changing a space's + * type on an existing record should not silently re-list a sisters-only reflection bay to + * the whole campus; if the audience should move too, send both. + */ +@Data +@Builder +@NoArgsConstructor +@AllArgsConstructor +public class UpdatePrayerSpaceRequest { + + @Size(max = 255) + private String name; + + @Size(max = 255) + private String tag; + + @Size(max = 255) + private String building; + + @Size(max = 255) + private String floor; + + @Size(max = 255) + private String roomInfo; + + @Positive(message = "Capacity must be positive") + private Integer capacity; + + private PrayerSpaceType type; + + private Audience audience; + + @DecimalMin(value = "-90.0", message = "Latitude must be between -90 and 90") + @DecimalMax(value = "90.0", message = "Latitude must be between -90 and 90") + private Double latitude; + + @DecimalMin(value = "-180.0", message = "Longitude must be between -180 and 180") + @DecimalMax(value = "180.0", message = "Longitude must be between -180 and 180") + private Double longitude; + + @URL(message = "Maps URL must be a valid URL") + private String mapsUrl; + + @URL(message = "Image URL must be a valid URL") + private String imageUrl; + + private String notes; + + private String directions; + + @Size(max = 255) + private String startingPoint; + + @PositiveOrZero(message = "Walk time cannot be negative") + private Integer walkTimeMinutes; + + @Size(max = 255) + private String entranceName; + + @Size(max = 255) + private String entranceDescription; + + /** Replaces the whole list when present. */ + private List<@NotBlank @Size(max = 255) String> amenities; + + /** Replaces the whole list when present. */ + @Valid + private List steps; + + /** Replaces the whole list when present. */ + private List<@NotBlank @Size(max = 255) String> tips; +} diff --git a/src/main/java/com/ibrasoft/lensbridge/dto/minbar/response/PrayerSpaceView.java b/src/main/java/com/ibrasoft/lensbridge/dto/minbar/response/PrayerSpaceView.java new file mode 100644 index 0000000..c5dd4a6 --- /dev/null +++ b/src/main/java/com/ibrasoft/lensbridge/dto/minbar/response/PrayerSpaceView.java @@ -0,0 +1,112 @@ +package com.ibrasoft.lensbridge.dto.minbar.response; + +import com.ibrasoft.lensbridge.model.minbar.Audience; +import com.ibrasoft.lensbridge.model.minbar.DirectionStep; +import com.ibrasoft.lensbridge.model.minbar.PrayerSpace; +import com.ibrasoft.lensbridge.model.minbar.PrayerSpaceType; +import lombok.*; + +import java.util.List; +import java.util.UUID; + +/** + * A prayer space on the wire. + *

+ * This exists because the endpoints used to return the entity. That leaked the JPA + * mapping into the public contract — {@code steps} is a lazy {@code @OneToMany}, so + * whether a caller got the walking directions depended on whether a transaction happened + * to be open, which is why the admin console has a comment saying the list endpoint omits + * them and re-fetches each space one at a time. A view assembled inside the service is + * the same shape every time. + *

+ * The fields are the stored parts, not rendered sentences: {@code walkTimeMinutes} and + * {@code startingPoint} rather than "2 min walk from the CCT main entrance". Clients + * compose the copy, which is what lets the app and the board word it differently without + * the server picking a side. + */ +@Data +@Builder +@NoArgsConstructor +@AllArgsConstructor +public class PrayerSpaceView { + + private UUID id; + private String name; + private String tag; + private String building; + private String floor; + private String roomInfo; + private Integer capacity; + private PrayerSpaceType type; + private Audience audience; + + /** Null together with {@code longitude} when nobody has pinned the space yet. */ + private Double latitude; + private Double longitude; + + private String mapsUrl; + private String imageUrl; + private String notes; + + /** Prose fallback, used when {@code steps} is empty. */ + private String directions; + + private String startingPoint; + private Integer walkTimeMinutes; + private String entranceName; + private String entranceDescription; + + private List amenities; + private List steps; + private List tips; + + /** One leg of the walk. */ + @Data + @Builder + @NoArgsConstructor + @AllArgsConstructor + public static class DirectionStepView { + private UUID id; + /** Zero-based position, so a client can render "step 3 of 5" without re-indexing. */ + private Integer order; + private String instruction; + /** The landmark that confirms you took the right turn. */ + private String subtext; + } + + public static PrayerSpaceView of(PrayerSpace space) { + return PrayerSpaceView.builder() + .id(space.getId()) + .name(space.getName()) + .tag(space.getTag()) + .building(space.getBuilding()) + .floor(space.getFloor()) + .roomInfo(space.getRoomInfo()) + .capacity(space.getCapacity()) + .type(space.getSpaceType()) + .audience(space.getAudience()) + .latitude(space.getLatitude()) + .longitude(space.getLongitude()) + .mapsUrl(space.getMapsUrl()) + .imageUrl(space.getImageUrl()) + .notes(space.getNotes()) + .directions(space.getDirections()) + .startingPoint(space.getStartingPoint()) + .walkTimeMinutes(space.getWalkTimeMinutes()) + .entranceName(space.getEntranceName()) + .entranceDescription(space.getEntranceDescription()) + .amenities(List.copyOf(space.getAmenities())) + .steps(space.getSteps().stream().map(PrayerSpaceView::stepOf).toList()) + .tips(List.copyOf(space.getTips())) + .build(); + } + + private static DirectionStepView stepOf(DirectionStep step) { + return DirectionStepView.builder() + .id(step.getId()) + .order(step.getStepOrder()) + .instruction(step.getInstruction()) + .subtext(step.getSubtext()) + .build(); + } +} diff --git a/src/main/java/com/ibrasoft/lensbridge/model/audit/AuditAction.java b/src/main/java/com/ibrasoft/lensbridge/model/audit/AuditAction.java index f5a44cc..6182e02 100644 --- a/src/main/java/com/ibrasoft/lensbridge/model/audit/AuditAction.java +++ b/src/main/java/com/ibrasoft/lensbridge/model/audit/AuditAction.java @@ -28,6 +28,13 @@ public enum AuditAction { UPDATE_POSTER("Update Poster"), DELETE_POSTER("Delete Poster"), + // Prayer Space Actions (Minbar) + // Worth auditing separately from board content: these are walking directions inside + // buildings, and a bad edit is only discovered by someone standing in the wrong hallway. + CREATE_PRAYER_SPACE("Create Prayer Space"), + UPDATE_PRAYER_SPACE("Update Prayer Space"), + DELETE_PRAYER_SPACE("Delete Prayer Space"), + // Promotable Social Media Actions CREATE_SOCIAL("Create Promotable Social Media"), UPDATE_SOCIAL("Update Promotable Social Media"), diff --git a/src/main/java/com/ibrasoft/lensbridge/model/auth/Permission.java b/src/main/java/com/ibrasoft/lensbridge/model/auth/Permission.java index 8def537..9323e16 100644 --- a/src/main/java/com/ibrasoft/lensbridge/model/auth/Permission.java +++ b/src/main/java/com/ibrasoft/lensbridge/model/auth/Permission.java @@ -36,6 +36,13 @@ public enum Permission implements GrantedAuthority { BOARD_WEEKLY_WRITE(Authority.BOARD_WEEKLY_WRITE), /** Ticker copy only. Deliberately not BOARD_CONFIG_WRITE, which can move the board's coordinates. */ BOARD_TICKER_WRITE(Authority.BOARD_TICKER_WRITE), + /** + * The prayer spaces the app navigates people to. Its own permission rather than + * BOARD_CONTENT_READ's write twin, because these are wayfinding instructions a + * student follows through a building at Maghrib -- getting them wrong sends someone + * to the wrong floor, which is a different kind of wrong from a stale poster. + */ + BOARD_PRAYER_SPACE_WRITE(Authority.BOARD_PRAYER_SPACE_WRITE), // ==================== MusallahBoard configuration ==================== @@ -107,6 +114,7 @@ public interface Authority { String BOARD_SOCIAL_WRITE = "board:social:write"; String BOARD_WEEKLY_WRITE = "board:weekly:write"; String BOARD_TICKER_WRITE = "board:ticker:write"; + String BOARD_PRAYER_SPACE_WRITE = "board:prayerspace:write"; String BOARD_CONFIG_READ = "board:config:read"; String BOARD_CONFIG_WRITE = "board:config:write"; diff --git a/src/main/java/com/ibrasoft/lensbridge/model/auth/RolePermissions.java b/src/main/java/com/ibrasoft/lensbridge/model/auth/RolePermissions.java index feae66a..c5c3230 100644 --- a/src/main/java/com/ibrasoft/lensbridge/model/auth/RolePermissions.java +++ b/src/main/java/com/ibrasoft/lensbridge/model/auth/RolePermissions.java @@ -44,6 +44,7 @@ final class RolePermissions { Permission.BOARD_SOCIAL_WRITE, Permission.BOARD_WEEKLY_WRITE, Permission.BOARD_TICKER_WRITE, + Permission.BOARD_PRAYER_SPACE_WRITE, Permission.BOARD_REFRESH)); // Owns the board product. Note the absence of everything under iam: — owning the diff --git a/src/main/java/com/ibrasoft/lensbridge/model/minbar/PrayerSpace.java b/src/main/java/com/ibrasoft/lensbridge/model/minbar/PrayerSpace.java index 619efeb..45a535a 100644 --- a/src/main/java/com/ibrasoft/lensbridge/model/minbar/PrayerSpace.java +++ b/src/main/java/com/ibrasoft/lensbridge/model/minbar/PrayerSpace.java @@ -2,11 +2,22 @@ import jakarta.persistence.*; import lombok.*; +import org.hibernate.annotations.BatchSize; import java.util.ArrayList; import java.util.List; import java.util.UUID; +/** + * A place on campus someone can pray. + *

+ * The directions fields are stored as the parts they are made of — a starting point, a + * walk time, an entrance, ordered steps — rather than as the sentences an app renders + * from them. A client composes "2 min walk from the CCT main entrance"; the database + * holds {@code startingPoint = "the CCT main entrance"} and {@code walkTimeMinutes = 2}. + * That is the same split the board payload uses, and it is what lets the console edit a + * walk time without re-parsing prose. + */ @Entity @Table(name = "prayer_spaces") @Data @Builder @NoArgsConstructor @AllArgsConstructor @@ -18,6 +29,7 @@ public class PrayerSpace { @Column(nullable = false) private String name; + /** Short caption under the name, e.g. "Main brothers' space". */ @Column(nullable = false) private String tag; @@ -26,18 +38,53 @@ public class PrayerSpace { @Column(nullable = false) private String building; - private String location; + /** Where in the building, in words: "Between offices 3026 & 3028". */ + @Column(name = "room_info") + private String roomInfo; private Integer capacity; + /** + * What kind of space this is — the label and the map filter. + * See {@link PrayerSpaceType} for why this is not {@link #audience}. + */ + @Enumerated(EnumType.STRING) + @Column(name = "space_type", nullable = false) + private PrayerSpaceType spaceType; + + /** Who the space is listed for. Defaults from {@link #spaceType} on create. */ @Enumerated(EnumType.STRING) @Column(nullable = false) private Audience audience; + /** + * Nullable, and the app has to cope: a reflection bay tucked inside a building has no + * meaningful pin, and a space can be listed before anyone has stood in it with a + * phone. Without both halves there is no distance, no marker, and no maps link. + */ + private Double latitude; + + private Double longitude; + private String mapsUrl; + @Column(name = "image_url") + private String imageUrl; + + /** Free-text operational note: "Gets busy during Dhuhr and Jummah." */ + @Column(columnDefinition = "TEXT") + private String notes; + // --- Directions metadata --- + /** + * Prose directions, for a space nobody has written {@link #steps} for yet. The app + * falls back to this when the step list is empty, so a new space is useful the moment + * it is created rather than only once someone has walked it. + */ + @Column(columnDefinition = "TEXT") + private String directions; + private String startingPoint; private Integer walkTimeMinutes; @@ -48,6 +95,7 @@ public class PrayerSpace { // --- Collections --- + @BatchSize(size = 32) @ElementCollection @CollectionTable(name = "prayer_space_amenities", joinColumns = @JoinColumn(name = "prayer_space_id"), @@ -57,11 +105,13 @@ public class PrayerSpace { @Builder.Default private List amenities = new ArrayList<>(); + @BatchSize(size = 32) @OneToMany(mappedBy = "prayerSpace", cascade = CascadeType.ALL, orphanRemoval = true) @OrderBy("stepOrder ASC") @Builder.Default private List steps = new ArrayList<>(); + @BatchSize(size = 32) @ElementCollection @CollectionTable(name = "prayer_space_tips", joinColumns = @JoinColumn(name = "prayer_space_id"), @@ -70,4 +120,23 @@ public class PrayerSpace { @OrderColumn(name = "sort_order") @Builder.Default private List tips = new ArrayList<>(); + + /** + * Replaces the walking directions with {@code replacement}, renumbering as it goes. + *

+ * Mutates the existing list rather than assigning a new one: {@code orphanRemoval} + * tracks the collection instance Hibernate handed us, and swapping it out is what + * raises "a collection with cascade=all-delete-orphan was no longer referenced". + * Steps have no identity worth preserving across an edit — nothing links to a step — + * so replacing wholesale is both correct and simpler than diffing by id. + */ + public void replaceSteps(List replacement) { + steps.clear(); + int order = 0; + for (DirectionStep step : replacement) { + step.setPrayerSpace(this); + step.setStepOrder(order++); + steps.add(step); + } + } } diff --git a/src/main/java/com/ibrasoft/lensbridge/model/minbar/PrayerSpaceType.java b/src/main/java/com/ibrasoft/lensbridge/model/minbar/PrayerSpaceType.java new file mode 100644 index 0000000..3c797b4 --- /dev/null +++ b/src/main/java/com/ibrasoft/lensbridge/model/minbar/PrayerSpaceType.java @@ -0,0 +1,71 @@ +package com.ibrasoft.lensbridge.model.minbar; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonValue; + +import java.util.Arrays; +import java.util.Optional; + +/** + * What kind of room this is. Deliberately not {@link Audience}, which the two were + * conflated as before. + *

+ * {@code Audience} answers "who is this listed for" — it is the filter the app applies + * to a signed-in user, and a sister never sees a brothers' musallah. {@code + * PrayerSpaceType} answers "what is it" — the label on the card, the colour of the pin, + * and the chips someone taps to narrow the map. The two agree for a gendered musallah + * and diverge for everything else: a multifaith room and a reflection bay are both shown + * to everyone, but they are not the same place and a student looking for somewhere quiet + * to pray between lectures wants to tell them apart. + *

+ * {@link #defaultAudience()} is what makes carrying both cheap: a create request that + * omits an audience gets the obvious one, so the only time anybody sets it by hand is the + * genuinely unusual case — a reflection bay that a building has designated sisters-only, + * say. + */ +public enum PrayerSpaceType { + /** A dedicated brothers' musallah. */ + BROTHERS("brothers", Audience.BROTHERS), + /** A dedicated sisters' musallah. */ + SISTERS("sisters", Audience.SISTERS), + /** A room shared with other faith groups. Open to everyone, and not exclusively ours. */ + MULTIFAITH("multifaith", Audience.BOTH), + /** A campus reflection bay: quiet, usually small, rarely equipped. */ + REFLECTION("reflection", Audience.BOTH); + + private final String typeName; + private final Audience defaultAudience; + + PrayerSpaceType(String typeName, Audience defaultAudience) { + this.typeName = typeName; + this.defaultAudience = defaultAudience; + } + + /** Who this kind of space is listed for when a request does not say. */ + public Audience defaultAudience() { + return defaultAudience; + } + + public static Optional from(String s) { + if (s == null) return Optional.empty(); + return Arrays.stream(values()) + .filter(t -> t.name().equalsIgnoreCase(s) || t.typeName.equalsIgnoreCase(s)) + .findFirst(); + } + + @JsonValue + public String toJson() { + return typeName; + } + + /** Accepts either case, matching {@link Audience#fromJson}. */ + @JsonCreator + public static PrayerSpaceType fromJson(String s) { + return from(s).orElseThrow(() -> new IllegalArgumentException("Unknown prayer space type: " + s)); + } + + @Override + public String toString() { + return typeName; + } +} diff --git a/src/main/java/com/ibrasoft/lensbridge/repository/sql/PrayerSpaceRepository.java b/src/main/java/com/ibrasoft/lensbridge/repository/sql/PrayerSpaceRepository.java index 7dda312..b4f2b7b 100644 --- a/src/main/java/com/ibrasoft/lensbridge/repository/sql/PrayerSpaceRepository.java +++ b/src/main/java/com/ibrasoft/lensbridge/repository/sql/PrayerSpaceRepository.java @@ -15,4 +15,7 @@ public interface PrayerSpaceRepository extends JpaRepository @Query("SELECT ps FROM PrayerSpace ps WHERE ps.audience = :aud OR ps.audience = com.ibrasoft.lensbridge.model.minbar.Audience.BOTH ORDER BY ps.name ASC") List findByAudienceOrBoth(@Param("aud") Audience audience); + + /** Every space regardless of audience — the console manages all of them. */ + List findAllByOrderByNameAsc(); } diff --git a/src/main/java/com/ibrasoft/lensbridge/service/PrayerSpaceService.java b/src/main/java/com/ibrasoft/lensbridge/service/PrayerSpaceService.java new file mode 100644 index 0000000..77b74ea --- /dev/null +++ b/src/main/java/com/ibrasoft/lensbridge/service/PrayerSpaceService.java @@ -0,0 +1,212 @@ +package com.ibrasoft.lensbridge.service; + +import com.ibrasoft.lensbridge.dto.minbar.request.CreatePrayerSpaceRequest; +import com.ibrasoft.lensbridge.dto.minbar.request.DirectionStepRequest; +import com.ibrasoft.lensbridge.dto.minbar.request.UpdatePrayerSpaceRequest; +import com.ibrasoft.lensbridge.dto.minbar.response.PrayerSpaceView; +import com.ibrasoft.lensbridge.dto.upload.response.ErrorResponse; +import com.ibrasoft.lensbridge.exception.ApiResponseException; +import com.ibrasoft.lensbridge.model.minbar.Audience; +import com.ibrasoft.lensbridge.model.minbar.DirectionStep; +import com.ibrasoft.lensbridge.model.minbar.PrayerSpace; +import com.ibrasoft.lensbridge.repository.sql.PrayerSpaceRepository; +import com.ibrasoft.lensbridge.util.Patch; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.http.HttpStatus; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import java.util.function.Consumer; + +/** + * Prayer spaces, read and written. + *

+ * Every method returns a {@link PrayerSpaceView} rather than the entity. Assembling the + * view inside the transaction is the point: the collections are lazy, so a controller + * handed the entity would get whatever the session happened to have initialized, which is + * how the list endpoint ended up silently dropping the walking directions. + */ +@Service +@RequiredArgsConstructor +@Slf4j +public class PrayerSpaceService { + + private final PrayerSpaceRepository prayerSpaceRepository; + + // ==================== Reads ==================== + + /** What the app shows a signed-in user: their audience, plus everything open to both. */ + @Transactional(readOnly = true) + public List getSpacesForAudience(Audience audience) { + return prayerSpaceRepository.findByAudienceOrBoth(audience).stream() + .map(PrayerSpaceView::of) + .toList(); + } + + /** Every space, for the console. */ + @Transactional(readOnly = true) + public List getAllSpaces() { + return prayerSpaceRepository.findAllByOrderByNameAsc().stream() + .map(PrayerSpaceView::of) + .toList(); + } + + @Transactional(readOnly = true) + public PrayerSpaceView getSpace(UUID id) { + return PrayerSpaceView.of(findOrThrow(id)); + } + + // ==================== Writes ==================== + + @Transactional + public PrayerSpaceView create(CreatePrayerSpaceRequest request) { + validateCoordinates(request.getLatitude(), request.getLongitude()); + + PrayerSpace space = PrayerSpace.builder() + .name(request.getName().trim()) + .tag(request.getTag().trim()) + .building(request.getBuilding().trim()) + .floor(blankToNull(request.getFloor())) + .roomInfo(blankToNull(request.getRoomInfo())) + .capacity(request.getCapacity()) + .spaceType(request.getType()) + // An unstated audience follows from the type; see PrayerSpaceType. + .audience(request.getAudience() != null + ? request.getAudience() + : request.getType().defaultAudience()) + .latitude(request.getLatitude()) + .longitude(request.getLongitude()) + .mapsUrl(blankToNull(request.getMapsUrl())) + .imageUrl(blankToNull(request.getImageUrl())) + .notes(blankToNull(request.getNotes())) + .directions(blankToNull(request.getDirections())) + .startingPoint(blankToNull(request.getStartingPoint())) + .walkTimeMinutes(request.getWalkTimeMinutes()) + .entranceName(blankToNull(request.getEntranceName())) + .entranceDescription(blankToNull(request.getEntranceDescription())) + .amenities(trimmedCopy(request.getAmenities())) + .tips(trimmedCopy(request.getTips())) + .build(); + + space.replaceSteps(toSteps(request.getSteps())); + + space = prayerSpaceRepository.save(space); + log.info("Created prayer space: id={}, name={}", space.getId(), space.getName()); + return PrayerSpaceView.of(space); + } + + @Transactional + public PrayerSpaceView update(UUID id, UpdatePrayerSpaceRequest request) { + PrayerSpace space = findOrThrow(id); + + Patch.apply(blankToNull(request.getName()), space::setName); + Patch.apply(blankToNull(request.getTag()), space::setTag); + Patch.apply(blankToNull(request.getBuilding()), space::setBuilding); + Patch.apply(request.getCapacity(), space::setCapacity); + Patch.apply(request.getType(), space::setSpaceType); + Patch.apply(request.getAudience(), space::setAudience); + Patch.apply(request.getWalkTimeMinutes(), space::setWalkTimeMinutes); + + // Optional text: an empty string clears the field, matching UpdatePosterRequest's + // signupUrl. A space can lose its room number or its photo without being deleted + // and recreated, and null still means "unchanged". + clearable(request.getFloor(), space::setFloor); + clearable(request.getRoomInfo(), space::setRoomInfo); + clearable(request.getMapsUrl(), space::setMapsUrl); + clearable(request.getImageUrl(), space::setImageUrl); + clearable(request.getNotes(), space::setNotes); + clearable(request.getDirections(), space::setDirections); + clearable(request.getStartingPoint(), space::setStartingPoint); + clearable(request.getEntranceName(), space::setEntranceName); + clearable(request.getEntranceDescription(), space::setEntranceDescription); + + Patch.apply(request.getLatitude(), space::setLatitude); + Patch.apply(request.getLongitude(), space::setLongitude); + validateCoordinates(space.getLatitude(), space.getLongitude()); + + // Lists replace wholesale. Mutated in place rather than reassigned: the amenity + // and tip collections are @ElementCollection instances Hibernate is tracking, and + // handing it a new ArrayList orphans the one it knows about. + replaceInPlace(space.getAmenities(), request.getAmenities()); + replaceInPlace(space.getTips(), request.getTips()); + if (request.getSteps() != null) { + space.replaceSteps(toSteps(request.getSteps())); + } + + space = prayerSpaceRepository.save(space); + log.info("Updated prayer space: id={}", id); + return PrayerSpaceView.of(space); + } + + @Transactional + public void delete(UUID id) { + PrayerSpace space = findOrThrow(id); + prayerSpaceRepository.delete(space); + log.info("Deleted prayer space: id={}, name={}", id, space.getName()); + } + + // ==================== Helpers ==================== + + private PrayerSpace findOrThrow(UUID id) { + return prayerSpaceRepository.findById(id) + .orElseThrow(() -> new ApiResponseException( + HttpStatus.NOT_FOUND, + ErrorResponse.of("Prayer space not found with id: " + id))); + } + + /** + * A pin needs both halves. One coordinate on its own is not a partial location, it is + * a point somewhere off the coast of Africa, so reject it rather than store it and let + * the map draw it. + */ + private void validateCoordinates(Double latitude, Double longitude) { + if ((latitude == null) != (longitude == null)) { + throw new ApiResponseException( + HttpStatus.BAD_REQUEST, + ErrorResponse.of("Latitude and longitude must be provided together")); + } + } + + private static List toSteps(List requests) { + return orEmpty(requests).stream() + .map(r -> DirectionStep.builder() + .instruction(r.getInstruction().trim()) + .subtext(blankToNull(r.getSubtext())) + .build()) + .toList(); + } + + private static void replaceInPlace(List target, List replacement) { + if (replacement == null) return; + target.clear(); + target.addAll(trimmedCopy(replacement)); + } + + private static List trimmedCopy(List values) { + List copy = new ArrayList<>(); + for (String value : orEmpty(values)) { + copy.add(value.trim()); + } + return copy; + } + + /** Applies a patch field where an empty string means "clear this". */ + private static void clearable(String value, Consumer setter) { + if (value == null) return; + setter.accept(blankToNull(value)); + } + + private static String blankToNull(String value) { + if (value == null) return null; + String trimmed = value.trim(); + return trimmed.isEmpty() ? null : trimmed; + } + + private static List orEmpty(List value) { + return value == null ? List.of() : value; + } +} diff --git a/src/main/resources/db/migration/postgresql/V8__prayer_space_type_and_coordinates.sql b/src/main/resources/db/migration/postgresql/V8__prayer_space_type_and_coordinates.sql new file mode 100644 index 0000000..c2cd069 --- /dev/null +++ b/src/main/resources/db/migration/postgresql/V8__prayer_space_type_and_coordinates.sql @@ -0,0 +1,82 @@ +-- +-- V8: give a prayer space a type, a pin on the map, and somewhere to put a photo, +-- and admit BOARD_PRAYER_SPACE_WRITE into the permission CHECK. +-- +-- Two things were conflated before this. `audience` was doing double duty as both "who is +-- this listed for" and "what kind of room is it", which cannot describe the campus: a +-- multifaith room and a reflection bay are both shown to everyone, and are not the same +-- place. `space_type` now answers the second question and `audience` keeps the first. +-- Existing rows backfill from audience -- a BROTHERS row was a brothers' musallah -- with +-- BOTH landing on MULTIFAITH, since nothing in the seed data was a reflection bay under +-- the old shape. Anything mislabelled is one PATCH away. +-- +-- `location` becomes `room_info`. It described where in the building the room is +-- ("Between offices 3026 & 3028"), which stops reading as "location" the moment the table +-- also holds latitude and longitude. +-- +-- Coordinates are nullable and are written as a pair (the service rejects one without the +-- other): a reflection bay tucked inside a building has no useful pin, and a space can be +-- listed before anyone has stood in it with a phone. +-- +-- The SQLite twin of this file must stay at the same version -- see docs/MIGRATIONS.md. + + +-- 1. Rename location -> room_info. +alter table prayer_spaces rename column location to room_info; + + +-- 2. Add space_type, backfilled from audience, then made NOT NULL. +alter table prayer_spaces add column space_type varchar(255); + +update prayer_spaces +set space_type = case audience + when 'BROTHERS' then 'BROTHERS' + when 'SISTERS' then 'SISTERS' + else 'MULTIFAITH' + end; + +alter table prayer_spaces alter column space_type set not null; + +alter table prayer_spaces + add constraint prayer_spaces_space_type_check + check (space_type in ('BROTHERS','SISTERS','MULTIFAITH','REFLECTION')); + + +-- 3. Map pin, photo, and the free-text fields the app renders when no steps exist yet. +alter table prayer_spaces add column latitude float(53); +alter table prayer_spaces add column longitude float(53); +alter table prayer_spaces add column image_url varchar(255); +alter table prayer_spaces add column notes TEXT; +alter table prayer_spaces add column directions TEXT; + + +-- 4. Widen the user_permissions CHECK for BOARD_PRAYER_SPACE_WRITE. +-- Same discovery-then-replace pattern as V6 and V7: drop whatever CHECK currently +-- constrains the column (prod's names predate Flyway and are not guaranteed to match a +-- fresh database's), then add a named replacement. + +do $$ +declare + constraint_name text; +begin + for constraint_name in + select con.conname + from pg_constraint con + join pg_class rel on rel.oid = con.conrelid + where rel.relname = 'user_permissions' + and con.contype = 'c' + loop + execute format('alter table user_permissions drop constraint %I', constraint_name); + end loop; +end $$; + +alter table user_permissions + add constraint user_permissions_permission_check check (permission in ( + 'MEDIA_UPLOAD_SELF','MEDIA_UPLOAD_MODERATE','MEDIA_UPLOAD_READ', + 'BOARD_CONTENT_READ','BOARD_POSTER_WRITE','BOARD_EVENT_WRITE','BOARD_SOCIAL_WRITE', + 'BOARD_WEEKLY_WRITE','BOARD_TICKER_WRITE','BOARD_PRAYER_SPACE_WRITE', + 'BOARD_CONFIG_READ','BOARD_CONFIG_WRITE','BOARD_REFRESH', + 'BOARD_DEVICE_READ','BOARD_DEVICE_ENROLL','BOARD_DEVICE_REVOKE','BOARD_TELEMETRY_SUBSCRIBE', + 'BOARD_COMMAND_BENIGN','BOARD_COMMAND_DISRUPTIVE','BOARD_COMMAND_INSPECT', + 'TCKET_SCAN','TCKET_MANAGE','TCKET_ADMIN', + 'IAM_USER_READ','IAM_USER_WRITE','IAM_ROLE_GRANT','AUDIT_READ')); diff --git a/src/main/resources/db/migration/sqlite/V8__prayer_space_type_and_coordinates.sql b/src/main/resources/db/migration/sqlite/V8__prayer_space_type_and_coordinates.sql new file mode 100644 index 0000000..b3250f3 --- /dev/null +++ b/src/main/resources/db/migration/sqlite/V8__prayer_space_type_and_coordinates.sql @@ -0,0 +1,83 @@ +-- +-- V8: give a prayer space a type, a pin on the map, and somewhere to put a photo, +-- and admit BOARD_PRAYER_SPACE_WRITE into the permission CHECK. +-- +-- See the Postgres twin for why space_type exists at all and why `location` becomes +-- `room_info`. The difference here is mechanical: SQLite cannot add a NOT NULL column to a +-- populated table without a DEFAULT, cannot promote a column to NOT NULL afterwards, and +-- cannot alter a CHECK constraint -- so both tables are rebuilt, the same pattern V4, V6 +-- and V7 use. +-- +-- Dropping prayer_spaces is safe here even though three child tables reference it: the +-- SQLite twin of V3 declares them without FK constraints (SQLite does not enforce foreign +-- keys by default), so their rows survive the swap and keep pointing at the same ids. +-- +-- The Postgres twin of this file must stay at the same version -- see docs/MIGRATIONS.md. + + +-- 1. Rebuild prayer_spaces: rename location -> room_info, add space_type (backfilled from +-- audience), coordinates, image, notes, and prose directions. + +create table prayer_spaces_new ( + capacity integer, + walk_time_minutes integer, + latitude float, + longitude float, + id blob not null, + audience varchar(255) not null check (audience in ('BROTHERS','SISTERS','BOTH')), + space_type varchar(255) not null check (space_type in ('BROTHERS','SISTERS','MULTIFAITH','REFLECTION')), + building varchar(255) not null, + entrance_description varchar(255), + entrance_name varchar(255), + floor varchar(255), + room_info varchar(255), + image_url varchar(255), + maps_url varchar(255), + name varchar(255) not null, + starting_point varchar(255), + tag varchar(255) not null, + notes TEXT, + directions TEXT, + primary key (id) +); + +insert into prayer_spaces_new ( + capacity, walk_time_minutes, latitude, longitude, id, audience, space_type, building, + entrance_description, entrance_name, floor, room_info, image_url, maps_url, name, + starting_point, tag, notes, directions) +select + capacity, walk_time_minutes, null, null, id, audience, + case audience + when 'BROTHERS' then 'BROTHERS' + when 'SISTERS' then 'SISTERS' + else 'MULTIFAITH' + end, + building, entrance_description, entrance_name, floor, location, null, maps_url, name, + starting_point, tag, null, null +from prayer_spaces; + +drop table prayer_spaces; + +alter table prayer_spaces_new rename to prayer_spaces; + + +-- 2. Rebuild user_permissions with BOARD_PRAYER_SPACE_WRITE in the CHECK. + +create table user_permissions_new ( + user_id blob not null, + permission varchar(255) not null check (permission in ( + 'MEDIA_UPLOAD_SELF','MEDIA_UPLOAD_MODERATE','MEDIA_UPLOAD_READ', + 'BOARD_CONTENT_READ','BOARD_POSTER_WRITE','BOARD_EVENT_WRITE','BOARD_SOCIAL_WRITE', + 'BOARD_WEEKLY_WRITE','BOARD_TICKER_WRITE','BOARD_PRAYER_SPACE_WRITE', + 'BOARD_CONFIG_READ','BOARD_CONFIG_WRITE','BOARD_REFRESH', + 'BOARD_DEVICE_READ','BOARD_DEVICE_ENROLL','BOARD_DEVICE_REVOKE','BOARD_TELEMETRY_SUBSCRIBE', + 'BOARD_COMMAND_BENIGN','BOARD_COMMAND_DISRUPTIVE','BOARD_COMMAND_INSPECT', + 'TCKET_SCAN','TCKET_MANAGE','TCKET_ADMIN', + 'IAM_USER_READ','IAM_USER_WRITE','IAM_ROLE_GRANT','AUDIT_READ')), + primary key (user_id, permission) +); + +insert into user_permissions_new (user_id, permission) + select user_id, permission from user_permissions; +drop table user_permissions; +alter table user_permissions_new rename to user_permissions; diff --git a/src/test/java/com/ibrasoft/lensbridge/controller/PrayerSpaceAdminControllerPermissionTest.java b/src/test/java/com/ibrasoft/lensbridge/controller/PrayerSpaceAdminControllerPermissionTest.java new file mode 100644 index 0000000..6cafe08 --- /dev/null +++ b/src/test/java/com/ibrasoft/lensbridge/controller/PrayerSpaceAdminControllerPermissionTest.java @@ -0,0 +1,220 @@ +package com.ibrasoft.lensbridge.controller; + +import com.ibrasoft.lensbridge.model.auth.Permission; +import com.ibrasoft.lensbridge.model.auth.Role; +import com.ibrasoft.lensbridge.service.AdminAuditService; +import com.ibrasoft.lensbridge.service.PrayerSpaceService; +import com.ibrasoft.lensbridge.service.UserService; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; +import org.springframework.context.annotation.Import; +import org.springframework.http.MediaType; +import org.springframework.security.test.context.support.WithAnonymousUser; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; + +import java.util.UUID; + +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +/** + * Security-only tests for {@link PrayerSpaceAdminController}. + *

+ * The split under test is reads from writes. Seeing where the musallahs are is not a + * privilege — the same data is served unauthenticated under {@code /api/minbar} — so reads + * sit on {@code board:content:read}, which every board role carries. Editing the walking + * directions somebody follows through a building at Maghrib is a different matter, and sits + * on its own {@code board:prayerspace:write}. A BOARD_VIEWER holding the read permission + * must therefore be able to list and not to write, which is the case a class-level + * annotation would have collapsed. + *

+ * CSRF is disabled in production, so no csrf() post-processor is needed. + */ +@WebMvcTest(controllers = PrayerSpaceAdminController.class) +@Import(MethodSecurityTestConfig.class) +class PrayerSpaceAdminControllerPermissionTest { + + private static final String BASE = "/api/admin/minbar/prayer-spaces"; + + @Autowired + private MockMvc mockMvc; + + @MockitoBean + private PrayerSpaceService prayerSpaceService; + @MockitoBean + private AdminAuditService auditService; + + /** + * Not used by this controller. WebConfig registers CurrentUserArgumentResolver, which + * needs a UserService, and the slice does not load the service layer. + */ + @MockitoBean + private UserService userService; + + private static String id() { + return UUID.randomUUID().toString(); + } + + private static MockHttpServletRequestBuilder createRequest() { + return post(BASE) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"name":"Brother's Musallah","tag":"Main brothers' space", + "building":"CCT","type":"brothers"} + """); + } + + private static MockHttpServletRequestBuilder updateRequest() { + return patch(BASE + "/" + id()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"capacity\":25}"); + } + + private void expectForbidden(MockHttpServletRequestBuilder request, Permission... held) throws Exception { + mockMvc.perform(request.with(TestAuthorities.asHolderOf(held))) + .andExpect(status().isForbidden()); + } + + /** + * Authorized means "not rejected by security" — the handler's own outcome is out of scope, + * and the service is a mock, so a 200 here proves nothing about behaviour and is not + * meant to. + */ + private void expectAllowed(MockHttpServletRequestBuilder request, Permission... held) throws Exception { + mockMvc.perform(request.with(TestAuthorities.asHolderOf(held))).andExpect(result -> { + int status = result.getResponse().getStatus(); + if (status == 401 || status == 403) { + throw new AssertionError("Expected the request to be authorized but got HTTP " + status); + } + }); + } + + private void expectAllowedAs(MockHttpServletRequestBuilder request, Role role) throws Exception { + mockMvc.perform(request.with(TestAuthorities.as(role))).andExpect(result -> { + int status = result.getResponse().getStatus(); + if (status == 401 || status == 403) { + throw new AssertionError("Expected " + role + " to be authorized but got HTTP " + status); + } + }); + } + + // ==================== Anonymous ==================== + + @Test + @WithAnonymousUser + void anonymousCannotReachTheAdminList() throws Exception { + mockMvc.perform(get(BASE)).andExpect(status().isUnauthorized()); + } + + @Test + @WithAnonymousUser + void anonymousCannotCreateASpace() throws Exception { + mockMvc.perform(createRequest()).andExpect(status().isUnauthorized()); + } + + // ==================== Reads ==================== + + @Test + void contentReadCanListEverySpace() throws Exception { + expectAllowed(get(BASE), Permission.BOARD_CONTENT_READ); + } + + @Test + void contentReadCanFetchOneSpace() throws Exception { + expectAllowed(get(BASE + "/" + id()), Permission.BOARD_CONTENT_READ); + } + + /** The write permission is not a superset of the read one; they are granted separately. */ + @Test + void prayerSpaceWriteAloneCannotList() throws Exception { + expectForbidden(get(BASE), Permission.BOARD_PRAYER_SPACE_WRITE); + } + + @Test + void anAdjacentBoardPermissionDoesNotConferReads() throws Exception { + expectForbidden(get(BASE), Permission.BOARD_POSTER_WRITE); + } + + // ==================== Writes ==================== + + @Test + void prayerSpaceWriteCanCreate() throws Exception { + expectAllowed(createRequest(), Permission.BOARD_PRAYER_SPACE_WRITE); + } + + @Test + void prayerSpaceWriteCanUpdate() throws Exception { + expectAllowed(updateRequest(), Permission.BOARD_PRAYER_SPACE_WRITE); + } + + @Test + void prayerSpaceWriteCanDelete() throws Exception { + expectAllowed(delete(BASE + "/" + id()), Permission.BOARD_PRAYER_SPACE_WRITE); + } + + /** + * The regression this file exists for. Reading the prayer spaces is deliberately cheap to + * grant; rewriting somebody's route to the musallah is not. + */ + @Test + void contentReadAloneCannotCreate() throws Exception { + expectForbidden(createRequest(), Permission.BOARD_CONTENT_READ); + } + + @Test + void contentReadAloneCannotUpdate() throws Exception { + expectForbidden(updateRequest(), Permission.BOARD_CONTENT_READ); + } + + @Test + void contentReadAloneCannotDelete() throws Exception { + expectForbidden(delete(BASE + "/" + id()), Permission.BOARD_CONTENT_READ); + } + + /** Board content and prayer spaces are separate grants, in both directions. */ + @Test + void aPosterEditorCannotRewriteDirections() throws Exception { + expectForbidden(updateRequest(), Permission.BOARD_POSTER_WRITE, Permission.BOARD_CONTENT_READ); + } + + // ==================== Role bundles ==================== + + @Test + void boardViewerCanReadButNotWrite() throws Exception { + expectAllowedAs(get(BASE), Role.BOARD_VIEWER); + mockMvc.perform(createRequest().with(TestAuthorities.as(Role.BOARD_VIEWER))) + .andExpect(status().isForbidden()); + } + + @Test + void boardEditorCanWrite() throws Exception { + expectAllowedAs(createRequest(), Role.BOARD_EDITOR); + expectAllowedAs(updateRequest(), Role.BOARD_EDITOR); + expectAllowedAs(delete(BASE + "/" + id()), Role.BOARD_EDITOR); + } + + @Test + void boardAdminCanWrite() throws Exception { + expectAllowedAs(createRequest(), Role.BOARD_ADMIN); + } + + @Test + void rootCanWrite() throws Exception { + expectAllowedAs(createRequest(), Role.ROOT); + } + + /** Owning ticketing is not owning the map. */ + @Test + void aTicketingAdminHasNoReachHere() throws Exception { + mockMvc.perform(get(BASE).with(TestAuthorities.as(Role.TCKET_ADMIN))) + .andExpect(status().isForbidden()); + mockMvc.perform(createRequest().with(TestAuthorities.as(Role.TCKET_ADMIN))) + .andExpect(status().isForbidden()); + } +} diff --git a/src/test/java/com/ibrasoft/lensbridge/service/PrayerSpaceServiceTest.java b/src/test/java/com/ibrasoft/lensbridge/service/PrayerSpaceServiceTest.java new file mode 100644 index 0000000..e1bfa96 --- /dev/null +++ b/src/test/java/com/ibrasoft/lensbridge/service/PrayerSpaceServiceTest.java @@ -0,0 +1,360 @@ +package com.ibrasoft.lensbridge.service; + +import com.ibrasoft.lensbridge.dto.minbar.request.CreatePrayerSpaceRequest; +import com.ibrasoft.lensbridge.dto.minbar.request.DirectionStepRequest; +import com.ibrasoft.lensbridge.dto.minbar.request.UpdatePrayerSpaceRequest; +import com.ibrasoft.lensbridge.dto.minbar.response.PrayerSpaceView; +import com.ibrasoft.lensbridge.dto.upload.response.ErrorResponse; +import com.ibrasoft.lensbridge.exception.ApiResponseException; +import com.ibrasoft.lensbridge.model.minbar.Audience; +import com.ibrasoft.lensbridge.model.minbar.DirectionStep; +import com.ibrasoft.lensbridge.model.minbar.PrayerSpace; +import com.ibrasoft.lensbridge.model.minbar.PrayerSpaceType; +import com.ibrasoft.lensbridge.repository.sql.PrayerSpaceRepository; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.http.HttpStatus; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class PrayerSpaceServiceTest { + + @Mock + private PrayerSpaceRepository prayerSpaceRepository; + + @InjectMocks + private PrayerSpaceService service; + + private CreatePrayerSpaceRequest.CreatePrayerSpaceRequestBuilder validCreate() { + return CreatePrayerSpaceRequest.builder() + .name("Brother's Musallah") + .tag("Main brothers' space") + .building("Communication, Culture & Technology (CCT)") + .type(PrayerSpaceType.BROTHERS); + } + + private PrayerSpace existing() { + PrayerSpace space = PrayerSpace.builder() + .id(UUID.randomUUID()) + .name("Deerfield Hall Reflection Bay") + .tag("Quiet bay") + .building("Deerfield Hall") + .floor("3rd Floor") + .roomInfo("Between offices 3026 & 3028") + .spaceType(PrayerSpaceType.REFLECTION) + .audience(Audience.BOTH) + .amenities(new ArrayList<>(List.of("Quiet space", "Seating"))) + .tips(new ArrayList<>(List.of("Bring your own prayer mat"))) + .steps(new ArrayList<>()) + .build(); + space.replaceSteps(List.of( + DirectionStep.builder().instruction("Take the elevator to the 3rd floor").build(), + DirectionStep.builder().instruction("Turn right").subtext("Past office 3024").build())); + return space; + } + + /** + * The message a caller actually sees. ApiResponseException's two-argument constructor + * passes null to super(), so the text lives in the body, not in getMessage(). + */ + private static String errorOf(Throwable e) { + return ((ErrorResponse) ((ApiResponseException) e).getBody()).getError(); + } + + /** Returns whatever was handed to save(), so assertions read the entity the service built. */ + private void echoSave() { + when(prayerSpaceRepository.save(any(PrayerSpace.class))) + .thenAnswer(invocation -> invocation.getArgument(0)); + } + + // ==================== Audience derivation ==================== + + @Test + void createDerivesAudienceFromTypeWhenNotGiven() { + echoSave(); + + PrayerSpaceView created = service.create(validCreate().build()); + + assertThat(created.getAudience()).isEqualTo(Audience.BROTHERS); + } + + @Test + void createDerivesBothForSpacesOpenToEveryone() { + echoSave(); + + assertThat(service.create(validCreate().type(PrayerSpaceType.MULTIFAITH).build()).getAudience()) + .isEqualTo(Audience.BOTH); + assertThat(service.create(validCreate().type(PrayerSpaceType.REFLECTION).build()).getAudience()) + .isEqualTo(Audience.BOTH); + } + + @Test + void createHonoursAnExplicitAudienceOverTheTypeDefault() { + echoSave(); + + PrayerSpaceView created = service.create( + validCreate().type(PrayerSpaceType.REFLECTION).audience(Audience.SISTERS).build()); + + assertThat(created.getType()).isEqualTo(PrayerSpaceType.REFLECTION); + assertThat(created.getAudience()).isEqualTo(Audience.SISTERS); + } + + /** + * Changing the type must not silently re-list the space. A reflection bay a building has + * designated sisters-only would otherwise be published to the whole campus by an edit that + * only meant to correct its label. + */ + @Test + void updateDoesNotReDeriveAudienceFromANewType() { + PrayerSpace space = existing(); + space.setAudience(Audience.SISTERS); + when(prayerSpaceRepository.findById(space.getId())).thenReturn(Optional.of(space)); + echoSave(); + + PrayerSpaceView updated = service.update(space.getId(), + UpdatePrayerSpaceRequest.builder().type(PrayerSpaceType.MULTIFAITH).build()); + + assertThat(updated.getType()).isEqualTo(PrayerSpaceType.MULTIFAITH); + assertThat(updated.getAudience()).isEqualTo(Audience.SISTERS); + } + + // ==================== Coordinates ==================== + + @Test + void createRejectsOneCoordinateWithoutTheOther() { + assertThatThrownBy(() -> service.create(validCreate().latitude(43.5489).build())) + .isInstanceOf(ApiResponseException.class) + .satisfies(e -> assertThat(errorOf(e)).contains("together")); + + verify(prayerSpaceRepository, never()).save(any()); + } + + @Test + void createAcceptsAPairOfCoordinates() { + echoSave(); + + PrayerSpaceView created = service.create( + validCreate().latitude(43.5489).longitude(-79.6632).build()); + + assertThat(created.getLatitude()).isEqualTo(43.5489); + assertThat(created.getLongitude()).isEqualTo(-79.6632); + } + + @Test + void createAcceptsNoCoordinatesAtAll() { + echoSave(); + + PrayerSpaceView created = service.create(validCreate().build()); + + assertThat(created.getLatitude()).isNull(); + assertThat(created.getLongitude()).isNull(); + } + + /** The pair is validated against the merged state, not the patch, so half a pin cannot slip in. */ + @Test + void updateRejectsALatitudeThatWouldLeaveTheSpaceHalfPinned() { + PrayerSpace space = existing(); + when(prayerSpaceRepository.findById(space.getId())).thenReturn(Optional.of(space)); + + assertThatThrownBy(() -> service.update(space.getId(), + UpdatePrayerSpaceRequest.builder().latitude(43.55).build())) + .isInstanceOf(ApiResponseException.class) + .satisfies(e -> assertThat(errorOf(e)).contains("together")); + + verify(prayerSpaceRepository, never()).save(any()); + } + + @Test + void updateAcceptsALatitudeWhenTheSpaceAlreadyHasALongitude() { + PrayerSpace space = existing(); + space.setLatitude(43.5000); + space.setLongitude(-79.6620); + when(prayerSpaceRepository.findById(space.getId())).thenReturn(Optional.of(space)); + echoSave(); + + PrayerSpaceView updated = service.update(space.getId(), + UpdatePrayerSpaceRequest.builder().latitude(43.5501).build()); + + assertThat(updated.getLatitude()).isEqualTo(43.5501); + assertThat(updated.getLongitude()).isEqualTo(-79.6620); + } + + // ==================== Patch semantics ==================== + + @Test + void updateLeavesOmittedFieldsAlone() { + PrayerSpace space = existing(); + when(prayerSpaceRepository.findById(space.getId())).thenReturn(Optional.of(space)); + echoSave(); + + PrayerSpaceView updated = service.update(space.getId(), + UpdatePrayerSpaceRequest.builder().capacity(6).build()); + + assertThat(updated.getCapacity()).isEqualTo(6); + assertThat(updated.getName()).isEqualTo("Deerfield Hall Reflection Bay"); + assertThat(updated.getFloor()).isEqualTo("3rd Floor"); + assertThat(updated.getAmenities()).containsExactly("Quiet space", "Seating"); + assertThat(updated.getSteps()).hasSize(2); + } + + @Test + void updateClearsAnOptionalTextFieldSentAsEmpty() { + PrayerSpace space = existing(); + when(prayerSpaceRepository.findById(space.getId())).thenReturn(Optional.of(space)); + echoSave(); + + PrayerSpaceView updated = service.update(space.getId(), + UpdatePrayerSpaceRequest.builder().roomInfo("").build()); + + assertThat(updated.getRoomInfo()).isNull(); + } + + @Test + void updateReplacesListsWholesaleAndAnEmptyListClearsThem() { + PrayerSpace space = existing(); + when(prayerSpaceRepository.findById(space.getId())).thenReturn(Optional.of(space)); + echoSave(); + + PrayerSpaceView updated = service.update(space.getId(), + UpdatePrayerSpaceRequest.builder() + .amenities(List.of("Prayer mats")) + .tips(List.of()) + .build()); + + assertThat(updated.getAmenities()).containsExactly("Prayer mats"); + assertThat(updated.getTips()).isEmpty(); + } + + /** + * The amenity list is an {@code @ElementCollection} Hibernate is tracking. Handing it a + * fresh ArrayList orphans the instance it knows about, so the service mutates in place — + * this asserts it actually does. + */ + @Test + void updateMutatesTheTrackedCollectionRatherThanReplacingIt() { + PrayerSpace space = existing(); + List tracked = space.getAmenities(); + when(prayerSpaceRepository.findById(space.getId())).thenReturn(Optional.of(space)); + echoSave(); + + service.update(space.getId(), + UpdatePrayerSpaceRequest.builder().amenities(List.of("Wudu area nearby")).build()); + + assertThat(space.getAmenities()).isSameAs(tracked); + assertThat(tracked).containsExactly("Wudu area nearby"); + } + + @Test + void updateRenumbersReplacementStepsFromZero() { + PrayerSpace space = existing(); + when(prayerSpaceRepository.findById(space.getId())).thenReturn(Optional.of(space)); + echoSave(); + + PrayerSpaceView updated = service.update(space.getId(), + UpdatePrayerSpaceRequest.builder() + .steps(List.of( + DirectionStepRequest.builder().instruction("Enter through the north doors").build(), + DirectionStepRequest.builder() + .instruction("Take the stairs") + .subtext("Not the elevator") + .build(), + DirectionStepRequest.builder().instruction("Turn left").build())) + .build()); + + assertThat(updated.getSteps()).extracting(PrayerSpaceView.DirectionStepView::getOrder) + .containsExactly(0, 1, 2); + assertThat(updated.getSteps()).extracting(PrayerSpaceView.DirectionStepView::getInstruction) + .containsExactly("Enter through the north doors", "Take the stairs", "Turn left"); + assertThat(updated.getSteps().get(1).getSubtext()).isEqualTo("Not the elevator"); + } + + @Test + void updateKeepsTheTrackedStepCollectionInstance() { + PrayerSpace space = existing(); + List tracked = space.getSteps(); + when(prayerSpaceRepository.findById(space.getId())).thenReturn(Optional.of(space)); + echoSave(); + + service.update(space.getId(), + UpdatePrayerSpaceRequest.builder() + .steps(List.of(DirectionStepRequest.builder().instruction("Walk in").build())) + .build()); + + assertThat(space.getSteps()).isSameAs(tracked); + assertThat(tracked).hasSize(1); + assertThat(tracked.get(0).getPrayerSpace()).isSameAs(space); + } + + @Test + void createTrimsWhitespaceAndDropsBlankOptionalText() { + echoSave(); + + PrayerSpaceView created = service.create(validCreate() + .name(" Brother's Musallah ") + .roomInfo(" ") + .amenities(List.of(" Prayer mats ")) + .steps(List.of(DirectionStepRequest.builder() + .instruction(" Enter through the glass doors ") + .subtext(" ") + .build())) + .build()); + + assertThat(created.getName()).isEqualTo("Brother's Musallah"); + assertThat(created.getRoomInfo()).isNull(); + assertThat(created.getAmenities()).containsExactly("Prayer mats"); + assertThat(created.getSteps().get(0).getInstruction()).isEqualTo("Enter through the glass doors"); + assertThat(created.getSteps().get(0).getSubtext()).isNull(); + } + + // ==================== Lookups ==================== + + @Test + void readingAMissingSpaceIsA404NotAnEmptyBody() { + UUID id = UUID.randomUUID(); + when(prayerSpaceRepository.findById(id)).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> service.getSpace(id)) + .isInstanceOf(ApiResponseException.class) + .satisfies(e -> assertThat(((ApiResponseException) e).getStatus()).isEqualTo(HttpStatus.NOT_FOUND)); + } + + @Test + void deletingAMissingSpaceIsA404() { + UUID id = UUID.randomUUID(); + when(prayerSpaceRepository.findById(id)).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> service.delete(id)).isInstanceOf(ApiResponseException.class); + verify(prayerSpaceRepository, never()).delete(any()); + } + + /** + * The app's list used to omit the walking directions, because the endpoint returned a lazy + * entity. Assembling the view in the service is what fixed that, so the list carrying steps + * is worth pinning down. + */ + @Test + void theAudienceListCarriesFullDirections() { + PrayerSpace space = existing(); + when(prayerSpaceRepository.findByAudienceOrBoth(Audience.SISTERS)).thenReturn(List.of(space)); + + List views = service.getSpacesForAudience(Audience.SISTERS); + + assertThat(views).hasSize(1); + assertThat(views.get(0).getSteps()).hasSize(2); + assertThat(views.get(0).getSteps().get(1).getSubtext()).isEqualTo("Past office 3024"); + } +} From b64baf32ccb11b67ac962f655e0e1d1ed32aa5d4 Mon Sep 17 00:00:00 2001 From: Ibrahim Chehab Date: Fri, 28 Aug 2026 12:15:43 -0400 Subject: [PATCH 2/2] Prayer spaces: clear a map pin, and actually audit the writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things found by exercising the endpoints against a real database rather than a mock. **A pin could be set but never removed.** For the optional text fields, sending `""` clears them; there was no equivalent for a number, because null already means "leave unchanged". So a space pinned to the wrong building could be corrected and never un-pinned, and the only way back was to delete it and type it in again. `clearCoordinates: true` on the patch request nulls both halves. Sending it alongside a latitude or longitude is a 400 — that asks for two contradictory things in one call. **None of the prayer-space writes were being audited.** V8 added the three `*_PRAYER_SPACE` actions to `AuditAction` and wired them into the controller, but did not widen `audit_events`' CHECK constraint, and `ddl-auto=validate` does not inspect CHECK constraints. Every audit insert failed, and `AdminAuditService` catches and logs rather than rethrowing — correctly, since a failed audit write must not roll back the operation it describes. The result was the worst of both: the edits went through and none of them were recorded. Only visible by reading the table. `LINK_TICKET_EVENT` and `UNLINK_TICKET_EVENT` turn out to have had the same bug since V5. They are fixed here too. V9 widens both CHECK constraints in both dialects and adds a `PRAYER_SPACE` entity type, so these stop being filed under `EVENT` alongside calendar entries. `MigrationEnumParityTest` is the part that matters: 178 cases asserting every constant of every `@Enumerated(STRING)` enum is named somewhere in each dialect's migrations. It is deliberately crude — it cannot prove a value landed in the right constraint — but whole-cloth omission is the mistake that keeps happening, and it is now a build failure with a message that says what to do. Verified it fails on the real defect before V9 fixes it. 794 tests pass. Co-Authored-By: Claude Opus 5 --- openapi.yaml | 3 + .../request/UpdatePrayerSpaceRequest.java | 11 ++ .../model/audit/AuditEntityType.java | 2 + .../lensbridge/service/AdminAuditService.java | 1 + .../service/PrayerSpaceService.java | 9 ++ .../V9__audit_prayer_space_actions.sql | 56 ++++++++++ .../sqlite/V9__audit_prayer_space_actions.sql | 58 ++++++++++ .../migration/MigrationEnumParityTest.java | 101 ++++++++++++++++++ .../service/PrayerSpaceServiceTest.java | 47 ++++++++ 9 files changed, 288 insertions(+) create mode 100644 src/main/resources/db/migration/postgresql/V9__audit_prayer_space_actions.sql create mode 100644 src/main/resources/db/migration/sqlite/V9__audit_prayer_space_actions.sql create mode 100644 src/test/java/com/ibrasoft/lensbridge/migration/MigrationEnumParityTest.java diff --git a/openapi.yaml b/openapi.yaml index d0f1bde..b28d4db 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -4551,6 +4551,7 @@ components: - UPLOAD - MUSALLAH_BOARD - DEVICE + - PRAYER_SPACE - EVENT timestamp: type: string @@ -6641,6 +6642,8 @@ components: type: integer format: int32 minimum: 0 + clearCoordinates: + type: boolean directions: type: string entranceDescription: diff --git a/src/main/java/com/ibrasoft/lensbridge/dto/minbar/request/UpdatePrayerSpaceRequest.java b/src/main/java/com/ibrasoft/lensbridge/dto/minbar/request/UpdatePrayerSpaceRequest.java index c669023..13a95d9 100644 --- a/src/main/java/com/ibrasoft/lensbridge/dto/minbar/request/UpdatePrayerSpaceRequest.java +++ b/src/main/java/com/ibrasoft/lensbridge/dto/minbar/request/UpdatePrayerSpaceRequest.java @@ -57,6 +57,17 @@ public class UpdatePrayerSpaceRequest { @DecimalMax(value = "180.0", message = "Longitude must be between -180 and 180") private Double longitude; + /** + * Removes the map pin entirely. + *

+ * The empty-string trick that clears a text field has no equivalent for a number, and + * null already means "unchanged" -- so without this a space that was pinned to the wrong + * building could be corrected but never un-pinned, and the only way back would be to + * delete it and type it in again. Ignored unless true, and rejected alongside a latitude + * or longitude in the same request, which asks for both at once. + */ + private Boolean clearCoordinates; + @URL(message = "Maps URL must be a valid URL") private String mapsUrl; diff --git a/src/main/java/com/ibrasoft/lensbridge/model/audit/AuditEntityType.java b/src/main/java/com/ibrasoft/lensbridge/model/audit/AuditEntityType.java index 1090874..a065a92 100644 --- a/src/main/java/com/ibrasoft/lensbridge/model/audit/AuditEntityType.java +++ b/src/main/java/com/ibrasoft/lensbridge/model/audit/AuditEntityType.java @@ -5,6 +5,8 @@ public enum AuditEntityType { UPLOAD("Upload"), MUSALLAH_BOARD("Musallah Board"), DEVICE("Device"), + /** A prayer space and its walking directions. */ + PRAYER_SPACE("Prayer Space"), EVENT("Event"); diff --git a/src/main/java/com/ibrasoft/lensbridge/service/AdminAuditService.java b/src/main/java/com/ibrasoft/lensbridge/service/AdminAuditService.java index a76ef8a..97169fa 100644 --- a/src/main/java/com/ibrasoft/lensbridge/service/AdminAuditService.java +++ b/src/main/java/com/ibrasoft/lensbridge/service/AdminAuditService.java @@ -120,6 +120,7 @@ private AuditEntityType toEntityType(String entityType) { case "poster", "calendarevent", "boardevent", "musallah board", "boardconfig", "weeklycontent" -> AuditEntityType.MUSALLAH_BOARD; case "device" -> AuditEntityType.DEVICE; + case "prayerspace" -> AuditEntityType.PRAYER_SPACE; default -> AuditEntityType.EVENT; }; } diff --git a/src/main/java/com/ibrasoft/lensbridge/service/PrayerSpaceService.java b/src/main/java/com/ibrasoft/lensbridge/service/PrayerSpaceService.java index 77b74ea..1153d23 100644 --- a/src/main/java/com/ibrasoft/lensbridge/service/PrayerSpaceService.java +++ b/src/main/java/com/ibrasoft/lensbridge/service/PrayerSpaceService.java @@ -124,6 +124,15 @@ public PrayerSpaceView update(UUID id, UpdatePrayerSpaceRequest request) { clearable(request.getEntranceName(), space::setEntranceName); clearable(request.getEntranceDescription(), space::setEntranceDescription); + if (Boolean.TRUE.equals(request.getClearCoordinates())) { + if (request.getLatitude() != null || request.getLongitude() != null) { + throw new ApiResponseException( + HttpStatus.BAD_REQUEST, + ErrorResponse.of("Cannot clear and set coordinates in the same request")); + } + space.setLatitude(null); + space.setLongitude(null); + } Patch.apply(request.getLatitude(), space::setLatitude); Patch.apply(request.getLongitude(), space::setLongitude); validateCoordinates(space.getLatitude(), space.getLongitude()); diff --git a/src/main/resources/db/migration/postgresql/V9__audit_prayer_space_actions.sql b/src/main/resources/db/migration/postgresql/V9__audit_prayer_space_actions.sql new file mode 100644 index 0000000..cb8942b --- /dev/null +++ b/src/main/resources/db/migration/postgresql/V9__audit_prayer_space_actions.sql @@ -0,0 +1,56 @@ +-- +-- V9: let the prayer-space actions actually be audited. +-- +-- V8 added CREATE_PRAYER_SPACE / UPDATE_PRAYER_SPACE / DELETE_PRAYER_SPACE to AuditAction and +-- wired them into the admin controller, but did not widen audit_events' CHECK constraints. The +-- inserts therefore failed the constraint, and AdminAuditService catches and logs rather than +-- rethrows -- by design, since a failed audit write must not roll back the operation the user +-- asked for. The result was the worst version of both: the writes succeeded and none of them +-- were recorded. Caught by inspecting the table after exercising the endpoints; nothing in the +-- API surface showed it. +-- +-- target_entity_type gains PRAYER_SPACE for the same reason. Without it AdminAuditService's +-- toEntityType() falls through to EVENT, which would have filed prayer-space edits under the +-- same heading as calendar events even once the action constraint allowed the row. +-- +-- MigrationEnumParityTest now fails the build when an enum constant is not named anywhere in +-- either migration directory, so the next value to be added cannot repeat this silently. +-- +-- The SQLite twin of this file must stay at the same version -- see docs/MIGRATIONS.md. + +do $$ +declare + constraint_name text; +begin + for constraint_name in + select con.conname + from pg_constraint con + join pg_class rel on rel.oid = con.conrelid + where rel.relname = 'audit_events' + and con.contype = 'c' + loop + execute format('alter table audit_events drop constraint %I', constraint_name); + end loop; +end $$; + +alter table audit_events + add constraint audit_events_action_check check (action in ( + 'APPROVE_UPLOAD','UNAPPROVE_UPLOAD','DELETE_UPLOAD','FEATURE_UPLOAD','UNFEATURE_UPLOAD', + 'CREATE_EVENT','UPDATE_EVENT','DELETE_EVENT', + 'CREATE_CALENDAR_EVENT','UPDATE_CALENDAR_EVENT','DELETE_CALENDAR_EVENT', + 'LINK_TICKET_EVENT','UNLINK_TICKET_EVENT', + 'CREATE_PRAYER_SPACE','UPDATE_PRAYER_SPACE','DELETE_PRAYER_SPACE', + 'CREATE_POSTER','UPDATE_POSTER','DELETE_POSTER', + 'CREATE_SOCIAL','UPDATE_SOCIAL','DELETE_SOCIAL', + 'SAVE_WEEKLY_CONTENT','DELETE_WEEKLY_CONTENT', + 'UPDATE_BOARD_CONFIG','UPDATE_BOARD_TICKER','REFRESH_BOARDS', + 'ISSUE_ENROLLMENT_TOKEN','REVOKE_DEVICE','ISSUE_DEVICE_COMMAND', + 'PROMOTE_USER','DEMOTE_USER','DISABLE_USER','ENABLE_USER', + 'VIEW_AUDIT_LOGS','EXPORT_DATA','SYSTEM_MAINTENANCE', + 'VERIFY_USER','UNVERIFY_USER','RESET_USER_PASSWORD','TRIGGER_PASSWORD_RESET_EMAIL', + 'ADD_USER_ROLE','REMOVE_USER_ROLE','GRANT_PERMISSION','REVOKE_PERMISSION', + 'ADD_USER','REMOVE_USER','UPDATE_USER')); + +alter table audit_events + add constraint audit_events_target_entity_type_check check (target_entity_type in ( + 'USER','UPLOAD','MUSALLAH_BOARD','DEVICE','PRAYER_SPACE','EVENT')); diff --git a/src/main/resources/db/migration/sqlite/V9__audit_prayer_space_actions.sql b/src/main/resources/db/migration/sqlite/V9__audit_prayer_space_actions.sql new file mode 100644 index 0000000..90b0fdd --- /dev/null +++ b/src/main/resources/db/migration/sqlite/V9__audit_prayer_space_actions.sql @@ -0,0 +1,58 @@ +-- +-- V9: let the prayer-space actions actually be audited. +-- +-- See the Postgres twin for what went wrong. Short version: V8 added the three +-- *_PRAYER_SPACE actions to AuditAction but not to audit_events' CHECK, so every audit insert +-- failed the constraint and AdminAuditService swallowed it -- deliberately, because a failed +-- audit write must not roll back the operation being audited. The edits went through and none +-- of them were recorded. +-- +-- LINK_TICKET_EVENT and UNLINK_TICKET_EVENT are added here too. They were introduced alongside +-- V5's tCketManage link and have been failing the same constraint ever since, unnoticed for the +-- same reason. +-- +-- SQLite cannot alter a CHECK constraint, so the table is rebuilt -- the same pattern as V4, V6, +-- V7 and V8. audit_events has no secondary indexes to recreate (the only entry in sqlite_master +-- is the implicit primary-key autoindex), and the SQLite twin of V1 declares no foreign key on +-- admin_id, so nothing else moves with it. +-- +-- The Postgres twin of this file must stay at the same version -- see docs/MIGRATIONS.md. + +create table audit_events_new ( + timestamp timestamp not null, + admin_id blob not null, + id blob not null, + target_entity_id blob, + details varchar(512), + action varchar(255) not null check (action in ( + 'APPROVE_UPLOAD','UNAPPROVE_UPLOAD','DELETE_UPLOAD','FEATURE_UPLOAD','UNFEATURE_UPLOAD', + 'CREATE_EVENT','UPDATE_EVENT','DELETE_EVENT', + 'CREATE_CALENDAR_EVENT','UPDATE_CALENDAR_EVENT','DELETE_CALENDAR_EVENT', + 'LINK_TICKET_EVENT','UNLINK_TICKET_EVENT', + 'CREATE_PRAYER_SPACE','UPDATE_PRAYER_SPACE','DELETE_PRAYER_SPACE', + 'CREATE_POSTER','UPDATE_POSTER','DELETE_POSTER', + 'CREATE_SOCIAL','UPDATE_SOCIAL','DELETE_SOCIAL', + 'SAVE_WEEKLY_CONTENT','DELETE_WEEKLY_CONTENT', + 'UPDATE_BOARD_CONFIG','UPDATE_BOARD_TICKER','REFRESH_BOARDS', + 'ISSUE_ENROLLMENT_TOKEN','REVOKE_DEVICE','ISSUE_DEVICE_COMMAND', + 'PROMOTE_USER','DEMOTE_USER','DISABLE_USER','ENABLE_USER', + 'VIEW_AUDIT_LOGS','EXPORT_DATA','SYSTEM_MAINTENANCE', + 'VERIFY_USER','UNVERIFY_USER','RESET_USER_PASSWORD','TRIGGER_PASSWORD_RESET_EMAIL', + 'ADD_USER_ROLE','REMOVE_USER_ROLE','GRANT_PERMISSION','REVOKE_PERMISSION', + 'ADD_USER','REMOVE_USER','UPDATE_USER')), + ip_address varchar(255), + target_entity_type varchar(255) check (target_entity_type in ( + 'USER','UPLOAD','MUSALLAH_BOARD','DEVICE','PRAYER_SPACE','EVENT')), + user_agent varchar(255), + primary key (id) +); + +insert into audit_events_new (timestamp, admin_id, id, target_entity_id, details, action, + ip_address, target_entity_type, user_agent) +select timestamp, admin_id, id, target_entity_id, details, action, + ip_address, target_entity_type, user_agent +from audit_events; + +drop table audit_events; + +alter table audit_events_new rename to audit_events; diff --git a/src/test/java/com/ibrasoft/lensbridge/migration/MigrationEnumParityTest.java b/src/test/java/com/ibrasoft/lensbridge/migration/MigrationEnumParityTest.java new file mode 100644 index 0000000..951bcd4 --- /dev/null +++ b/src/test/java/com/ibrasoft/lensbridge/migration/MigrationEnumParityTest.java @@ -0,0 +1,101 @@ +package com.ibrasoft.lensbridge.migration; + +import com.ibrasoft.lensbridge.model.audit.AuditAction; +import com.ibrasoft.lensbridge.model.audit.AuditEntityType; +import com.ibrasoft.lensbridge.model.auth.Permission; +import com.ibrasoft.lensbridge.model.auth.Role; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import java.io.IOException; +import java.net.URISyntaxException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Every {@code @Enumerated(STRING)} column in this schema is constrained by a {@code CHECK} + * listing its legal values, and {@code ddl-auto=validate} does not inspect CHECK constraints. + * So adding a constant to one of these enums and forgetting the migration compiles, starts, + * passes every other test — and then fails at INSERT time, in whichever environment first + * tries to write the new value. + *

+ * That is not hypothetical. {@code LINK_TICKET_EVENT} and {@code UNLINK_TICKET_EVENT} were + * added with V5 and never reached a CHECK constraint; every attempt to audit a ticket link + * failed silently for as long as the feature has existed, because + * {@code AdminAuditService.logAuditEvent} catches and logs rather than rethrowing — correctly, + * since a failed audit write must not roll back the operation it describes. The three + * {@code *_PRAYER_SPACE} actions repeated the mistake one migration later. Both were found by + * reading the table, not by anything failing. + *

+ * The check here is deliberately crude: it asserts the constant's name appears somewhere + * in the dialect's migration text. It cannot tell you the value landed in the right constraint, + * or that a later migration did not narrow it back out — that is review's job. What it catches + * is the whole-cloth omission, which is the mistake that actually keeps happening. + */ +class MigrationEnumParityTest { + + private static final String SQLITE = "db/migration/sqlite"; + private static final String POSTGRESQL = "db/migration/postgresql"; + + /** One case: a constant that must appear in one dialect's migrations. */ + record Case(String dialect, String enumName, String constant, String sql) { + @Override + public String toString() { + return enumName + "." + constant + " in " + dialect; + } + } + + private static String migrationTextOf(String location) throws IOException, URISyntaxException { + Path dir = Paths.get(Objects.requireNonNull( + MigrationEnumParityTest.class.getClassLoader().getResource(location), + "Missing migration directory: " + location).toURI()); + StringBuilder combined = new StringBuilder(); + try (Stream files = Files.list(dir)) { + for (Path file : files.filter(p -> p.getFileName().toString().endsWith(".sql")).toList()) { + combined.append(Files.readString(file)).append('\n'); + } + } + return combined.toString(); + } + + static Stream constrainedConstants() throws Exception { + List cases = new ArrayList<>(); + for (String dialect : List.of(SQLITE, POSTGRESQL)) { + String sql = migrationTextOf(dialect); + for (AuditAction action : AuditAction.values()) { + cases.add(new Case(dialect, "AuditAction", action.name(), sql)); + } + for (AuditEntityType type : AuditEntityType.values()) { + cases.add(new Case(dialect, "AuditEntityType", type.name(), sql)); + } + for (Permission permission : Permission.values()) { + cases.add(new Case(dialect, "Permission", permission.name(), sql)); + } + for (Role role : Role.values()) { + cases.add(new Case(dialect, "Role", role.name(), sql)); + } + } + return cases.stream(); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("constrainedConstants") + @DisplayName("every enum constant stored as a string is named in the migrations") + void everyConstantAppearsInTheMigrations(Case testCase) { + assertThat(testCase.sql()) + .as("%s.%s is persisted as a string into a CHECK-constrained column, but no " + + "migration under %s mentions it. Writing that value will fail the " + + "constraint at runtime — and for audit writes it fails silently. " + + "Add it to a migration in BOTH dialect directories.", + testCase.enumName(), testCase.constant(), testCase.dialect()) + .contains("'" + testCase.constant() + "'"); + } +} diff --git a/src/test/java/com/ibrasoft/lensbridge/service/PrayerSpaceServiceTest.java b/src/test/java/com/ibrasoft/lensbridge/service/PrayerSpaceServiceTest.java index e1bfa96..8ba2d34 100644 --- a/src/test/java/com/ibrasoft/lensbridge/service/PrayerSpaceServiceTest.java +++ b/src/test/java/com/ibrasoft/lensbridge/service/PrayerSpaceServiceTest.java @@ -193,6 +193,53 @@ void updateAcceptsALatitudeWhenTheSpaceAlreadyHasALongitude() { assertThat(updated.getLongitude()).isEqualTo(-79.6620); } + @Test + void updateCanClearThePinEntirely() { + PrayerSpace space = existing(); + space.setLatitude(43.5000); + space.setLongitude(-79.6620); + when(prayerSpaceRepository.findById(space.getId())).thenReturn(Optional.of(space)); + echoSave(); + + PrayerSpaceView updated = service.update(space.getId(), + UpdatePrayerSpaceRequest.builder().clearCoordinates(true).build()); + + assertThat(updated.getLatitude()).isNull(); + assertThat(updated.getLongitude()).isNull(); + } + + @Test + void updateRejectsClearingAndSettingCoordinatesAtOnce() { + PrayerSpace space = existing(); + when(prayerSpaceRepository.findById(space.getId())).thenReturn(Optional.of(space)); + + assertThatThrownBy(() -> service.update(space.getId(), UpdatePrayerSpaceRequest.builder() + .clearCoordinates(true) + .latitude(43.55) + .longitude(-79.66) + .build())) + .isInstanceOf(ApiResponseException.class) + .satisfies(e -> assertThat(errorOf(e)).contains("same request")); + + verify(prayerSpaceRepository, never()).save(any()); + } + + /** false is not a request to do anything, so an existing pin survives it. */ + @Test + void updateLeavesThePinAloneWhenClearCoordinatesIsFalse() { + PrayerSpace space = existing(); + space.setLatitude(43.5000); + space.setLongitude(-79.6620); + when(prayerSpaceRepository.findById(space.getId())).thenReturn(Optional.of(space)); + echoSave(); + + PrayerSpaceView updated = service.update(space.getId(), + UpdatePrayerSpaceRequest.builder().clearCoordinates(false).build()); + + assertThat(updated.getLatitude()).isEqualTo(43.5000); + assertThat(updated.getLongitude()).isEqualTo(-79.6620); + } + // ==================== Patch semantics ==================== @Test