From b54216d068018cd509d5c93f31cbaa38a90b3357 Mon Sep 17 00:00:00 2001 From: George Dawoud Date: Sat, 7 Feb 2026 13:23:31 -0800 Subject: [PATCH 1/3] Fix CSV and directory export to use family address fallback When exporting CSV or generating church directory, persons without personal addresses now fall back to their family's address data. This ensures members are included in exports with complete address information even if they haven't entered personal address details. Fixes #7937 --- .../ui/reports/standard.csv.reports.spec.js | 62 +++++++++++++++++++ src/CSVCreateFile.php | 39 ++++++------ src/Reports/DirectoryReport.php | 17 ++--- 3 files changed, 91 insertions(+), 27 deletions(-) diff --git a/cypress/e2e/ui/reports/standard.csv.reports.spec.js b/cypress/e2e/ui/reports/standard.csv.reports.spec.js index 48d096beae..1b45c7ad71 100644 --- a/cypress/e2e/ui/reports/standard.csv.reports.spec.js +++ b/cypress/e2e/ui/reports/standard.csv.reports.spec.js @@ -24,6 +24,68 @@ describe("csv export", () => { expect(response.body).to.not.include("Parse error"); }); }); + + it("should include address columns in CSV export with family fallback", () => { + // Test default format (format=0) includes address fields when requested + cy.request({ + method: "POST", + url: "/CSVCreateFile.php", + form: true, + body: { + output: "csv", + format: "default", + familyonly: "false", + Source: "all", + Address1: "1", + Address2: "1", + City: "1", + State: "1", + Zip: "1" + } + }).then((response) => { + expect(response.status).to.eq(200); + expect(response.headers["content-type"]).to.include("text/csv"); + + // Verify CSV header contains address columns + const lines = response.body.split('\n'); + expect(lines.length).to.be.greaterThan(1); + const header = lines[0].toLowerCase(); + expect(header).to.include("address 1"); + expect(header).to.include("city"); + expect(header).to.include("state"); + }); + }); + + it("should include address columns in rollup format CSV export", () => { + // Test rollup format includes address fields when requested + cy.request({ + method: "POST", + url: "/CSVCreateFile.php", + form: true, + body: { + output: "csv", + format: "rollup", + familyonly: "false", + Source: "all", + Address1: "1", + Address2: "1", + City: "1", + State: "1", + Zip: "1" + } + }).then((response) => { + expect(response.status).to.eq(200); + expect(response.headers["content-type"]).to.include("text/csv"); + + // Verify CSV header contains address columns + const lines = response.body.split('\n'); + expect(lines.length).to.be.greaterThan(1); + const header = lines[0].toLowerCase(); + expect(header).to.include("address 1"); + expect(header).to.include("city"); + expect(header).to.include("state"); + }); + }); }); describe("advanced deposit report", () => { diff --git a/src/CSVCreateFile.php b/src/CSVCreateFile.php index f2fa8c35d7..03edf2db68 100644 --- a/src/CSVCreateFile.php +++ b/src/CSVCreateFile.php @@ -374,31 +374,32 @@ extract($aRow); $person = PersonQuery::create()->findOneById($per_ID); - // Use person data only - each person must enter their own information + // Use person data with fallback to family data for addresses + // This ensures members without personal addresses still get exported with their family's address if ($sFormat === 'rollup') { - // Even in rollup format, use person data (no family inheritance) - $sHomePhone = $per_HomePhone ?? ''; + // Rollup format: use person data with family fallback for address/contact fields + $sHomePhone = !empty($per_HomePhone) ? $per_HomePhone : ($fam_HomePhone ?? ''); $sWorkPhone = $per_WorkPhone ?? ''; $sCellPhone = $per_CellPhone ?? ''; - $sCountry = $per_Country ?? ''; - $sAddress1 = $per_Address1 ?? ''; - $sAddress2 = $per_Address2 ?? ''; - $sCity = $per_City ?? ''; - $sState = $per_State ?? ''; - $sZip = $per_Zip ?? ''; - $sEmail = $per_Email ?? ''; + $sCountry = !empty($per_Country) ? $per_Country : ($fam_Country ?? ''); + $sAddress1 = !empty($per_Address1) ? $per_Address1 : ($fam_Address1 ?? ''); + $sAddress2 = !empty($per_Address2) ? $per_Address2 : ($fam_Address2 ?? ''); + $sCity = !empty($per_City) ? $per_City : ($fam_City ?? ''); + $sState = !empty($per_State) ? $per_State : ($fam_State ?? ''); + $sZip = !empty($per_Zip) ? $per_Zip : ($fam_Zip ?? ''); + $sEmail = !empty($per_Email) ? $per_Email : ($fam_Email ?? ''); } else { - // Individual data - use person data only - $sHomePhone = $per_HomePhone ?? ''; + // Default format: use person data with family fallback for address/contact fields + $sHomePhone = !empty($per_HomePhone) ? $per_HomePhone : ($fam_HomePhone ?? ''); $sWorkPhone = $per_WorkPhone ?? ''; $sCellPhone = $per_CellPhone ?? ''; - $sCountry = $per_Country ?? ''; - $sAddress1 = $per_Address1 ?? ''; - $sAddress2 = $per_Address2 ?? ''; - $sCity = $per_City ?? ''; - $sState = $per_State ?? ''; - $sZip = $per_Zip ?? ''; - $sEmail = $per_Email ?? ''; + $sCountry = !empty($per_Country) ? $per_Country : ($fam_Country ?? ''); + $sAddress1 = !empty($per_Address1) ? $per_Address1 : ($fam_Address1 ?? ''); + $sAddress2 = !empty($per_Address2) ? $per_Address2 : ($fam_Address2 ?? ''); + $sCity = !empty($per_City) ? $per_City : ($fam_City ?? ''); + $sState = !empty($per_State) ? $per_State : ($fam_State ?? ''); + $sZip = !empty($per_Zip) ? $per_Zip : ($fam_Zip ?? ''); + $sEmail = !empty($per_Email) ? $per_Email : ($fam_Email ?? ''); } // Check if we're filtering out people with incomplete addresses diff --git a/src/Reports/DirectoryReport.php b/src/Reports/DirectoryReport.php index 940ea3bbf4..e44b85ae09 100644 --- a/src/Reports/DirectoryReport.php +++ b/src/Reports/DirectoryReport.php @@ -246,16 +246,17 @@ $pdf->sRecordName .= ' ' . MiscUtils::formatBirthDate($per_BirthYear, $per_BirthMonth, $per_BirthDay, $per_Flags); } - // Use person data only - each person must enter their own information - $sAddress1 = $per_Address1 ?? ''; - $sAddress2 = $per_Address2 ?? ''; - $sCity = $per_City ?? ''; - $sState = $per_State ?? ''; - $sZip = $per_Zip ?? ''; - $sHomePhone = $per_HomePhone ?? ''; + // Use person data with fallback to family data for addresses + // This ensures members without personal addresses still appear in the directory with their family's address + $sAddress1 = !empty($per_Address1) ? $per_Address1 : ($fam_Address1 ?? ''); + $sAddress2 = !empty($per_Address2) ? $per_Address2 : ($fam_Address2 ?? ''); + $sCity = !empty($per_City) ? $per_City : ($fam_City ?? ''); + $sState = !empty($per_State) ? $per_State : ($fam_State ?? ''); + $sZip = !empty($per_Zip) ? $per_Zip : ($fam_Zip ?? ''); + $sHomePhone = !empty($per_HomePhone) ? $per_HomePhone : ($fam_HomePhone ?? ''); $sWorkPhone = $per_WorkPhone ?? ''; $sCellPhone = $per_CellPhone ?? ''; - $sEmail = $per_Email ?? ''; + $sEmail = !empty($per_Email) ? $per_Email : ($fam_Email ?? ''); if ($bDirAddress) { if (strlen($sAddress1)) { From 428459365c099968b0e9dc20d9177a960e7d25b1 Mon Sep 17 00:00:00 2001 From: George Dawoud Date: Sat, 7 Feb 2026 13:53:06 -0800 Subject: [PATCH 2/3] Address PR review comments - Fix POST parameter name: use 'Format' (capital F) to match CSVCreateFile.php - Deduplicate address fallback code: remove conditional since rollup and default use identical address/contact field assignments --- .../ui/reports/standard.csv.reports.spec.js | 6 +-- src/CSVCreateFile.php | 37 ++++++------------- 2 files changed, 14 insertions(+), 29 deletions(-) diff --git a/cypress/e2e/ui/reports/standard.csv.reports.spec.js b/cypress/e2e/ui/reports/standard.csv.reports.spec.js index 1b45c7ad71..545900390d 100644 --- a/cypress/e2e/ui/reports/standard.csv.reports.spec.js +++ b/cypress/e2e/ui/reports/standard.csv.reports.spec.js @@ -26,14 +26,14 @@ describe("csv export", () => { }); it("should include address columns in CSV export with family fallback", () => { - // Test default format (format=0) includes address fields when requested + // Test default format includes address fields when requested cy.request({ method: "POST", url: "/CSVCreateFile.php", form: true, body: { output: "csv", - format: "default", + Format: "default", familyonly: "false", Source: "all", Address1: "1", @@ -64,7 +64,7 @@ describe("csv export", () => { form: true, body: { output: "csv", - format: "rollup", + Format: "rollup", familyonly: "false", Source: "all", Address1: "1", diff --git a/src/CSVCreateFile.php b/src/CSVCreateFile.php index 03edf2db68..d09582ec2b 100644 --- a/src/CSVCreateFile.php +++ b/src/CSVCreateFile.php @@ -374,33 +374,18 @@ extract($aRow); $person = PersonQuery::create()->findOneById($per_ID); - // Use person data with fallback to family data for addresses + // Use person data with fallback to family data for addresses/contact // This ensures members without personal addresses still get exported with their family's address - if ($sFormat === 'rollup') { - // Rollup format: use person data with family fallback for address/contact fields - $sHomePhone = !empty($per_HomePhone) ? $per_HomePhone : ($fam_HomePhone ?? ''); - $sWorkPhone = $per_WorkPhone ?? ''; - $sCellPhone = $per_CellPhone ?? ''; - $sCountry = !empty($per_Country) ? $per_Country : ($fam_Country ?? ''); - $sAddress1 = !empty($per_Address1) ? $per_Address1 : ($fam_Address1 ?? ''); - $sAddress2 = !empty($per_Address2) ? $per_Address2 : ($fam_Address2 ?? ''); - $sCity = !empty($per_City) ? $per_City : ($fam_City ?? ''); - $sState = !empty($per_State) ? $per_State : ($fam_State ?? ''); - $sZip = !empty($per_Zip) ? $per_Zip : ($fam_Zip ?? ''); - $sEmail = !empty($per_Email) ? $per_Email : ($fam_Email ?? ''); - } else { - // Default format: use person data with family fallback for address/contact fields - $sHomePhone = !empty($per_HomePhone) ? $per_HomePhone : ($fam_HomePhone ?? ''); - $sWorkPhone = $per_WorkPhone ?? ''; - $sCellPhone = $per_CellPhone ?? ''; - $sCountry = !empty($per_Country) ? $per_Country : ($fam_Country ?? ''); - $sAddress1 = !empty($per_Address1) ? $per_Address1 : ($fam_Address1 ?? ''); - $sAddress2 = !empty($per_Address2) ? $per_Address2 : ($fam_Address2 ?? ''); - $sCity = !empty($per_City) ? $per_City : ($fam_City ?? ''); - $sState = !empty($per_State) ? $per_State : ($fam_State ?? ''); - $sZip = !empty($per_Zip) ? $per_Zip : ($fam_Zip ?? ''); - $sEmail = !empty($per_Email) ? $per_Email : ($fam_Email ?? ''); - } + $sHomePhone = !empty($per_HomePhone) ? $per_HomePhone : ($fam_HomePhone ?? ''); + $sWorkPhone = $per_WorkPhone ?? ''; + $sCellPhone = $per_CellPhone ?? ''; + $sCountry = !empty($per_Country) ? $per_Country : ($fam_Country ?? ''); + $sAddress1 = !empty($per_Address1) ? $per_Address1 : ($fam_Address1 ?? ''); + $sAddress2 = !empty($per_Address2) ? $per_Address2 : ($fam_Address2 ?? ''); + $sCity = !empty($per_City) ? $per_City : ($fam_City ?? ''); + $sState = !empty($per_State) ? $per_State : ($fam_State ?? ''); + $sZip = !empty($per_Zip) ? $per_Zip : ($fam_Zip ?? ''); + $sEmail = !empty($per_Email) ? $per_Email : ($fam_Email ?? ''); // Check if we're filtering out people with incomplete addresses if (!($bSkipIncompleteAddr && (strlen($sCity) === 0 || strlen($sState) === 0 || strlen($sZip) === 0 || (strlen($sAddress1) === 0 && strlen($sAddress2) === 0)))) { From fcbba06dec064cb152a31401ec117a7d00ceff0d Mon Sep 17 00:00:00 2001 From: George Dawoud Date: Sat, 7 Feb 2026 14:23:45 -0800 Subject: [PATCH 3/3] Address PR review comments for CSV export tests - Add PHP error checks (Fatal error, Parse error) to CSV export tests - Verify at least one data row exists in export (validates fallback works) - Add code comment noting similar logic in DirectoryReports.php for future consolidation consideration --- .../ui/reports/standard.csv.reports.spec.js | 18 ++++++++++++++++++ src/CSVCreateFile.php | 2 ++ 2 files changed, 20 insertions(+) diff --git a/cypress/e2e/ui/reports/standard.csv.reports.spec.js b/cypress/e2e/ui/reports/standard.csv.reports.spec.js index 545900390d..3086d92ac4 100644 --- a/cypress/e2e/ui/reports/standard.csv.reports.spec.js +++ b/cypress/e2e/ui/reports/standard.csv.reports.spec.js @@ -27,6 +27,7 @@ describe("csv export", () => { it("should include address columns in CSV export with family fallback", () => { // Test default format includes address fields when requested + // This also validates the family fallback logic for addresses cy.request({ method: "POST", url: "/CSVCreateFile.php", @@ -46,6 +47,10 @@ describe("csv export", () => { expect(response.status).to.eq(200); expect(response.headers["content-type"]).to.include("text/csv"); + // Verify no PHP errors in response + expect(response.body).to.not.include("Fatal error"); + expect(response.body).to.not.include("Parse error"); + // Verify CSV header contains address columns const lines = response.body.split('\n'); expect(lines.length).to.be.greaterThan(1); @@ -53,6 +58,11 @@ describe("csv export", () => { expect(header).to.include("address 1"); expect(header).to.include("city"); expect(header).to.include("state"); + + // Verify at least one data row exists (family fallback should populate addresses) + // Filter out empty lines + const dataLines = lines.slice(1).filter(line => line.trim().length > 0); + expect(dataLines.length).to.be.greaterThan(0, "Expected at least one data row in export"); }); }); @@ -77,6 +87,10 @@ describe("csv export", () => { expect(response.status).to.eq(200); expect(response.headers["content-type"]).to.include("text/csv"); + // Verify no PHP errors in response + expect(response.body).to.not.include("Fatal error"); + expect(response.body).to.not.include("Parse error"); + // Verify CSV header contains address columns const lines = response.body.split('\n'); expect(lines.length).to.be.greaterThan(1); @@ -84,6 +98,10 @@ describe("csv export", () => { expect(header).to.include("address 1"); expect(header).to.include("city"); expect(header).to.include("state"); + + // Verify at least one data row exists + const dataLines = lines.slice(1).filter(line => line.trim().length > 0); + expect(dataLines.length).to.be.greaterThan(0, "Expected at least one data row in export"); }); }); }); diff --git a/src/CSVCreateFile.php b/src/CSVCreateFile.php index d09582ec2b..d395478b9b 100644 --- a/src/CSVCreateFile.php +++ b/src/CSVCreateFile.php @@ -376,6 +376,8 @@ // Use person data with fallback to family data for addresses/contact // This ensures members without personal addresses still get exported with their family's address + // Note: Similar logic exists in DirectoryReports.php - consider centralizing in PersonService + // if this pattern needs to be reused elsewhere (see issue #7937) $sHomePhone = !empty($per_HomePhone) ? $per_HomePhone : ($fam_HomePhone ?? ''); $sWorkPhone = $per_WorkPhone ?? ''; $sCellPhone = $per_CellPhone ?? '';