Each events save is different transaction

scripts/migrate-event-address-nullable.sql

@@ -0,0 +1,16 @@
+-- Migration: allow events.address to be NULL
+-- Run on any environment with strict schema management (staging / production).
+-- Dev uses ddl-auto=update, but Hibernate's "update" mode never DROPS an existing NOT NULL
+-- constraint, so this must be applied by hand even on dev DBs created before this change.
+--
+-- WHY:
+--   Scraped events sometimes arrive without an address. Import previously failed the whole scrape
+--   batch on the events.address NOT NULL constraint — one address-less row rolled back every event
+--   in that cycle, so nothing reached the ops portal. address is now optional in the entity: the
+--   event is kept for admin review and geocoded later once an admin supplies an address via the
+--   ops portal edit endpoint (PUT /admin/event/{id}).
+--
+-- ROLLBACK (only safe once no rows have address IS NULL):
+--   ALTER TABLE events ALTER COLUMN address SET NOT NULL;
+
+ALTER TABLE events ALTER COLUMN address DROP NOT NULL;

src/main/java/org/smalltech/hashtaglocal_backend/entity/EventEntity.java

@@ -79,8 +79,12 @@ public class EventEntity {
   /**
    * Raw address string copied directly from the event data source (e.g., the Excel sheet). Kept as
    * a human-readable fallback while `location_id` is still null.
+   *
+   * <p>Nullable: scraped events sometimes arrive without an address. We keep the event anyway (for
+   * admin review) rather than dropping it — geocoding skips events with a null address until an
+   * admin supplies one via the ops portal edit endpoint.
    */
-  @Column(length = 1024, nullable = false)
+  @Column(length = 1024)
   private String address;
 
   /** URL to the original event page on the source platform. */

src/main/java/org/smalltech/hashtaglocal_backend/service/EventImportService.java

@@ -1,22 +1,17 @@
 package org.smalltech.hashtaglocal_backend.service;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Locale;
 import java.util.regex.Pattern;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.smalltech.hashtaglocal_backend.dto.ScrapeEventDTO;
-import org.smalltech.hashtaglocal_backend.entity.EventApprovalEntity;
 import org.smalltech.hashtaglocal_backend.entity.EventEntity;
 import org.smalltech.hashtaglocal_backend.entity.MediaEntity;
-import org.smalltech.hashtaglocal_backend.model.EventApprovalStatus;
 import org.smalltech.hashtaglocal_backend.model.EventPortalModel;
 import org.smalltech.hashtaglocal_backend.model.EventTypeModel;
-import org.smalltech.hashtaglocal_backend.repository.EventApprovalRepository;
 import org.smalltech.hashtaglocal_backend.repository.EventRepository;
 import org.springframework.stereotype.Service;
-import org.springframework.transaction.annotation.Transactional;
 
 /**
  * Handles importing events from the scrape service JSON response.
@@ -52,22 +47,25 @@ public class EventImportService {
   private final EventService eventService;
   private final EventRepository eventRepository;
   private final EventImageService eventImageService;
-  private final EventApprovalRepository eventApprovalRepository;
 
   /**
    * Processes a list of events from the scrape service, deduplicates against the database, and
-   * bulk-saves all new events in a single transaction.
+   * saves each new event (with its {@code PENDING} approval row) in its own transaction.
+   *
+   * <p>Events are persisted independently, so a single bad row — a constraint violation, a failed
+   * image upload, etc. — only skips that event instead of rolling back the whole batch. A missing
+   * address does <b>not</b> drop the event: it is kept for admin review and geocoded later once an
+   * address is supplied via the ops portal.
    *
    * @param scrapeEvents raw event DTOs from the scrape service response
-   * @return the number of events actually saved (duplicates excluded)
+   * @return the number of events actually saved (duplicates and failures excluded)
    */
-  @Transactional
   public int importFromScrapeResponse(List<ScrapeEventDTO> scrapeEvents) {
     if (scrapeEvents == null || scrapeEvents.isEmpty()) {
       return 0;
     }
 
-    List<EventEntity> toSave = new ArrayList<>();
+    int imported = 0;
 
     for (ScrapeEventDTO dto : scrapeEvents) {
       try {
@@ -84,8 +82,7 @@ public int importFromScrapeResponse(List<ScrapeEventDTO> scrapeEvents) {
           continue;
         }
         // fromString() returns null for portals it doesn't recognise. portal is a NOT NULL
-        // column, and toSave is bulk-inserted in one transaction, so letting a null portal
-        // through here would roll back — and silently drop — the entire import batch.
+        // column, so letting a null through would fail the insert for this event.
         if (EventPortalModel.fromString(dto.getPortal()) == null) {
           log.warn(
               "Skipping event '{}' — unrecognized portal '{}'", dto.getName(), dto.getPortal());
@@ -104,33 +101,21 @@ public int importFromScrapeResponse(List<ScrapeEventDTO> scrapeEvents) {
           continue;
         }
 
-        toSave.add(toEntity(dto, media));
+        // Persist this event (plus its PENDING approval) in its own transaction. Isolating each
+        // save means one problematic event never takes the rest of the batch down with it.
+        eventService.saveWithPendingApproval(toEntity(dto, media));
+        imported++;
       } catch (Exception e) {
         log.warn("Skipping event '{}' due to error: {}", dto.getName(), e.getMessage());
       }
     }
 
-    List<EventEntity> saved = eventService.saveAll(toSave);
-
-    // Create a PENDING approval row for every newly imported event so it lands in the
-    // admin review queue before appearing on the public site.
-    List<EventApprovalEntity> approvals =
-        saved.stream()
-            .map(
-                e ->
-                    EventApprovalEntity.builder()
-                        .eventId(e.getId())
-                        .status(EventApprovalStatus.PENDING)
-                        .build())
-            .toList();
-    eventApprovalRepository.saveAll(approvals);
-
     log.info(
         "Imported {} new events ({} received, {} skipped)",
-        toSave.size(),
+        imported,
         scrapeEvents.size(),
-        scrapeEvents.size() - toSave.size());
-    return toSave.size();
+        scrapeEvents.size() - imported);
+    return imported;
   }
 
   private EventEntity toEntity(ScrapeEventDTO dto, MediaEntity media) {
@@ -141,12 +126,25 @@ private EventEntity toEntity(ScrapeEventDTO dto, MediaEntity media) {
         .type(parseEventType(dto.getType()))
         .startTime(dto.getStartTime())
         .endTime(dto.getEndTime())
-        .address(dto.getAddress())
+        .address(normalizeAddress(dto.getAddress()))
         .link(stripUtmParams(dto.getLink()))
         .media(media)
         .build();
   }
 
+  /**
+   * Normalises the scraped address: trims surrounding whitespace and collapses a blank or missing
+   * value to {@code null}. Storing null (rather than an empty string) keeps the geocoding query
+   * ({@code findByLocationIsNullAndAddressIsNotNull}) from repeatedly picking up un-geocodable
+   * events.
+   */
+  private String normalizeAddress(String address) {
+    if (address == null || address.isBlank()) {
+      return null;
+    }
+    return address.strip();
+  }
+
   private String stripUtmParams(String link) {
     if (link == null || link.isBlank()) {
       return link;

src/main/java/org/smalltech/hashtaglocal_backend/service/EventService.java

@@ -72,6 +72,25 @@ public List<EventEntity> saveAll(List<EventEntity> events) {
     return eventRepository.saveAll(events);
   }
 
+  /**
+   * Persists a single event together with a fresh {@code PENDING} approval row in one transaction,
+   * so the event lands in the admin review queue atomically.
+   *
+   * <p>Used by the scraper import: each event is saved independently, so a single failing row (a
+   * constraint violation, a bad field, etc.) rolls back only that event instead of the whole
+   * import batch.
+   */
+  @Transactional
+  public EventEntity saveWithPendingApproval(EventEntity event) {
+    EventEntity saved = eventRepository.save(event);
+    eventApprovalRepository.save(
+        EventApprovalEntity.builder()
+            .eventId(saved.getId())
+            .status(EventApprovalStatus.PENDING)
+            .build());
+    return saved;
+  }
+
   /**
    * Returns all {@code APPROVED} events that have a resolved location, mapped to {@link EventData}.
    *

src/test/java/org/smalltech/hashtaglocal_backend/service/EventImportServiceTest.java

@@ -28,7 +28,6 @@
 import org.smalltech.hashtaglocal_backend.model.EventPortalModel;
 import org.smalltech.hashtaglocal_backend.model.EventTypeModel;
 import org.smalltech.hashtaglocal_backend.model.MediaTypeModel;
-import org.smalltech.hashtaglocal_backend.repository.EventApprovalRepository;
 import org.smalltech.hashtaglocal_backend.repository.EventRepository;
 
 /**
@@ -44,7 +43,6 @@ class EventImportServiceTest {
   @Mock private EventService eventService;
   @Mock private EventRepository eventRepository;
   @Mock private EventImageService eventImageService;
-  @Mock private EventApprovalRepository eventApprovalRepository;
 
   @InjectMocks private EventImportService eventImportService;
 
@@ -79,11 +77,11 @@ private ScrapeEventDTO dto(String name, String type, String portal, LocalDateTim
         .build();
   }
 
-  @SuppressWarnings("unchecked")
   private List<EventEntity> capturedSavedEvents() {
-    ArgumentCaptor<List<EventEntity>> captor = ArgumentCaptor.forClass(List.class);
-    verify(eventService).saveAll(captor.capture());
-    return captor.getValue();
+    ArgumentCaptor<EventEntity> captor = ArgumentCaptor.forClass(EventEntity.class);
+    // atLeast(0): captures every per-event save, and tolerates the zero-save cases (all skipped).
+    verify(eventService, atLeast(0)).saveWithPendingApproval(captor.capture());
+    return captor.getAllValues();
   }
 
   // ---------------------------------------------------------------------------
@@ -153,9 +151,8 @@ void mapsPortalStringToEventPortalModel(String raw, String expected, String desc
         List.of(dto("Test Event", "TREKANDPLOG", raw, START_TIME)));
 
     if (expected == null) {
-      // Unrecognised/null portals are skipped, not saved: portal is a NOT NULL column and the
-      // batch is bulk-inserted in one transaction, so letting a null portal through would roll
-      // back the whole import. See EventImportService#importFromScrapeResponse.
+      // Unrecognised/null portals are skipped, not saved: portal is a NOT NULL column, so letting a
+      // null portal through would fail the insert. See EventImportService#importFromScrapeResponse.
       assertTrue(
           capturedSavedEvents().isEmpty(),
           "event with unrecognised portal '" + raw + "' should be skipped, not saved");
@@ -224,7 +221,7 @@ void skipsDuplicateEvent() {
             List.of(dto("Trek and Plog", "TREKANDPLOG", "Team everest", START_TIME)));
 
     assertEquals(0, count);
-    verify(eventService).saveAll(List.of());
+    verify(eventService, never()).saveWithPendingApproval(any());
   }
 
   @Test
@@ -281,4 +278,48 @@ void returnsZeroForNullOrEmptyList() {
     assertEquals(0, eventImportService.importFromScrapeResponse(List.of()));
     verifyNoInteractions(eventRepository);
   }
+
+  @Test
+  @DisplayName("Event with a blank/missing address is kept, with address normalised to null")
+  void keepsEventWithMissingAddress() {
+    when(eventRepository.existsByNameAndStartTime(any(), any())).thenReturn(false);
+    ScrapeEventDTO noAddress =
+        ScrapeEventDTO.builder()
+            .name("Trek and Plog")
+            .organisation("Org")
+            .portal("Team everest")
+            .type("TREKANDPLOG")
+            .startTime(START_TIME)
+            .address("   ") // blank — must not cause the event to be dropped
+            .link("https://example.com")
+            .image("https://example.com/image.jpg")
+            .build();
+
+    int count = eventImportService.importFromScrapeResponse(List.of(noAddress));
+
+    assertEquals(1, count, "a missing address must not drop the event");
+    assertNull(
+        capturedSavedEvents().get(0).getAddress(), "blank address should be normalised to null");
+  }
+
+  @Test
+  @DisplayName("One event failing to save does not abort the rest of the batch")
+  void oneFailingEventDoesNotAbortBatch() {
+    LocalDateTime t1 = LocalDateTime.of(2026, 2, 21, 5, 0);
+    LocalDateTime t2 = LocalDateTime.of(2026, 3, 7, 0, 0);
+    when(eventRepository.existsByNameAndStartTime(any(), any())).thenReturn(false);
+    // First per-event save blows up; the second must still be attempted and succeed.
+    when(eventService.saveWithPendingApproval(any()))
+        .thenThrow(new RuntimeException("constraint violation"))
+        .thenReturn(EventEntity.builder().build());
+
+    int count =
+        eventImportService.importFromScrapeResponse(
+            List.of(
+                dto("Bad Event", "TREKANDPLOG", "Team everest", t1),
+                dto("Good Event", "TREKANDPLOG", "Team everest", t2)));
+
+    assertEquals(1, count, "only the good event counts as imported");
+    verify(eventService, times(2)).saveWithPendingApproval(any());
+  }
 }