From 712904bc97815189e824682c3aef307c744d6a8a Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Fri, 25 Sep 2026 09:35:18 +0200 Subject: [PATCH 1/4] Inject database and login into coordinates, which raise on errors --- .../Controllers/MapsController.php | 39 ++- lib/GaletteMaps/Coordinates.php | 257 +++++++----------- .../tests/units/MapsController.php | 30 +- tests/GaletteMaps/tests/units/Coordinates.php | 60 ++-- 4 files changed, 181 insertions(+), 205 deletions(-) diff --git a/lib/GaletteMaps/Controllers/MapsController.php b/lib/GaletteMaps/Controllers/MapsController.php index 9234efd..c40326f 100644 --- a/lib/GaletteMaps/Controllers/MapsController.php +++ b/lib/GaletteMaps/Controllers/MapsController.php @@ -34,6 +34,9 @@ class MapsController extends AbstractPluginController #[Inject("Plugin Galette Maps")] protected array $module_info; + #[Inject] + protected Coordinates $coordinates; + /** * Member dependencies to load; groups are loaded on demand by access checks * @@ -78,9 +81,9 @@ public function map(Request $request, Response $response): Response ]; try { - $params['list'] = (new Coordinates())->listCoords(); + $params['list'] = $this->coordinates->listVisible(); } catch (\Throwable $e) { - //already logged + Analog::log('Unable to list coordinates | ' . $e->getMessage(), Analog::ERROR); $this->flash->addMessageNow( 'error_detected', _T('Coordinates has not been loaded. Maybe plugin tables does not exists in the database?', 'maps') @@ -137,12 +140,11 @@ public function localizeMember(Request $request, Response $response, ?int $id = } $can_edit = $member->canEdit($this->login); - $coords = new Coordinates(); - $mcoords = $coords->getCoords($member->id); + $mcoords = $this->coordinates->get($member->id); $towns = false; //towns are only proposed to choose a location - if ($can_edit && count($mcoords) === 0 && trim($member->town ?? '') !== '') { + if ($can_edit && $mcoords === null && trim($member->town ?? '') !== '') { try { $towns = (new NominatimTowns($this->preferences))->search( $member->town, @@ -175,7 +177,7 @@ public function localizeMember(Request $request, Response $response, ?int $id = if ($towns !== false) { $params['towns'] = $towns; - } elseif (count($mcoords) > 0) { + } elseif ($mcoords !== null) { $params['town'] = $mcoords; } @@ -319,11 +321,15 @@ public function ILiveHere(Request $request, Response $response, ?int $id = null) if ($error === null) { $post = $request->getParsedBody(); - $coords = new Coordinates(); if (isset($post['remove'])) { - if ($coords->removeCoords($id)) { + try { + $this->coordinates->remove($id); $message = _T('Coordinates has been removed!', 'maps'); - } else { + } catch (\Throwable $e) { + Analog::log( + 'Unable to remove coordinates of member #' . $id . ' | ' . $e->getMessage(), + Analog::ERROR + ); $error = _T('Coordinates has not been removed :(', 'maps'); $status = 500; } @@ -338,11 +344,18 @@ public function ILiveHere(Request $request, Response $response, ?int $id = null) ) { $error = _T('Invalid coordinates.', 'maps'); $status = 400; - } elseif ($coords->setCoords($id, $latitude, $longitude)) { - $message = _T('New coordinates has been stored!', 'maps'); } else { - $error = _T('Coordinates has not been stored :(', 'maps'); - $status = 500; + try { + $this->coordinates->set($id, $latitude, $longitude); + $message = _T('New coordinates has been stored!', 'maps'); + } catch (\Throwable $e) { + Analog::log( + 'Unable to store coordinates of member #' . $id . ' | ' . $e->getMessage(), + Analog::ERROR + ); + $error = _T('Coordinates has not been stored :(', 'maps'); + $status = 500; + } } } } diff --git a/lib/GaletteMaps/Coordinates.php b/lib/GaletteMaps/Coordinates.php index aa414fa..17c2ce3 100644 --- a/lib/GaletteMaps/Coordinates.php +++ b/lib/GaletteMaps/Coordinates.php @@ -10,15 +10,16 @@ namespace GaletteMaps; -use Analog\Analog; -use ArrayObject; use Galette\Core\Db; +use Galette\Core\Login; use Galette\Entity\Adherent; -use Laminas\Db\Sql\Expression; /** * Members GPS coordinates * + * Errors are not caught here: callers know what the user was trying to do, + * and log it along with the database message. + * * @author Johan Cwiklinski */ @@ -28,120 +29,98 @@ class Coordinates public const string PK = 'id_adh'; /** - * Retrieve member coordinates + * Constructor + * + * @param Db $zdb Database instance + * @param Login $login Logged-in user, whose rights filter the list + */ + public function __construct( + private readonly Db $zdb, + private readonly Login $login + ) { + } + + /** + * Get member coordinates * * @param int $id Member id * - * @return array|ArrayObject + * @return ?array{latitude: string, longitude: string} null when member has no coordinates */ - public function getCoords(int $id): array|ArrayObject + public function get(int $id): ?array { - /** @var Db $zdb */ - global $zdb; - - try { - $select = $zdb->select($this->getTableName()); - $select->where([self::PK => $id]); - $results = $zdb->execute($select); - - if ($results->count() > 0) { - return $results->current(); - } else { - return []; - } - } catch (\Exception $e) { - if ($e->getCode() == '42S02') { - Analog::log( - 'Maps coordinates table does not exists', - Analog::WARNING - ); - } else { - Analog::log( - 'Unable to retrieve members coordinates for "' - . $id . '". | ' . $e->getMessage(), - Analog::WARNING - ); - } - throw $e; + $select = $this->zdb->select($this->getTableName()); + $select->columns(['latitude', 'longitude'])->where([self::PK => $id]); + $row = $this->zdb->execute($select)->current(); + + if ($row === null) { + return null; } + + return [ + 'latitude' => (string)$row['latitude'], + 'longitude' => (string)$row['longitude'] + ]; } /** - * Returns list of all know coordinates, filtered on publicly - * visible profile for non admins and non staff + * Get coordinates of the members logged-in user can see on the map + * + * Staff and administrators see every active member; others see active, + * up-to-date members who display their information, and their own position. * - * @return array> + * @return array */ - public function listCoords(): array + public function listVisible(): array { - global $zdb, $login; - - try { - $select = $zdb->select($this->getTableName(), 'c'); - $select->join( - [ - 'a' => PREFIX_DB . Adherent::TABLE - ], - 'a.' . self::PK . '=' . 'c.' . self::PK, - //only what the map displays - ['nom_adh', 'prenom_adh', 'pseudo_adh', 'societe_adh'] - ); - $where = $select->where; - $where->equalTo('a.activite_adh', new Expression('true')); - - if ( - !$login->isAdmin() - && !$login->isStaff() - && !$login->isSuperAdmin() - ) { - //limit query to public up-to-date profiles, and to logged-in member own one - $visible = $where->nest(); - $public = $visible->nest(); - $public->nest() - ->greaterThanOrEqualTo('a.date_echeance', date('Y-m-d')) - ->or->equalTo('a.bool_exempt_adh', new Expression('true')) - ->unnest(); - $public->and->equalTo('a.bool_display_info', new Expression('true')); - $public->unnest(); - if ($login->isLogged()) { - $visible->or->equalTo('a.' . Adherent::PK, $login->id); - } - $visible->unnest(); - } - - $results = $zdb->execute($select); - - $res = []; - foreach ($results as $r) { - $m = [ - 'id_adh' => (int)$r->{self::PK}, - 'lat' => $r->latitude, - 'lng' => $r->longitude, - 'name' => Adherent::getNameWithCase($r->nom_adh, $r->prenom_adh), - 'nickname' => $r->pseudo_adh - ]; - if (trim($r->societe_adh ?? '') !== '') { - $m['company'] = $r->societe_adh; - } - $res[] = $m; + $select = $this->zdb->select($this->getTableName(), 'c'); + $select->join( + [ + 'a' => PREFIX_DB . Adherent::TABLE + ], + 'a.' . self::PK . '=' . 'c.' . self::PK, + //only what the map displays + ['nom_adh', 'prenom_adh', 'pseudo_adh', 'societe_adh'] + ); + $where = $select->where; + $where->equalTo('a.activite_adh', right: true); + + if ( + !$this->login->isAdmin() + && !$this->login->isStaff() + && !$this->login->isSuperAdmin() + ) { + //limit query to public up-to-date profiles, and to logged-in member own one + $visible = $where->nest(); + $public = $visible->nest(); + $public->nest() + ->greaterThanOrEqualTo('a.date_echeance', date('Y-m-d')) + ->or->equalTo('a.bool_exempt_adh', right: true) + ->unnest(); + $public->and->equalTo('a.bool_display_info', right: true); + $public->unnest(); + if ($this->login->isLogged()) { + $visible->or->equalTo('a.' . Adherent::PK, $this->login->id); } + $visible->unnest(); + } - return $res; - } catch (\Exception $e) { - if ($e->getCode() == '42S02') { - Analog::log( - 'Maps coordinates table does not exists', - Analog::WARNING - ); - } else { - Analog::log( - 'Unable to retrieve members coordinates list "' - . '". | ' . $e->getMessage(), - Analog::WARNING - ); + $res = []; + foreach ($this->zdb->execute($select) as $r) { + $m = [ + 'id_adh' => (int)$r[self::PK], + 'lat' => (string)$r['latitude'], + 'lng' => (string)$r['longitude'], + 'name' => Adherent::getNameWithCase($r['nom_adh'], $r['prenom_adh']), + 'nickname' => $r['pseudo_adh'] + ]; + if (trim($r['societe_adh'] ?? '') !== '') { + $m['company'] = $r['societe_adh']; } - throw $e; + $res[] = $m; } + + return $res; } /** @@ -151,71 +130,35 @@ public function listCoords(): array * @param float $latitude Latitude * @param float $longitude Longitude */ - public function setCoords(int $id, float $latitude, float $longitude): bool + public function set(int $id, float $latitude, float $longitude): void { - global $zdb; - - try { - $coords = $this->getCoords($id); - if (count($coords) === 0) { - //coordinates does not exist yet - $insert = $zdb->insert($this->getTableName()); - $insert->values( - [ - self::PK => $id, - 'latitude' => $latitude, - 'longitude' => $longitude - ] - ); - $results = $zdb->execute($insert); - } else { - //coordinates already exists, just update - $update = $zdb->update($this->getTableName()); - $update->set( - [ - 'latitude' => $latitude, - 'longitude' => $longitude - ] - )->where( - [self::PK => $id] - ); - //no row is affected when the position does not change - $zdb->execute($update); - return true; - } - return ($results->count() > 0); - } catch (\Exception $e) { - Analog::log( - 'Unable to set coordinates | ' . $e->getMessage(), - Analog::ERROR - ); - return false; + $values = [ + 'latitude' => $latitude, + 'longitude' => $longitude + ]; + + if ($this->get($id) === null) { + $insert = $this->zdb->insert($this->getTableName()); + $insert->values([self::PK => $id] + $values); + $this->zdb->execute($insert); + } else { + //no row is affected when the position does not change: not an error + $update = $this->zdb->update($this->getTableName()); + $update->set($values)->where([self::PK => $id]); + $this->zdb->execute($update); } } /** - * Remove member coordinates + * Remove member coordinates; removing nothing is not an error * * @param int $id Member id */ - public function removeCoords(int $id): bool + public function remove(int $id): void { - global $zdb; - - try { - $delete = $zdb->delete($this->getTableName()); - $delete->where([self::PK => $id]); - //removing nothing is not an error - $zdb->execute($delete); - return true; - } catch (\Exception $e) { - Analog::log( - 'Unable to remove coordinates for member ' - . $id . ' | ' . $e->getMessage(), - Analog::ERROR - ); - return false; - } + $delete = $this->zdb->delete($this->getTableName()); + $delete->where([self::PK => $id]); + $this->zdb->execute($delete); } /** diff --git a/tests/GaletteMaps/Controllers/tests/units/MapsController.php b/tests/GaletteMaps/Controllers/tests/units/MapsController.php index c46a246..f74152e 100644 --- a/tests/GaletteMaps/Controllers/tests/units/MapsController.php +++ b/tests/GaletteMaps/Controllers/tests/units/MapsController.php @@ -107,16 +107,16 @@ public function testMemberCannotChangeOtherMemberCoords(): void //member two speaks Catalan, member one gets messages in English $this->getMemberOne(); $member_two = $this->getMemberTwo(); - $coords = new Coordinates(); - $this->assertTrue($coords->setCoords($member_two->id, 48.85, 2.35)); + $coords = new Coordinates($this->zdb, $this->login); + $coords->set($member_two->id, 48.85, 2.35); $this->logMember($this->dataAdherentOne()); $this->expectCoordsRefused($this->postCoords($member_two->id), $member_two->id); $this->expectCoordsRefused($this->postCoords($member_two->id, ['remove' => '1']), $member_two->id); - $this->assertEquals( - ['id_adh' => $member_two->id, 'latitude' => '48.850000', 'longitude' => '2.350000'], - (array)$coords->getCoords($member_two->id) + $this->assertSame( + ['latitude' => '48.850000', 'longitude' => '2.350000'], + $coords->get($member_two->id) ); } @@ -136,7 +136,7 @@ public function testMemberChangesOwnCoords(): void json_decode((string)$test_response->getBody(), true) ); } - $this->assertCount(3, (array)(new Coordinates())->getCoords($member_one->id)); + $this->assertNotNull((new Coordinates($this->zdb, $this->login))->get($member_one->id)); } /** @@ -149,12 +149,12 @@ public function testManagerChangesCoordsAsCoreAllows(): void $this->logMember($this->dataAdherentTwo()); $this->expectCoordsRefused($this->postCoords($member_one->id), $member_one->id); - $this->assertSame([], (new Coordinates())->getCoords($member_one->id)); + $this->assertNull((new Coordinates($this->zdb, $this->login))->get($member_one->id)); $this->preferences->pref_bool_groupsmanagers_edit_member = true; $test_response = $this->postCoords($member_one->id); $this->assertSame(200, $test_response->getStatusCode()); - $this->assertCount(3, (array)(new Coordinates())->getCoords($member_one->id)); + $this->assertNotNull((new Coordinates($this->zdb, $this->login))->get($member_one->id)); } /** @@ -180,7 +180,7 @@ public function testInvalidCoords(): void json_decode((string)$test_response->getBody(), true) ); } - $this->assertSame([], (new Coordinates())->getCoords($member_one->id)); + $this->assertNull((new Coordinates($this->zdb, $this->login))->get($member_one->id)); //bounds are included $test_response = $this->postCoords(null, ['latitude' => '-90', 'longitude' => '180']); @@ -235,7 +235,7 @@ public function testMemberCannotShowOtherMemberCoords(): void //member two speaks Catalan, member one gets messages in English $this->getMemberOne(); $member_two = $this->getMemberTwo(); - $this->assertTrue((new Coordinates())->setCoords($member_two->id, 48.85, 2.35)); + (new Coordinates($this->zdb, $this->login))->set($member_two->id, 48.85, 2.35); $this->logMember($this->dataAdherentOne()); $request = $this->createRequest('maps_localize_member', ['id' => (string)$member_two->id]); @@ -260,7 +260,7 @@ public function testManagerShowsCoords(): void { $member_one = $this->getMemberOne(); $this->makeMemberTwoManager([$member_one]); - $this->assertTrue((new Coordinates())->setCoords($member_one->id, 48.85, 2.35)); + (new Coordinates($this->zdb, $this->login))->set($member_one->id, 48.85, 2.35); $this->logMember($this->dataAdherentTwo()); $request = $this->createRequest('maps_localize_member', ['id' => (string)$member_one->id]); @@ -294,7 +294,7 @@ public function testMapEscapesNames(): void 'societe_adh' => '', ])->where([Adherent::PK => $member_one->id]); $this->zdb->execute($update); - $this->assertTrue((new Coordinates())->setCoords($member_one->id, 48.85, 2.35)); + (new Coordinates($this->zdb, $this->login))->set($member_one->id, 48.85, 2.35); $this->logSuperAdmin(); $test_response = $this->app->handle($this->createRequest('maps_map')); @@ -313,7 +313,7 @@ public function testMapEscapesNames(): void public function testPublicMap(): void { $member_one = $this->getMemberOne(); - $this->assertTrue((new Coordinates())->setCoords($member_one->id, 48.85, 2.35)); + (new Coordinates($this->zdb, $this->login))->set($member_one->id, 48.85, 2.35); $request = $this->createRequest('maps_map'); $this->preferences->pref_bool_publicpages = false; @@ -355,7 +355,7 @@ public function testOwnPage(): void $this->assertStringNotContainsString('id="removecoords"', $body); $this->assertStringContainsString('onMapClick', $body); - $this->assertTrue((new Coordinates())->setCoords($member_one->id, 48.85, 2.35)); + (new Coordinates($this->zdb, $this->login))->set($member_one->id, 48.85, 2.35); $test_response = $this->app->handle($request); $this->assertSame(200, $test_response->getStatusCode()); $body = (string)$test_response->getBody(); @@ -495,7 +495,7 @@ public function testSuperAdminCoords(): void $test_response = $this->postCoords($member_one->id); $this->assertSame(200, $test_response->getStatusCode()); - $this->assertCount(3, (array)(new Coordinates())->getCoords($member_one->id)); + $this->assertNotNull((new Coordinates($this->zdb, $this->login))->get($member_one->id)); $test_response = $this->postCoords(null); $this->assertSame(400, $test_response->getStatusCode()); diff --git a/tests/GaletteMaps/tests/units/Coordinates.php b/tests/GaletteMaps/tests/units/Coordinates.php index 2c88d5f..9713d80 100644 --- a/tests/GaletteMaps/tests/units/Coordinates.php +++ b/tests/GaletteMaps/tests/units/Coordinates.php @@ -37,23 +37,22 @@ public function tearDown(): void public function testCoordinates(): void { $member = $this->getMemberOne(); - $coords = new \GaletteMaps\Coordinates(); - $this->assertSame([], $coords->getCoords($member->id)); - $this->assertSame([], $coords->listCoords()); + $coords = new \GaletteMaps\Coordinates($this->zdb, $this->login); + $this->assertNull($coords->get($member->id)); + $this->assertSame([], $coords->listVisible()); $this->logSuperAdmin(); - $this->assertSame([], $coords->getCoords($member->id)); - $this->assertSame([], $coords->listCoords()); + $this->assertNull($coords->get($member->id)); + $this->assertSame([], $coords->listVisible()); //set coordinates for member one - $this->assertTrue($coords->setCoords($member->id, 50.362038, 3.472998)); - $this->assertEquals( + $coords->set($member->id, 50.362038, 3.472998); + $this->assertSame( [ - 'id_adh' => $member->id, 'latitude' => '50.362038', 'longitude' => '3.472998' ], - (array)$coords->getCoords($member->id) + $coords->get($member->id) ); $this->assertEquals( [ @@ -65,15 +64,15 @@ public function testCoordinates(): void 'nickname' => 'ubertrand' ] ], - $coords->listCoords() + $coords->listVisible() ); //update coordinates for member one - $this->assertTrue($coords->setCoords($member->id, 51.362038, 3.572998)); + $coords->set($member->id, 51.362038, 3.572998); //remove coordinates for member one - $this->assertTrue($coords->removeCoords($member->id)); - $this->assertSame([], $coords->getCoords($member->id)); + $coords->remove($member->id); + $this->assertNull($coords->get($member->id)); } /** @@ -82,9 +81,9 @@ public function testCoordinates(): void public function testSetSamePosition(): void { $member = $this->getMemberOne(); - $coords = new \GaletteMaps\Coordinates(); - $this->assertTrue($coords->setCoords($member->id, 50.362038, 3.472998)); - $this->assertTrue($coords->setCoords($member->id, 50.362038, 3.472998)); + $coords = new \GaletteMaps\Coordinates($this->zdb, $this->login); + $coords->set($member->id, 50.362038, 3.472998); + $coords->set($member->id, 50.362038, 3.472998); } /** @@ -115,7 +114,7 @@ private function setVisibility(int $id_adh, bool $active, bool $public, bool $up */ private function listedIds(): array { - $ids = array_column((new \GaletteMaps\Coordinates())->listCoords(), 'id_adh'); + $ids = array_column((new \GaletteMaps\Coordinates($this->zdb, $this->login))->listVisible(), 'id_adh'); sort($ids); return $ids; } @@ -127,9 +126,9 @@ public function testListVisibility(): void { $member_one = $this->getMemberOne(); $member_two = $this->getMemberTwo(); - $coords = new \GaletteMaps\Coordinates(); - $this->assertTrue($coords->setCoords($member_one->id, 50.36, 3.47)); - $this->assertTrue($coords->setCoords($member_two->id, 48.85, 2.35)); + $coords = new \GaletteMaps\Coordinates($this->zdb, $this->login); + $coords->set($member_one->id, 50.36, 3.47); + $coords->set($member_two->id, 48.85, 2.35); $this->setVisibility($member_one->id, active: true, public: false, uptodate: false); $this->setVisibility($member_two->id, active: true, public: true, uptodate: true); @@ -148,4 +147,25 @@ public function testListVisibility(): void $this->assertSame([], $this->listedIds()); $this->login->logout(); } + + /** + * Storing coordinates of a member that does not exist raises + */ + public function testSetMissingMember(): void + { + $coords = new \GaletteMaps\Coordinates($this->zdb, $this->login); + //a failing query aborts the whole transaction on PostgreSQL + $this->zdb->db->query('SAVEPOINT maps_set', \Laminas\Db\Adapter\Adapter::QUERY_MODE_EXECUTE); + try { + $coords->set(999999, 50.36, 3.47); + $this->fail('An exception was expected'); + } catch (\Throwable $e) { + $this->assertNotInstanceOf(\PHPUnit\Framework\AssertionFailedError::class, $e); + } finally { + $this->zdb->db->query('ROLLBACK TO SAVEPOINT maps_set', \Laminas\Db\Adapter\Adapter::QUERY_MODE_EXECUTE); + } + $this->assertNull($coords->get(999999)); + //logged by Db + $this->expectLogEntry(\Analog\Analog::ERROR, 'Query error: INSERT INTO'); + } } From d3e2ab54ec7a68804aa34175fe95b35274ff384a Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Fri, 25 Sep 2026 09:36:28 +0200 Subject: [PATCH 2/4] Inject login into plugin class --- lib/GaletteMaps/PluginGaletteMaps.php | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/lib/GaletteMaps/PluginGaletteMaps.php b/lib/GaletteMaps/PluginGaletteMaps.php index 77f51e1..c6970f1 100644 --- a/lib/GaletteMaps/PluginGaletteMaps.php +++ b/lib/GaletteMaps/PluginGaletteMaps.php @@ -30,7 +30,9 @@ class PluginGaletteMaps extends GalettePlugin implements InstallableInterface, MenuProviderInterface, DashboardProviderInterface, MemberActionProviderInterface, PreferencesProviderInterface { #[Inject] - private readonly Db $zdb; //@phpstan-ignore-line injected from DI + private readonly Db $zdb; //@phpstan-ignore property.uninitializedReadonly, property.onlyRead (injected from DI) + #[Inject] + private readonly Login $login; //@phpstan-ignore property.uninitializedReadonly, property.onlyRead (injected from DI) /** * Get the preferences the plugin declares @@ -49,11 +51,9 @@ public function getPreferences(): array */ public function getMenus(): array { - /** @var Login $login */ - global $login; $menus = []; - if ($login->isAdmin()) { + if ($this->login->isAdmin()) { $menus['configuration'] = [ 'items' => [ [ @@ -66,7 +66,7 @@ public function getMenus(): array ]; } - if ($login->isLogged() && !$login->isSuperAdmin()) { + if ($this->login->isLogged() && !$this->login->isSuperAdmin()) { $menus['myaccount'] = [ 'items' => [ [ @@ -107,10 +107,7 @@ public function getPublicMenus(): array */ public function getMyDashboards(): array { - /** @var Login $login */ - global $login; - - if ($login->isSuperAdmin()) { + if ($this->login->isSuperAdmin()) { return []; } @@ -119,7 +116,7 @@ public function getMyDashboards(): array 'label' => _T("My localization", "maps"), 'route' => [ 'name' => 'maps_localize_member', - 'args' => ["id" => $login->id] + 'args' => ["id" => $this->login->id] ], 'icon' => 'map' ] From a00f18a6d2c351f7e07e7d5ca559cad65b3f329a Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Fri, 25 Sep 2026 09:37:47 +0200 Subject: [PATCH 3/4] Inject towns search into controller --- .../Controllers/MapsController.php | 5 ++- .../tests/units/MapsController.php | 33 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/lib/GaletteMaps/Controllers/MapsController.php b/lib/GaletteMaps/Controllers/MapsController.php index c40326f..a35ce1b 100644 --- a/lib/GaletteMaps/Controllers/MapsController.php +++ b/lib/GaletteMaps/Controllers/MapsController.php @@ -37,6 +37,9 @@ class MapsController extends AbstractPluginController #[Inject] protected Coordinates $coordinates; + #[Inject] + protected NominatimTowns $nominatim; + /** * Member dependencies to load; groups are loaded on demand by access checks * @@ -146,7 +149,7 @@ public function localizeMember(Request $request, Response $response, ?int $id = //towns are only proposed to choose a location if ($can_edit && $mcoords === null && trim($member->town ?? '') !== '') { try { - $towns = (new NominatimTowns($this->preferences))->search( + $towns = $this->nominatim->search( $member->town, $member->country ); diff --git a/tests/GaletteMaps/Controllers/tests/units/MapsController.php b/tests/GaletteMaps/Controllers/tests/units/MapsController.php index f74152e..eea4eb6 100644 --- a/tests/GaletteMaps/Controllers/tests/units/MapsController.php +++ b/tests/GaletteMaps/Controllers/tests/units/MapsController.php @@ -202,6 +202,39 @@ public function testRemoveMissingCoords(): void ); } + /** + * Towns matching member town are proposed when member has no coordinates + */ + public function testTownsProposed(): void + { + $member_one = $this->getMemberOne(); + $update = $this->zdb->update(Adherent::TABLE); + $update->set(['ville_adh' => 'Valenciennes'])->where([Adherent::PK => $member_one->id]); + $this->zdb->execute($update); + $places = [ + ['lat' => '50.3620', 'lon' => '3.4729', 'display_name' => 'Valenciennes', 'address' => ['city' => 'Valenciennes']], + ['lat' => '45.1', 'lon' => '1.2', 'display_name' => 'Somewhere', 'address' => ['village' => 'Petit Valenciennes']], + ]; + $client = new \GuzzleHttp\Client([ + 'handler' => \GuzzleHttp\HandlerStack::create( + new \GuzzleHttp\Handler\MockHandler([new \GuzzleHttp\Psr7\Response(200, [], (string)json_encode($places))]) + ) + ]); + $this->container->set( + \GaletteMaps\NominatimTowns::class, + new \GaletteMaps\NominatimTowns($this->preferences, $client) + ); + + $this->logMember($this->dataAdherentOne()); + $test_response = $this->app->handle($this->createRequest('maps_mymap')); + $this->assertSame(200, $test_response->getStatusCode()); + $body = (string)$test_response->getBody(); + $this->assertStringContainsString('id="possible_towns"', $body); + $this->assertStringContainsString('50.3620/3.4729', $body); + $this->assertStringContainsString('<b>Petit</b> Valenciennes', $body); + $this->assertStringNotContainsString('Petit', $body); + } + /** * Unreachable towns search does not prevent to display the map */ From ee3fe4116a7d80c5bf006bb48b27868ec74a5899 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Fri, 25 Sep 2026 09:39:08 +0200 Subject: [PATCH 4/4] Answer coordinates changes with core JSON helper --- .../Controllers/MapsController.php | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/lib/GaletteMaps/Controllers/MapsController.php b/lib/GaletteMaps/Controllers/MapsController.php index a35ce1b..1f6f1f3 100644 --- a/lib/GaletteMaps/Controllers/MapsController.php +++ b/lib/GaletteMaps/Controllers/MapsController.php @@ -363,18 +363,13 @@ public function ILiveHere(Request $request, Response $response, ?int $id = null) } } - $response = $response - ->withStatus($status) - ->withHeader('Content-type', 'application/json'); - - $res = [ - 'res' => $error === null, - 'message' => ($error ?? $message) - ]; - - $body = $response->getBody(); - $body->write(json_encode($res)); - - return $response; + return $this->withJson( + $response, + [ + 'res' => $error === null, + 'message' => ($error ?? $message) + ], + $status + ); } }