From 91a6951eeaa444b0ed33ba6ac07677e5f30a6be7 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Fri, 25 Sep 2026 07:34:42 +0200 Subject: [PATCH 1/8] Remove dead code and obsolete files --- .composer-require-checker.config.json | 119 ------------ .scrutinizer.yml | 173 ------------------ _routes.php | 11 -- bin/release | 3 - bin/twig-cache | 114 ------------ .../Controllers/MapsController.php | 2 - package-lock.json | 4 +- package.json | 4 +- 8 files changed, 4 insertions(+), 426 deletions(-) delete mode 100644 .composer-require-checker.config.json delete mode 100644 .scrutinizer.yml delete mode 100755 bin/twig-cache diff --git a/.composer-require-checker.config.json b/.composer-require-checker.config.json deleted file mode 100644 index 841670d..0000000 --- a/.composer-require-checker.config.json +++ /dev/null @@ -1,119 +0,0 @@ -{ - "php-core-extensions": [ - "core", "standard", - - "ctype", "date", "fileinfo", "filter", "iconv", "json", "pcre", "reflection", "session", "simplexml", "spl", "xml", - - "curl", "gd", "intl", "mbstring", "mysqli", "zlib", "pdo", "pgsql", "gettext", - - "exif", "pcntl" - ], - "symbol-whitelist": [ - "// PHP symbols", - "null", - "false", "true", - "static", "self", "parent", - "array", "bool", "callable", "float", "int", "iterable", "object", "string", "void", - - "// random native PHP functions", - "// these are moved in random native extension in PHP 8.2, but this extension does not exists in previous PHP versions", - "mt_rand", - "rand", - "random_int", - - "// Galette constants (not detected as they are dynamically declared)", - "GALETTE_ATTACHMENTS_PATH", - "GALETTE_ADAPTATIVE_CARDS", - "GALETTE_BASE_PATH", - "GALETTE_CACHE_DIR", - "GALETTE_CARD_COLS", - "GALETTE_CARD_HEIGHT", - "GALETTE_CARD_ROWS", - "GALETTE_CARD_WIDTH", - "GALETTE_COMPAT_VERSION", - "GALETTE_SYSCONFIG_PATH", - "GALETTE_CONFIG_PATH", - "GALETTE_DATA_PATH", - "GALETTE_DB_VERSION", - "GALETTE_DOCUMENTS_PATH", - "GALETTE_DOWNLOADS_URI", - "GALETTE_EXPORTS_PATH", - "GALETTE_FILES_PATH", - "GALETTE_HOSTED", - "GALETTE_IMPORTS_PATH", - "GALETTE_LOGS_PATH", - "GALETTE_MODE", - "GALETTE_DEBUG", - "GALETTE_NIGHTLY", - "GALETTE_PHOTOS_PATH", - "GALETTE_PLUGINS_PATH", - "GALETTE_PLUGINS_DATA_PATH", - "GALETTE_ROOT", - "GALETTE_TELEMETRY_URI", - "GALETTE_TEMPIMAGES_PATH", - "GALETTE_THEME", - "GALETTE_THEMES_PATH", - "GALETTE_VERSION", - "GALETTE_X_FORWARDED_FOR_INDEX", - "_CURRENT_THEME_PATH", - "GALETTE_URI", - "GALETTE_MYSQL_MIN", - "GALETTE_MARIADB_MIN", - "GALETTE_PGSQL_MIN", - "GALETTE_DISPLAY_VERSION", - "GALETTE_TESTS", - "GALETTE_PHP_MIN", - - "// Galette db constants (not detected as they are dynamically declared)", - "HOST_DB", - "NAME_DB", - "PORT_DB", - "PREFIX_DB", - "PWD_DB", - "TYPE_DB", - "USER_DB", - - "//XHProf constants", - - "//know but not detected Galette function,", - "remove_remarks", - "isValidWebUrl", - "remove_remarks", - "split_sql_file", - "_T", - "__", - "_Tn", - "_Tx", - "_Tnx", - - "// XHProf PECL extension", - "XHProfRuns_Default", - "xhprof_disable", - "xhprof_enable", - "GALETTE_XHPROF_PATH", - "XHPROF_PATH", - "XHPROF_URL", - "XHPROF_FLAGS_CPU", - "XHPROF_FLAGS_MEMORY", - "XHPROF_FLAGS_NO_BUILTINS", - "XHPROF_HOST", - - "// DI functions", - "DI\\autowire", - "DI\\Container", - "DI\\ContainerBuilder", - "DI\\Bridge\\Slim\\App", - "DI\\Attribute\\Inject", - - "// Not detected, do not know why.", - "Psr\\Http\\Server\\RequestHandlerInterface", - "Install" - ], - "scan-files": [ - "*.php", - "lib/**/.php", - "scripts/*.php", - "scripts/**/*.php", - "webroot/*.php" - ] -} diff --git a/.scrutinizer.yml b/.scrutinizer.yml deleted file mode 100644 index 08e4730..0000000 --- a/.scrutinizer.yml +++ /dev/null @@ -1,173 +0,0 @@ -#tools: -# external_code_coverage: -# timeout: 600 # Timeout in seconds. - -build: - dependencies: - override: - - composer self-update - - git clone --depth=1 https://github.com/galette/galette -b develop galette && pushd galette - - nodes: - - analysis: - environment: - php: - version: 8.2 - - project_setup: - override: - - 'true' - tests: - override: - - php-scrutinizer-run - - js-scrutinizer-run - - cache: - directories: - - vendor # Cache for already installed composer package -> speed up composer install - - galette/vendor # Same ^ - - bin # As vendor directory is cached, bin directory must be also cached (as some dependency will not installed if they are already present and so, related binary will not be linked) - - ~/.composer # Composer home directory (avoid fetching already fetched packages) -filter: - excluded_paths: - - 'galette/*' - - 'tests/*' - - 'bin/*' - - '*.min.js' - dependency_paths: - - "galette/vendor/" - - "vendor/" - -checks: - php: - fix_php_opening_tag: false - remove_php_closing_tag: false - one_class_per_file: false - side_effects_or_types: false - no_mixed_inline_html: false - require_braces_around_control_structures: false - php5_style_constructor: false - no_global_keyword: false - avoid_usage_of_logical_operators: false - psr2_class_declaration: false - no_underscore_prefix_in_properties: false - no_underscore_prefix_in_methods: false - blank_line_after_namespace_declaration: false - single_namespace_per_use: false - psr2_switch_declaration: false - psr2_control_structure_declaration: false - avoid_superglobals: false - security_vulnerabilities: false - no_exit: false - fix_use_statements: - remove_unused: true - preserve_multiple: false - preserve_blanklines: true - order_alphabetically: false - javascript: true - -coding_style: - php: - indentation: - general: - use_tabs: false - size: 4 - switch: - indent_case: true - spaces: - general: - linefeed_character: newline - before_parentheses: - function_declaration: false - closure_definition: true - function_call: false - if: true - for: true - while: true - switch: true - catch: true - array_initializer: false - around_operators: - assignment: true - logical: true - equality: true - relational: true - bitwise: true - additive: true - multiplicative: true - shift: true - unary_additive: false - concatenation: true - negation: false - before_left_brace: - class: true - function: true - if: true - else: true - for: true - while: true - do: true - switch: true - try: true - catch: true - finally: true - before_keywords: - else: true - while: true - catch: true - finally: true - within: - brackets: false - array_initializer: false - grouping: false - function_call: false - function_declaration: false - if: false - for: false - while: false - switch: false - catch: false - type_cast: false - ternary_operator: - before_condition: true - after_condition: true - before_alternative: true - after_alternative: true - in_short_version: false - other: - before_comma: false - after_comma: true - before_semicolon: false - after_semicolon: true - after_type_cast: false - braces: - classes_functions: - class: undefined - function: undefined - closure: undefined - if: - opening: undefined - always: true - else_on_new_line: false - for: - opening: undefined - always: true - while: - opening: undefined - always: true - do_while: - opening: undefined - always: true - while_on_new_line: false - switch: - opening: undefined - try: - opening: undefined - catch_on_new_line: false - finally_on_new_line: false - upper_lower_casing: - keywords: - general: undefined - constants: - true_false_null: undefined diff --git a/_routes.php b/_routes.php index 158349b..e408107 100644 --- a/_routes.php +++ b/_routes.php @@ -25,17 +25,6 @@ return $handler->handle($request); }; -/*$app->addMiddleware(function (\Psr\Http\Message\ServerRequestInterface $request, \Psr\Http\Server\RequestHandlerInterface $handler) use ($container) { - //check if JS has been generated - if (!file_exists(__DIR__ . '/webroot/maps-main.bundle.min.js')) { - $container->get('flash')->addMessageNow( - 'error_detected', - _T('Javascript libraries has not been built!', 'maps') - ); - } - return $handler->handle($request); -});*/ - $app->get( '/localize-member/{id:\d+}', [MapsController::class, 'localizeMember'] diff --git a/bin/release b/bin/release index 4ab39d5..5e2c359 100755 --- a/bin/release +++ b/bin/release @@ -410,13 +410,10 @@ def add_libs(rel_name, galette_archive): # development only, nothing a Galette installation needs — the npm sources # included, the build output is what ships todrop = [ - '.composer-require-checker.config.json', '.docheader', '.gitignore', '.php-cs-fixer.dist.php', '.phpcs.xml', - '.scrutinizer.yml', - 'composer.json.checker', 'gulpfile.js', 'package.json', 'package-lock.json', diff --git a/bin/twig-cache b/bin/twig-cache deleted file mode 100755 index 96a1eab..0000000 --- a/bin/twig-cache +++ /dev/null @@ -1,114 +0,0 @@ -#!/bin/php -isDir() && !$fpath->isLink()) ? rmdir($fpath->getPathname()) : unlink($fpath->getPathname()); - } - - rmdir($path); - return true; - } - return false; -} - -/** - * Return a custom Twig cache handler. - * This handler is useful to be able to preserve filenames of compiled files. - * - * @param string $directory - * - * @return CacheInterface - */ -function getTwigCacheHandler(string $directory): CacheInterface -{ - return new class($directory) extends FilesystemCache { - - private string $directory; - - public function __construct(string $directory, int $options = 0) - { - $this->directory = rtrim($directory, DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR; - parent::__construct($directory, $options); - } - - public function generateKey(string $name, string $className): string - { - return $this->directory . $name; - } - }; -} - -$directory = sprintf('%s/../templates/default', __DIR__); -$cache_dir = sprintf('%s/../tempcache', __DIR__); -$cache = getTwigCacheHandler($cache_dir); -if (file_exists($cache_dir)) { - rmdir_recursive($cache_dir); -} -mkdir($cache_dir); - -$iterator = new RecursiveIteratorIterator( - new RecursiveDirectoryIterator($directory), - RecursiveIteratorIterator::LEAVES_ONLY -); - -$loader = new FilesystemLoader($directory); -$twig = new Environment( - $loader, - [ - 'cache' => $cache, - 'auto_reload' => true, - ] -); - -$twig_functions = [ - '__', - '_T', - '_Tn', - '_Tx', - '_Tnx', - 'url_for', - 'memberName', - 'callstatic', - 'is_current_url', - 'get_class', - 'base_path', - 'file_exists' -]; - -foreach ($twig_functions as $function) { - $twig->addFunction(new TwigFunction($function, $function)); -} - -/** @var SplFileInfo $file */ -foreach ($iterator as $file) { - if ($file->isFile()) { - $twig->load(str_replace($directory . '/', '', $file)); - } -} diff --git a/lib/GaletteMaps/Controllers/MapsController.php b/lib/GaletteMaps/Controllers/MapsController.php index 4d1240f..eac9558 100644 --- a/lib/GaletteMaps/Controllers/MapsController.php +++ b/lib/GaletteMaps/Controllers/MapsController.php @@ -60,7 +60,6 @@ public function map(Request $request, Response $response): Response $list = $coords->listCoords(); $params = [ - 'require_dialog' => true, 'page_title' => _T('Maps', 'maps'), 'module_id' => $this->getModuleId(), 'tiles' => TileProviders::resolve($this->preferences) @@ -136,7 +135,6 @@ public function localizeMember(Request $request, Response $response, ?int $id = ), 'member' => $member, 'can_edit' => $can_edit, - 'require_dialog' => true, 'adh_map' => true, 'module_id' => $this->getModuleId(), 'tiles' => TileProviders::resolve($this->preferences) diff --git a/package-lock.json b/package-lock.json index 2b4faeb..02e610a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "plugin-maps", - "version": "2.0.0", + "version": "2.2.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "plugin-maps", - "version": "2.0.0", + "version": "2.2.1", "license": "GPL-3.0-or-later", "dependencies": { "@maplibre/maplibre-gl-leaflet": "^0.1.4", diff --git a/package.json b/package.json index 0b3be67..2b8d81f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "plugin-maps", - "version": "2.0.0", + "version": "2.2.1", "description": "A [Galette](https://galette.eu) plugin to display maps", "main": "index.js", "scripts": { @@ -9,7 +9,7 @@ }, "repository": { "type": "git", - "url": "ssh://git.tuxfamily.org/gitroot/galette/plugin-maps.git" + "url": "https://github.com/galette-plugins/plugin-maps.git" }, "author": "Johan Cwiklinski ", "license": "GPL-3.0-or-later", From a1586ea33ec5c485c03c94aa0ff1bc937b730bec Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Fri, 25 Sep 2026 07:36:22 +0200 Subject: [PATCH 2/8] Do not report storing an unchanged position as a failure --- lib/GaletteMaps/Coordinates.php | 4 +++- .../Controllers/tests/units/MapsController.php | 5 ++--- tests/GaletteMaps/tests/units/Coordinates.php | 11 +++++++++++ 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/GaletteMaps/Coordinates.php b/lib/GaletteMaps/Coordinates.php index 067ab5a..73bd25c 100644 --- a/lib/GaletteMaps/Coordinates.php +++ b/lib/GaletteMaps/Coordinates.php @@ -213,7 +213,9 @@ public function setCoords(int $id, float $latitude, float $longitude): bool )->where( [self::PK => $id] ); - $results = $zdb->execute($update); + //no row is affected when the position does not change + $zdb->execute($update); + return true; } return ($results->count() > 0); } catch (\Exception $e) { diff --git a/tests/GaletteMaps/Controllers/tests/units/MapsController.php b/tests/GaletteMaps/Controllers/tests/units/MapsController.php index 321d060..dc03275 100644 --- a/tests/GaletteMaps/Controllers/tests/units/MapsController.php +++ b/tests/GaletteMaps/Controllers/tests/units/MapsController.php @@ -126,9 +126,8 @@ public function testMemberChangesOwnCoords(): void $member_one = $this->getMemberOne(); $this->logMember($this->dataAdherentOne()); - //storing the same position again reports a failure, hence a different latitude - foreach ([[null, '50.362038'], [$member_one->id, '51.5']] as [$id_adh, $latitude]) { - $test_response = $this->postCoords($id_adh, ['latitude' => $latitude, 'longitude' => '3.472998']); + foreach ([null, $member_one->id] as $id_adh) { + $test_response = $this->postCoords($id_adh); $this->assertSame(200, $test_response->getStatusCode()); $this->assertSame( ['res' => true, 'message' => 'New coordinates has been stored!'], diff --git a/tests/GaletteMaps/tests/units/Coordinates.php b/tests/GaletteMaps/tests/units/Coordinates.php index 49d6a9d..f989816 100644 --- a/tests/GaletteMaps/tests/units/Coordinates.php +++ b/tests/GaletteMaps/tests/units/Coordinates.php @@ -75,4 +75,15 @@ public function testCoordinates(): void $this->assertTrue($coords->removeCoords($member->id)); $this->assertSame([], $coords->getCoords($member->id)); } + + /** + * Storing the same position again is not a failure + */ + 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)); + } } From 988f9951b600aef573750eb843957596dd157bbd Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Fri, 25 Sep 2026 07:36:22 +0200 Subject: [PATCH 3/8] Apply active status to the logged-in member own position on the map --- lib/GaletteMaps/Coordinates.php | 64 +++++-------------- tests/GaletteMaps/tests/units/Coordinates.php | 62 ++++++++++++++++++ 2 files changed, 77 insertions(+), 49 deletions(-) diff --git a/lib/GaletteMaps/Coordinates.php b/lib/GaletteMaps/Coordinates.php index 73bd25c..ce39d69 100644 --- a/lib/GaletteMaps/Coordinates.php +++ b/lib/GaletteMaps/Coordinates.php @@ -15,8 +15,6 @@ use Galette\Core\Db; use Galette\Entity\Adherent; use Laminas\Db\Sql\Expression; -use Laminas\Db\Sql\Predicate\PredicateSet; -use Laminas\Db\Sql\Predicate\Operator; /** * Members GPS coordinates @@ -87,60 +85,28 @@ public function listCoords(): array 'a.' . self::PK . '=' . 'c.' . self::PK, //only what the map displays ['nom_adh', 'prenom_adh', 'pseudo_adh', 'societe_adh'] - )->where->equalTo( - 'activite_adh', - new Expression('true') ); + $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 - $select->where( - [ - new PredicateSet( - [ - new Operator( - 'date_echeance', - '>=', - date('Y-m-d') - ), - new Operator( - 'bool_exempt_adh', - '=', - new Expression('true') - ) - ], - PredicateSet::OP_OR - ), - new PredicateSet( - [ - new Operator( - 'bool_display_info', - '=', - new Expression('true') - ) - ] - ) - ] - ); - - if ($login->isLogged() && !$login->isSuperAdmin()) { - $select->where( - new PredicateSet( - [ - new Operator( - 'a.' . Adherent::PK, - '=', - $login->id - ) - ] - ), - PredicateSet::OP_OR - ); + //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); @@ -243,7 +209,7 @@ public function removeCoords(int $id): bool return ($del->count() > 0); } catch (\Exception $e) { Analog::log( - 'Unable to set coordinates for member ' + 'Unable to remove coordinates for member ' . $id . ' | ' . $e->getMessage(), Analog::ERROR ); diff --git a/tests/GaletteMaps/tests/units/Coordinates.php b/tests/GaletteMaps/tests/units/Coordinates.php index f989816..3ec6a8d 100644 --- a/tests/GaletteMaps/tests/units/Coordinates.php +++ b/tests/GaletteMaps/tests/units/Coordinates.php @@ -86,4 +86,66 @@ public function testSetSamePosition(): void $this->assertTrue($coords->setCoords($member->id, 50.362038, 3.472998)); $this->assertTrue($coords->setCoords($member->id, 50.362038, 3.472998)); } + + /** + * Set member visibility fields + * + * @param int $id_adh Member ID + * @param bool $active Is member active + * @param bool $public Does member display its information + * @param bool $uptodate Is member up to date + */ + private function setVisibility(int $id_adh, bool $active, bool $public, bool $uptodate): void + { + $bool = fn(bool $value): \Laminas\Db\Sql\Expression => new \Laminas\Db\Sql\Expression($value ? 'true' : 'false'); + $update = $this->zdb->update(\Galette\Entity\Adherent::TABLE); + $update->set([ + 'activite_adh' => $bool($active), + 'bool_display_info' => $bool($public), + 'bool_exempt_adh' => $bool(false), + 'date_echeance' => $uptodate ? date('Y-m-d', strtotime('+1 month')) : date('Y-m-d', strtotime('-1 month')), + ])->where([\Galette\Entity\Adherent::PK => $id_adh]); + $this->zdb->execute($update); + } + + /** + * Get IDs of listed members + * + * @return array + */ + private function listedIds(): array + { + $ids = array_column((new \GaletteMaps\Coordinates())->listCoords(), 'id_adh'); + sort($ids); + return $ids; + } + + /** + * Visitors see public up-to-date profiles, members also see their own one, if active + */ + 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)); + + $this->setVisibility($member_one->id, active: true, public: false, uptodate: false); + $this->setVisibility($member_two->id, active: true, public: true, uptodate: true); + $this->assertSame([$member_two->id], $this->listedIds()); + + $this->assertTrue($this->login->login($this->dataAdherentOne()['login_adh'], $this->dataAdherentOne()['mdp_adh'])); + $expected = [$member_one->id, $member_two->id]; + sort($expected); + $this->assertSame($expected, $this->listedIds()); + + //an inactive member is never listed, not even to itself + $this->setVisibility($member_one->id, active: false, public: false, uptodate: false); + $this->assertSame([$member_two->id], $this->listedIds()); + + $this->setVisibility($member_two->id, active: false, public: true, uptodate: true); + $this->assertSame([], $this->listedIds()); + $this->login->logout(); + } } From ab3d779c1ee3986ec977d77071d4eba08ed17e46 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Fri, 25 Sep 2026 07:38:13 +0200 Subject: [PATCH 4/8] Show the map with an error message when coordinates cannot be loaded --- lib/GaletteMaps/Controllers/MapsController.php | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/GaletteMaps/Controllers/MapsController.php b/lib/GaletteMaps/Controllers/MapsController.php index eac9558..eba0346 100644 --- a/lib/GaletteMaps/Controllers/MapsController.php +++ b/lib/GaletteMaps/Controllers/MapsController.php @@ -56,19 +56,18 @@ private function getMemberDeps(): array */ public function map(Request $request, Response $response): Response { - $coords = new Coordinates(); - $list = $coords->listCoords(); - $params = [ 'page_title' => _T('Maps', 'maps'), 'module_id' => $this->getModuleId(), - 'tiles' => TileProviders::resolve($this->preferences) + 'tiles' => TileProviders::resolve($this->preferences), + 'list' => [] ]; - if ($list !== false) { - $params['list'] = $list; - } else { - $this->flash->addMessage( + try { + $params['list'] = (new Coordinates())->listCoords(); + } catch (\Throwable $e) { + //already logged + $this->flash->addMessageNow( 'error_detected', _T('Coordinates has not been loaded. Maybe plugin tables does not exists in the database?', 'maps') ); From 2be7d6ed73365d29e16cdbf93798cb4024d014aa Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Fri, 25 Sep 2026 07:38:13 +0200 Subject: [PATCH 5/8] Validate coordinates and answer errors with HTTP status codes --- .../Controllers/MapsController.php | 31 +++++++------ lib/GaletteMaps/Coordinates.php | 5 +- templates/default/common_scripts.html.twig | 7 +-- templates/default/mymap.html.twig | 4 +- .../tests/units/MapsController.php | 46 +++++++++++++++++++ 5 files changed, 71 insertions(+), 22 deletions(-) diff --git a/lib/GaletteMaps/Controllers/MapsController.php b/lib/GaletteMaps/Controllers/MapsController.php index eba0346..bc7f5f2 100644 --- a/lib/GaletteMaps/Controllers/MapsController.php +++ b/lib/GaletteMaps/Controllers/MapsController.php @@ -261,6 +261,7 @@ public function ILiveHere(Request $request, Response $response, ?int $id = null) Analog::INFO ); $error = _T('Superadmin cannot be localized.', 'maps'); + $status = 400; } else { $id ??= (int)$this->login->id; $member = new Adherent($this->zdb, $id, $this->getMemberDeps()); @@ -280,29 +281,29 @@ public function ILiveHere(Request $request, Response $response, ?int $id = null) $post = $request->getParsedBody(); $coords = new Coordinates(); if (isset($post['remove'])) { - $res = $coords->removeCoords($id); - if ($res > 0) { + if ($coords->removeCoords($id)) { $message = _T('Coordinates has been removed!', 'maps'); } else { $error = _T('Coordinates has not been removed :(', 'maps'); + $status = 500; } - } elseif ( - isset($post['latitude']) - && isset($post['longitude']) - ) { - $res = $coords->setCoords( - $id, - (float)$post['latitude'], - (float)$post['longitude'] - ); - - if ($res === true) { + } else { + $latitude = filter_var($post['latitude'] ?? null, FILTER_VALIDATE_FLOAT); + $longitude = filter_var($post['longitude'] ?? null, FILTER_VALIDATE_FLOAT); + if ( + $latitude === false + || $longitude === false + || abs($latitude) > 90 + || abs($longitude) > 180 + ) { + $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; } - } else { - $error = _T('Something went wrong :(', 'maps'); } } diff --git a/lib/GaletteMaps/Coordinates.php b/lib/GaletteMaps/Coordinates.php index ce39d69..aa414fa 100644 --- a/lib/GaletteMaps/Coordinates.php +++ b/lib/GaletteMaps/Coordinates.php @@ -205,8 +205,9 @@ public function removeCoords(int $id): bool try { $delete = $zdb->delete($this->getTableName()); $delete->where([self::PK => $id]); - $del = $zdb->execute($delete); - return ($del->count() > 0); + //removing nothing is not an error + $zdb->execute($delete); + return true; } catch (\Exception $e) { Analog::log( 'Unable to remove coordinates for member ' diff --git a/templates/default/common_scripts.html.twig b/templates/default/common_scripts.html.twig index de0affb..d13b7db 100644 --- a/templates/default/common_scripts.html.twig +++ b/templates/default/common_scripts.html.twig @@ -51,7 +51,8 @@ function _iLiveHere(_id){ $('#' + _id).click(function(e, f,g){ var _a = $(this); - var _latlng = _a.data('latlng'); + //a click on a copy of the world gives a longitude beyond 180 + var _latlng = _a.data('latlng').wrap(); $.ajax({ url: '{% if mymap is defined %}{{ url_for("maps_ilivehere") }}{% else %}{{ url_for("maps_ilivehere", {"id": member.id}) }}{% endif %}', type: 'POST', @@ -80,8 +81,8 @@ }, }).modal('show'); }, - error: function(){ - alert("{{ _T("An error occurred during 'I live here' process :(", "maps")|e("js") }}") + error: function(xhr){ + alert(xhr.responseJSON && xhr.responseJSON.message ? xhr.responseJSON.message : "{{ _T("An error occurred during 'I live here' process :(", "maps")|e("js") }}"); } }); return false; diff --git a/templates/default/mymap.html.twig b/templates/default/mymap.html.twig index a1e05a5..75f16be 100644 --- a/templates/default/mymap.html.twig +++ b/templates/default/mymap.html.twig @@ -134,8 +134,8 @@ }).modal('show'); }, "500") }, - error: function() { - window.alert("{{ _T("An error occurred removing coordinates :(", "maps")|e('js') }}"); + error: function(xhr) { + window.alert(xhr.responseJSON && xhr.responseJSON.message ? xhr.responseJSON.message : "{{ _T("An error occurred removing coordinates :(", "maps")|e('js') }}"); } }); } diff --git a/tests/GaletteMaps/Controllers/tests/units/MapsController.php b/tests/GaletteMaps/Controllers/tests/units/MapsController.php index dc03275..dab592d 100644 --- a/tests/GaletteMaps/Controllers/tests/units/MapsController.php +++ b/tests/GaletteMaps/Controllers/tests/units/MapsController.php @@ -155,6 +155,51 @@ public function testManagerChangesCoordsAsCoreAllows(): void $this->assertCount(3, (array)(new Coordinates())->getCoords($member_one->id)); } + /** + * Coordinates out of bounds, not numeric or missing are refused + */ + public function testInvalidCoords(): void + { + $member_one = $this->getMemberOne(); + $this->logMember($this->dataAdherentOne()); + + $invalid = [ + ['latitude' => '91', 'longitude' => '3'], + ['latitude' => '50', 'longitude' => '-180.5'], + ['latitude' => 'north', 'longitude' => '3'], + ['latitude' => '50'], + [], + ]; + foreach ($invalid as $data) { + $test_response = $this->postCoords(null, $data); + $this->assertSame(400, $test_response->getStatusCode(), print_r($data, true)); + $this->assertSame( + ['res' => false, 'message' => 'Invalid coordinates.'], + json_decode((string)$test_response->getBody(), true) + ); + } + $this->assertSame([], (new Coordinates())->getCoords($member_one->id)); + + //bounds are included + $test_response = $this->postCoords(null, ['latitude' => '-90', 'longitude' => '180']); + $this->assertSame(200, $test_response->getStatusCode()); + } + + /** + * Removing coordinates of a member that has none is not an error + */ + public function testRemoveMissingCoords(): void + { + $this->getMemberOne(); + $this->logMember($this->dataAdherentOne()); + $test_response = $this->postCoords(null, ['remove' => '1']); + $this->assertSame(200, $test_response->getStatusCode()); + $this->assertSame( + ['res' => true, 'message' => 'Coordinates has been removed!'], + json_decode((string)$test_response->getBody(), true) + ); + } + /** * A member cannot display coordinates of another member */ @@ -248,6 +293,7 @@ public function testSuperAdminCoords(): void $this->assertCount(3, (array)(new Coordinates())->getCoords($member_one->id)); $test_response = $this->postCoords(null); + $this->assertSame(400, $test_response->getStatusCode()); $this->assertSame( ['res' => false, 'message' => 'Superadmin cannot be localized.'], json_decode((string)$test_response->getBody(), true) From 62012d8ed61f436c47058b1c512c1b5f8141012b Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Fri, 25 Sep 2026 07:40:35 +0200 Subject: [PATCH 6/8] Align test docblock parameters --- tests/GaletteMaps/tests/units/Coordinates.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/GaletteMaps/tests/units/Coordinates.php b/tests/GaletteMaps/tests/units/Coordinates.php index 3ec6a8d..2c88d5f 100644 --- a/tests/GaletteMaps/tests/units/Coordinates.php +++ b/tests/GaletteMaps/tests/units/Coordinates.php @@ -90,9 +90,9 @@ public function testSetSamePosition(): void /** * Set member visibility fields * - * @param int $id_adh Member ID - * @param bool $active Is member active - * @param bool $public Does member display its information + * @param int $id_adh Member ID + * @param bool $active Is member active + * @param bool $public Does member display its information * @param bool $uptodate Is member up to date */ private function setVisibility(int $id_adh, bool $active, bool $public, bool $uptodate): void From e70549646ff673080853da498c62c716594c923f Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Fri, 25 Sep 2026 07:40:35 +0200 Subject: [PATCH 7/8] Query Nominatim over HTTPS with a timeout, without failing the page --- .../Controllers/MapsController.php | 16 +- lib/GaletteMaps/NominatimTowns.php | 146 ++++++++---------- .../tests/units/MapsController.php | 25 +++ .../tests/units/NominatimTowns.php | 110 +++++++++++++ 4 files changed, 212 insertions(+), 85 deletions(-) create mode 100644 tests/GaletteMaps/tests/units/NominatimTowns.php diff --git a/lib/GaletteMaps/Controllers/MapsController.php b/lib/GaletteMaps/Controllers/MapsController.php index bc7f5f2..804b01c 100644 --- a/lib/GaletteMaps/Controllers/MapsController.php +++ b/lib/GaletteMaps/Controllers/MapsController.php @@ -116,13 +116,21 @@ public function localizeMember(Request $request, Response $response, ?int $id = $towns = false; //towns are only proposed to choose a location - if ($can_edit && count($mcoords) === 0) { - if ($member->town != '') { - $t = new NominatimTowns($this->preferences); - $towns = $t->search( + if ($can_edit && count($mcoords) === 0 && trim($member->town ?? '') !== '') { + try { + $towns = (new NominatimTowns($this->preferences))->search( $member->town, $member->country ); + } catch (\RuntimeException $e) { + Analog::log( + 'Unable to search towns for member #' . $member->id . ' | ' . $e->getMessage(), + Analog::WARNING + ); + $this->flash->addMessageNow( + 'warning_detected', + _T('Town search is not available for now, you can still search or click on the map.', 'maps') + ); } } diff --git a/lib/GaletteMaps/NominatimTowns.php b/lib/GaletteMaps/NominatimTowns.php index 333d4d1..5847e72 100644 --- a/lib/GaletteMaps/NominatimTowns.php +++ b/lib/GaletteMaps/NominatimTowns.php @@ -12,6 +12,9 @@ use Analog\Analog; use Galette\Core\Preferences; +use GuzzleHttp\Client; +use GuzzleHttp\ClientInterface; +use GuzzleHttp\Exception\GuzzleException; /** * Towns GPS coordinates via nominatim @@ -21,23 +24,22 @@ class NominatimTowns { - private Preferences $preferences; + private const string URI = 'https://nominatim.openstreetmap.org/search'; - /** @var array */ - private array $query_options = [ - 'format' => 'xml', - 'addressdetails' => '1' - ]; - private string $uri = 'http://nominatim.openstreetmap.org/search'; + private Preferences $preferences; + private ClientInterface $client; /** * Constructor * - * @param Preferences $preferences Preferences instance + * @param Preferences $preferences Preferences instance + * @param ?ClientInterface $client HTTP client, a default one is built if null */ - public function __construct(Preferences $preferences) + public function __construct(Preferences $preferences, ?ClientInterface $client = null) { $this->preferences = $preferences; + //a slow answer must not hold the page + $this->client = $client ?? new Client(['timeout' => 5.0, 'connect_timeout' => 2.0]); } /** @@ -47,96 +49,78 @@ public function __construct(Preferences $preferences) * @param ?string $country Country name (optional) * * @return array> + * + * @throws \RuntimeException when the service cannot be queried */ public function search(string $town, ?string $country = null): array { - if (!$town || trim($town) === '') { - throw new \RuntimeException( - "Town has not been specified!" - ); + if (trim($town) === '') { + return []; } - $options = $this->query_options; - $options['city'] = $town; - if ($country !== null) { - $options['country'] = $country; + $query = [ + 'format' => 'jsonv2', + 'addressdetails' => '1', + 'city' => $town + ]; + if ($country !== null && trim($country) !== '') { + $query['country'] = $country; } - $url_options = []; - foreach ($options as $key => $value) { - $url_options[] = $key . '=' . urlencode($value); - } - - $url = $this->uri . '?' . implode('&', $url_options); - $ch = curl_init($url); - curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1); - curl_setopt($ch, CURLOPT_FOLLOWLOCATION, true); - curl_setopt($ch, CURLOPT_USERAGENT, 'GaletteMaps/' . $this->preferences->pref_nom); - - $response = curl_exec($ch); - if ($response === false) { - throw new \RuntimeException( - "Error on nominatim request:\n\tURI:" . $url - . "\n\tOptions:\n" . print_r($options, true) + try { + $response = $this->client->request( + 'GET', + self::URI, + [ + 'query' => $query, + 'headers' => [ + //usage policy requires to identify the application + 'User-Agent' => sprintf( + 'GaletteMaps (%s; %s)', + $this->preferences->pref_nom, + $this->preferences->getURL() + ) + ] + ] ); - } - - //get request infos - $infos = curl_getinfo($ch); - if ($infos['http_code'] !== 200) { - //At this point, core has been created, but is failing - //to load in solr. + $places = json_decode((string)$response->getBody(), true, flags: JSON_THROW_ON_ERROR); + } catch (GuzzleException|\JsonException $e) { throw new \RuntimeException( - "Error on nominatim:\n\tURI: " . $url - . "\n\Options: " . print_r($options, true) + 'Error on nominatim request for "' . $town . '": ' . $e->getMessage(), + previous: $e ); } - $xml = new \SimpleXMLElement($response); - $towns = $xml->xpath('//place'); + if (!is_array($places)) { + throw new \RuntimeException('Unexpected nominatim answer for "' . $town . '"'); + } $results = []; - foreach ($towns as $town) { - if ($town->city || $town->town || $town->village) { - $unique = true; - foreach ($results as $elt) { - if ( - $elt['latitude'] == (string)$town['lat'] - && $elt['longitude'] == (string)$town['lon'] - ) { - $unique = false; - Analog::log( - 'Town is already in list, ignore.', - Analog::INFO - ); - } - } - - if ($unique === true) { - $full_name = null; - if ($town->city) { - $full_name = (string)$town->city; - } elseif ($town->town) { - $full_name = (string)$town->town; - } elseif ($town->village) { - $full_name = (string)$town->village; - } else { - $full_name = (string)$town['display_name']; - } - - $results[] = [ - 'full_name' => $full_name, - 'latitude' => (string)$town['lat'], - 'longitude' => (string)$town['lon'] - ]; - } - } else { + foreach ($places as $place) { + $address = $place['address'] ?? []; + $full_name = $address['city'] ?? $address['town'] ?? $address['village'] ?? null; + if ($full_name === null) { Analog::log( - 'Nominatim result "' . $town['display_name'] - . '" is not a town', + 'Nominatim result "' . ($place['display_name'] ?? '') . '" is not a town', Analog::INFO ); + continue; } + + $latitude = (string)$place['lat']; + $longitude = (string)$place['lon']; + foreach ($results as $elt) { + if ($elt['latitude'] === $latitude && $elt['longitude'] === $longitude) { + Analog::log('Town is already in list, ignore.', Analog::INFO); + continue 2; + } + } + + $results[] = [ + 'full_name' => (string)$full_name, + 'latitude' => $latitude, + 'longitude' => $longitude + ]; } return $results; diff --git a/tests/GaletteMaps/Controllers/tests/units/MapsController.php b/tests/GaletteMaps/Controllers/tests/units/MapsController.php index dab592d..a7596b1 100644 --- a/tests/GaletteMaps/Controllers/tests/units/MapsController.php +++ b/tests/GaletteMaps/Controllers/tests/units/MapsController.php @@ -200,6 +200,31 @@ public function testRemoveMissingCoords(): void ); } + /** + * Unreachable towns search does not prevent to display the map + */ + public function testTownsSearchUnavailable(): void + { + $member_one = $this->getMemberOne(); + $this->assertNotEmpty($member_one->town); + $this->logMember($this->dataAdherentOne()); + + //no proxy listens there: the request fails at once + putenv('HTTPS_PROXY=http://127.0.0.1:1'); + try { + $test_response = $this->app->handle($this->createRequest('maps_mymap')); + } finally { + putenv('HTTPS_PROXY'); + } + $this->assertSame(200, $test_response->getStatusCode()); + $this->assertStringContainsString( + 'Town search is not available for now, you can still search or click on the map.', + (string)$test_response->getBody() + ); + $this->expectLogEntry(Analog::WARNING, 'Unable to search towns for member #' . $member_one->id); + $this->expectNoLogEntry(); + } + /** * A member cannot display coordinates of another member */ diff --git a/tests/GaletteMaps/tests/units/NominatimTowns.php b/tests/GaletteMaps/tests/units/NominatimTowns.php new file mode 100644 index 0000000..8e83743 --- /dev/null +++ b/tests/GaletteMaps/tests/units/NominatimTowns.php @@ -0,0 +1,110 @@ + + */ +class NominatimTowns extends GaletteTestCase +{ + /** @var array> */ + private array $requests = []; + + /** + * Get a search instance answering given responses + * + * @param array $responses Responses + */ + private function getSearch(array $responses): \GaletteMaps\NominatimTowns + { + $this->requests = []; + $stack = HandlerStack::create(new MockHandler($responses)); + $stack->push(Middleware::history($this->requests)); + return new \GaletteMaps\NominatimTowns($this->preferences, new Client(['handler' => $stack])); + } + + /** + * Towns are extracted from the answer, other places and duplicates are ignored + */ + public function testSearch(): void + { + $places = [ + ['lat' => '50.3620', 'lon' => '3.4729', 'display_name' => 'Valenciennes, Nord', 'address' => ['city' => 'Valenciennes']], + ['lat' => '50.3620', 'lon' => '3.4729', 'display_name' => 'Valenciennes again', 'address' => ['town' => 'Valenciennes']], + ['lat' => '45.1', 'lon' => '1.2', 'display_name' => 'Somewhere', 'address' => ['village' => 'Petit Valenciennes']], + ['lat' => '44.0', 'lon' => '2.0', 'display_name' => 'Rue de Valenciennes', 'address' => ['road' => 'Rue de Valenciennes']], + ]; + $search = $this->getSearch([new Response(200, [], (string)json_encode($places))]); + + $this->assertSame( + [ + ['full_name' => 'Valenciennes', 'latitude' => '50.3620', 'longitude' => '3.4729'], + ['full_name' => 'Petit Valenciennes', 'latitude' => '45.1', 'longitude' => '1.2'], + ], + $search->search('Valenciennes', 'France') + ); + $this->expectLogEntry(\Analog::INFO, 'Town is already in list, ignore.'); + $this->expectLogEntry(\Analog::INFO, 'Nominatim result "Rue de Valenciennes" is not a town'); + + $this->assertCount(1, $this->requests); + /** @var Request $request */ + $request = $this->requests[0]['request']; + $this->assertSame('https', $request->getUri()->getScheme()); + $this->assertSame('nominatim.openstreetmap.org', $request->getUri()->getHost()); + parse_str($request->getUri()->getQuery(), $query); + $this->assertSame( + ['format' => 'jsonv2', 'addressdetails' => '1', 'city' => 'Valenciennes', 'country' => 'France'], + $query + ); + $this->assertStringStartsWith('GaletteMaps (', $request->getHeaderLine('User-Agent')); + } + + /** + * An empty town does not query the service + */ + public function testEmptyTown(): void + { + $search = $this->getSearch([]); + $this->assertSame([], $search->search(' ')); + $this->assertCount(0, $this->requests); + } + + /** + * Service failures are reported with an exception + */ + public function testFailures(): void + { + $failures = [ + new Response(503), + new Response(200, [], 'not json'), + new \GuzzleHttp\Exception\ConnectException('Timeout', new Request('GET', 'https://nominatim.openstreetmap.org')), + ]; + foreach ($failures as $failure) { + $search = $this->getSearch([$failure]); + try { + $search->search('Valenciennes'); + $this->fail('An exception was expected'); + } catch (\RuntimeException $e) { + $this->assertStringStartsWith('Error on nominatim request for "Valenciennes"', $e->getMessage()); + } + } + } +} From 2946679bd1ebf6739a05e1131da6efe86d9238af Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Fri, 25 Sep 2026 08:30:58 +0200 Subject: [PATCH 8/8] Keep spaces around multiple catch types, as phpcs expects --- .php-cs-fixer.dist.php | 2 ++ lib/GaletteMaps/NominatimTowns.php | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index 1c9b4c3..7cb77df 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -18,6 +18,8 @@ 'cast_spaces' => ['space' => 'none'], 'single_line_empty_body' => false, 'no_unused_imports' => true, + // spaces around multiple catch types only, as phpcs expects - https://mlocati.github.io/php-cs-fixer-configurator/#version:3.90|fixer:types_spaces + 'types_spaces' => ['space' => 'none', 'space_multiple_catch' => 'single'], // rules for phpdoc // Removes @param, @return and @var tags that don't provide any useful information - https://mlocati.github.io/php-cs-fixer-configurator/#version:3.90|fixer:no_superfluous_phpdoc_tags 'no_superfluous_phpdoc_tags' => [ diff --git a/lib/GaletteMaps/NominatimTowns.php b/lib/GaletteMaps/NominatimTowns.php index 5847e72..278b452 100644 --- a/lib/GaletteMaps/NominatimTowns.php +++ b/lib/GaletteMaps/NominatimTowns.php @@ -84,7 +84,7 @@ public function search(string $town, ?string $country = null): array ] ); $places = json_decode((string)$response->getBody(), true, flags: JSON_THROW_ON_ERROR); - } catch (GuzzleException|\JsonException $e) { + } catch (GuzzleException | \JsonException $e) { throw new \RuntimeException( 'Error on nominatim request for "' . $town . '": ' . $e->getMessage(), previous: $e