fix: surface real errors in self-update + search, restore sub-entity saves on member update
Database Portability Tests / Unit Tests (PlatformHelper) (pull_request) Failing after 38s
Database Portability Tests / Integration (mysql) (pull_request) Has been skipped
Database Portability Tests / Integration (postgres) (pull_request) Has been skipped
Database Portability Tests / Integration (sqlite) (pull_request) Has been skipped
Database Portability Tests / Verify no MySQL-specific SQL (pull_request) Successful in 4s

Three bugs that silently masked real problems:

1. Self-update check (Backup view): backend returned an `error` field but the
   UI only rendered a generic "Pruefung fehlgeschlagen" badge, hiding the
   real cause. SelfUpdateService now classifies the exception (DNS, timeout,
   TLS, network, HTTP, parse) into a user-friendly diagnostic and the UI
   surfaces it with optional technical details (target URL + raw exception).
   Helps diagnose hotspot / cellular issues where the Gitea fetch fails
   behind a carrier firewall, captive portal, or IPv6-only APN.

2. Member search (global SearchBar): the catch block set results to [],
   which rendered as "Keine Ergebnisse" regardless of whether the request
   truly returned no matches or actually failed (500 / 401 / timeout / NC
   brute-force throttle). Now distinguishes by HTTP status and shows a
   dedicated error panel with a retry button. Also added a request-race
   guard so a slow response for an earlier query can't overwrite a newer
   one's results.

3. Member update silently discarded addresses / phones / emails: the
   frontend payload stripped sub-entities, and the backend update() never
   read them either. Fix layers:
   - MemberService::update() now syncs `addresses`/`phones`/`emails`
     arrays: items with an existing id are updated, new items are
     inserted, and existing items missing from the payload are deleted.
     Absent keys are untouched (backwards compatible).
   - MemberDetail.vue includes sub-entities in the update payload.

Tests: 4 new MemberService tests covering sync add / update / delete /
noop paths. All 1122 unit tests pass. Version bumped to 0.2.9.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
shahondin1624
2026-04-17 21:30:41 +02:00
parent fa702e30f3
commit 2fdd8bb083
9 changed files with 400 additions and 6 deletions
+1 -1
View File
@@ -5,7 +5,7 @@
<name>Mitgliederverwaltung</name>
<summary>Mitgliederverwaltung für Pfadfindervereine</summary>
<description><![CDATA[Verwaltung von Mitgliedern, Familien, Beiträgen, Lagern und mehr für Pfadfindervereine. Integriert sich in Nextcloud Kalender, Kontakte und Dateien.]]></description>
<version>0.2.8</version>
<version>0.2.9</version>
<licence>agpl</licence>
<author>shahondin1624</author>
<namespace>Mitgliederverwaltung</namespace>
+113
View File
@@ -370,6 +370,10 @@ class MemberService {
$member = $this->memberMapper->findById($id);
$oldData = $member->jsonSerialize();
// Preserve the raw payload so we can still see addresses/phones/emails
// after filtering the member-column data below.
$rawData = $data;
// Filter data to only known editable fields to prevent pollution
// from read-only/system fields (createdAt, updatedAt, deletedAt, etc.)
$editableFields = [
@@ -456,11 +460,120 @@ class MemberService {
/** @var Member $member */
$member = $this->memberMapper->update($member);
// Sync sub-entities if the caller included them in the payload.
// Absent keys (undefined) leave existing sub-entities untouched.
if (array_key_exists('addresses', $rawData) && is_array($rawData['addresses'])) {
$this->syncAddresses($id, $rawData['addresses']);
}
if (array_key_exists('phones', $rawData) && is_array($rawData['phones'])) {
$this->syncPhones($id, $rawData['phones']);
}
if (array_key_exists('emails', $rawData) && is_array($rawData['emails'])) {
$this->syncEmails($id, $rawData['emails']);
}
$this->auditService->logUpdate($oldData, $member->jsonSerialize(), 'member', $id);
return $this->find($id);
}
/**
* Sync a member's addresses against an incoming array.
* Rules:
* - Items with an existing id ∈ current DB set → updated
* - Items without id (or with unknown id) → created
* - Existing DB items whose id is not referenced → deleted
*
* @param array[] $incoming
* @throws Exception
*/
private function syncAddresses(int $memberId, array $incoming): void {
$existing = $this->addressMapper->findByMemberId($memberId);
$existingById = [];
foreach ($existing as $addr) {
$existingById[$addr->getId()] = $addr;
}
$keptIds = [];
foreach ($incoming as $item) {
$id = isset($item['id']) ? (int)$item['id'] : 0;
if ($id > 0 && isset($existingById[$id])) {
$this->updateAddress($id, $item);
$keptIds[$id] = true;
} else {
$this->addAddress($memberId, $item);
}
}
foreach ($existingById as $id => $addr) {
if (!isset($keptIds[$id])) {
$this->deleteAddress($id);
}
}
}
/**
* Sync a member's phones (same rules as {@see syncAddresses}).
*
* @param array[] $incoming
* @throws Exception
*/
private function syncPhones(int $memberId, array $incoming): void {
$existing = $this->phoneMapper->findByMemberId($memberId);
$existingById = [];
foreach ($existing as $p) {
$existingById[$p->getId()] = $p;
}
$keptIds = [];
foreach ($incoming as $item) {
$id = isset($item['id']) ? (int)$item['id'] : 0;
if ($id > 0 && isset($existingById[$id])) {
$this->updatePhone($id, $item);
$keptIds[$id] = true;
} else {
$this->addPhone($memberId, $item);
}
}
foreach ($existingById as $id => $p) {
if (!isset($keptIds[$id])) {
$this->deletePhone($id);
}
}
}
/**
* Sync a member's emails (same rules as {@see syncAddresses}).
*
* @param array[] $incoming
* @throws Exception
*/
private function syncEmails(int $memberId, array $incoming): void {
$existing = $this->emailMapper->findByMemberId($memberId);
$existingById = [];
foreach ($existing as $e) {
$existingById[$e->getId()] = $e;
}
$keptIds = [];
foreach ($incoming as $item) {
$id = isset($item['id']) ? (int)$item['id'] : 0;
if ($id > 0 && isset($existingById[$id])) {
$this->updateEmail($id, $item);
$keptIds[$id] = true;
} else {
$this->addEmail($memberId, $item);
}
}
foreach ($existingById as $id => $e) {
if (!isset($keptIds[$id])) {
$this->deleteEmail($id);
}
}
}
// ── Delete (soft) ────────────────────────────────────────────────
/**
+56 -1
View File
@@ -60,13 +60,17 @@ class SelfUpdateService {
'exception' => $e,
'app' => self::APP_ID,
]);
$classification = $this->classifyNetworkError($e);
return [
'available' => false,
'currentVersion' => $currentVersion,
'latestVersion' => null,
'releaseUrl' => null,
'publishedAt' => null,
'error' => $e->getMessage(),
'error' => $classification['message'],
'errorType' => $classification['type'],
'errorDetail' => $e->getMessage(),
'targetUrl' => self::GITEA_BASE,
];
}
@@ -437,6 +441,57 @@ class SelfUpdateService {
*
* @return array{success: bool, output: string|null, error: string|null}
*/
/**
* Classify a network exception into a user-friendly diagnostic hint.
*
* @return array{type: string, message: string}
*/
private function classifyNetworkError(\Throwable $e): array {
$raw = strtolower($e->getMessage());
$host = parse_url(self::GITEA_BASE, PHP_URL_HOST) ?: 'Gitea-Server';
if (str_contains($raw, 'could not resolve host') || str_contains($raw, 'name resolution') || str_contains($raw, 'getaddrinfo')) {
return [
'type' => 'dns',
'message' => "DNS-Aufloesung fuer {$host} fehlgeschlagen. Pruefe DNS des Servers (z. B. Mobilfunk-/Hotspot-DNS blockiert die Domain).",
];
}
if (str_contains($raw, 'timed out') || str_contains($raw, 'timeout') || str_contains($raw, 'operation timed out')) {
return [
'type' => 'timeout',
'message' => "Zeitueberschreitung beim Verbindungsaufbau zu {$host} (Timeout: 15 s). Bei Mobilfunk/Hotspot ist die Latenz oft zu hoch.",
];
}
if (str_contains($raw, 'ssl') || str_contains($raw, 'certificate') || str_contains($raw, 'tls')) {
return [
'type' => 'tls',
'message' => "TLS-Fehler bei {$host}. Moegliche Ursache: Captive Portal, HTTPS-Inspection oder abgelaufenes Zertifikat.",
];
}
if (str_contains($raw, 'connection refused') || str_contains($raw, 'could not connect') || str_contains($raw, 'no route to host') || str_contains($raw, 'network is unreachable')) {
return [
'type' => 'network',
'message' => "Verbindung zu {$host} nicht moeglich. Der Server hat keinen Zugriff auf diese Domain (Firewall, Routing, IPv6-only-Mobilfunk ohne IPv4-Fallback?).",
];
}
if (preg_match('/\b(4\d\d|5\d\d)\b/', $raw, $m)) {
return [
'type' => 'http',
'message' => "HTTP-Fehler {$m[1]} beim Abruf von {$host}.",
];
}
if (str_contains($raw, 'json') || str_contains($raw, 'decode')) {
return [
'type' => 'parse',
'message' => "Antwort von {$host} konnte nicht als JSON gelesen werden (evtl. Login-Seite eines Captive Portals).",
];
}
return [
'type' => 'unknown',
'message' => "Update-Pruefung fehlgeschlagen: " . $e->getMessage(),
];
}
private function runOccUpgrade(): array {
$serverRoot = defined('OC_SERVERROOT') ? OC_SERVERROOT : '/var/www/html';
$occPath = $serverRoot . '/occ';
+76
View File
@@ -30,6 +30,15 @@
Suche...
</div>
<!-- Error (shown instead of "no results" when the request failed) -->
<div v-else-if="error" class="search-bar__error">
<strong>Suche fehlgeschlagen</strong>
<div class="search-bar__error-msg">{{ error }}</div>
<button class="search-bar__retry" @mousedown.prevent="performSearch">
Erneut versuchen
</button>
</div>
<!-- No results -->
<div v-else-if="results.length === 0 && query.trim().length >= 2" class="search-bar__empty">
Keine Ergebnisse für "{{ query }}"
@@ -80,16 +89,19 @@ const router = useRouter()
const query = ref('')
const results = ref([])
const loading = ref(false)
const error = ref(null)
const showResults = ref(false)
const selectedIndex = ref(-1)
const inputRef = ref(null)
let searchTimeout = null
let inflightRequestId = 0
// ── Search logic ────────────────────────────────────────────────────
function onInput() {
selectedIndex.value = -1
error.value = null
clearTimeout(searchTimeout)
if (query.value.trim().length < 2) {
@@ -105,20 +117,53 @@ function onInput() {
async function performSearch() {
if (query.value.trim().length < 2) return
const requestId = ++inflightRequestId
loading.value = true
error.value = null
try {
const url = generateUrl('/apps/mitgliederverwaltung/api/v1/members/search')
const response = await axios.get(url, {
params: { q: query.value.trim(), limit: 10 },
timeout: 10000,
})
// Drop stale responses — only the most recent request wins
if (requestId !== inflightRequestId) return
results.value = response.data.data || []
} catch (err) {
if (requestId !== inflightRequestId) return
console.error('Search failed:', err)
results.value = []
error.value = formatSearchError(err)
} finally {
if (requestId === inflightRequestId) {
loading.value = false
}
}
}
function formatSearchError(err) {
if (err?.code === 'ECONNABORTED' || /timeout/i.test(err?.message || '')) {
return 'Zeitüberschreitung der Anfrage (10 s). Server ist überlastet oder die Verbindung ist langsam.'
}
const status = err?.response?.status
if (status === 401 || status === 403) {
return 'Sitzung abgelaufen. Bitte die Seite neu laden und erneut anmelden.'
}
if (status === 429) {
return 'Zu viele Anfragen. Nextcloud hat die Suche vorübergehend gesperrt — kurz warten und erneut versuchen.'
}
if (status >= 500 && status < 600) {
const serverMsg = err.response?.data?.error || 'Serverfehler'
return `HTTP ${status}: ${serverMsg}`
}
if (status) {
return `HTTP ${status}: ${err.response?.data?.error || err.message}`
}
if (err?.message) {
return `Netzwerkfehler: ${err.message}`
}
return 'Unbekannter Fehler.'
}
// ── Dropdown visibility ─────────────────────────────────────────────
@@ -146,6 +191,7 @@ function clearQuery() {
query.value = ''
results.value = []
selectedIndex.value = -1
error.value = null
nextTick(() => {
inputRef.value?.focus()
})
@@ -270,6 +316,36 @@ function formatStatus(status) {
text-align: center;
}
.search-bar__error {
padding: 12px 16px;
color: var(--color-error);
background: color-mix(in srgb, var(--color-error) 10%, transparent);
font-size: 0.85em;
border-radius: var(--border-radius);
margin: 4px;
}
.search-bar__error strong {
display: block;
margin-bottom: 4px;
}
.search-bar__error-msg {
color: var(--color-text);
margin-bottom: 8px;
word-break: break-word;
}
.search-bar__retry {
background: var(--color-error);
color: white;
border: none;
border-radius: var(--border-radius);
padding: 4px 10px;
cursor: pointer;
font-size: 0.85em;
}
.search-bar__results {
list-style: none;
margin: 0;
+1 -1
View File
@@ -31,7 +31,7 @@ app.use(router)
// @nextcloud/vue v9 reads appName/appVersion via Vue's inject(),
// not via webpack DefinePlugin globals.
app.provide('appName', 'mitgliederverwaltung')
app.provide('appVersion', '0.2.8')
app.provide('appVersion', '0.2.9')
app.mount('#mitgliederverwaltung')
+18 -1
View File
@@ -28,12 +28,24 @@
<span class="backup-page__badge">Aktuell</span>
</template>
<template v-else-if="store.updateInfo.error">
<span class="backup-page__badge backup-page__badge--none">
<span class="backup-page__badge backup-page__badge--none"
:title="store.updateInfo.errorDetail || store.updateInfo.error">
Pruefung fehlgeschlagen
</span>
</template>
</template>
</div>
<div v-if="store.updateInfo?.error" class="backup-page__update-error">
<strong>Update-Check fehlgeschlagen</strong>
<p>{{ store.updateInfo.error }}</p>
<details v-if="store.updateInfo.errorDetail">
<summary>Technische Details</summary>
<pre class="backup-page__update-error-detail">{{ store.updateInfo.errorDetail }}</pre>
<div v-if="store.updateInfo.targetUrl">
Ziel: <code>{{ store.updateInfo.targetUrl }}</code>
</div>
</details>
</div>
<div v-if="store.updateInfo?.releaseBody" class="backup-page__update-notes">
{{ store.updateInfo.releaseBody }}
</div>
@@ -293,6 +305,11 @@ function formatTrigger(trigger) {
.backup-page__update { padding: 12px 16px; background: var(--color-background-dark); border-radius: var(--border-radius-large); }
.backup-page__update-status { display: flex; align-items: center; gap: 12px; margin-bottom: 8px; }
.backup-page__update-notes { font-size: 0.9em; color: var(--color-text-lighter); margin-bottom: 12px; white-space: pre-line; }
.backup-page__update-error { margin: 8px 0 12px; padding: 10px 14px; border-radius: var(--border-radius); background: color-mix(in srgb, var(--color-error) 10%, transparent); border: 1px solid color-mix(in srgb, var(--color-error) 40%, transparent); font-size: 0.9em; }
.backup-page__update-error strong { color: var(--color-error); }
.backup-page__update-error p { margin: 4px 0 8px; }
.backup-page__update-error summary { cursor: pointer; color: var(--color-text-lighter); font-size: 0.85em; }
.backup-page__update-error-detail { white-space: pre-wrap; word-break: break-word; font-size: 0.8em; margin: 4px 0; padding: 6px 8px; background: var(--color-background-dark); border-radius: var(--border-radius); }
.backup-page__update-actions { display: flex; gap: 8px; }
.backup-page__create { display: flex; gap: 16px; align-items: flex-end; }
+3
View File
@@ -365,6 +365,9 @@ async function save() {
einwilligungDatum: d.einwilligungDatum,
juleicaNummer: d.juleicaNummer,
juleicaAblaufdatum: d.juleicaAblaufdatum,
addresses: d.addresses || [],
phones: d.phones || [],
emails: d.emails || [],
}
await store.updateMember(Number(props.id), data)
await loadMember()
+130
View File
@@ -551,6 +551,136 @@ class MemberServiceTest extends TestCase {
$this->assertSame('Moritz', $result['vorname']);
}
public function testUpdateAddsNewAddressWhenSyncingEmptyList(): void {
$member = $this->createMember(1);
$this->memberMapper->method('findById')->with(1)->willReturn($member);
$this->memberMapper->method('update')->willReturnArgument(0);
$this->addressMapper->method('findByMemberId')->willReturn([]);
$this->phoneMapper->method('findByMemberId')->willReturn([]);
$this->emailMapper->method('findByMemberId')->willReturn([]);
$inserted = new Address();
$inserted->setStrasse('Musterstr. 1');
$inserted->setPlz('12345');
$inserted->setOrt('Berlin');
$inserted->setLand('Deutschland');
$inserted->setMemberId(1);
$inserted->setIsPrimary(false);
$this->addressMapper->expects($this->once())
->method('insert')
->willReturn($inserted);
$this->addressMapper->expects($this->never())->method('update');
$this->addressMapper->expects($this->never())->method('delete');
$result = $this->service->update(1, [
'vorname' => 'Moritz',
'addresses' => [
['strasse' => 'Musterstr. 1', 'plz' => '12345', 'ort' => 'Berlin', 'land' => 'Deutschland'],
],
]);
$this->assertSame('Moritz', $result['vorname']);
}
public function testUpdateUpdatesExistingAddressById(): void {
$member = $this->createMember(1);
$existing = new Address();
$existing->setId(42);
$existing->setMemberId(1);
$existing->setStrasse('Alt');
$existing->setPlz('00000');
$existing->setOrt('Alt');
$existing->setLand('Deutschland');
$existing->setIsPrimary(false);
$this->memberMapper->method('findById')->with(1)->willReturn($member);
$this->memberMapper->method('update')->willReturnArgument(0);
$this->addressMapper->method('findByMemberId')->willReturn([$existing]);
$this->phoneMapper->method('findByMemberId')->willReturn([]);
$this->emailMapper->method('findByMemberId')->willReturn([]);
$this->addressMapper->method('find')->with(42)->willReturn($existing);
$this->addressMapper->expects($this->once())
->method('update')
->willReturnArgument(0);
$this->addressMapper->expects($this->never())->method('insert');
$this->addressMapper->expects($this->never())->method('delete');
$this->service->update(1, [
'addresses' => [
['id' => 42, 'strasse' => 'Neu 1', 'plz' => '12345', 'ort' => 'Neu'],
],
]);
}
public function testUpdateDeletesAddressesNotInPayload(): void {
$member = $this->createMember(1);
$keep = new Address();
$keep->setId(42);
$keep->setMemberId(1);
$keep->setStrasse('Keep');
$keep->setPlz('11111');
$keep->setOrt('Keep');
$keep->setLand('Deutschland');
$keep->setIsPrimary(false);
$remove = new Address();
$remove->setId(43);
$remove->setMemberId(1);
$remove->setStrasse('Remove');
$remove->setPlz('22222');
$remove->setOrt('Remove');
$remove->setLand('Deutschland');
$remove->setIsPrimary(false);
$this->memberMapper->method('findById')->with(1)->willReturn($member);
$this->memberMapper->method('update')->willReturnArgument(0);
$this->addressMapper->method('findByMemberId')->willReturn([$keep, $remove]);
$this->phoneMapper->method('findByMemberId')->willReturn([]);
$this->emailMapper->method('findByMemberId')->willReturn([]);
$this->addressMapper->method('find')->willReturnMap([
[42, $keep],
[43, $remove],
]);
$this->addressMapper->expects($this->once())->method('update')->willReturnArgument(0);
$this->addressMapper->expects($this->once())->method('delete')->with($remove);
$this->addressMapper->expects($this->never())->method('insert');
$this->service->update(1, [
'addresses' => [
['id' => 42, 'strasse' => 'Keep', 'plz' => '11111', 'ort' => 'Keep'],
],
]);
}
public function testUpdateWithoutSubEntitiesKeysDoesNotTouchAddresses(): void {
$member = $this->createMember(1);
$this->memberMapper->method('findById')->with(1)->willReturn($member);
$this->memberMapper->method('update')->willReturnArgument(0);
$this->addressMapper->method('findByMemberId')->willReturn([]);
$this->phoneMapper->method('findByMemberId')->willReturn([]);
$this->emailMapper->method('findByMemberId')->willReturn([]);
// Only mapper calls allowed: no insert/update/delete on sub-entity mappers
$this->addressMapper->expects($this->never())->method('insert');
$this->addressMapper->expects($this->never())->method('update');
$this->addressMapper->expects($this->never())->method('delete');
$this->phoneMapper->expects($this->never())->method('insert');
$this->phoneMapper->expects($this->never())->method('update');
$this->phoneMapper->expects($this->never())->method('delete');
$this->emailMapper->expects($this->never())->method('insert');
$this->emailMapper->expects($this->never())->method('update');
$this->emailMapper->expects($this->never())->method('delete');
$this->service->update(1, ['vorname' => 'Moritz']);
}
public function testUpdateImportedMemberSucceeds(): void {
// Simulate a member created by import (has all fields set via import path)
$member = $this->createMember(1, 'Max', 'Mustermann', '2010-01-15', '2020-01-01');
+1 -1
View File
@@ -41,7 +41,7 @@ module.exports = {
new VueLoaderPlugin(),
new webpack.DefinePlugin({
appName: JSON.stringify('mitgliederverwaltung'),
appVersion: JSON.stringify('0.2.8'),
appVersion: JSON.stringify('0.2.9'),
}),
new webpack.optimize.LimitChunkCountPlugin({
maxChunks: 1,