From 66fb2ef4d7930b60a2dbafe6830887a4087731d8 Mon Sep 17 00:00:00 2001 From: edwh Date: Tue, 30 Jun 2026 22:34:16 +0100 Subject: [PATCH 01/34] RES-1995: map of groups (on Laravel 10) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reworks the groups page to show a map of groups, split out from PR #862 so it lands on the current Laravel 10 develop (the Laravel 13 upgrade is a separate PR). - New Vue components: GroupMap, GroupMarker, GroupMapAndList, GroupInfoModal - GroupsPage/GroupsTable/GroupsRequiringModeration reworked for map + filter bar - store/groups.js: group-summary fetch for the map; dedup - API: /api/v2/groups/summary endpoint + lat/lng on the groups list, GroupSummary resource - Group filter-bar translation keys across locales; leaflet-control-geocoder dep - Jest: store root + leaflet/vue-awesome mocks; new component+store tests Stays on Laravel 10 — no composer/framework/docker changes. Co-Authored-By: Claude Opus 4.8 --- app/Http/Controllers/API/GroupController.php | 73 +++- app/Http/Controllers/GroupController.php | 40 +- app/Http/Resources/GroupSummary.php | 75 +++- jest.config.json | 7 +- lang/de/groups.php | 7 +- lang/en/groups.php | 23 +- lang/en/networks.php | 2 - lang/fr-BE/groups.php | 21 +- lang/fr-BE/networks.php | 2 - lang/fr/groups.php | 21 +- lang/fr/networks.php | 2 - lang/it/groups.php | 7 +- lang/ne/groups.php | 7 +- lang/nl-BE/groups.php | 7 +- lang/no/groups.php | 7 +- package-lock.json | 15 + package.json | 1 + resources/global/css/_global.scss | 16 + resources/js/app.js | 8 +- resources/js/components/GroupInfoModal.vue | 93 +++++ resources/js/components/GroupMap.test.js | 129 ++++++ resources/js/components/GroupMap.vue | 369 ++++++++++++++++++ resources/js/components/GroupMapAndList.vue | 137 +++++++ resources/js/components/GroupMarker.vue | 80 ++++ resources/js/components/GroupsPage.test.js | 82 ++++ resources/js/components/GroupsPage.vue | 144 +++---- .../GroupsRequiringModeration.test.js | 103 +++++ .../components/GroupsRequiringModeration.vue | 17 +- resources/js/components/GroupsTable.test.js | 70 ++++ resources/js/components/GroupsTable.vue | 247 ++++++------ resources/js/constants.js | 5 +- resources/js/store/groups.js | 59 ++- resources/js/store/groups.test.js | 98 +++++ resources/views/group/index.blade.php | 6 +- routes/api.php | 1 + routes/web.php | 1 + tests/Feature/Groups/BasicTest.php | 12 +- tests/Feature/Groups/GroupViewTest.php | 26 +- tests/Feature/Networks/NetworkTest.php | 7 +- tests/Unit/GroupFilterTranslationsTest.php | 44 +++ tests/__mocks__/leaflet-control-geocoder.js | 10 + tests/__mocks__/vue-awesome.js | 4 + 42 files changed, 1740 insertions(+), 345 deletions(-) create mode 100644 resources/js/components/GroupInfoModal.vue create mode 100644 resources/js/components/GroupMap.test.js create mode 100644 resources/js/components/GroupMap.vue create mode 100644 resources/js/components/GroupMapAndList.vue create mode 100644 resources/js/components/GroupMarker.vue create mode 100644 resources/js/components/GroupsPage.test.js create mode 100644 resources/js/components/GroupsRequiringModeration.test.js create mode 100644 resources/js/components/GroupsTable.test.js create mode 100644 resources/js/store/groups.test.js create mode 100644 tests/Unit/GroupFilterTranslationsTest.php create mode 100644 tests/__mocks__/leaflet-control-geocoder.js create mode 100644 tests/__mocks__/vue-awesome.js diff --git a/app/Http/Controllers/API/GroupController.php b/app/Http/Controllers/API/GroupController.php index c4b5abfc21..61cb6197f9 100644 --- a/app/Http/Controllers/API/GroupController.php +++ b/app/Http/Controllers/API/GroupController.php @@ -269,8 +269,8 @@ public static function listNamesv2(Request $request) { 'includeArchived' => ['string', 'in:true,false'], ]); - // We only return the group id and name, for speed. - $query = Group::select('idgroups', 'name', 'archived_at'); + // We only return a small number of attributes, for speed. + $query = Group::select('idgroups', 'name', 'latitude', 'longitude', 'archived_at'); if (!$request->has('includeArchived') || $request->get('includeArchived') == 'false') { $query = $query->whereNull('archived_at'); @@ -283,6 +283,8 @@ public static function listNamesv2(Request $request) { $ret[] = [ 'id' => $group->idgroups, 'name' => $group->name, + 'lat' => $group->latitude, + 'lng' => $group->longitude, 'archived_at' => $group->archived_at ? Carbon::parse($group->archived_at)->toIso8601String() : null ]; } @@ -292,6 +294,73 @@ public static function listNamesv2(Request $request) { ]; } + /** + * @OA\Get( + * path="/api/v2/groups/summary", + * operationId="getGroupSummariesv2", + * tags={"Groups"}, + * summary="Get list of groups with summary information", + * @OA\Parameter( + * name="archived", + * description="Include archived groups", + * required=false, + * in="path", + * @OA\Schema( + * type="boolean" + * ) + * ), + * @OA\Parameter( + * name="includeNextEvent", + * description="Include the next event for the group. This makes the call slower. Default false.", + * required=false, + * in="query", + * @OA\Schema( + * type="boolean" + * ) + * ), + * @OA\Parameter( + * name="includeCounts", + * description="Include the counts of hosts and restarters. This makes the call slower. Default false.", + * required=false, + * in="query", + * @OA\Schema( + * type="boolean" + * ) + * ), + * @OA\Response( + * response=200, + * description="Successful operation", + * @OA\JsonContent( + * @OA\Property( + * property="data", + * title="data", + * description="An array of events", + * type="array", + * @OA\Items( + * @OA\Schema( + * ref="#/components/schemas/GroupSummary" + * ), + * ) + * ) + * ) + * ), + * ) + */ + + public static function listSummaryv2(Request $request) { + $request->validate([ + 'archived' => ['string', 'in:true,false'], + ]); + + $query = Group::all(); + + $groups = $query->all(); + + return [ + 'data' => \App\Http\Resources\GroupSummaryCollection::make($groups) + ]; + } + /** * @OA\Get( * path="/api/v2/groups/tags", diff --git a/app/Http/Controllers/GroupController.php b/app/Http/Controllers/GroupController.php index 7ae9c17d39..7d742b4000 100644 --- a/app/Http/Controllers/GroupController.php +++ b/app/Http/Controllers/GroupController.php @@ -64,7 +64,7 @@ private function indexVariations($tab, $network) // Look for groups we have joined, not just been invited to. We have to explicitly test on deleted_at because // the normal filtering out of soft deletes won't happen for joins. - $your_groups =array_column(Group::with(['networks']) + $your_groups = array_column(Group::with(['networks']) ->join('users_groups', 'users_groups.group', '=', 'groups.idgroups') ->leftJoin('events', 'events.group', '=', 'groups.idgroups') ->where('users_groups.user', $user->id) @@ -75,13 +75,40 @@ private function indexVariations($tab, $network) ->get() ->toArray(), 'idgroups'); - // We pass a high limit to the groups nearby; there is a distance limit which will normally kick in first. - $groups_near_you = array_column($user->groupsNearby(1000), 'idgroups'); + $nearby_groups = []; + $min_lat = 90; + $max_lat = -90; + $min_lng = 180; + $max_lng = -180; + + if ($user->latitude || $user->longitude || $user->country_code) { + // We pass a high limit to the groups nearby; there is a distance limit which will normally kick in first. + $nearby_groups = $user->groupsNearby(1000); + + // Now find the lat/lng bounding box which contains these groups. + foreach ($nearby_groups as $group) { + if ($group->latitude < $min_lat) { + $min_lat = $group->latitude; + } + if ($group->latitude > $max_lat) { + $max_lat = $group->latitude; + } + if ($group->longitude < $min_lng) { + $min_lng = $group->longitude; + } + if ($group->longitude > $max_lng) { + $max_lng = $group->longitude; + } + } + } return view('group.index', [ - 'groups' => GroupController::expandGroups($groups, $your_groups, $groups_near_you), + 'your_groups' => $your_groups, + 'nearby_groups' => [ [ $min_lat, $min_lng ], [ $max_lat, $max_lng ] ], 'your_area' => $user->location, - 'tab' => $tab, + 'your_lat' => $user->latitude, + 'your_lng' => $user->longitude, + 'tab' => (!$tab || $tab === 'mine') ? 'mine' : 'other', 'network' => $network, 'networks' => $networks, 'all_group_tags' => $all_group_tags, @@ -481,7 +508,7 @@ public function delete($id): RedirectResponse } } - public static function expandGroups($groups, $your_groupids, $nearby_groupids) + public static function expandGroups($groups, $your_groupids) { $ret = []; $user = Auth::user(); @@ -534,7 +561,6 @@ public static function expandGroups($groups, $your_groupids, $nearby_groupids) ]; }), 'following' => in_array($group->idgroups, $your_groupids), - 'nearby' => in_array($group->idgroups, $nearby_groupids), 'archived_at' => $group->archived_at ? Carbon::parse($group->archived_at)->toIso8601String() : null ]; } diff --git a/app/Http/Resources/GroupSummary.php b/app/Http/Resources/GroupSummary.php index b4af5cf306..224a56f088 100644 --- a/app/Http/Resources/GroupSummary.php +++ b/app/Http/Resources/GroupSummary.php @@ -5,6 +5,7 @@ use Illuminate\Http\Request; use Carbon\Carbon; use Illuminate\Http\Resources\Json\JsonResource; +use Cache; /** * @OA\Schema( @@ -63,6 +64,18 @@ * ref="#/components/schemas/EventSummary" * ), * @OA\Property( + * property="hosts", + * title="hosts", + * description="The number of hosts of this group (if requested via API call flag).", + * type="number", + * ), + * @OA\Property( + * property="restarters", + * title="hosts", + * description="The number of restarters in this group (if requested via API call flag).", + * type="number", + * ), + * @OA\Property( * property="summary", * title="summary", * description="Indicates that this is a summary result, not full group information.", @@ -96,27 +109,53 @@ public function toArray(Request $request): array 'summary' => true ]; + if ($request->get('includeCounts', false)) { + $ret['hosts'] = $this->resource->all_confirmed_hosts_count; + $ret['restarters'] = $this->resource->all_confirmed_restarters_count; + } + if ($request->get('includeNextEvent', false)) { - // Get next approved event for group. - $nextevent = \App\Group::find($this->idgroups)->getNextUpcomingEvent(); + // Get next approved event for group. We cache all upcoming events to speed up the case where we + // are fetching many groups. + if (Cache::has('future_events')) { + $upcoming = Cache::get('future_events'); + } else { + $future = \App\Party::future()->get(); - if ($nextevent) { - // Using the resource for the nested event causes infinite loops. Just add the model attributes we - // need directly. - $ret['next_event'] = [ - 'id' => $nextevent->idevents, - 'start' => $nextevent->event_start_utc, - 'end' => $nextevent->event_end_utc, - 'timezone' => $nextevent->timezone, - 'title' => $nextevent->venue ?? $nextevent->location, - 'location' => $nextevent->location, - 'online' => $nextevent->online, - 'lat' => $nextevent->latitude, - 'lng' => $nextevent->longitude, - 'updated_at' => $nextevent->updated_at->toIso8601String(), - 'summary' => true - ]; + // Can't serialise the whole event, and we only need a few fields. + $upcoming = []; + + foreach ($future as $event) { + $upcoming[] = [ + 'id' => $event->idevents, + 'group_id' => $event->group, + 'start' => $event->event_start_utc, + 'end' => $event->event_end_utc, + 'timezone' => $event->timezone, + 'title' => $event->venue ?? $event->location, + 'location' => $event->location, + 'online' => $event->online, + 'lat' => $event->latitude, + 'lng' => $event->longitude, + 'updated_at' => $event->updated_at->toIso8601String(), + 'summary' => true + ]; + } + + Cache::put('future_events', $upcoming, 60); + } + + // Find the next event for this group. + $nextevent = null; + + foreach ($upcoming as $event) { + if ($event['group_id'] == $this->idgroups) { + $nextevent = $event; + break; + } } + + $ret['next_event'] = $nextevent; } return($ret); diff --git a/jest.config.json b/jest.config.json index 92ccff1a17..3bec585c1c 100644 --- a/jest.config.json +++ b/jest.config.json @@ -12,7 +12,8 @@ }, "roots": [ "/resources/js/components", - "/resources/js/misc" + "/resources/js/misc", + "/resources/js/store" ], "modulePaths": [ "" @@ -23,6 +24,8 @@ "setupFilesAfterEnv": ["/tests/jest.setup.js"], "testEnvironment": "jsdom", "moduleNameMapper": { - "^resources/js/mixins/lang.js$": "/tests/__mocks__/resources/js/mixins/lang.js" + "^resources/js/mixins/lang.js$": "/tests/__mocks__/resources/js/mixins/lang.js", + "^leaflet-control-geocoder/.*$": "/tests/__mocks__/leaflet-control-geocoder.js", + "^vue-awesome/.*$": "/tests/__mocks__/vue-awesome.js" } } \ No newline at end of file diff --git a/lang/de/groups.php b/lang/de/groups.php index 1629d89db6..0b3eea052d 100644 --- a/lang/de/groups.php +++ b/lang/de/groups.php @@ -3,14 +3,12 @@ return [ 'group' => 'Group', 'groups' => 'Groups', - 'all_groups' => 'All groups', - 'search_name' => 'Search name', 'add_groups' => 'Add a group', 'add_groups_content' => 'Tell us more about your group, so we can create a page for you and help you publicise to potential volunteers and participants.', 'create_groups' => 'Create new group', 'create_group' => 'Create group', 'groups_title1' => 'Your groups', - 'groups_title2' => 'Groups nearest to you', + 'groups_title2' => 'Other groups', 'groups_name' => 'Name', 'groups_name_of' => 'Name of group', 'groups_about_group' => 'Tell us about your group', @@ -51,4 +49,7 @@ 'tag-6' => 'Tag 6', 'tag-7' => 'Tag 7', 'tag-8' => 'Tag 8', + 'marker_title' => 'Click for more information', + 'goto_group' => 'Go to group', + 'next_event' => 'Next event', ]; diff --git a/lang/en/groups.php b/lang/en/groups.php index 4071759763..ceed335efb 100644 --- a/lang/en/groups.php +++ b/lang/en/groups.php @@ -5,16 +5,14 @@ 'headline_stats_dropdown' => 'Headline stats', 'co2_equivalence_visualisation_dropdown' => 'CO2 equivalence visualisation', 'group_admin_only' => 'Admin only', - 'search_name' => 'Search name', 'group' => 'Group', 'groups' => 'Groups', - 'all_groups' => 'All Groups', 'add_groups' => 'Add a group', 'add_groups_content' => 'Tell us more about your group, so we can create a page for you and help you publicise to potential volunteers and participants.', 'create_groups' => 'Add a new group', 'create_group' => 'Create group', 'groups_title1' => 'Your Groups', - 'groups_title2' => 'Other groups nearby', + 'groups_title2' => 'Other groups', 'groups_name' => 'Name', 'groups_name_of' => 'Name of group', 'groups_about_group' => 'Tell us about your group', @@ -104,7 +102,6 @@ 'repairable_items' => 'Repairable items', 'end_of_life_items' => 'End-of-life items', 'no_unpowered_stats' => 'At the moment, these stats are only displayed for powered items. We hope to include unpowered items soon.', - 'all_groups_mobile' => 'All', 'create_groups_mobile2' => 'Add new', 'groups_title1_mobile' => 'Yours', 'groups_title2_mobile' => 'Nearest', @@ -112,13 +109,13 @@ 'no_groups_mine' => 'If you can\'t see any here yet, why not follow your nearest group to hear about their upcoming repair events?', 'no_groups_nearest_no_location' => '

You do not currently have a town/city set. You can set one in your profile.

You can also view all groups.

', 'no_groups_nearest_with_location' => '

There are no groups within 50 km of your location. You can see all groups here. Or why not start your own? Learn what running your own repair event involves.

', - 'group_count' => 'There is :count group.|There are :count groups.', - 'search_name_placeholder' => 'Search name...', - 'search_location_placeholder' => 'Search location...', - 'search_country_placeholder' => 'Country...', - 'search_tags_placeholder' => 'Tag', - 'show_filters' => 'Show Filters', - 'hide_filters' => 'Hide Filters', + 'group_count' => 'There is :count group. Zoom out to see more.|There are :count groups. Zoom out to see more.', + 'search_name_placeholder' => 'Search by name', + 'search_location_placeholder' => 'Search by location', + 'search_country_placeholder' => 'Filter by country', + 'search_tags_placeholder' => 'Filter by tags', + 'show_filters' => 'Show filters', + 'hide_filters' => 'Hide filters', 'leave_group_button' => 'Unfollow group', 'leave_group_button_mobile' => 'Unfollow', 'leave_group_confirm' => 'Please confirm that you want to unfollow this group.', @@ -141,7 +138,6 @@ 'archive_group_confirm' => 'Please confirm that you want to archive :name.', 'delete_succeeded' => 'Group :name has been deleted.', 'nearest_groups' => 'These are the groups that are within 50 km of :location', - 'nearest_groups_change' => '(change)', 'invitation_pending' => 'You have an invitation to this group. Please click here if you would like to join.', 'geocode_failed' => 'Location not found. If you are unable to find the location of your group, please try a more general location (such as village/town), or a specific street address, rather than a building name.', 'discourse_title' => 'This is a discussion group for anyone who follows :group. @@ -180,4 +176,7 @@ 'export.events.items_end_of_life' => 'Items end-of-life', 'export.events.items_kg_waste_prevented' => 'kg waste prevented', 'export.events.items_kg_co2_prevent' => 'kg CO2 prevented', + 'marker_title' => 'Click for more information', + 'goto_group' => 'Go to group', + 'next_event' => 'Next event', ]; diff --git a/lang/en/networks.php b/lang/en/networks.php index e3b20c0207..e0e1f4f27f 100644 --- a/lang/en/networks.php +++ b/lang/en/networks.php @@ -53,8 +53,6 @@ 'add_groups_warning_none_selected' => 'No groups selected.', 'add_groups_success' => '{1} :number group added.|[2,*] :number groups added.', 'view_groups_menuitem' => 'View groups', - 'groups_count' => '{0} There are currently no groups in the :name network.|{1} There is currently :count group in the :name network.|[2,*] There are currently :count groups in the :name network.', - 'view_groups_link' => 'View these groups.', 'none' => 'None', ], 'edit' => [ diff --git a/lang/fr-BE/groups.php b/lang/fr-BE/groups.php index 23b1d9e0cc..96c22a9e16 100644 --- a/lang/fr-BE/groups.php +++ b/lang/fr-BE/groups.php @@ -3,14 +3,12 @@ return [ 'group' => 'Repair Café', 'groups' => 'Repair Cafés', - 'all_groups' => 'Tous les Repair Cafés', - 'search_name' => 'Chercher nom', 'add_groups' => 'Ajouter un nouveau Repair Café', 'add_groups_content' => 'Dites-en nous plus sur votre repair café, afin que nous puissions vous créer une page et vous aider à trouver de potentiels nouveaux bénévoles et participants', 'create_groups' => 'Créer nouveau Repair Café', 'create_group' => 'Créer un nouveau Repair Café', 'groups_title1' => 'Vos Repair Cafés', - 'groups_title2' => 'Repair Cafés proches de chez vous', + 'groups_title2' => 'Autres Repair Cafés', 'groups_name' => 'Nom', 'groups_name_of' => 'Nom du Repair Café', 'groups_about_group' => 'Parlez-nous de votre Repair Café', @@ -105,11 +103,10 @@ 'volunteers_attended' => 'Bénévoles ayant participé', 'volunteers_confirmed' => 'Bénévoles confirmés', 'volunteers_invited' => 'Bénévoles invités', - 'all_groups_mobile' => 'Tous', 'create_groups_mobile2' => 'Ajouter nouveau', 'groups_title1_mobile' => 'Le vôtre', 'groups_title2_mobile' => 'Le plus proche', - 'group_count' => 'Il y a :count Repair Café. Il y a :count Repair Cafés.', + 'group_count' => 'Il y a :count Repair Café. Dézoomez pour en voir plus.|Il y a :count Repair Cafés. Dézoomez pour en voir plus.', 'hide_filters' => 'Cacher les filtres', 'join_group_button_mobile' => 'Suivre', 'leave_group_button' => 'Ne plus suivre ce Repair Café', @@ -121,11 +118,11 @@ 'no_groups_nearest_no_location' => '

Vous n\'avez pas défini de village/ville. Vous pouvez en ajouter un.e dans votre profil.

Vous pouvez aussi voir tous les Repair Cafés.

', 'no_groups_nearest_with_location' => '

Il n\'y a apparemment pas encore de Repair Cafés listé proche de chez vous.

Voulez-vous créer ou ajouter un Repair Café? Regardez comment faire dans nos ressources.

', 'no_unpowered_stats' => 'Pour l\'instant, ces statistiques sont seulement affichées pour les appareils électriques. Nous espérons pouvoir inclure les appareils non-électriques sous peu.', - 'search_country_placeholder' => 'Pays', - 'search_location_placeholder' => 'Rechercher localisation...', - 'search_name_placeholder' => 'Rechercher nom...', - 'search_tags_placeholder' => 'Tag', - 'show_filters' => 'Montrer les filtres', + 'search_country_placeholder' => 'Filtrer par pays', + 'search_location_placeholder' => 'Rechercher par localisation', + 'search_name_placeholder' => 'Rechercher par nom', + 'search_tags_placeholder' => 'Filtrer par tag', + 'show_filters' => 'Afficher les filtres', 'all' => 'Tous', 'nearby' => 'Proche', 'no_other_events' => 'Il n\'y a actuellement aucun autre événement à venir', @@ -150,7 +147,6 @@ Apprenez à utiliser ce groupe ici : :help.', 'invitation_pending' => 'Vous avez une invitation à rejoindre ce Repair Café. Cliquez ici si vous voulez le rejoindre.', 'nearest_groups' => 'Ce sont les Repair Cafés qui se trouvent dans un rayon de 50km autour de :location', - 'nearest_groups_change' => '(change)', 'talk_group' => 'Voir la conversation de Repair Café', 'talk_group_add_title' => 'Bienvenue sur :group_name', 'editing' => 'Modification de', @@ -183,4 +179,7 @@ 'export.events.items_end_of_life' => 'Total des appareils en fin de vie', 'export.events.items_kg_waste_prevented' => 'kg déchets évités', 'export.events.items_kg_co2_prevent' => 'kg emissions de CO2 évitées', + 'marker_title' => 'Cliquez pour plus d\'informations', + 'goto_group' => 'Aller au Repair Café', + 'next_event' => 'Prochain événement', ]; diff --git a/lang/fr-BE/networks.php b/lang/fr-BE/networks.php index 86e2e28363..d20e1f1751 100644 --- a/lang/fr-BE/networks.php +++ b/lang/fr-BE/networks.php @@ -60,8 +60,6 @@ 'add_groups_success' => '{1} :number repair café ajouté.|[2,*] :number repair cafés ajoutés.', 'add_groups_warning_none_selected' => 'Pas de repair café(s) sélectionné(s)', 'view_groups_menuitem' => 'Voir les repair cafés', - 'groups_count' => '{0} Il n\'y a actuellement aucun repair café dans le réseau :name.|{1} Il y a actuellement :count repair café dans le réseau :name.|[2,*] Il y a actuellement :count repair cafés dans le réseau :name.', - 'view_groups_link' => 'Voir ces repair cafés.', 'none' => 'Aucun', ], ]; diff --git a/lang/fr/groups.php b/lang/fr/groups.php index 5315fa74a7..387f8aaf1c 100644 --- a/lang/fr/groups.php +++ b/lang/fr/groups.php @@ -3,14 +3,12 @@ return [ 'group' => 'Repair Café', 'groups' => 'Repair Cafés', - 'all_groups' => 'Tous les Repair Cafés', - 'search_name' => 'Chercher nom', 'add_groups' => 'Ajouter un nouveau Repair Café', 'add_groups_content' => 'Dites-en nous plus sur votre repair café, afin que nous puissions vous créer une page et vous aider à trouver de potentiels nouveaux bénévoles et participants', 'create_groups' => 'Créer nouveau Repair Café', 'create_group' => 'Créer un nouveau Repair Café', 'groups_title1' => 'Vos Repair Cafés', - 'groups_title2' => 'Repair Cafés proches de chez vous', + 'groups_title2' => 'Autres Repair Cafés', 'groups_name' => 'Nom', 'groups_name_of' => 'Nom du Repair Café', 'groups_about_group' => 'Parlez-nous de votre Repair Café', @@ -105,11 +103,10 @@ 'volunteers_attended' => 'Bénévoles ayant participé', 'volunteers_confirmed' => 'Bénévoles confirmés', 'volunteers_invited' => 'Bénévoles invités', - 'all_groups_mobile' => 'Tous', 'create_groups_mobile2' => 'Ajouter nouveau', 'groups_title1_mobile' => 'Le vôtre', 'groups_title2_mobile' => 'Le plus proche', - 'group_count' => 'Il y a :count Repair Café. Il y a :count Repair Cafés.', + 'group_count' => 'Il y a :count Repair Café. Dézoomez pour en voir plus.|Il y a :count Repair Cafés. Dézoomez pour en voir plus.', 'hide_filters' => 'Cacher les filtres', 'join_group_button_mobile' => 'Suivre', 'leave_group_button' => 'Ne plus suivre ce Repair Café', @@ -121,11 +118,11 @@ 'no_groups_nearest_no_location' => '

Vous n\'avez pas défini de village/ville. Vous pouvez en ajouter un.e dans votre profil.

Vous pouvez aussi voir tous les Repair Cafés.

', 'no_groups_nearest_with_location' => '

Il n\'y a apparemment pas encore de Repair Cafés listé proche de chez vous.

Voulez-vous créer ou ajouter un Repair Café? Regardez comment faire dans nos ressources.

', 'no_unpowered_stats' => 'Pour l\'instant, ces statistiques sont seulement affichées pour les appareils électriques. Nous espérons pouvoir inclure les appareils non-électriques sous peu.', - 'search_country_placeholder' => 'Pays', - 'search_location_placeholder' => 'Rechercher localisation...', - 'search_name_placeholder' => 'Rechercher nom...', - 'search_tags_placeholder' => 'Tag', - 'show_filters' => 'Montrer les filtres', + 'search_country_placeholder' => 'Filtrer par pays', + 'search_location_placeholder' => 'Rechercher par localisation', + 'search_name_placeholder' => 'Rechercher par nom', + 'search_tags_placeholder' => 'Filtrer par tag', + 'show_filters' => 'Afficher les filtres', 'all' => 'Tous', 'nearby' => 'Proche', 'no_other_events' => 'Il n\'y a actuellement aucun autre événement à venir', @@ -150,7 +147,6 @@ Apprenez à utiliser ce groupe ici : :help.', 'invitation_pending' => 'Vous avez une invitation à rejoindre ce Repair Café. Cliquez ici si vous voulez le rejoindre.', 'nearest_groups' => 'Ce sont les Repair Cafés qui se trouvent dans un rayon de 50km autour de :location', - 'nearest_groups_change' => '(change)', 'talk_group' => 'Voir la conversation de Repair Café', 'talk_group_add_title' => 'Bienvenue sur :group_name', 'editing' => 'Modification de', @@ -183,4 +179,7 @@ 'export.events.items_end_of_life' => 'Total des appareils en fin de vie', 'export.events.items_kg_waste_prevented' => 'kg déchets évités', 'export.events.items_kg_co2_prevent' => 'kg emissions de CO2 évitées', + 'marker_title' => 'Cliquez pour plus d\'informations', + 'goto_group' => 'Aller au Repair Café', + 'next_event' => 'Prochain événement', ]; diff --git a/lang/fr/networks.php b/lang/fr/networks.php index 86e2e28363..d20e1f1751 100644 --- a/lang/fr/networks.php +++ b/lang/fr/networks.php @@ -60,8 +60,6 @@ 'add_groups_success' => '{1} :number repair café ajouté.|[2,*] :number repair cafés ajoutés.', 'add_groups_warning_none_selected' => 'Pas de repair café(s) sélectionné(s)', 'view_groups_menuitem' => 'Voir les repair cafés', - 'groups_count' => '{0} Il n\'y a actuellement aucun repair café dans le réseau :name.|{1} Il y a actuellement :count repair café dans le réseau :name.|[2,*] Il y a actuellement :count repair cafés dans le réseau :name.', - 'view_groups_link' => 'Voir ces repair cafés.', 'none' => 'Aucun', ], ]; diff --git a/lang/it/groups.php b/lang/it/groups.php index 2a453fcb60..5b265335eb 100644 --- a/lang/it/groups.php +++ b/lang/it/groups.php @@ -3,14 +3,12 @@ return [ 'group' => 'Gruppo', 'groups' => 'Gruppi', - 'all_groups' => 'Tutti i gruppi', - 'search_name' => 'Cerca nome', 'add_groups' => 'Aggiungi un gruppo', 'add_groups_content' => 'Raccontaci di più sul tuo gruppo, in modo che possiamo creare una pagina per te e aiutarti a trovare volontari e partecipanti.', 'create_groups' => 'Crea nuovo gruppo', 'create_group' => 'Crea gruppo', 'groups_title1' => 'Tuoi gruppi', - 'groups_title2' => 'Gruppi vicini a te', + 'groups_title2' => 'Altri gruppi', 'groups_name' => 'Nome', 'groups_name_of' => 'Nome del gruppo', 'groups_about_group' => 'Parlaci del tuo gruppo', @@ -61,4 +59,7 @@ 'shareable_link_box' => 'Invita attraverso link condivisibile', 'type_shareable_link_message' => 'Condividi il link sopra per invitare le persone a unirsi a questo gruppo. Se una persona con cui condividi il collegamento non ha ancora un account, le verrà richiesto di registrarsi.', 'upcoming_none_planned' => 'Nessuno pianificato', + 'marker_title' => 'Click for more information', + 'goto_group' => 'Go to group', + 'next_event' => 'Next event', ]; diff --git a/lang/ne/groups.php b/lang/ne/groups.php index f8e5f293b3..32a43fda83 100644 --- a/lang/ne/groups.php +++ b/lang/ne/groups.php @@ -4,7 +4,6 @@ 'about_group_name_header' => 'Over :group', 'add_groups' => 'Een groep toevoegen', 'add_groups_content' => 'Vertel ons wat meer over je groep. Zo kunnen we pagina voor je aanmaken en je helpen om vrijwilligers en deelnemers te bereiken.', - 'all_groups' => 'Alle groepen', 'approve_group' => 'Groep goedkeuren', 'area' => 'Gebied', 'co2_equivalence_visualisation_dropdown' => 'Visualisatie van het equivalent CO2', @@ -25,7 +24,7 @@ 'groups_name' => 'Naam', 'groups_name_of' => 'Naam van de groep', 'groups_title1' => 'Jouw groepen', - 'groups_title2' => 'Groepen dichtst bij jou in de buurt', + 'groups_title2' => 'Andere groepen', 'groups_website' => 'Jouw website', 'groups_website_small' => 'Heb je geen website? Voeg gerust een Facebookgroep of iets gelijkaardigs toe', 'group_admin_only' => 'Enkel voor beheerders', @@ -41,7 +40,6 @@ 'message_example_text' => '', 'message_header' => 'Uitnodiging', 'restarter_column_table' => 'Restarter', - 'search_name' => 'Naam zoeken', 'send_invite_button' => 'Uitnodigingen versturen', 'share_stats_header' => 'Je statistieken delen', 'share_stats_message' => 'Door kapotte toestellen te repareren :group heel wat CO2 uitstoot en afval vermeden. Help ons om dit goede nieuws te verspreiden en deel het op je website.', @@ -55,4 +53,7 @@ 'tag-7' => 'Label 7', 'tag-8' => 'Label 8', 'type_email_addresses_message' => 'Typ hier de e-mailadressen van de mensen die je wilt uitnodigen om lid te worden van je groep. Zet een tab of komma achter elk e-mailadres.', + 'marker_title' => 'Click for more information', + 'goto_group' => 'Go to group', + 'next_event' => 'Next event', ]; diff --git a/lang/nl-BE/groups.php b/lang/nl-BE/groups.php index b2cd84bf21..7585c29f08 100644 --- a/lang/nl-BE/groups.php +++ b/lang/nl-BE/groups.php @@ -4,7 +4,6 @@ 'about_group_name_header' => 'Over :group', 'add_groups' => 'Een groep toevoegen', 'add_groups_content' => 'Vertel ons wat meer over je groep. Zo kunnen we pagina voor je aanmaken en je helpen om vrijwilligers en deelnemers te bereiken.', - 'all_groups' => 'Alle groepen', 'approve_group' => 'Groep goedkeuren', 'area' => 'Gebied', 'co2_equivalence_visualisation_dropdown' => 'Visualisatie van het equivalent CO2', @@ -25,7 +24,7 @@ 'groups_name' => 'Naam', 'groups_name_of' => 'Naam van de groep', 'groups_title1' => 'Jouw groepen', - 'groups_title2' => 'Groepen dichtst bij jou in de buurt', + 'groups_title2' => 'Andere groepen', 'groups_website' => 'Jouw website', 'groups_website_small' => 'Heb je geen website? Voeg gerust een Facebookgroep of iets gelijkaardigs toe', 'group_admin_only' => 'Enkel voor beheerders', @@ -41,7 +40,6 @@ 'message_example_text' => '', 'message_header' => 'Uitnodiging', 'restarter_column_table' => 'Restarter', - 'search_name' => 'Naam zoeken', 'send_invite_button' => 'Uitnodigingen versturen', 'share_stats_header' => 'Je statistieken delen', 'share_stats_message' => 'Door kapotte toestellen te repareren heeft :group heel wat CO2 uitstoot en afval vermeden. Help ons om dit goede nieuws te verspreiden en deel het op je website.', @@ -71,4 +69,7 @@ 'postcode' => 'Postnummer van het Repair Café', 'events' => 'Activiteiten', 'group_facts' => 'Groepsresultaten', + 'marker_title' => 'Click for more information', + 'goto_group' => 'Go to group', + 'next_event' => 'Next event', ]; diff --git a/lang/no/groups.php b/lang/no/groups.php index 2607fc6e1e..ddbf2b6f33 100644 --- a/lang/no/groups.php +++ b/lang/no/groups.php @@ -3,14 +3,12 @@ return [ 'group' => 'Group', 'groups' => 'Groups', - 'all_groups' => 'All groups', - 'search_name' => 'Search name', 'add_groups' => 'Add a group', 'add_groups_content' => 'Tell us more about your group, so we can create a page for you and help you publicise to potential volunteers and participants.', 'create_groups' => 'Create new group', 'create_group' => 'Create group', 'groups_title1' => 'Your groups', - 'groups_title2' => 'Groups nearest to you', + 'groups_title2' => 'Other groups', 'groups_name' => 'Name', 'groups_name_of' => 'Name of group', 'groups_about_group' => 'Tell us about your group', @@ -51,4 +49,7 @@ 'tag-6' => 'Tag 6', 'tag-7' => 'Tag 7', 'tag-8' => 'Tag 8', + 'marker_title' => 'Click for more information', + 'goto_group' => 'Go to group', + 'next_event' => 'Next event', ]; diff --git a/package-lock.json b/package-lock.json index 991ec4ca76..d6eff8aa56 100644 --- a/package-lock.json +++ b/package-lock.json @@ -29,6 +29,7 @@ "js-cookie": "^2.2.0", "lang.js": "^1.1.14", "leaflet": "^1.4.0", + "leaflet-control-geocoder": "^1.13.0", "lodash-es": "^4.17.21", "moment": "^2.29.4", "moment-timezone": "^0.5.35", @@ -14300,6 +14301,14 @@ "resolved": "https://registry.npmjs.org/leaflet/-/leaflet-1.8.0.tgz", "integrity": "sha512-gwhMjFCQiYs3x/Sf+d49f10ERXaEFCPr+nVTryhAW8DWbMGqJqt9G4XuIaHmFW08zYvhgdzqXGr8AlW8v8dQkA==" }, + "node_modules/leaflet-control-geocoder": { + "version": "1.13.0", + "resolved": "https://registry.npmjs.org/leaflet-control-geocoder/-/leaflet-control-geocoder-1.13.0.tgz", + "integrity": "sha512-mgYGx/2WA5CcvhP+IJtw7VvJwSGAe5zxX+TKe6ruYkLj2W5I5V/K/nQiLvsUtqifBojBGoKIPNZ8m0mXJNIudg==", + "optionalDependencies": { + "open-location-code": "^1.0.0" + } + }, "node_modules/leven": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/leven/-/leven-3.1.0.tgz", @@ -18149,6 +18158,12 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/open-location-code": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/open-location-code/-/open-location-code-1.0.3.tgz", + "integrity": "sha512-DBm14BSn40Ee241n80zIFXIT6+y8Tb0I+jTdosLJ8Sidvr2qONvymwqymVbHV2nS+1gkDZ5eTNpnOIVV0Kn2fw==", + "optional": true + }, "node_modules/open/node_modules/is-wsl": { "version": "2.2.0", "resolved": "https://registry.npmjs.org/is-wsl/-/is-wsl-2.2.0.tgz", diff --git a/package.json b/package.json index 48d6b32144..7bee43d1ad 100644 --- a/package.json +++ b/package.json @@ -78,6 +78,7 @@ "js-cookie": "^2.2.0", "lang.js": "^1.1.14", "leaflet": "^1.4.0", + "leaflet-control-geocoder": "^1.13.0", "lodash-es": "^4.17.21", "moment": "^2.29.4", "moment-timezone": "^0.5.35", diff --git a/resources/global/css/_global.scss b/resources/global/css/_global.scss index 1fe59bae2a..f91512a7aa 100644 --- a/resources/global/css/_global.scss +++ b/resources/global/css/_global.scss @@ -141,4 +141,20 @@ h2 { .fa-fw { width: 1rem; height: 1rem; +} + +.fa-spin { + animation-name: spin; + animation-duration: 4s; + animation-timing-function: linear; + animation-iteration-count: infinite; +} + +@keyframes spin { + 0% { + transform: rotate(0deg); + } + 100% { + transform: rotate(360deg); + } } \ No newline at end of file diff --git a/resources/js/app.js b/resources/js/app.js index 4cc54ab738..dec3fd8fe5 100644 --- a/resources/js/app.js +++ b/resources/js/app.js @@ -55,6 +55,9 @@ import RichTextEditor from './components/RichTextEditor.vue' import Notifications from './components/Notifications.vue' import GroupTimeZone from './components/GroupTimeZone.vue' import StatsShare from './components/StatsShare.vue' +import GroupMapAndList from './components/GroupMapAndList.vue' +import GroupMarker from './components/GroupMarker.vue' +import GroupInfoModal from './components/GroupInfoModal.vue' import CategoriesTable from './components/CategoriesTable.vue' import RolesTable from './components/RolesTable.vue' import EmailValidation from './components/EmailValidation.vue' @@ -419,6 +422,9 @@ function initializeJQuery() { 'categories-table': CategoriesTable, 'roles-table': RolesTable, 'emailvalidation': EmailValidation, + 'groupmapandlist': GroupMapAndList, + 'groupmarker': GroupMarker, + 'groupinfomodal': GroupInfoModal, } }) }) @@ -1073,6 +1079,6 @@ function initAutocomplete() { // All jQuery initialization moved to initializeJQuery() function above // Sentry initialization is also inside the initializeJQuery() function -// Start jQuery initialization (called earlier on line 509, don't duplicate here) +// Start jQuery initialization (called earlier in file) // initializeJQuery(); diff --git a/resources/js/components/GroupInfoModal.vue b/resources/js/components/GroupInfoModal.vue new file mode 100644 index 0000000000..833b86c5ac --- /dev/null +++ b/resources/js/components/GroupInfoModal.vue @@ -0,0 +1,93 @@ + + + diff --git a/resources/js/components/GroupMap.test.js b/resources/js/components/GroupMap.test.js new file mode 100644 index 0000000000..37c0da868c --- /dev/null +++ b/resources/js/components/GroupMap.test.js @@ -0,0 +1,129 @@ +import Vue from "vue" +import { BootstrapVue } from 'bootstrap-vue' +Vue.use(BootstrapVue) + +import { shallowMount, createLocalVue } from '@vue/test-utils' +import Vuex from 'vuex' +import L from 'leaflet' +import LangMixin from 'resources/js/mixins/lang.js' +import GroupMap from './GroupMap.vue' + +// GroupMap uses the global `L` (window.L) for LatLng / LatLngBounds. +global.L = L + +const localVue = createLocalVue() +localVue.use(Vuex) +localVue.mixin(LangMixin) + +// The groups page sends this inverted whole-world box when the user has no +// location set (min_lat 90 > max_lat -90). +const WORLD = [[90, 180], [-90, -180]] + +function makeStore(groups = []) { + return new Vuex.Store({ + modules: { + groups: { + namespaced: true, + getters: { list: () => groups }, + }, + }, + }) +} + +function fakeMap(size = { x: 688, y: 400 }) { + return { + invalidateSize: jest.fn(), + fitBounds: jest.fn(), + flyToBounds: jest.fn(), + getSize: () => size, + getBounds: () => L.latLngBounds([[50, -1], [52, 1]]), + getZoom: () => 5, + getCenter: () => ({ lat: 0, lng: 0 }), + } +} + +function mountMap(initialBounds, groups) { + return shallowMount(GroupMap, { + localVue, + store: makeStore(groups), + propsData: { initialBounds }, + }) +} + +describe('GroupMap visibility handling', () => { + // Regression (grey map): when the map is created inside a hidden tab its + // Leaflet container is 0x0. When the tab becomes visible nothing tells + // Leaflet to re-measure, so tiles never fill the now-visible area and most + // of the map shows as grey. + test('calls invalidateSize when its container is resized (becomes visible)', () => { + let resizeCb = null + let observed = false + global.ResizeObserver = class { + constructor(cb) { resizeCb = cb } + observe() { observed = true } + unobserve() {} + disconnect() {} + } + + const wrapper = mountMap([], []) + const map = fakeMap() + wrapper.vm.mapObject = map + wrapper.vm.$refs.map = { mapObject: map } + + expect(observed).toBe(true) + expect(typeof resizeCb).toBe('function') + + resizeCb([{ contentRect: { width: 688, height: 400 } }]) + + expect(map.invalidateSize).toHaveBeenCalled() + }) +}) + +describe('GroupMap.hasLocation', () => { + test('is false for the inverted whole-world box (no user location)', () => { + expect(mountMap(WORLD, []).vm.hasLocation).toBe(false) + }) + + test('is true for a real bounding box', () => { + expect(mountMap([[51.0, -0.8], [51.8, 0.4]], []).vm.hasLocation).toBe(true) + }) +}) + +describe('GroupMap.zoomToGroups', () => { + const groups = [ + { id: 1, location: { lat: 51.5, lng: -0.1 } }, + { id: 2, location: { lat: 53.4, lng: -2.2 } }, + { id: 3, location: { lat: 55.9, lng: -3.2 } }, + ] + + test('does not frame the map while it is still 0x0 (off-screen), so it can retry later', () => { + const wrapper = mountMap(WORLD, groups) + const map = fakeMap({ x: 0, y: 0 }) + wrapper.vm.mapObject = map + wrapper.vm.$refs.map = { mapObject: map } + wrapper.vm.zoomedToGroups = false + + wrapper.vm.zoomToGroups() + + expect(map.fitBounds).not.toHaveBeenCalled() + expect(wrapper.vm.zoomedToGroups).toBe(false) + }) + + test('frames ALL groups via fitBounds (not flyToBounds) when the user has no location', () => { + const wrapper = mountMap(WORLD, groups) + const map = fakeMap({ x: 688, y: 400 }) + wrapper.vm.mapObject = map + wrapper.vm.$refs.map = { mapObject: map } + wrapper.vm.zoomedToGroups = false + + wrapper.vm.zoomToGroups() + + expect(map.flyToBounds).not.toHaveBeenCalled() + expect(map.fitBounds).toHaveBeenCalledTimes(1) + + const bounds = map.fitBounds.mock.calls[0][0] + // The framed bounds must contain every group, including the furthest ones. + expect(bounds.contains([51.5, -0.1])).toBe(true) + expect(bounds.contains([55.9, -3.2])).toBe(true) + }) +}) diff --git a/resources/js/components/GroupMap.vue b/resources/js/components/GroupMap.vue new file mode 100644 index 0000000000..d4a7462eb5 --- /dev/null +++ b/resources/js/components/GroupMap.vue @@ -0,0 +1,369 @@ + + + diff --git a/resources/js/components/GroupMapAndList.vue b/resources/js/components/GroupMapAndList.vue new file mode 100644 index 0000000000..4bae399b42 --- /dev/null +++ b/resources/js/components/GroupMapAndList.vue @@ -0,0 +1,137 @@ + + + diff --git a/resources/js/components/GroupMarker.vue b/resources/js/components/GroupMarker.vue new file mode 100644 index 0000000000..4befada9a2 --- /dev/null +++ b/resources/js/components/GroupMarker.vue @@ -0,0 +1,80 @@ + + + diff --git a/resources/js/components/GroupsPage.test.js b/resources/js/components/GroupsPage.test.js new file mode 100644 index 0000000000..84b387012e --- /dev/null +++ b/resources/js/components/GroupsPage.test.js @@ -0,0 +1,82 @@ +import Vue from "vue" +import { BootstrapVue } from 'bootstrap-vue' +Vue.use(BootstrapVue) + +import { mount, createLocalVue } from '@vue/test-utils' +import Vuex from 'vuex' +import LangMixin from 'resources/js/mixins/lang.js' +import GroupsPage from './GroupsPage.vue' + +const localVue = createLocalVue() +localVue.use(Vuex) + +function makeStore() { + return new Vuex.Store({ + modules: { + groups: { + namespaced: true, + getters: { list: () => [] }, + actions: { fetch: () => Promise.resolve() }, + }, + }, + }) +} + +const groupsTableStub = { + name: 'GroupsTable', + props: { + groupids: { type: Array }, + tab: { type: Number, default: 0 }, + yourArea: { type: String, default: null }, + networks: { type: Array, default: null }, + allGroupTags: { type: Array, default: null }, + showTags: { type: Boolean, default: false }, + }, + template: '
', +} + +const groupMapStub = { name: 'GroupMapAndList', template: '
' } + +function makeWrapper(props = {}) { + return mount(GroupsPage, { + localVue, + store: makeStore(), + mixins: [LangMixin], + propsData: { + yourGroups: [1, 2], + nearbyGroups: [], + networks: [{ id: 10, name: 'Test' }], + allGroupTags: [{ id: 1, tag_name: 'Foo' }], + ...props, + }, + stubs: { GroupsTable: groupsTableStub, GroupMapAndList: groupMapStub }, + }) +} + +async function flushTabs(wrapper) { + // b-tab has `lazy`, so the tab's content only renders after the tab activates + // on the first Vue tick. + await wrapper.vm.$nextTick() + await wrapper.vm.$nextTick() +} + +test('forwards showTags / networks / allGroupTags to the inner GroupsTable so tag badges render on the "your groups" tab', async () => { + const networks = [{ id: 10, name: 'Test' }] + const allGroupTags = [{ id: 1, tag_name: 'Foo' }, { id: 2, tag_name: 'Bar' }] + const wrapper = makeWrapper({ showTags: true, networks, allGroupTags }) + await flushTabs(wrapper) + + const table = wrapper.findComponent(groupsTableStub) + expect(table.exists()).toBe(true) + expect(table.props('showTags')).toBe(true) + expect(table.props('networks')).toEqual(networks) + expect(table.props('allGroupTags')).toEqual(allGroupTags) +}) + +test('forwards yourArea (bound, not the literal string "yourArea") to GroupsTable', async () => { + const wrapper = makeWrapper({ yourArea: 'London' }) + await flushTabs(wrapper) + + const table = wrapper.findComponent(groupsTableStub) + expect(table.props('yourArea')).toBe('London') +}) diff --git a/resources/js/components/GroupsPage.vue b/resources/js/components/GroupsPage.vue index 4dd8ebd953..33ff2d138b 100644 --- a/resources/js/components/GroupsPage.vue +++ b/resources/js/components/GroupsPage.vue @@ -27,11 +27,14 @@
@@ -43,51 +46,24 @@ {{ __('groups.groups_title2') }}
-

- {{ nearestGroups }} - {{ __('groups.nearest_groups_change') }}. -

- +
- - - -
\ No newline at end of file diff --git a/resources/js/components/NetworkPage.vue b/resources/js/components/NetworkPage.vue index ce10ff577b..59cb4264eb 100644 --- a/resources/js/components/NetworkPage.vue +++ b/resources/js/components/NetworkPage.vue @@ -12,7 +12,6 @@
- {{ __('networks.show.view_groups_menuitem') }} {{ __('networks.show.add_groups_menuitem') }} {{ __('groups.export_event_list') }} diff --git a/tests/Feature/Networks/NetworkTest.php b/tests/Feature/Networks/NetworkTest.php index d37edf5de0..b256b88299 100644 --- a/tests/Feature/Networks/NetworkTest.php +++ b/tests/Feature/Networks/NetworkTest.php @@ -264,10 +264,6 @@ public function network_page(): void $response = $this->get('/networks/' . $network->id); $response->assertSee($coordinator->name); - // Group should not show on network page yet. - $response = $this->get('/group/network/' . $network->id); - $response->assertDontSee('"networks":[' . $network->id . ']'); - // Add the group. $response = $this->get('/networks/' . $network->id); $crawler = new Crawler($response->getContent()); @@ -285,13 +281,15 @@ public function network_page(): void ]); $response->assertRedirect(); - // Groups are now fetched client-side from /api/v2/groups/summary, so - // the group name is no longer server-rendered here. The network scope - // must still reach the Vue layer: /group/network/{id} passes the - // network id, which GroupsPage forwards to the map/list to filter by. + // /group/network/{id} is retired: coordinators see their groups on the + // network page itself (map + list). Old links redirect there. $response = $this->get('/group/network/' . $network->id); + $response->assertRedirect('/networks/' . $network->id); + + // The network page embeds the map/list scoped to this network. + $response = $this->get('/networks/' . $network->id); $response->assertSuccessful(); - $response->assertSee(':network="' . $network->id . '"', false); + $response->assertSee(':network="{"id":' . $network->id, false); // And the group must be in the API data the page will fetch. $response = $this->get('/api/v2/groups/summary'); From 792931fccce6bb7668a8f9398cceb74e32f162e3 Mon Sep 17 00:00:00 2001 From: edwh Date: Tue, 14 Jul 2026 20:41:20 +0100 Subject: [PATCH 06/34] RES-1995: minimal=true trims the groups summary to what the map uses On production-sized data (1,159 groups) the full summary response is 807KB raw / 121KB gzipped and takes ~4s to build server-side; half the payload is fields the map page never reads. minimal=true collapses networks to plain ids, next_event to id/start/title/summary, location to location/country/lat/lng, and drops updated_at. The client requests it; the default shape is unchanged (pinned by test). Caching was considered instead but rejected: traffic is too low for a short-TTL cache to be warm when it matters. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01TzY1phBjyT3HogpeS53zon --- app/Http/Controllers/API/GroupController.php | 9 ++++ app/Http/Resources/GroupSummary.php | 39 ++++++++++++-- resources/js/store/groups.js | 2 +- resources/js/store/groups.test.js | 1 + tests/Feature/Groups/GroupSummaryApiTest.php | 56 ++++++++++++++++++++ 5 files changed, 101 insertions(+), 6 deletions(-) diff --git a/app/Http/Controllers/API/GroupController.php b/app/Http/Controllers/API/GroupController.php index 69f95990d0..e75ea21a29 100644 --- a/app/Http/Controllers/API/GroupController.php +++ b/app/Http/Controllers/API/GroupController.php @@ -327,6 +327,15 @@ public static function listNamesv2(Request $request) { * type="boolean" * ) * ), + * @OA\Parameter( + * name="minimal", + * description="Trim each group to the fields the groups map page uses: networks become plain ids, next_event keeps only id/start/title, and location keeps only location/country/lat/lng. Default false.", + * required=false, + * in="query", + * @OA\Schema( + * type="boolean" + * ) + * ), * @OA\Response( * response=200, * description="Successful operation", diff --git a/app/Http/Resources/GroupSummary.php b/app/Http/Resources/GroupSummary.php index 07813a5eeb..248e892c0f 100644 --- a/app/Http/Resources/GroupSummary.php +++ b/app/Http/Resources/GroupSummary.php @@ -44,10 +44,13 @@ * @OA\Property( * property="networks", * title="networks", - * description="An array of networks of which the group is a member.", + * description="An array of networks of which the group is a member. With minimal=true this is an array of network ids only.", * type="array", * @OA\Items( - * ref="#/components/schemas/NetworkSummary" + * oneOf={ + * @OA\Schema(ref="#/components/schemas/NetworkSummary"), + * @OA\Schema(type="integer") + * } * ) * ), * @OA\Property( @@ -109,12 +112,24 @@ class GroupSummary extends JsonResource */ public function toArray(Request $request): array { + // minimal=true trims the response to what the groups map page actually + // consumes: on a production-sized dataset the full shape is ~2x the + // bytes and a large share of the serialisation time. + $minimal = $request->get('minimal', false); + $ret = [ 'id' => $this->idgroups, 'name' => $this->name, 'image' => $this->groupImage && is_object($this->groupImage) && is_object($this->groupImage->image) ? $this->groupImage->image->path : null, - 'location' => new GroupLocation($this), - 'networks' => new NetworkSummaryCollection($this->resource->networks), + 'location' => $minimal ? [ + 'location' => $this->location, + 'country' => \App\Helpers\Fixometer::getCountryFromCountryCode($this->country_code), + 'lat' => $this->latitude, + 'lng' => $this->longitude, + ] : new GroupLocation($this), + 'networks' => $minimal + ? $this->resource->networks->pluck('id')->all() + : new NetworkSummaryCollection($this->resource->networks), // Tags drive the badges and the tag filter on the groups list. // Only included when the caller eager-loaded them, so other users // of this resource don't pick up an N+1. @@ -127,11 +142,14 @@ public function toArray(Request $request): array ]; }); }), - 'updated_at' => Carbon::parse($this->updated_at)->toIso8601String(), 'archived_at' => $this->archived_at ? Carbon::parse($this->archived_at)->toIso8601String() : null, 'summary' => true ]; + if (! $minimal) { + $ret['updated_at'] = Carbon::parse($this->updated_at)->toIso8601String(); + } + if ($request->get('includeCounts', false)) { $ret['hosts'] = $this->resource->all_confirmed_hosts_count; $ret['restarters'] = $this->resource->all_confirmed_restarters_count; @@ -178,6 +196,17 @@ public function toArray(Request $request): array } } + if ($minimal && $nextevent) { + // Only what the list and modal display; id/summary are kept + // because the EventSummary schema requires them. + $nextevent = [ + 'id' => $nextevent['id'], + 'start' => $nextevent['start'], + 'title' => $nextevent['title'], + 'summary' => true, + ]; + } + $ret['next_event'] = $nextevent; } diff --git a/resources/js/store/groups.js b/resources/js/store/groups.js index 85c5ebb18e..20be29ba5b 100644 --- a/resources/js/store/groups.js +++ b/resources/js/store/groups.js @@ -153,7 +153,7 @@ export default { if (params && params.details) { // We want more details. Ask for archived groups too: the list shows // them with an "archived" badge, as the old server-rendered page did. - url = '/api/v2/groups/summary?locale=' + getLocale() + '&includeNextEvent=true&includeCounts=true&archived=true' + url = '/api/v2/groups/summary?locale=' + getLocale() + '&includeNextEvent=true&includeCounts=true&archived=true&minimal=true' } else { // Just the name and lat/lng. url = '/api/v2/groups/names?locale=' + getLocale() diff --git a/resources/js/store/groups.test.js b/resources/js/store/groups.test.js index 24bb5ac14a..3ce2959d3c 100644 --- a/resources/js/store/groups.test.js +++ b/resources/js/store/groups.test.js @@ -103,6 +103,7 @@ test('the details list fetch asks for archived groups (shown with a badge, as th await groups.actions.list({ commit }, { details: true }) expect(axios.get.mock.calls[0][0]).toContain('archived=true') + expect(axios.get.mock.calls[0][0]).toContain('minimal=true') }) // GroupsTable renders its rows from the groups/list store; groups that only diff --git a/tests/Feature/Groups/GroupSummaryApiTest.php b/tests/Feature/Groups/GroupSummaryApiTest.php index 56dde04dca..24ebd7e2f8 100644 --- a/tests/Feature/Groups/GroupSummaryApiTest.php +++ b/tests/Feature/Groups/GroupSummaryApiTest.php @@ -4,6 +4,8 @@ use App\Group; use App\GroupTags; +use App\Network; +use App\Party; use Carbon\Carbon; use DB; use Tests\TestCase; @@ -77,4 +79,58 @@ public function testQueryCountDoesNotScaleWithGroupCount(): void "Summary endpoint queries scale with group count: $queriesForFew -> $queriesForMany" ); } + + public function testMinimalParamReturnsOnlyWhatTheMapPageUses(): void + { + $network = Network::factory()->create(); + $tag = GroupTags::factory()->create(); + $group = Group::factory()->create(['name' => 'Minimal Group']); + $group->addTag($tag); + $network->addGroup($group); + Party::factory()->create([ + 'group' => $group->idgroups, + 'event_start_utc' => Carbon::now()->addDays(3)->toIso8601String(), + 'event_end_utc' => Carbon::now()->addDays(3)->addHours(2)->toIso8601String(), + 'approved' => true, + ]); + \Cache::forget('future_events'); + + $response = $this->get('/api/v2/groups/summary?minimal=true&includeNextEvent=true&includeCounts=true'); + $response->assertSuccessful(); + $g = collect($response->json('data'))->firstWhere('id', $group->idgroups); + $this->assertNotNull($g); + + // Exactly the fields the map page consumes - nothing else. + $this->assertEqualsCanonicalizing( + ['id', 'name', 'image', 'location', 'networks', 'group_tags_full', + 'archived_at', 'summary', 'hosts', 'restarters', 'next_event'], + array_keys($g) + ); + $this->assertEqualsCanonicalizing( + ['location', 'country', 'lat', 'lng'], + array_keys($g['location']) + ); + // Networks collapse to plain ids. + $this->assertEquals([$network->id], $g['networks']); + // next_event keeps only what the list and modal show (plus the + // id/summary fields its schema requires). + $this->assertEqualsCanonicalizing( + ['id', 'start', 'title', 'summary'], + array_keys($g['next_event']) + ); + } + + public function testDefaultShapeUnchangedWithoutMinimal(): void + { + $network = Network::factory()->create(); + $group = Group::factory()->create(); + $network->addGroup($group); + + $response = $this->get('/api/v2/groups/summary'); + $g = collect($response->json('data'))->firstWhere('id', $group->idgroups); + + $this->assertArrayHasKey('updated_at', $g); + $this->assertEquals($network->id, $g['networks'][0]['id']); + $this->assertArrayHasKey('area', $g['location']); + } } From a686030136eaaea81b1a3e55163f3ab94e713ca1 Mon Sep 17 00:00:00 2001 From: edwh Date: Wed, 15 Jul 2026 09:06:13 +0100 Subject: [PATCH 07/34] RES-1995: split map data from list data - index + on-demand hydration Per review: the map should not wait for the full groups payload, and the list should not load rows nobody has scrolled to. The names index (/api/v2/groups/names) now carries country, network ids and tag ids - everything the map markers and client-side filters need - and the list hydrates only its visible rows through a new ids= parameter on /groups/summary (batched, capped at 200, mirroring the existing infinite-scroll pagination rather than inventing server-side pages). API surface review: the unreleased minimal= parameter is removed in favour of this - net change is one new parameter (ids) and three fields on the existing names index; no new endpoints. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01TzY1phBjyT3HogpeS53zon --- app/Http/Controllers/API/GroupController.php | 38 ++++++++-- app/Http/Resources/GroupSummary.php | 39 ++-------- resources/js/components/GroupMap.vue | 3 +- resources/js/components/GroupMarker.vue | 3 + resources/js/components/GroupsTable.vue | 29 ++++---- resources/js/store/groups.js | 75 +++++++++++++++++--- resources/js/store/groups.test.js | 3 +- tests/Feature/Groups/GroupSummaryApiTest.php | 74 ++++++++++--------- 8 files changed, 164 insertions(+), 100 deletions(-) diff --git a/app/Http/Controllers/API/GroupController.php b/app/Http/Controllers/API/GroupController.php index e75ea21a29..71008e6b4e 100644 --- a/app/Http/Controllers/API/GroupController.php +++ b/app/Http/Controllers/API/GroupController.php @@ -269,14 +269,28 @@ public static function listNamesv2(Request $request) { 'includeArchived' => ['string', 'in:true,false'], ]); - // We only return a small number of attributes, for speed. - $query = Group::select('idgroups', 'name', 'latitude', 'longitude', 'archived_at'); + // We only return a small number of attributes, for speed: this index + // drives the groups map (positions/tooltips) and the client-side + // name/country/network/tag filters, with full rows hydrated on demand + // via /groups/summary?ids=. + $query = Group::select('idgroups', 'name', 'latitude', 'longitude', 'country_code', 'archived_at'); if (!$request->has('includeArchived') || $request->get('includeArchived') == 'false') { $query = $query->whereNull('archived_at'); } $groups = $query->get(); + + // Two cheap lookups instead of per-group relation loads. + $networkIds = \DB::table('group_network') + ->whereIn('group_id', $groups->pluck('idgroups')) + ->get() + ->groupBy('group_id'); + $tagIds = \DB::table('grouptags_groups') + ->whereIn('group', $groups->pluck('idgroups')) + ->get() + ->groupBy('group'); + $ret = []; foreach ($groups as $group) { @@ -285,6 +299,9 @@ public static function listNamesv2(Request $request) { 'name' => $group->name, 'lat' => $group->latitude, 'lng' => $group->longitude, + 'country' => \App\Helpers\Fixometer::getCountryFromCountryCode($group->country_code), + 'network_ids' => $networkIds->has($group->idgroups) ? $networkIds[$group->idgroups]->pluck('network_id')->all() : [], + 'tag_ids' => $tagIds->has($group->idgroups) ? $tagIds[$group->idgroups]->pluck('group_tag')->all() : [], 'archived_at' => $group->archived_at ? Carbon::parse($group->archived_at)->toIso8601String() : null ]; } @@ -328,12 +345,12 @@ public static function listNamesv2(Request $request) { * ) * ), * @OA\Parameter( - * name="minimal", - * description="Trim each group to the fields the groups map page uses: networks become plain ids, next_event keeps only id/start/title, and location keeps only location/country/lat/lng. Default false.", + * name="ids", + * description="Comma-separated group ids. When present, only these groups are returned (used by the groups list to hydrate the visible rows). Maximum 200 ids.", * required=false, * in="query", * @OA\Schema( - * type="boolean" + * type="string" * ) * ), * @OA\Response( @@ -359,6 +376,11 @@ public static function listNamesv2(Request $request) { public static function listSummaryv2(Request $request) { $request->validate([ 'archived' => ['string', 'in:true,false'], + 'ids' => ['string', 'regex:/^\d+(,\d+)*$/', function ($attribute, $value, $fail) { + if (count(explode(',', $value)) > 200) { + $fail('A maximum of 200 ids may be requested at once.'); + } + }], ]); // Eager-load everything the GroupSummary resource touches, otherwise @@ -369,6 +391,12 @@ public static function listSummaryv2(Request $request) { $query = $query->whereNull('archived_at'); } + // The groups list hydrates just its visible rows this way, instead of + // paying to serialise every group on page load. + if ($request->filled('ids')) { + $query = $query->whereIn('idgroups', explode(',', $request->get('ids'))); + } + $groups = $query->get(); return [ diff --git a/app/Http/Resources/GroupSummary.php b/app/Http/Resources/GroupSummary.php index 248e892c0f..07813a5eeb 100644 --- a/app/Http/Resources/GroupSummary.php +++ b/app/Http/Resources/GroupSummary.php @@ -44,13 +44,10 @@ * @OA\Property( * property="networks", * title="networks", - * description="An array of networks of which the group is a member. With minimal=true this is an array of network ids only.", + * description="An array of networks of which the group is a member.", * type="array", * @OA\Items( - * oneOf={ - * @OA\Schema(ref="#/components/schemas/NetworkSummary"), - * @OA\Schema(type="integer") - * } + * ref="#/components/schemas/NetworkSummary" * ) * ), * @OA\Property( @@ -112,24 +109,12 @@ class GroupSummary extends JsonResource */ public function toArray(Request $request): array { - // minimal=true trims the response to what the groups map page actually - // consumes: on a production-sized dataset the full shape is ~2x the - // bytes and a large share of the serialisation time. - $minimal = $request->get('minimal', false); - $ret = [ 'id' => $this->idgroups, 'name' => $this->name, 'image' => $this->groupImage && is_object($this->groupImage) && is_object($this->groupImage->image) ? $this->groupImage->image->path : null, - 'location' => $minimal ? [ - 'location' => $this->location, - 'country' => \App\Helpers\Fixometer::getCountryFromCountryCode($this->country_code), - 'lat' => $this->latitude, - 'lng' => $this->longitude, - ] : new GroupLocation($this), - 'networks' => $minimal - ? $this->resource->networks->pluck('id')->all() - : new NetworkSummaryCollection($this->resource->networks), + 'location' => new GroupLocation($this), + 'networks' => new NetworkSummaryCollection($this->resource->networks), // Tags drive the badges and the tag filter on the groups list. // Only included when the caller eager-loaded them, so other users // of this resource don't pick up an N+1. @@ -142,14 +127,11 @@ public function toArray(Request $request): array ]; }); }), + 'updated_at' => Carbon::parse($this->updated_at)->toIso8601String(), 'archived_at' => $this->archived_at ? Carbon::parse($this->archived_at)->toIso8601String() : null, 'summary' => true ]; - if (! $minimal) { - $ret['updated_at'] = Carbon::parse($this->updated_at)->toIso8601String(); - } - if ($request->get('includeCounts', false)) { $ret['hosts'] = $this->resource->all_confirmed_hosts_count; $ret['restarters'] = $this->resource->all_confirmed_restarters_count; @@ -196,17 +178,6 @@ public function toArray(Request $request): array } } - if ($minimal && $nextevent) { - // Only what the list and modal display; id/summary are kept - // because the EventSummary schema requires them. - $nextevent = [ - 'id' => $nextevent['id'], - 'start' => $nextevent['start'], - 'title' => $nextevent['title'], - 'summary' => true, - ]; - } - $ret['next_event'] = $nextevent; } diff --git a/resources/js/components/GroupMap.vue b/resources/js/components/GroupMap.vue index dff7f9dd27..f38c38af72 100644 --- a/resources/js/components/GroupMap.vue +++ b/resources/js/components/GroupMap.vue @@ -89,7 +89,8 @@ export default { return groups } - return groups.filter((g) => (g.networks || []).some((n) => n.id === this.network)) + // Networks may be summary objects ([{id}]) or plain ids (the names index). + return groups.filter((g) => (g.networks || []).some((n) => (n && n.id !== undefined ? n.id : n) === this.network)) }, mappableGroups() { // A group with no geocode would put a marker at null island (0,0) — diff --git a/resources/js/components/GroupMarker.vue b/resources/js/components/GroupMarker.vue index 52f75bd236..8e67953680 100644 --- a/resources/js/components/GroupMarker.vue +++ b/resources/js/components/GroupMarker.vue @@ -78,6 +78,9 @@ export default { }, methods: { openModal() { + // The store may only hold the lean index entry for this group; fetch + // the full row so the modal can show location and next event. + this.$store.dispatch('groups/hydrate', { ids: [this.id] }) this.showModal = true }, markerHover(over) { diff --git a/resources/js/components/GroupsTable.vue b/resources/js/components/GroupsTable.vue index 31bfe02d53..dcf503a847 100644 --- a/resources/js/components/GroupsTable.vue +++ b/resources/js/components/GroupsTable.vue @@ -272,21 +272,18 @@ export default { }, }, watch: { - async itemsToShow(newVal) { - // We may need to fetch the group over the API if not in store. - // - // This is for the "your groups" or "other groups nearby" case. For "all groups" it would result in too - // many API calls, so we fetch those in a single slow API call. - newVal.forEach(async (g) => { - const group = this.$store.getters['groups/get'](g.id) + itemsToShow: { + immediate: true, + handler(newVal) { + // Hydrate the visible rows in one batched call (image, location + // text, counts, next event, tag names). The store skips ids that + // are already hydrated or in flight. + const ids = newVal.map(g => g.id) - if (!group || !group.location) { - await this.$store.dispatch('groups/fetch', { - id: g.id, - includeStats: false - }) + if (ids.length) { + this.$store.dispatch('groups/hydrate', { ids }) } - }) + } } }, methods: { @@ -388,8 +385,10 @@ export default { if (!this.allGroupTags || !tags) { return [] } - const visibleTagIds = this.allGroupTags.map(t => t.id) - return tags.filter(t => visibleTagIds.includes(t.id)) + // Resolve names from allGroupTags: index entries only carry tag ids + // until the row is hydrated. + const byId = new Map(this.allGroupTags.map(t => [t.id, t])) + return tags.filter(t => byId.has(t.id)).map(t => t.name ? t : byId.get(t.id)) } }, } diff --git a/resources/js/store/groups.js b/resources/js/store/groups.js index 20be29ba5b..b27ed7328b 100644 --- a/resources/js/store/groups.js +++ b/resources/js/store/groups.js @@ -43,6 +43,10 @@ export default { // List of groups indexed by group id. Use object rather than array so that it's sparse. list: {}, + // Ids with a full-row fetch in flight, so watchers firing repeatedly + // don't duplicate hydration calls. + hydrating: {}, + // Groups requiring moderation. moderate: {}, @@ -68,6 +72,15 @@ export default { } }, mutations: { + setHydrating(state, params) { + params.ids.forEach(id => { + if (params.value) { + Vue.set(state.hydrating, id, true) + } else { + Vue.delete(state.hydrating, id) + } + }) + }, set(state, params) { Vue.set(state.list, params.id || params.idgroups, params) }, @@ -148,25 +161,65 @@ export default { } }, async list({commit}, params) { - let url - - if (params && params.details) { - // We want more details. Ask for archived groups too: the list shows - // them with an "archived" badge, as the old server-rendered page did. - url = '/api/v2/groups/summary?locale=' + getLocale() + '&includeNextEvent=true&includeCounts=true&archived=true&minimal=true' - } else { - // Just the name and lat/lng. - url = '/api/v2/groups/names?locale=' + getLocale() - } + // The names index is enough to draw the map and run the client-side + // filters; the table hydrates the rows it actually shows via + // hydrate(). Archived groups are included: the list badges them, as + // the old server-rendered page did. + const url = '/api/v2/groups/names?locale=' + getLocale() + + (params && params.details ? '&includeArchived=true' : '') let ret = await axios.get(url) if (ret) { commit('setList', { - groups: ret.data.data + groups: ret.data.data.map(g => ({ + id: g.id, + name: g.name, + lat: g.lat, + lng: g.lng, + location: { + location: null, + country: g.country, + lat: g.lat, + lng: g.lng, + }, + networks: g.network_ids || [], + group_tags_full: (g.tag_ids || []).map(id => ({ id })), + archived_at: g.archived_at, + })) }) } }, + async hydrate({commit, state}, params) { + // Fetch full rows (image, location text, counts, next event, tag names) + // for just the given ids - one batched call per visible page of the + // list, instead of serialising every group up front. + const ids = params.ids.filter(id => { + const g = state.list[id] + return (!g || !g.summary) && !state.hydrating[id] + }) + + if (!ids.length) { + return + } + + commit('setHydrating', { ids, value: true }) + + try { + // The API caps ids at 200 per call. + for (let i = 0; i < ids.length; i += 200) { + const chunk = ids.slice(i, i + 200) + const ret = await axios.get('/api/v2/groups/summary?locale=' + getLocale() + + '&includeNextEvent=true&includeCounts=true&archived=true&ids=' + chunk.join(',')) + + if (ret && ret.data) { + ret.data.data.forEach(g => commit('set', g)) + } + } + } finally { + commit('setHydrating', { ids, value: false }) + } + }, async listTags({commit, rootGetters}) { const apiToken = rootGetters['auth/apiToken'] let url = '/api/v2/groups/tags?locale=' + getLocale() diff --git a/resources/js/store/groups.test.js b/resources/js/store/groups.test.js index 3ce2959d3c..ee0251d454 100644 --- a/resources/js/store/groups.test.js +++ b/resources/js/store/groups.test.js @@ -102,8 +102,7 @@ test('the details list fetch asks for archived groups (shown with a badge, as th await groups.actions.list({ commit }, { details: true }) - expect(axios.get.mock.calls[0][0]).toContain('archived=true') - expect(axios.get.mock.calls[0][0]).toContain('minimal=true') + expect(axios.get.mock.calls[0][0]).toContain('includeArchived=true') }) // GroupsTable renders its rows from the groups/list store; groups that only diff --git a/tests/Feature/Groups/GroupSummaryApiTest.php b/tests/Feature/Groups/GroupSummaryApiTest.php index 24ebd7e2f8..74ec210dd2 100644 --- a/tests/Feature/Groups/GroupSummaryApiTest.php +++ b/tests/Feature/Groups/GroupSummaryApiTest.php @@ -80,57 +80,67 @@ public function testQueryCountDoesNotScaleWithGroupCount(): void ); } - public function testMinimalParamReturnsOnlyWhatTheMapPageUses(): void + public function testIdsParamHydratesOnlyThoseGroups(): void { $network = Network::factory()->create(); $tag = GroupTags::factory()->create(); - $group = Group::factory()->create(['name' => 'Minimal Group']); - $group->addTag($tag); - $network->addGroup($group); + $a = Group::factory()->create(['name' => 'Hydrate A']); + $b = Group::factory()->create(['name' => 'Hydrate B']); + $other = Group::factory()->create(['name' => 'Not Asked For']); + $a->addTag($tag); + $network->addGroup($a); Party::factory()->create([ - 'group' => $group->idgroups, + 'group' => $a->idgroups, 'event_start_utc' => Carbon::now()->addDays(3)->toIso8601String(), 'event_end_utc' => Carbon::now()->addDays(3)->addHours(2)->toIso8601String(), 'approved' => true, ]); \Cache::forget('future_events'); - $response = $this->get('/api/v2/groups/summary?minimal=true&includeNextEvent=true&includeCounts=true'); + $response = $this->get('/api/v2/groups/summary?ids=' . $a->idgroups . ',' . $b->idgroups + . '&includeNextEvent=true&includeCounts=true&archived=true'); $response->assertSuccessful(); - $g = collect($response->json('data'))->firstWhere('id', $group->idgroups); - $this->assertNotNull($g); + $data = collect($response->json('data')); + + // Only the requested groups, with the full row shape the list needs. + $this->assertEqualsCanonicalizing([$a->idgroups, $b->idgroups], $data->pluck('id')->all()); + $ga = $data->firstWhere('id', $a->idgroups); + $this->assertEquals($network->id, $ga['networks'][0]['id']); + $this->assertNotNull($ga['next_event']); + $this->assertArrayHasKey('hosts', $ga); + $this->assertEquals($tag->id, $ga['group_tags_full'][0]['id']); + } - // Exactly the fields the map page consumes - nothing else. - $this->assertEqualsCanonicalizing( - ['id', 'name', 'image', 'location', 'networks', 'group_tags_full', - 'archived_at', 'summary', 'hosts', 'restarters', 'next_event'], - array_keys($g) - ); - $this->assertEqualsCanonicalizing( - ['location', 'country', 'lat', 'lng'], - array_keys($g['location']) - ); - // Networks collapse to plain ids. - $this->assertEquals([$network->id], $g['networks']); - // next_event keeps only what the list and modal show (plus the - // id/summary fields its schema requires). - $this->assertEqualsCanonicalizing( - ['id', 'start', 'title', 'summary'], - array_keys($g['next_event']) - ); + public function testIdsParamRejectsMoreThanTwoHundred(): void + { + $this->expectException(\Illuminate\Validation\ValidationException::class); + $this->get('/api/v2/groups/summary?ids=' . implode(',', range(1, 201))); } - public function testDefaultShapeUnchangedWithoutMinimal(): void + public function testNamesIndexCarriesFilterFields(): void { $network = Network::factory()->create(); - $group = Group::factory()->create(); + $tag = GroupTags::factory()->create(); + $group = Group::factory()->create([ + 'name' => 'Index Group', + 'country_code' => 'GB', + ]); + $group->addTag($tag); $network->addGroup($group); - $response = $this->get('/api/v2/groups/summary'); + $response = $this->get('/api/v2/groups/names?includeArchived=true'); + $response->assertSuccessful(); $g = collect($response->json('data'))->firstWhere('id', $group->idgroups); + $this->assertNotNull($g); - $this->assertArrayHasKey('updated_at', $g); - $this->assertEquals($network->id, $g['networks'][0]['id']); - $this->assertArrayHasKey('area', $g['location']); + // The map/list index: identity + position + everything the client-side + // filters need, and nothing heavier. + $this->assertEqualsCanonicalizing( + ['id', 'name', 'lat', 'lng', 'archived_at', 'country', 'network_ids', 'tag_ids'], + array_keys($g) + ); + $this->assertEquals('United Kingdom', $g['country']); + $this->assertEquals([$network->id], $g['network_ids']); + $this->assertEquals([$tag->id], $g['tag_ids']); } } From 3979bec4150a12081c4120c05c0a987ef1d49543 Mon Sep 17 00:00:00 2001 From: edwh Date: Wed, 15 Jul 2026 09:07:09 +0100 Subject: [PATCH 08/34] Document the new names-index fields in the OpenAPI annotation Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01TzY1phBjyT3HogpeS53zon --- app/Http/Controllers/API/GroupController.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/Http/Controllers/API/GroupController.php b/app/Http/Controllers/API/GroupController.php index 71008e6b4e..0b6ca938df 100644 --- a/app/Http/Controllers/API/GroupController.php +++ b/app/Http/Controllers/API/GroupController.php @@ -257,6 +257,12 @@ public static function getGroupList(): JsonResponse * type="object", * @OA\Property(property="id", type="integer", example=1), * @OA\Property(property="name", type="string", example="Group Name"), + * @OA\Property(property="lat", type="number", nullable=true, example=51.5), + * @OA\Property(property="lng", type="number", nullable=true, example=-0.12), + * @OA\Property(property="country", type="string", nullable=true, example="United Kingdom"), + * @OA\Property(property="network_ids", type="array", @OA\Items(type="integer")), + * @OA\Property(property="tag_ids", type="array", @OA\Items(type="integer")), + * @OA\Property(property="archived_at", type="string", format="date-time", nullable=true), * ) * ) * ) From e040d41d94d856af7e3b2038ccdf003b8747cba6 Mon Sep 17 00:00:00 2001 From: edwh Date: Wed, 15 Jul 2026 09:20:58 +0100 Subject: [PATCH 09/34] Tests for the hydrate action and index-entry shaping Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01TzY1phBjyT3HogpeS53zon --- resources/js/store/groups.test.js | 63 +++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/resources/js/store/groups.test.js b/resources/js/store/groups.test.js index ee0251d454..ee643f42f0 100644 --- a/resources/js/store/groups.test.js +++ b/resources/js/store/groups.test.js @@ -154,3 +154,66 @@ describe('getModerationRequired', () => { expect(store.getters['groups/get'](7).location).toEqual({ lat: 1, lng: 2 }) }) }) + +describe('hydrate', () => { + test('fetches full rows for un-hydrated ids in one batched call', async () => { + axios.get.mockResolvedValueOnce({ data: { data: [{ id: 1, summary: true }, { id: 2, summary: true }] } }) + const commits = [] + const state = { list: {}, hydrating: {} } + + await groups.actions.hydrate( + { commit: (type, params) => commits.push([type, params]), state }, + { ids: [1, 2] } + ) + + expect(axios.get).toHaveBeenCalledTimes(1) + const url = axios.get.mock.calls[0][0] + expect(url).toContain('/api/v2/groups/summary') + expect(url).toContain('ids=1,2') + expect(url).toContain('includeNextEvent=true') + expect(url).toContain('includeCounts=true') + expect(commits.filter(c => c[0] === 'set').map(c => c[1].id)).toEqual([1, 2]) + }) + + test('skips ids already hydrated or in flight', async () => { + const state = { + // A full row has summary: true; a bare index entry does not. + list: { 1: { id: 1, summary: true }, 2: { id: 2 } }, + hydrating: { 2: true }, + } + + await groups.actions.hydrate({ commit: () => {}, state }, { ids: [1, 2] }) + + expect(axios.get).not.toHaveBeenCalled() + }) + + test('chunks requests at the API cap of 200 ids', async () => { + axios.get.mockResolvedValue({ data: { data: [] } }) + const state = { list: {}, hydrating: {} } + const ids = Array.from({ length: 250 }, (_, i) => i + 1) + + await groups.actions.hydrate({ commit: () => {}, state }, { ids }) + + expect(axios.get).toHaveBeenCalledTimes(2) + expect(axios.get.mock.calls[0][0]).toContain('ids=1,') + expect(axios.get.mock.calls[1][0]).toContain('ids=201,') + }) +}) + +test('the index fetch shapes entries for the map, filters and table', async () => { + axios.get.mockResolvedValueOnce({ + data: { data: [{ id: 7, name: 'G', lat: 51, lng: 0, country: 'United Kingdom', network_ids: [3], tag_ids: [9], archived_at: null }] } + }) + const commits = [] + + await groups.actions.list( + { commit: (type, params) => commits.push([type, params]) }, + { details: true } + ) + + expect(axios.get.mock.calls[0][0]).toContain('/api/v2/groups/names') + const g = commits.find(c => c[0] === 'setList')[1].groups[0] + expect(g.location).toEqual({ location: null, country: 'United Kingdom', lat: 51, lng: 0 }) + expect(g.networks).toEqual([3]) + expect(g.group_tags_full).toEqual([{ id: 9 }]) +}) From 6a819446b79af4530633bb0d0afe0c252f32d94f Mon Sep 17 00:00:00 2001 From: edwh Date: Wed, 15 Jul 2026 09:51:23 +0100 Subject: [PATCH 10/34] Names index: cast lat/lng and pivot ids to their documented types The new OpenAPI property types exposed that decimals and the varchar pivot columns serialise as strings; the response validator (rightly) rejected them. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01TzY1phBjyT3HogpeS53zon --- app/Http/Controllers/API/GroupController.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/Http/Controllers/API/GroupController.php b/app/Http/Controllers/API/GroupController.php index 0b6ca938df..a191f5c575 100644 --- a/app/Http/Controllers/API/GroupController.php +++ b/app/Http/Controllers/API/GroupController.php @@ -303,11 +303,12 @@ public static function listNamesv2(Request $request) { $ret[] = [ 'id' => $group->idgroups, 'name' => $group->name, - 'lat' => $group->latitude, - 'lng' => $group->longitude, + 'lat' => $group->latitude !== null ? (float) $group->latitude : null, + 'lng' => $group->longitude !== null ? (float) $group->longitude : null, 'country' => \App\Helpers\Fixometer::getCountryFromCountryCode($group->country_code), - 'network_ids' => $networkIds->has($group->idgroups) ? $networkIds[$group->idgroups]->pluck('network_id')->all() : [], - 'tag_ids' => $tagIds->has($group->idgroups) ? $tagIds[$group->idgroups]->pluck('group_tag')->all() : [], + 'network_ids' => $networkIds->has($group->idgroups) ? $networkIds[$group->idgroups]->pluck('network_id')->map(fn ($id) => (int) $id)->all() : [], + // The pivot columns are varchars; the API contract is integers. + 'tag_ids' => $tagIds->has($group->idgroups) ? $tagIds[$group->idgroups]->pluck('group_tag')->map(fn ($id) => (int) $id)->all() : [], 'archived_at' => $group->archived_at ? Carbon::parse($group->archived_at)->toIso8601String() : null ]; } From 1dbf435be9f1b846818edb63121fd3cda15ffcaf Mon Sep 17 00:00:00 2001 From: edwh Date: Wed, 15 Jul 2026 13:21:44 +0100 Subject: [PATCH 11/34] Map search: import the geocoder stylesheet; translate its strings The control creates its "Nothing found." element at initialisation and relies on the plugin stylesheet to keep it hidden until a search fails - without the import it showed from page load, next to an unstyled icon button. Also styles the genuine error state to match the site and translates the placeholder and error message (en/fr/fr-BE). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01TzY1phBjyT3HogpeS53zon --- lang/en/groups.php | 2 ++ lang/fr-BE/groups.php | 2 ++ lang/fr/groups.php | 2 ++ resources/js/components/GroupMap.vue | 19 ++++++++++++++++++- 4 files changed, 24 insertions(+), 1 deletion(-) diff --git a/lang/en/groups.php b/lang/en/groups.php index 76ca8c25f9..c2aad0e79f 100644 --- a/lang/en/groups.php +++ b/lang/en/groups.php @@ -177,6 +177,8 @@ 'export.events.items_kg_waste_prevented' => 'kg waste prevented', 'export.events.items_kg_co2_prevent' => 'kg CO2 prevented', 'marker_title' => 'Click for more information', + 'search_place' => 'Search for a place...', + 'search_nothing_found' => 'Nothing found.', 'goto_group' => 'Go to group', 'next_event' => 'Next event', ]; diff --git a/lang/fr-BE/groups.php b/lang/fr-BE/groups.php index 4a0a3b9300..c6b7567ae0 100644 --- a/lang/fr-BE/groups.php +++ b/lang/fr-BE/groups.php @@ -180,6 +180,8 @@ 'export.events.items_kg_waste_prevented' => 'kg déchets évités', 'export.events.items_kg_co2_prevent' => 'kg emissions de CO2 évitées', 'marker_title' => 'Cliquez pour plus d\'informations', + 'search_place' => 'Rechercher un lieu...', + 'search_nothing_found' => 'Aucun résultat.', 'goto_group' => 'Aller au Repair Café', 'next_event' => 'Prochain événement', ]; diff --git a/lang/fr/groups.php b/lang/fr/groups.php index 7ff3715816..4eef9e69b1 100644 --- a/lang/fr/groups.php +++ b/lang/fr/groups.php @@ -180,6 +180,8 @@ 'export.events.items_kg_waste_prevented' => 'kg déchets évités', 'export.events.items_kg_co2_prevent' => 'kg emissions de CO2 évitées', 'marker_title' => 'Cliquez pour plus d\'informations', + 'search_place' => 'Rechercher un lieu...', + 'search_nothing_found' => 'Aucun résultat.', 'goto_group' => 'Aller au Repair Café', 'next_event' => 'Prochain événement', ]; diff --git a/resources/js/components/GroupMap.vue b/resources/js/components/GroupMap.vue index f38c38af72..cd64a82ec5 100644 --- a/resources/js/components/GroupMap.vue +++ b/resources/js/components/GroupMap.vue @@ -22,6 +22,9 @@ import map from '../mixins/map' import { Geocoder } from 'leaflet-control-geocoder/src/control' import { Photon } from 'leaflet-control-geocoder/src/geocoders/photon' +// The control builds its "nothing found" element at init; this stylesheet is +// what keeps it hidden until a search actually fails (and styles the button). +import 'leaflet-control-geocoder/dist/Control.Geocoder.css' import GroupMarker from './GroupMarker.vue' export default { @@ -181,7 +184,8 @@ export default { if (this.mapObject) { try { new Geocoder({ - placeholder: 'Search for a place...', + placeholder: this.__('groups.search_place'), + errorMessage: this.__('groups.search_nothing_found'), defaultMarkGeocode: false, geocoder: new Photon({ nameProperties: [ @@ -347,6 +351,19 @@ export default { right: 30px; } +// Belt and braces over the plugin CSS: never show the error element unless +// the control has flagged a failed search. +:deep(.leaflet-control-geocoder-form-no-error) { + display: none; +} + +:deep(.leaflet-control-geocoder-error) { + display: block; + padding: 0.375rem 1rem 0.5rem; + font-size: 0.875rem; + color: #6c757d; +} + @media screen and (max-width: 360px) { :deep(.leaflet-control-geocoder-form input) { max-width: 200px; From c5689fce457a0c62a1f7581cee1ef80319f578ad Mon Sep 17 00:00:00 2001 From: edwh Date: Tue, 21 Jul 2026 11:34:20 +0100 Subject: [PATCH 12/34] Map feedback: pin anchoring, instant tooltip, and the right "next event" Three items from Neil's testing document that had a clear root cause. The pins looked mispositioned - a group in Ulverston appeared to be out in Morecambe Bay. GroupMarker built its icon with L.icon({iconUrl}) and nothing else; with no iconSize/iconAnchor Leaflet puts the image's top-left corner on the coordinate, so the whole 25x41 teardrop hung down and to the right of the place it was meant to be pointing at. Set Leaflet's own defaults for this image so the tip of the pin is the anchor. The group name took about a second to appear on hover because it was the native `title` attribute, whose delay belongs to the browser and can't be tuned. Use a Leaflet tooltip instead, which shows as soon as the pointer arrives; the name stays on the marker as `alt` for screen readers. "Next event" showed the event furthest in the future rather than the soonest - Ulverston RC's December event instead of its August one. scopeUndeleted() applies an ORDER BY event_start_utc DESC, and orderBy() appends rather than replaces, so scopeFuture() was compiling to "ORDER BY event_start_utc DESC, event_start_utc ASC" - the DESC winning. scopeFutureForUser() already guards against this with reorder(); do the same here. This also fixes the upcoming event lists on the group and event pages, which were ordered furthest-first for the same reason. Tests: the existing next_event coverage only ever gave a group one future event, which can't tell soonest from furthest. Added a group with two. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB --- app/Party.php | 8 ++- resources/js/app.js | 1 + resources/js/components/GroupMarker.test.js | 22 +++++++ resources/js/components/GroupMarker.vue | 20 +++++- tests/Feature/Groups/GroupViewTest.php | 67 +++++++++++++++++++++ 5 files changed, 115 insertions(+), 3 deletions(-) diff --git a/app/Party.php b/app/Party.php index cfc6a8a4fd..377d4e51d7 100644 --- a/app/Party.php +++ b/app/Party.php @@ -213,7 +213,13 @@ public function scopePast($query) { public function scopeFuture($query) { // A future event is an event where the start time is greater than now. $query = $query->undeleted(); - $query = $query->where('event_start_utc', '>', date('Y-m-d H:i:s'))->orderBy('event_start_utc','ASC'); + // undeleted() has already applied an ORDER BY event_start_utc DESC, and + // orderBy() appends rather than replaces. Without the reorder() we end up + // with "ORDER BY event_start_utc DESC, event_start_utc ASC", where the DESC + // wins - so callers asking for the *next* event got the one furthest in the + // future instead. scopeFutureForUser() already does this. + $query = $query->where('event_start_utc', '>', date('Y-m-d H:i:s')) + ->reorder()->orderBy('event_start_utc','ASC'); return $query; } diff --git a/resources/js/app.js b/resources/js/app.js index dec3fd8fe5..e28bda451a 100644 --- a/resources/js/app.js +++ b/resources/js/app.js @@ -436,6 +436,7 @@ function initializeJQuery() { Vue.component('l-map', leafletModule.LMap); Vue.component('l-marker', leafletModule.LMarker); Vue.component('l-tile-layer', leafletModule.LTileLayer); + Vue.component('l-tooltip', leafletModule.LTooltip); }).catch((e) => { console.warn('Vue2-Leaflet components not available, using fallback:', e.message); }); diff --git a/resources/js/components/GroupMarker.test.js b/resources/js/components/GroupMarker.test.js index c658c34110..8198c24568 100644 --- a/resources/js/components/GroupMarker.test.js +++ b/resources/js/components/GroupMarker.test.js @@ -46,6 +46,15 @@ describe('GroupMarker icon', () => { expect(mountMarker({ hover: true }).vm.icon.options.iconUrl).toBe('/images/vendor/leaflet/dist/marker-icon.png') }) + // Neil's PR feedback: pins looked mispositioned - a group in Ulverston appeared + // to be out in Morecambe Bay. With no iconSize/iconAnchor, Leaflet puts the + // image's top-left corner on the coordinate rather than the tip of the pin. + test('anchors the tip of the pin to the coordinate, not its top-left corner', () => { + const options = mountMarker().vm.icon.options + expect(options.iconSize).toEqual([25, 41]) + expect(options.iconAnchor).toEqual([12, 41]) + }) + test('recolours via a CSS class: green for groups you follow, red on hover', () => { expect(mountMarker().vm.icon.options.className).toBe('') expect(mountMarker({ highlight: true }).vm.icon.options.className).toBe('group-marker-yours') @@ -54,6 +63,19 @@ describe('GroupMarker icon', () => { }) }) +// Neil's PR feedback: the group name took too long to appear on hover. It was +// the native `title` attribute, whose delay is the browser's and can't be tuned; +// a Leaflet tooltip shows as soon as the pointer arrives. +describe('GroupMarker tooltip', () => { + test('renders the name in a Leaflet tooltip rather than a native title', () => { + const wrapper = mountMarker() + + const tooltip = wrapper.find('l-tooltip') + expect(tooltip.exists()).toBe(true) + expect(tooltip.text()).toContain('Test Group') + }) +}) + // Neil's PR feedback: hovering a pin should highlight the matching list row // (the reverse of row-hover → red pin). describe('GroupMarker hover emission', () => { diff --git a/resources/js/components/GroupMarker.vue b/resources/js/components/GroupMarker.vue index 8e67953680..fcd0db51eb 100644 --- a/resources/js/components/GroupMarker.vue +++ b/resources/js/components/GroupMarker.vue @@ -4,12 +4,17 @@ mouseover drives the highlight. --> + > + + {{ label }} +
@@ -63,9 +68,20 @@ export default { return L.icon({ iconUrl: '/images/vendor/leaflet/dist/marker-icon.png', + // Without iconSize/iconAnchor Leaflet pins the image's top-left corner to + // the coordinate, so the whole 25x41 teardrop hangs down and to the right + // and the pin appears to point at somewhere else entirely. These are + // Leaflet's own defaults for this image: anchor the tip of the pin. + iconSize: [25, 41], + iconAnchor: [12, 41], + // Measured from the anchor (the tip), so the tooltip clears the pin head. + tooltipAnchor: [0, -41], className: className, }) }, + label() { + return this.group.name + ' - ' + this.__('groups.marker_title') + }, group() { return this.$store.getters['groups/get'](this.id) }, diff --git a/tests/Feature/Groups/GroupViewTest.php b/tests/Feature/Groups/GroupViewTest.php index 802f89546d..e7e0bfa61a 100644 --- a/tests/Feature/Groups/GroupViewTest.php +++ b/tests/Feature/Groups/GroupViewTest.php @@ -7,6 +7,7 @@ use App\Party; use App\Role; use Carbon\Carbon; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\DB; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Tests\TestCase; @@ -196,6 +197,72 @@ public function testGroupIndexNextEventIsEagerLoaded(): void $this->assertNotNull($group2['next_event'], 'Group Beta should have a next_event'); } + public function testNextEventIsTheSoonestNotTheFurthestAway(): void + { + $this->loginAsTestUser(Role::ADMINISTRATOR); + + // A group with more than one upcoming event. The existing coverage only + // ever gave a group a single future event, which can't tell "soonest" from + // "furthest away" - and the summary API was returning the latter. + $id = $this->createGroup('Group With Two Events'); + + $soon = Carbon::parse('1pm tomorrow'); + $later = Carbon::parse('1pm +5 months'); + + // Created furthest-first, so a test that happens to pass on insertion order + // rather than on the ordering we asked for would still fail here. + Party::factory()->create([ + 'group' => $id, + 'approved' => true, + 'event_start_utc' => $later->toIso8601String(), + 'event_end_utc' => $later->copy()->addHours(2)->toIso8601String(), + ]); + $soonest = Party::factory()->create([ + 'group' => $id, + 'approved' => true, + 'event_start_utc' => $soon->toIso8601String(), + 'event_end_utc' => $soon->copy()->addHours(2)->toIso8601String(), + ]); + + // The upcoming events are cached globally, not per group. + Cache::forget('future_events'); + + $response = $this->get('/api/v2/groups/summary?includeNextEvent=true'); + $response->assertSuccessful(); + + $group = collect($response->json('data'))->firstWhere('id', $id); + $this->assertNotNull($group, "Group (id=$id) not found in API response"); + $this->assertNotNull($group['next_event'], 'Group should have a next_event'); + $this->assertEquals($soonest->idevents, $group['next_event']['id'], 'next_event should be the soonest upcoming event, not the furthest away'); + } + + public function testFutureScopeReturnsSoonestFirst(): void + { + // The summary API relies on Party::future() coming back in ascending date + // order. scopeUndeleted() applies a DESC order and orderBy() appends, so + // this needs an explicit reorder() to hold. + $this->loginAsTestUser(Role::ADMINISTRATOR); + $id = $this->createGroup('Group For Scope Ordering'); + + foreach (['1pm +3 months', '1pm tomorrow', '1pm +5 months'] as $when) { + $at = Carbon::parse($when); + Party::factory()->create([ + 'group' => $id, + 'approved' => true, + 'event_start_utc' => $at->toIso8601String(), + 'event_end_utc' => $at->copy()->addHours(2)->toIso8601String(), + ]); + } + + $starts = Party::future()->forGroup($id)->get()->pluck('event_start_utc')->map(function ($d) { + return Carbon::parse($d)->timestamp; + })->all(); + + $sorted = $starts; + sort($sorted); + $this->assertEquals($sorted, $starts, 'Party::future() should return the soonest event first'); + } + public function testGroupIndexQueryCountScalesWithO1NotN(): void { $this->loginAsTestUser(Role::ADMINISTRATOR); From ebfeaed32c0862d662e8318d4b8d8afeccee4d8d Mon Sep 17 00:00:00 2001 From: edwh Date: Tue, 21 Jul 2026 13:21:32 +0100 Subject: [PATCH 13/34] Preload the map's place search with the user's own area Neil's PR feedback: as well as centring the map on your town, put that town in the "Search for a place..." box, as a visual hint that the map has already been searched for you rather than an empty box that looks like nothing has happened. The controller already works out the user's area and the blade already hands it to GroupsPage, but it stopped there - it now carries on down to the map. The geocoder is a Leaflet control rather than a Vue component, so we keep a reference to it when it's created and set its query once it exists; presetSearch is a no-op until then. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB --- resources/js/components/GroupMap.test.js | 36 +++++++++++++++++-- resources/js/components/GroupMap.vue | 18 +++++++++- .../js/components/GroupMapAndList.test.js | 9 +++++ resources/js/components/GroupMapAndList.vue | 6 ++++ resources/js/components/GroupsPage.test.js | 8 +++++ resources/js/components/GroupsPage.vue | 1 + 6 files changed, 75 insertions(+), 3 deletions(-) diff --git a/resources/js/components/GroupMap.test.js b/resources/js/components/GroupMap.test.js index ad3f780d3f..f94d95bfd5 100644 --- a/resources/js/components/GroupMap.test.js +++ b/resources/js/components/GroupMap.test.js @@ -42,11 +42,11 @@ function fakeMap(size = { x: 688, y: 400 }) { } } -function mountMap(initialBounds, groups) { +function mountMap(initialBounds, groups, props = {}) { return shallowMount(GroupMap, { localVue, store: makeStore(groups), - propsData: { initialBounds }, + propsData: { initialBounds, ...props }, }) } @@ -155,6 +155,38 @@ describe('GroupMap options', () => { }) }) +// Neil's PR feedback: as well as centring the map on your town, put that town in +// the "Search for a place..." box - a visual hint that the map has already been +// searched for you. +describe('GroupMap place search preload', () => { + test('preloads the search box with the area the map was centred on', () => { + const wrapper = mountMap(WORLD, [], { yourArea: 'Ulverston' }) + const geocoder = { setQuery: jest.fn() } + wrapper.vm.geocoder = geocoder + + wrapper.vm.presetSearch() + + expect(geocoder.setQuery).toHaveBeenCalledWith('Ulverston') + }) + + test('leaves the box empty when the user has no area set', () => { + const wrapper = mountMap(WORLD, [], { yourArea: '' }) + const geocoder = { setQuery: jest.fn() } + wrapper.vm.geocoder = geocoder + + wrapper.vm.presetSearch() + + expect(geocoder.setQuery).not.toHaveBeenCalled() + }) + + // The search box is built inside the Leaflet control, so there's nothing to + // preload until the map is ready. + test('does nothing when the geocoder control has not been created yet', () => { + const wrapper = mountMap(WORLD, [], { yourArea: 'Ulverston' }) + expect(() => wrapper.vm.presetSearch()).not.toThrow() + }) +}) + describe('GroupMap markers', () => { test('only renders markers for groups with coordinates', () => { const wrapper = mountMap(WORLD, [ diff --git a/resources/js/components/GroupMap.vue b/resources/js/components/GroupMap.vue index cd64a82ec5..a76a852deb 100644 --- a/resources/js/components/GroupMap.vue +++ b/resources/js/components/GroupMap.vue @@ -61,12 +61,18 @@ export default { type: Number, required: false, default: null, + }, + yourArea: { + type: String, + required: false, + default: '', } }, data() { return { moved: false, mapObject: null, + geocoder: null, zoom: this.minZoom, destroyed: false, mapIdle: 0, @@ -183,7 +189,7 @@ export default { if (this.mapObject) { try { - new Geocoder({ + this.geocoder = new Geocoder({ placeholder: this.__('groups.search_place'), errorMessage: this.__('groups.search_nothing_found'), defaultMarkGeocode: false, @@ -213,6 +219,8 @@ export default { } }) .addTo(this.mapObject) + + this.presetSearch() } catch (e) { // This is usually caused by leaflet. console.log('Ignore leaflet exception', e) @@ -221,6 +229,14 @@ export default { this.idle() }, + presetSearch() { + // We've already centred the map on the user's area, so show that area in + // the search box too - a hint that the map has been searched for them, + // rather than an empty box that looks like nothing has happened. + if (this.geocoder && this.yourArea) { + this.geocoder.setQuery(this.yourArea) + } + }, idle() { this.mapObject = this.$refs.map.mapObject this.mapIdle++ diff --git a/resources/js/components/GroupMapAndList.test.js b/resources/js/components/GroupMapAndList.test.js index 9f43876e0a..5b81c4b0c4 100644 --- a/resources/js/components/GroupMapAndList.test.js +++ b/resources/js/components/GroupMapAndList.test.js @@ -38,6 +38,7 @@ const groupMapStub = { initialBounds: { type: Array }, network: { type: Number, default: null }, yourGroups: { type: Array, default: () => [] }, + yourArea: { type: String, default: '' }, hover: { type: Number, default: null }, }, template: '
', @@ -99,6 +100,14 @@ test('forwards network and filter context to the map and the table', async () => expect(table.props('showTags')).toBe(true) }) +// The search box is preloaded with the user's area, so the town has to survive +// the whole way down from the page to the map. +test('forwards yourArea to the map so the search box can be preloaded', async () => { + const wrapper = await makeWrapper({ yourArea: 'Ulverston' }) + + expect(wrapper.findComponent(groupMapStub).props('yourArea')).toBe('Ulverston') +}) + describe('effectiveGroupIds', () => { test('falls back to the full (network-filtered) list before the map has reported', async () => { const wrapper = await makeWrapper({ network: 5 }) diff --git a/resources/js/components/GroupMapAndList.vue b/resources/js/components/GroupMapAndList.vue index 26c1922336..e7073f613c 100644 --- a/resources/js/components/GroupMapAndList.vue +++ b/resources/js/components/GroupMapAndList.vue @@ -14,6 +14,7 @@ :bounds.sync="bounds" :network="network" :your-groups="yourGroups" + :your-area="yourArea" :hover="hover" @update:hover="hover = $event" @groups="groupsChanged($event)" @@ -69,6 +70,11 @@ export default { required: false, default: () => [], }, + yourArea: { + type: String, + required: false, + default: '', + }, showFilters: { type: Boolean, required: false, diff --git a/resources/js/components/GroupsPage.test.js b/resources/js/components/GroupsPage.test.js index c916649080..0c7f78946a 100644 --- a/resources/js/components/GroupsPage.test.js +++ b/resources/js/components/GroupsPage.test.js @@ -41,6 +41,7 @@ const groupMapStub = { props: { initialBounds: { type: Array }, yourGroups: { type: Array, default: () => [] }, + yourArea: { type: String, default: '' }, network: { type: Number, default: null }, showFilters: { type: Boolean, default: false }, canManageTags: { type: Boolean, default: false }, @@ -129,6 +130,13 @@ test('forwards network + filter context to the map list on a network view', asyn expect(map.props('networks')).toBeNull() }) +test('forwards yourArea to the map list so the place search is preloaded', async () => { + const wrapper = makeWrapper({ tab: 'other', nearbyGroups: WORLD_BOUNDS, yourArea: 'Ulverston' }) + await flushTabs(wrapper) + + expect(wrapper.findComponent(groupMapStub).props('yourArea')).toBe('Ulverston') +}) + test('passes the networks list for the network filter on the plain groups page', async () => { const networks = [{ id: 10, name: 'Test' }] const wrapper = makeWrapper({ tab: 'other', nearbyGroups: WORLD_BOUNDS, networks }) diff --git a/resources/js/components/GroupsPage.vue b/resources/js/components/GroupsPage.vue index 84930b3965..025bd7b82f 100644 --- a/resources/js/components/GroupsPage.vue +++ b/resources/js/components/GroupsPage.vue @@ -52,6 +52,7 @@ Date: Tue, 21 Jul 2026 13:47:46 +0100 Subject: [PATCH 14/34] Only approved events count as a group's next event The summary API took any future event, but the group's own page uses Group::getNextUpcomingEvent(), which only counts approved ones. So an event still awaiting moderation was advertised as the group's next event on the public map while the group's page ignored it. The comment above the code already said "Get next approved event for group" - it just never did. Filtered at the query rather than in the loop, so unapproved events don't sit in the cache at all. The cache key changes with the contents: the old key could otherwise keep serving unapproved events for up to a minute after deploy. Tests: an unapproved event that falls sooner must be skipped in favour of a later approved one, and a group whose only upcoming event is unapproved must report no next event at all. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB --- app/Http/Resources/GroupSummary.php | 12 +- tests/Feature/Groups/GroupSummaryApiTest.php | 2 +- tests/Feature/Groups/GroupViewTest.php | 63 +- tests/clover.xml | 10933 +++++++++++++++++ 4 files changed, 11004 insertions(+), 6 deletions(-) create mode 100644 tests/clover.xml diff --git a/app/Http/Resources/GroupSummary.php b/app/Http/Resources/GroupSummary.php index 07813a5eeb..7f3b50b391 100644 --- a/app/Http/Resources/GroupSummary.php +++ b/app/Http/Resources/GroupSummary.php @@ -140,10 +140,14 @@ public function toArray(Request $request): array if ($request->get('includeNextEvent', false)) { // Get next approved event for group. We cache all upcoming events to speed up the case where we // are fetching many groups. - if (Cache::has('future_events')) { - $upcoming = Cache::get('future_events'); + // + // Only approved events count, matching Group::getNextUpcomingEvent() which the group's own page uses. + // Without this an event still awaiting moderation was advertised as the group's next event on the + // public map while the group's page ignored it. + if (Cache::has('future_approved_events')) { + $upcoming = Cache::get('future_approved_events'); } else { - $future = \App\Party::future()->get(); + $future = \App\Party::future()->where('approved', true)->get(); // Can't serialise the whole event, and we only need a few fields. $upcoming = []; @@ -165,7 +169,7 @@ public function toArray(Request $request): array ]; } - Cache::put('future_events', $upcoming, 60); + Cache::put('future_approved_events', $upcoming, 60); } // Find the next event for this group. diff --git a/tests/Feature/Groups/GroupSummaryApiTest.php b/tests/Feature/Groups/GroupSummaryApiTest.php index 74ec210dd2..3cc8b06b26 100644 --- a/tests/Feature/Groups/GroupSummaryApiTest.php +++ b/tests/Feature/Groups/GroupSummaryApiTest.php @@ -95,7 +95,7 @@ public function testIdsParamHydratesOnlyThoseGroups(): void 'event_end_utc' => Carbon::now()->addDays(3)->addHours(2)->toIso8601String(), 'approved' => true, ]); - \Cache::forget('future_events'); + \Cache::forget('future_approved_events'); $response = $this->get('/api/v2/groups/summary?ids=' . $a->idgroups . ',' . $b->idgroups . '&includeNextEvent=true&includeCounts=true&archived=true'); diff --git a/tests/Feature/Groups/GroupViewTest.php b/tests/Feature/Groups/GroupViewTest.php index e7e0bfa61a..8c392cd6cf 100644 --- a/tests/Feature/Groups/GroupViewTest.php +++ b/tests/Feature/Groups/GroupViewTest.php @@ -225,7 +225,7 @@ public function testNextEventIsTheSoonestNotTheFurthestAway(): void ]); // The upcoming events are cached globally, not per group. - Cache::forget('future_events'); + Cache::forget('future_approved_events'); $response = $this->get('/api/v2/groups/summary?includeNextEvent=true'); $response->assertSuccessful(); @@ -236,6 +236,67 @@ public function testNextEventIsTheSoonestNotTheFurthestAway(): void $this->assertEquals($soonest->idevents, $group['next_event']['id'], 'next_event should be the soonest upcoming event, not the furthest away'); } + public function testNextEventIgnoresUnapprovedEvents(): void + { + $this->loginAsTestUser(Role::ADMINISTRATOR); + + // The group page only ever counts approved events as the "next" one + // (Group::getNextUpcomingEvent), but the summary API used to take any + // future event - so an unapproved event showed on the public map as the + // group's next event while the group's own page ignored it. + $id = $this->createGroup('Group With An Unapproved Event'); + + $soon = Carbon::parse('1pm tomorrow'); + $later = Carbon::parse('1pm +3 months'); + + Party::factory()->create([ + 'group' => $id, + 'approved' => false, + 'event_start_utc' => $soon->toIso8601String(), + 'event_end_utc' => $soon->copy()->addHours(2)->toIso8601String(), + ]); + $approved = Party::factory()->create([ + 'group' => $id, + 'approved' => true, + 'event_start_utc' => $later->toIso8601String(), + 'event_end_utc' => $later->copy()->addHours(2)->toIso8601String(), + ]); + + Cache::forget('future_approved_events'); + + $response = $this->get('/api/v2/groups/summary?includeNextEvent=true'); + $response->assertSuccessful(); + + $group = collect($response->json('data'))->firstWhere('id', $id); + $this->assertNotNull($group, "Group (id=$id) not found in API response"); + $this->assertNotNull($group['next_event'], 'Group should have a next_event'); + $this->assertEquals($approved->idevents, $group['next_event']['id'], 'next_event should skip the unapproved event and use the approved one'); + } + + public function testNextEventIsNullWhenTheOnlyUpcomingEventIsUnapproved(): void + { + $this->loginAsTestUser(Role::ADMINISTRATOR); + + $id = $this->createGroup('Group With Only An Unapproved Event'); + + $soon = Carbon::parse('1pm tomorrow'); + Party::factory()->create([ + 'group' => $id, + 'approved' => false, + 'event_start_utc' => $soon->toIso8601String(), + 'event_end_utc' => $soon->copy()->addHours(2)->toIso8601String(), + ]); + + Cache::forget('future_approved_events'); + + $response = $this->get('/api/v2/groups/summary?includeNextEvent=true'); + $response->assertSuccessful(); + + $group = collect($response->json('data'))->firstWhere('id', $id); + $this->assertNotNull($group, "Group (id=$id) not found in API response"); + $this->assertNull($group['next_event'], 'An unapproved event must not be advertised as the next event'); + } + public function testFutureScopeReturnsSoonestFirst(): void { // The summary API relies on Party::future() coming back in ascending date diff --git a/tests/clover.xml b/tests/clover.xml new file mode 100644 index 0000000000..99d7adfc5c --- /dev/null +++ b/tests/clover.xml @@ -0,0 +1,10933 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From b91d5c0a9f91f5064fe66990444709676879d29e Mon Sep 17 00:00:00 2001 From: edwh Date: Tue, 21 Jul 2026 14:01:17 +0100 Subject: [PATCH 15/34] Let a from-scratch database finish migrating Binary logging is on and the app user has no SUPER privilege, so the recreate_repair_status_triggers migration fails with "ERROR 1419: You do not have the SUPER privilege and binary logging is enabled". Existing databases don't hit it because their volumes pre-date that migration, but a fresh volume can never finish migrating - so a new developer, or anyone who removes the volume, gets a database that cannot be built. CircleCI already works around this with a SET GLOBAL before it migrates. Setting log_bin_trust_function_creators in my.cnf instead means it applies to a fresh volume without that manual step, and survives a restart, which a SET GLOBAL does not. The same setup step grants the app user SELECT on mysql.time_zone_name, which CONVERT_TZ() with named zones needs - without it the conversion silently returns NULL. That one has to run against the database rather than the server, so it goes in an init script, which the mysql image runs once when the volume is first created. Verified by destroying the volume and migrating with no manual SET GLOBAL. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB --- docker-compose.yml | 2 ++ mysql/init/01-grants.sql | 9 +++++++++ mysql/my.cnf | 11 ++++++++++- 3 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 mysql/init/01-grants.sql diff --git a/docker-compose.yml b/docker-compose.yml index 8a97348c4f..3c3b4cc3a6 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -135,6 +135,8 @@ services: volumes: - dbdata:/var/lib/mysql - ./mysql/my.cnf:/etc/mysql/my.cnf + # Run once when the data volume is first created - see the file for why. + - ./mysql/init:/docker-entrypoint-initdb.d:ro networks: - app-network diff --git a/mysql/init/01-grants.sql b/mysql/init/01-grants.sql new file mode 100644 index 0000000000..cdb7e97908 --- /dev/null +++ b/mysql/init/01-grants.sql @@ -0,0 +1,9 @@ +-- Run once, by the mysql image, when the data volume is first created. +-- +-- CONVERT_TZ() with named zones (e.g. CONVERT_TZ(event_start_utc, 'GMT', timezone)) +-- reads mysql.time_zone_name, which the app user cannot select from by default - +-- the conversion then silently returns NULL rather than erroring. CircleCI issues +-- this same grant before running the suite; doing it here keeps a local +-- from-scratch database consistent with CI. +GRANT SELECT ON mysql.time_zone_name TO 'restarters'@'%'; +FLUSH PRIVILEGES; diff --git a/mysql/my.cnf b/mysql/my.cnf index 038590de40..4372993b42 100644 --- a/mysql/my.cnf +++ b/mysql/my.cnf @@ -5,4 +5,13 @@ general_log_file = /var/lib/mysql/general.log # Disable ONLY_FULL_GROUP_BY for compatibility with getItemTypes() query # This query uses window functions in a way that's incompatible with strict GROUP BY mode -sql_mode = "STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION" \ No newline at end of file +sql_mode = "STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION" + +# Let the app user create the repair_status triggers. Binary logging is on and +# the app user has no SUPER privilege, so without this the +# recreate_repair_status_triggers migration fails with "ERROR 1419: You do not +# have the SUPER privilege and binary logging is enabled" and a from-scratch +# database can never finish migrating. CircleCI does the same thing with a +# SET GLOBAL before it migrates; setting it here means a fresh volume works +# without that manual step, and that it survives a restart. +log_bin_trust_function_creators = 1 \ No newline at end of file From f6485a506aadca305d1f68b3076b3acfef9582cd Mon Sep 17 00:00:00 2001 From: edwh Date: Tue, 21 Jul 2026 14:01:31 +0100 Subject: [PATCH 16/34] Comments should say why the code is as it is, not what changed Dropped the attribution to who reported each issue and the past tense that went with it. A comment is read by someone looking at the code now, not at its history, which git already has. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB --- app/Http/Resources/GroupSummary.php | 4 ++-- app/Party.php | 4 ++-- resources/js/components/GroupInfoModal.test.js | 3 +-- resources/js/components/GroupMarker.test.js | 16 ++++++++-------- resources/js/components/GroupsTable.test.js | 2 +- 5 files changed, 14 insertions(+), 15 deletions(-) diff --git a/app/Http/Resources/GroupSummary.php b/app/Http/Resources/GroupSummary.php index 7f3b50b391..cacd66e331 100644 --- a/app/Http/Resources/GroupSummary.php +++ b/app/Http/Resources/GroupSummary.php @@ -142,8 +142,8 @@ public function toArray(Request $request): array // are fetching many groups. // // Only approved events count, matching Group::getNextUpcomingEvent() which the group's own page uses. - // Without this an event still awaiting moderation was advertised as the group's next event on the - // public map while the group's page ignored it. + // Without this an event still awaiting moderation would be advertised as the group's next event on + // the public map while the group's page ignored it. if (Cache::has('future_approved_events')) { $upcoming = Cache::get('future_approved_events'); } else { diff --git a/app/Party.php b/app/Party.php index 377d4e51d7..4800630ec5 100644 --- a/app/Party.php +++ b/app/Party.php @@ -216,8 +216,8 @@ public function scopeFuture($query) { // undeleted() has already applied an ORDER BY event_start_utc DESC, and // orderBy() appends rather than replaces. Without the reorder() we end up // with "ORDER BY event_start_utc DESC, event_start_utc ASC", where the DESC - // wins - so callers asking for the *next* event got the one furthest in the - // future instead. scopeFutureForUser() already does this. + // wins - so a caller asking for the *next* event would get the one furthest + // in the future. scopeFutureForUser() does the same. $query = $query->where('event_start_utc', '>', date('Y-m-d H:i:s')) ->reorder()->orderBy('event_start_utc','ASC'); return $query; diff --git a/resources/js/components/GroupInfoModal.test.js b/resources/js/components/GroupInfoModal.test.js index aec1e60fe0..eaad8b97f4 100644 --- a/resources/js/components/GroupInfoModal.test.js +++ b/resources/js/components/GroupInfoModal.test.js @@ -28,8 +28,7 @@ function makeStore () { }) } -// Neil's PR feedback: the logo and name should navigate to the group, not -// just the "Go to group" button. +// The logo and name navigate to the group, not just the "Go to group" button. test('modal title logo and name link to the group page', () => { const wrapper = shallowMount(GroupInfoModal, { localVue, diff --git a/resources/js/components/GroupMarker.test.js b/resources/js/components/GroupMarker.test.js index 8198c24568..2a3392d3f2 100644 --- a/resources/js/components/GroupMarker.test.js +++ b/resources/js/components/GroupMarker.test.js @@ -46,9 +46,9 @@ describe('GroupMarker icon', () => { expect(mountMarker({ hover: true }).vm.icon.options.iconUrl).toBe('/images/vendor/leaflet/dist/marker-icon.png') }) - // Neil's PR feedback: pins looked mispositioned - a group in Ulverston appeared - // to be out in Morecambe Bay. With no iconSize/iconAnchor, Leaflet puts the - // image's top-left corner on the coordinate rather than the tip of the pin. + // With no iconSize/iconAnchor, Leaflet puts the image's top-left corner on the + // coordinate rather than the tip of the pin, so the marker points somewhere + // other than the place it marks. test('anchors the tip of the pin to the coordinate, not its top-left corner', () => { const options = mountMarker().vm.icon.options expect(options.iconSize).toEqual([25, 41]) @@ -63,9 +63,9 @@ describe('GroupMarker icon', () => { }) }) -// Neil's PR feedback: the group name took too long to appear on hover. It was -// the native `title` attribute, whose delay is the browser's and can't be tuned; -// a Leaflet tooltip shows as soon as the pointer arrives. +// The name must come from a Leaflet tooltip rather than the native `title` +// attribute: `title` appears only after the browser's own delay, which can't be +// tuned, whereas a tooltip shows as soon as the pointer arrives. describe('GroupMarker tooltip', () => { test('renders the name in a Leaflet tooltip rather than a native title', () => { const wrapper = mountMarker() @@ -76,8 +76,8 @@ describe('GroupMarker tooltip', () => { }) }) -// Neil's PR feedback: hovering a pin should highlight the matching list row -// (the reverse of row-hover → red pin). +// Hovering a pin highlights the matching list row - the reverse of the +// row-hover → red pin direction. describe('GroupMarker hover emission', () => { test('mouseover emits update:hover with the group id, mouseout clears it', async () => { const wrapper = mountMarker() diff --git a/resources/js/components/GroupsTable.test.js b/resources/js/components/GroupsTable.test.js index 8b49737050..ff1fae4dac 100644 --- a/resources/js/components/GroupsTable.test.js +++ b/resources/js/components/GroupsTable.test.js @@ -201,7 +201,7 @@ describe('GroupsTable follow/unfollow button', () => { }) }) -// Neil's PR feedback: hovering a map pin should highlight the matching row. +// Hovering a map pin highlights the matching row. describe('GroupsTable pin-hover row highlight', () => { test('row gets the highlight class when hover matches its id', () => { const wrapper = mountTable([group], { hover: 1 }) From 69366923bb6052f9102d8c25596181787c517179 Mon Sep 17 00:00:00 2001 From: edwh Date: Tue, 21 Jul 2026 14:01:43 +0100 Subject: [PATCH 17/34] Open the map on the user's country when they have no town groupsNearby() needs coordinates, which a user who has set only their country doesn't have - it returns nothing, the bounding box stays at its "no location" value and the map opens on the whole world. Frame the groups in their country instead. A country with no groups in it has nothing to frame, so that still falls back to showing everything. A box round a country isn't the same kind of thing as a box round the groups near you, though, and the map was treating them alike: any valid box meant "zoom to the 5 groups nearest the centre", which for a country lands somewhere arbitrary in the middle of it. So the map now distinguishes the two, using the user's own coordinates - already worked out by the controller and passed to nothing until now. Those coordinates also replace the centre of the bounding box when picking the nearest groups, which is what "nearest to you" should have meant all along. Big countries will frame wide. That's accepted for now: it still says whether there is anything in your country at all, and the place search is right there. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB --- app/Http/Controllers/GroupController.php | 13 ++++ resources/js/components/GroupMap.test.js | 67 ++++++++++++++++++- resources/js/components/GroupMap.vue | 33 +++++++-- .../js/components/GroupMapAndList.test.js | 15 ++++- resources/js/components/GroupMapAndList.vue | 12 ++++ resources/js/components/GroupsPage.test.js | 11 +++ resources/js/components/GroupsPage.vue | 12 ++++ resources/views/group/index.blade.php | 2 + tests/Feature/Groups/GroupViewTest.php | 60 +++++++++++++++++ 9 files changed, 216 insertions(+), 9 deletions(-) diff --git a/app/Http/Controllers/GroupController.php b/app/Http/Controllers/GroupController.php index 3e0d89c228..c837f1b150 100644 --- a/app/Http/Controllers/GroupController.php +++ b/app/Http/Controllers/GroupController.php @@ -83,6 +83,19 @@ private function indexVariations($tab, $network) // We pass a high limit to the groups nearby; there is a distance limit which will normally kick in first. $nearby_groups = $user->groupsNearby(1000); + if (empty($nearby_groups) && $user->country_code) { + // groupsNearby() needs coordinates, which a user who has only set their country doesn't have. We + // can still open the map on something better than the whole world by framing the groups in their + // country. If there aren't any then we leave the bounding box alone, which the map reads as "no + // location" and falls back to showing every group. + $nearby_groups = Group::whereNull('archived_at') + ->where('approved', true) + ->where('country_code', $user->country_code) + ->whereNotNull('latitude') + ->whereNotNull('longitude') + ->get(); + } + // Now find the lat/lng bounding box which contains these groups. foreach ($nearby_groups as $group) { if ($group->latitude < $min_lat) { diff --git a/resources/js/components/GroupMap.test.js b/resources/js/components/GroupMap.test.js index f94d95bfd5..f50e33d16d 100644 --- a/resources/js/components/GroupMap.test.js +++ b/resources/js/components/GroupMap.test.js @@ -155,9 +155,70 @@ describe('GroupMap options', () => { }) }) -// Neil's PR feedback: as well as centring the map on your town, put that town in -// the "Search for a place..." box - a visual hint that the map has already been -// searched for you. +// A user who has set only their country gets a box around that country's groups. +// Framing the 5 nearest groups to the middle of a country would be useless - the +// centre of France is not near anyone in particular - so that only happens for a +// user who has coordinates of their own. +describe('GroupMap country-level framing', () => { + const groups = [ + { id: 1, location: { lat: 51.5, lng: -0.1 } }, + { id: 2, location: { lat: 53.4, lng: -2.2 } }, + { id: 3, location: { lat: 55.9, lng: -3.2 } }, + ] + const COUNTRY = [[50.0, -5.0], [56.0, 1.0]] + + test('fits the country box as given when the user has no coordinates of their own', () => { + const wrapper = mountMap(COUNTRY, groups) + const map = fakeMap() + wrapper.vm.mapObject = map + wrapper.vm.$refs.map = { mapObject: map } + wrapper.vm.zoomedToGroups = false + + wrapper.vm.zoomToGroups() + + expect(map.fitBounds).toHaveBeenCalledTimes(1) + expect(wrapper.vm.hasUserPoint).toBe(false) + // The whole country box, not a tight box round a few groups near its middle. + const bounds = map.fitBounds.mock.calls[0][0] + expect(L.latLngBounds(bounds).contains([50.0, -5.0])).toBe(true) + expect(L.latLngBounds(bounds).contains([56.0, 1.0])).toBe(true) + }) + + test('frames the groups nearest the user when they do have coordinates', () => { + const wrapper = mountMap(COUNTRY, groups, { yourLat: 51.5, yourLng: -0.1 }) + const map = fakeMap() + wrapper.vm.mapObject = map + wrapper.vm.$refs.map = { mapObject: map } + wrapper.vm.zoomedToGroups = false + + expect(wrapper.vm.hasUserPoint).toBe(true) + + wrapper.vm.zoomToGroups() + + const bounds = map.fitBounds.mock.calls[0][0] + // Centred on the user's own point, not the middle of the country box. + expect(bounds.contains([51.5, -0.1])).toBe(true) + }) + + // Country with no groups in it: the page can't build a box, so we still get the + // inverted world box and fall back to showing everything. + test('falls back to framing all groups when the country has none', () => { + const wrapper = mountMap(WORLD, groups) + const map = fakeMap() + wrapper.vm.mapObject = map + wrapper.vm.$refs.map = { mapObject: map } + wrapper.vm.zoomedToGroups = false + + wrapper.vm.zoomToGroups() + + const bounds = map.fitBounds.mock.calls[0][0] + expect(bounds.contains([51.5, -0.1])).toBe(true) + expect(bounds.contains([55.9, -3.2])).toBe(true) + }) +}) + +// The map is centred on the user's town, and that town also goes in the "Search +// for a place..." box, as a hint that the map has already been searched for them. describe('GroupMap place search preload', () => { test('preloads the search box with the area the map was centred on', () => { const wrapper = mountMap(WORLD, [], { yourArea: 'Ulverston' }) diff --git a/resources/js/components/GroupMap.vue b/resources/js/components/GroupMap.vue index a76a852deb..fdedfdfd5f 100644 --- a/resources/js/components/GroupMap.vue +++ b/resources/js/components/GroupMap.vue @@ -66,6 +66,16 @@ export default { type: String, required: false, default: '', + }, + yourLat: { + type: Number, + required: false, + default: null, + }, + yourLng: { + type: Number, + required: false, + default: null, } }, data() { @@ -121,6 +131,12 @@ export default { } return +b[0][0] <= +b[1][0] }, + hasUserPoint() { + // A location of the user's own, as opposed to a box round their country. + // Only this justifies zooming in to the groups nearest them. + return this.yourLat !== null && this.yourLng !== null && + !isNaN(+this.yourLat) && !isNaN(+this.yourLng) + }, }, created() { this.bounds = this.initialBounds @@ -304,10 +320,19 @@ export default { const lngOf = (g) => +(g.location && g.location.lng != null ? g.location.lng : g.lng) let framed - if (this.hasLocation) { - // The user has a location, so the map is already centred on their area. - // Frame the 5 groups closest to the centre. - const center = this.mapObject.getCenter() + if (!this.hasUserPoint && this.hasLocation) { + // A box round the user's country: show the country as we were given it. + // Zooming to the groups nearest its centre would land somewhere + // arbitrary in the middle of the country, near nobody in particular. + this.bounds = this.initialBounds + this.mapObject.fitBounds(this.initialBounds) + return + } + + if (this.hasUserPoint) { + // Frame the 5 groups closest to the user themselves. Using the centre + // of the bounding box instead would drift away from where they are. + const center = new L.LatLng(+this.yourLat, +this.yourLng) framed = this.allGroups .map((group) => { const distance = Math.sqrt((latOf(group) - center.lat) ** 2 + (lngOf(group) - center.lng) ** 2) diff --git a/resources/js/components/GroupMapAndList.test.js b/resources/js/components/GroupMapAndList.test.js index 5b81c4b0c4..c0e9e90951 100644 --- a/resources/js/components/GroupMapAndList.test.js +++ b/resources/js/components/GroupMapAndList.test.js @@ -39,6 +39,8 @@ const groupMapStub = { network: { type: Number, default: null }, yourGroups: { type: Array, default: () => [] }, yourArea: { type: String, default: '' }, + yourLat: { type: Number, default: null }, + yourLng: { type: Number, default: null }, hover: { type: Number, default: null }, }, template: '
', @@ -108,6 +110,15 @@ test('forwards yourArea to the map so the search box can be preloaded', async () expect(wrapper.findComponent(groupMapStub).props('yourArea')).toBe('Ulverston') }) +// The map only zooms to the groups nearest the user if it knows where they are. +test('forwards the user\'s own coordinates to the map', async () => { + const wrapper = await makeWrapper({ yourLat: 54.19, yourLng: -3.09 }) + + const map = wrapper.findComponent(groupMapStub) + expect(map.props('yourLat')).toBe(54.19) + expect(map.props('yourLng')).toBe(-3.09) +}) + describe('effectiveGroupIds', () => { test('falls back to the full (network-filtered) list before the map has reported', async () => { const wrapper = await makeWrapper({ network: 5 }) @@ -131,8 +142,8 @@ describe('effectiveGroupIds', () => { }) }) -// Neil's PR feedback: pin hover flows back up from the map and down into the -// table, so the matching row highlights. +// Pin hover flows back up from the map and down into the table, so the matching +// row highlights. test('map update:hover lands in the table hover prop', async () => { const wrapper = await makeWrapper() wrapper.findComponent({ name: 'GroupMap' }).vm.$emit('update:hover', 42) diff --git a/resources/js/components/GroupMapAndList.vue b/resources/js/components/GroupMapAndList.vue index e7073f613c..bed471f90a 100644 --- a/resources/js/components/GroupMapAndList.vue +++ b/resources/js/components/GroupMapAndList.vue @@ -15,6 +15,8 @@ :network="network" :your-groups="yourGroups" :your-area="yourArea" + :your-lat="yourLat" + :your-lng="yourLng" :hover="hover" @update:hover="hover = $event" @groups="groupsChanged($event)" @@ -75,6 +77,16 @@ export default { required: false, default: '', }, + yourLat: { + type: Number, + required: false, + default: null, + }, + yourLng: { + type: Number, + required: false, + default: null, + }, showFilters: { type: Boolean, required: false, diff --git a/resources/js/components/GroupsPage.test.js b/resources/js/components/GroupsPage.test.js index 0c7f78946a..1101e611b1 100644 --- a/resources/js/components/GroupsPage.test.js +++ b/resources/js/components/GroupsPage.test.js @@ -42,6 +42,8 @@ const groupMapStub = { initialBounds: { type: Array }, yourGroups: { type: Array, default: () => [] }, yourArea: { type: String, default: '' }, + yourLat: { type: Number, default: null }, + yourLng: { type: Number, default: null }, network: { type: Number, default: null }, showFilters: { type: Boolean, default: false }, canManageTags: { type: Boolean, default: false }, @@ -137,6 +139,15 @@ test('forwards yourArea to the map list so the place search is preloaded', async expect(wrapper.findComponent(groupMapStub).props('yourArea')).toBe('Ulverston') }) +test('forwards the user\'s own coordinates to the map list', async () => { + const wrapper = makeWrapper({ tab: 'other', nearbyGroups: WORLD_BOUNDS, yourLat: 54.19, yourLng: -3.09 }) + await flushTabs(wrapper) + + const map = wrapper.findComponent(groupMapStub) + expect(map.props('yourLat')).toBe(54.19) + expect(map.props('yourLng')).toBe(-3.09) +}) + test('passes the networks list for the network filter on the plain groups page', async () => { const networks = [{ id: 10, name: 'Test' }] const wrapper = makeWrapper({ tab: 'other', nearbyGroups: WORLD_BOUNDS, networks }) diff --git a/resources/js/components/GroupsPage.vue b/resources/js/components/GroupsPage.vue index 025bd7b82f..6cbce499df 100644 --- a/resources/js/components/GroupsPage.vue +++ b/resources/js/components/GroupsPage.vue @@ -53,6 +53,8 @@ :initial-bounds="nearbyGroups" :your-groups="yourGroups" :your-area="yourArea" + :your-lat="yourLat" + :your-lng="yourLng" :network="network" show-filters :can-manage-tags="showTags" @@ -101,6 +103,16 @@ export default { required: false, default: null }, + yourLat: { + type: Number, + required: false, + default: null + }, + yourLng: { + type: Number, + required: false, + default: null + }, canCreate: { type: Boolean, required: false, diff --git a/resources/views/group/index.blade.php b/resources/views/group/index.blade.php index 8a576f6715..15e62523e7 100644 --- a/resources/views/group/index.blade.php +++ b/resources/views/group/index.blade.php @@ -62,6 +62,8 @@ :your-groups="{{ json_encode($your_groups, JSON_INVALID_UTF8_IGNORE) }}" :nearby-groups="{{ json_encode($nearby_groups, JSON_INVALID_UTF8_IGNORE) }}" your-area="{{ $your_area }}" + :your-lat="{{ $your_lat !== null ? $your_lat : 'null' }}" + :your-lng="{{ $your_lng !== null ? $your_lng : 'null' }}" :can-create="{{ $can_create ? 'true' : 'false' }}" tab="{{ $tab }}" :network="{{ $network ? $network : 'null' }}" diff --git a/tests/Feature/Groups/GroupViewTest.php b/tests/Feature/Groups/GroupViewTest.php index 8c392cd6cf..3df7417db8 100644 --- a/tests/Feature/Groups/GroupViewTest.php +++ b/tests/Feature/Groups/GroupViewTest.php @@ -197,6 +197,66 @@ public function testGroupIndexNextEventIsEagerLoaded(): void $this->assertNotNull($group2['next_event'], 'Group Beta should have a next_event'); } + public function testGroupsPageFramesTheCountryForAUserWithNoTown(): void + { + // A user who has only set their country has no coordinates, so + // groupsNearby() can't help - the map used to open on the whole world. + // Frame the groups in their country instead. + $this->loginAsTestUser(Role::ADMINISTRATOR); + $id = $this->createGroup('Group In Country', 'https://therestartproject.org', 'London'); + + $group = Group::find($id); + $group->country_code = 'GB'; + $group->latitude = 54.19; + $group->longitude = -3.09; + $group->approved = true; + $group->save(); + + $user = \App\User::factory()->restarter()->create([ + 'country_code' => 'GB', + 'location' => null, + 'latitude' => null, + 'longitude' => null, + ]); + $this->actingAs($user); + + $props = $this->getVueProperties($this->get('/group/other')); + $bounds = json_decode($props[1][':nearby-groups'], true); + + // [[min_lat, min_lng], [max_lat, max_lng]] - a real box, not the inverted + // world box [[90,180],[-90,-180]] that means "no location". + $this->assertLessThanOrEqual($bounds[1][0], $bounds[0][0], 'Should be a real bounding box, not the inverted world box'); + $this->assertEqualsWithDelta(54.19, $bounds[0][0], 0.001); + $this->assertEqualsWithDelta(-3.09, $bounds[0][1], 0.001); + } + + public function testGroupsPageFallsBackToTheWorldWhenTheCountryHasNoGroups(): void + { + $this->loginAsTestUser(Role::ADMINISTRATOR); + $id = $this->createGroup('Group Somewhere Else'); + + $group = Group::find($id); + $group->country_code = 'GB'; + $group->latitude = 54.19; + $group->longitude = -3.09; + $group->save(); + + // Country with no groups in it: nothing to frame, so we fall back to the + // inverted world box, which the map reads as "show me everything". + $user = \App\User::factory()->restarter()->create([ + 'country_code' => 'NZ', + 'location' => null, + 'latitude' => null, + 'longitude' => null, + ]); + $this->actingAs($user); + + $props = $this->getVueProperties($this->get('/group/other')); + $bounds = json_decode($props[1][':nearby-groups'], true); + + $this->assertEquals([[90, 180], [-90, -180]], $bounds); + } + public function testNextEventIsTheSoonestNotTheFurthestAway(): void { $this->loginAsTestUser(Role::ADMINISTRATOR); From 5e40c7f9c8ce5a3ee70a1ac713c7e79016789dbc Mon Sep 17 00:00:00 2001 From: edwh Date: Tue, 21 Jul 2026 14:21:19 +0100 Subject: [PATCH 18/34] Separate a group's name from its archived badge The template compiler condenses the newline between the name link and the badge, so they render with nothing between them and the badge sits hard against the last letter of the name. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB --- resources/js/components/GroupsTable.vue | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/resources/js/components/GroupsTable.vue b/resources/js/components/GroupsTable.vue index dcf503a847..21e8868dc1 100644 --- a/resources/js/components/GroupsTable.vue +++ b/resources/js/components/GroupsTable.vue @@ -51,7 +51,9 @@ - - @@ -97,26 +82,9 @@ @@ -130,19 +98,25 @@ import { DATE_FORMAT, DEFAULT_PROFILE } from '../constants' import images from '../mixins/images' import moment from 'moment' import GroupsTableFilters from './GroupsTableFilters.vue' -import ConfirmModal from './ConfirmModal.vue' import GroupArchivedBadge from "./GroupArchivedBadge.vue"; import InfiniteLoading from 'vue-infinite-loading' export default { - components: {GroupArchivedBadge, ConfirmModal, GroupsTableFilters, InfiniteLoading}, + components: {GroupArchivedBadge, GroupsTableFilters, InfiniteLoading}, mixins: [images], props: { groupids: { type: Array, required: true }, + // Where the map is centred, so the list can be ordered by what's nearest to + // the middle of what the user is looking at. Null away from the map. + centre: { + type: Object, + required: false, + default: null + }, // Group id whose row should be highlighted (set when its map pin is hovered). hover: { type: Number, @@ -164,11 +138,6 @@ export default { required: false, default: null }, - yourGroups: { - type: Array, - required: false, - default: () => [], - }, approve: { type: Boolean, required: false, @@ -179,11 +148,6 @@ export default { required: false, default: false }, - networks: { - type: Array, - required: false, - default: null - }, allGroupTags: { type: Array, required: false, @@ -198,25 +162,28 @@ export default { data () { return { searchName: null, - searchLocation: null, - searchNetwork: null, - searchCountry: null, searchTags: null, searchShow: false, - fields: [ + show: 3 + } + }, + computed: { + fields() { + const fields = [ { key: 'group_image', label: 'Group Image', tdClass: 'image'}, { key: 'group_name', label: 'Group Name', sortable: true }, { key: 'location', label: 'Location', tdClass: "hidecell", thClass: "hidecell" }, - { key: 'hosts', label: 'Hosts', sortable: true, tdClass: "hidecell text-center", thClass: "hidecell text-center pl-3" }, - { key: 'restarters', label: 'Restarters', sortable: true, tdClass: "hidecell text-center", thClass: "hidecell text-center pl-3" }, { key: 'next_event', label: 'Next Event', sortable: true, tdClass: "hidecell event", thClass: "hidecell" }, - { key: 'following' , label: 'Follow' } - ], - show: 3, - left: [] - } - }, - computed: { + ] + + if (this.approve) { + // Moderation reuses this table, and its rows need a way through to the + // group that is waiting to be approved. + fields.push({ key: 'following', label: 'Moderate' }) + } + + return fields + }, defaultProfile() { return DEFAULT_PROFILE }, @@ -234,20 +201,6 @@ export default { items = items.filter(g => g.name && g.name.toLowerCase().includes(name)) } - if (this.searchLocation) { - const loc = this.searchLocation.toLowerCase() - items = items.filter(g => g.location && g.location.location && g.location.location.toLowerCase().includes(loc)) - } - - if (this.searchCountry) { - items = items.filter(g => g.location && g.location.country === this.searchCountry.country) - } - - if (this.searchNetwork) { - // Groups may hold networks as summary objects ([{id}]) or as plain ids. - items = items.filter(g => (g.networks || []).some(n => (n && n.id !== undefined ? n.id : n) === this.searchNetwork)) - } - if (this.searchTags && this.searchTags.length) { const tagIds = this.searchTags.map(t => t.id) items = items.filter(g => { @@ -259,9 +212,21 @@ export default { return items }, itemsToShow() { - // Sort before slicing so the first page is the alphabetically-first - // groups, not whatever happened to be first in the store. + // Sort before slicing, so the first page is the first groups in order + // rather than whichever ones happened to load first. const items = [...this.filteredItems].sort((a, b) => { + if (this.centre) { + // Nearest the middle of the map first: someone looking at a map wants + // what's in front of them, and alphabetical order says nothing about + // where a group is. Groups we can't place go last. + const da = this.distanceFromCentre(a) + const db = this.distanceFromCentre(b) + + if (da !== db) { + return da - db + } + } + return a.name.localeCompare(b.name) }) @@ -289,6 +254,18 @@ export default { } }, methods: { + distanceFromCentre(group) { + // Only ever compared with each other, so the flat-earth approximation is + // fine and avoids the cost of a great-circle calculation per row. + const lat = group.location && group.location.lat != null ? group.location.lat : group.lat + const lng = group.location && group.location.lng != null ? group.location.lng : group.lng + + if (lat == null || lng == null || isNaN(+lat) || isNaN(+lng)) { + return Number.MAX_VALUE + } + + return Math.sqrt((+lat - this.centre.lat) ** 2 + (+lng - this.centre.lng) ** 2) + }, eventStart(event) { // next_event is an object ({start}) from the v2 APIs but a plain date // string in the moderation store (newToOld). @@ -327,14 +304,6 @@ export default { } else { return new moment(this.eventStart(aRow.next_event)).unix() - new moment(this.eventStart(bRow.next_event)).unix() } - } else if (key === 'hosts' || key === 'restarters') { - if (parseInt(a) < parseInt(b)) { - return -1 - } else if (parseInt(a) > parseInt(b)) { - return 1 - } else { - return 0 - } } else { return String(a).localeCompare(String(b), compareLocale, compareOptions) } @@ -347,16 +316,6 @@ export default { $state.complete() } }, - leaveGroup(idgroups) { - this.$refs['confirmLeave-' + idgroups].show() - }, - async leaveConfirmed(idgroups) { - await this.$store.dispatch('groups/unfollow', { - idgroups: idgroups - }) - - this.left.push(idgroups) - }, distance(dist ) { if (dist < 5) { return Math.round(dist * 10) / 10 @@ -364,11 +323,6 @@ export default { return Math.round(dist) } }, - yourGroup(id) { - // `left` tracks groups unfollowed in this session, so the button flips - // without waiting for fresh server data. - return this.yourGroups.includes(id) && !this.left.includes(id) - }, toggleFilters() { this.searchShow = !this.searchShow }, diff --git a/resources/js/components/GroupsTableFilters.test.js b/resources/js/components/GroupsTableFilters.test.js index c15a0b4b52..f6d776a5da 100644 --- a/resources/js/components/GroupsTableFilters.test.js +++ b/resources/js/components/GroupsTableFilters.test.js @@ -23,19 +23,46 @@ function mountFilters(props = {}) { }) } -// On a network-scoped view the list is already filtered to one network, so an -// empty network dropdown is just confusing — hide it when there are no -// networks to choose from. -test('hides the network dropdown when no networks are supplied', () => { - const wrapper = mountFilters({ networks: null }) - const placeholders = wrapper.findAll('.stub-multiselect').wrappers +function placeholdersOf(wrapper) { + return wrapper.findAll('.stub-multiselect').wrappers .map(w => w.attributes('data-placeholder')) +} + +// Searching by place is what the map's own "Search for a place..." box is for, +// and country is a narrower version of the same thing. A network filter only +// makes sense to someone who already knows the networks, and the network pages +// scope the list themselves. +test('does not offer location, country or network filters', () => { + const wrapper = mountFilters({ networks: [{ id: 1, name: 'Restarters' }], showTags: true, allGroupTags: [] }) + + const placeholders = placeholdersOf(wrapper) expect(placeholders).not.toContain('networks.network') + expect(placeholders).not.toContain('groups.search_country_placeholder') + + const inputPlaceholders = wrapper.findAll('b-form-input-stub').wrappers + .map(w => w.attributes('placeholder')) + expect(inputPlaceholders).not.toContain('groups.search_location_placeholder') }) -test('shows the network dropdown when networks are supplied', () => { - const wrapper = mountFilters({ networks: [{ id: 1, name: 'Restarters' }] }) - const placeholders = wrapper.findAll('.stub-multiselect').wrappers - .map(w => w.attributes('data-placeholder')) - expect(placeholders).toContain('networks.network') +test('searches by name', () => { + const inputs = mountFilters().findAll('b-form-input-stub') + + expect(inputs).toHaveLength(1) + expect(inputs.at(0).attributes('placeholder')).toBe('groups.search_name_placeholder') +}) + +test('emits the name to filter on', async () => { + const wrapper = mountFilters() + + wrapper.vm.searchName = 'Ulverston' + await wrapper.vm.$nextTick() + + expect(wrapper.emitted('update:name').pop()).toEqual(['Ulverston']) +}) + +// Tags are only visible to admins and network coordinators. +test('offers the tag filter only to those who can see tags', () => { + expect(placeholdersOf(mountFilters({ showTags: false }))).toEqual([]) + expect(placeholdersOf(mountFilters({ showTags: true, allGroupTags: [] }))) + .toEqual(['groups.search_tags_placeholder']) }) diff --git a/resources/js/components/GroupsTableFilters.vue b/resources/js/components/GroupsTableFilters.vue index 6a6badd7bf..a4ab7f543e 100644 --- a/resources/js/components/GroupsTableFilters.vue +++ b/resources/js/components/GroupsTableFilters.vue @@ -23,64 +23,11 @@ :selectedLabel="__('partials.remove')" open-direction="bottom" /> - - -
\ No newline at end of file + diff --git a/resources/views/group/index.blade.php b/resources/views/group/index.blade.php index 15e62523e7..2fe1adb739 100644 --- a/resources/views/group/index.blade.php +++ b/resources/views/group/index.blade.php @@ -67,7 +67,6 @@ :can-create="{{ $can_create ? 'true' : 'false' }}" tab="{{ $tab }}" :network="{{ $network ? $network : 'null' }}" - :networks="{{ json_encode($networks, JSON_INVALID_UTF8_IGNORE) }}" :all-group-tags="{{ json_encode($all_group_tags, JSON_INVALID_UTF8_IGNORE) }}" :show-tags="{{ $show_tags ? 'true' : 'false' }}" /> diff --git a/tests/Feature/Groups/BasicTest.php b/tests/Feature/Groups/BasicTest.php index 5d59d79c4e..ee61e982cf 100644 --- a/tests/Feature/Groups/BasicTest.php +++ b/tests/Feature/Groups/BasicTest.php @@ -3,7 +3,6 @@ namespace Tests\Feature\Groups; use App\Group; -use App\Network; use App\User; use DB; use Hash; @@ -41,7 +40,6 @@ public function testPageLoads($url, $tab): void ':can-create' => 'true', 'tab' => $tab, ':network' => 'null', - ':networks' => '[{"id":' . Network::first()->id . ',"name":"Restarters","description":null,"website":null,"default_language":"en","timezone":"Europe\\/London","created_at":"2021-05-24 12:19:37","updated_at":"2021-05-24 12:19:37","events_push_to_wordpress":0,"include_in_zapier":0,"users_push_to_drip":0,"shortname":"restarters","discourse_group":null,"auto_approve_events":0,"logo":null}]', ':show-tags' => 'false', ], ]); diff --git a/tests/clover.xml b/tests/clover.xml index 99d7adfc5c..cb81ed310c 100644 --- a/tests/clover.xml +++ b/tests/clover.xml @@ -1,6 +1,6 @@ - - + + @@ -63,18 +63,18 @@ - + - - - + + + - - + + @@ -131,8 +131,8 @@ - - + + @@ -150,8 +150,8 @@ - - + + @@ -208,8 +208,8 @@ - - + + @@ -228,9 +228,9 @@ - - - + + + @@ -242,9 +242,9 @@ - - - + + + @@ -266,9 +266,9 @@ - - - + + + @@ -290,8 +290,8 @@ - - + + @@ -347,8 +347,8 @@ - - + + @@ -421,8 +421,8 @@ - - + + @@ -448,8 +448,8 @@ - - + + @@ -480,8 +480,8 @@ - - + + @@ -492,8 +492,8 @@ - - + + @@ -513,8 +513,8 @@ - - + + @@ -546,8 +546,8 @@ - - + + @@ -590,8 +590,8 @@ - - + + @@ -638,9 +638,9 @@ - - - + + + @@ -649,9 +649,9 @@ - - - + + + @@ -696,10 +696,10 @@ - - - - + + + + @@ -757,10 +757,10 @@ - - - - + + + + @@ -798,10 +798,10 @@ - - - - + + + + @@ -852,8 +852,8 @@ - - + + @@ -902,8 +902,8 @@ - - + + @@ -926,8 +926,8 @@ - - + + @@ -944,8 +944,8 @@ - - + + @@ -964,8 +964,8 @@ - - + + @@ -982,8 +982,8 @@ - - + + @@ -1000,8 +1000,8 @@ - - + + @@ -1012,58 +1012,58 @@ - - - - - - - - - - - + + + + + + + + + + + - + - - - + + + - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + - - - + + + - - - + + + - + - + @@ -1080,36 +1080,36 @@ - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + - - + + - + - + - - + + @@ -1212,7 +1212,7 @@ - + @@ -1284,11 +1284,11 @@ - + - - - + + + @@ -1309,12 +1309,12 @@ - + - - - - + + + + @@ -1357,12 +1357,12 @@ - + - - - - + + + + @@ -1386,40 +1386,40 @@ - + - - - - + + + + - + - - + + - + - + - - - - + + + + - - + + @@ -1430,25 +1430,25 @@ - - + + - + - - + + - - + + - - + + @@ -1478,186 +1478,186 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - + + + + + + - - - - + + + + - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - + + + + + + + + + + + + + - - - - - - - + + + + + + + - - + + - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + @@ -1733,9 +1733,9 @@ - - - + + + @@ -1746,19 +1746,19 @@ - - - - - - - - - - - - - + + + + + + + + + + + + + @@ -1785,7 +1785,7 @@ - + @@ -1795,18 +1795,18 @@ - + - - - - + + + + - - + + @@ -1814,7 +1814,7 @@ - + @@ -1826,34 +1826,34 @@ - + - - - - - - - + + + + + + + - - + + - - - + + + - - - + + + - - - - - + + + + + @@ -1869,53 +1869,53 @@ - - - - - + + + + + - - - - - - - + + + + + + + - - + + - - - + + + - - - + + + - - - + + + - - + + - - - - + + + + - + @@ -1933,30 +1933,30 @@ - - + + - + - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + @@ -1985,13 +1985,13 @@ - - + + - - - + + + @@ -2000,11 +2000,11 @@ - - - - - + + + + + @@ -2032,34 +2032,34 @@ - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + - - - - - - - - - - + + + + + + + + + + @@ -2114,13 +2114,13 @@ - - - - - - - + + + + + + + @@ -2131,30 +2131,30 @@ - - - + + + - + - - - - + + + + - - - - - - + + + + + + - - - - - - + + + + + + @@ -2193,17 +2193,17 @@ - - - - - - - + + + + + + + - + @@ -2312,15 +2312,15 @@ - - - - + + + + - - - + + + @@ -2337,7 +2337,7 @@ - + @@ -2375,12 +2375,12 @@ - - - - - - + + + + + + @@ -2401,29 +2401,29 @@ - + - - - - - - + + + + + + - - - - - - - + + + + + + + - + @@ -2773,162 +2773,162 @@ - + - - - - - - - - - - - - - - + + + + + + + + + + + + + + - - - - - - + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - + + + + + + + - - - + + + - - - - - - + + + + + + - - - - - - - + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -2941,13 +2941,13 @@ - - - + + + - + - + @@ -2964,358 +2964,358 @@ - - - - - - - - - + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - + + + + - - - - - - - - - - + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - + + + + + + + + + + + + - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + - + - - - + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - + + + + + + + + + + + + - - - - - - + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -3330,22 +3330,22 @@ - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + @@ -3354,17 +3354,17 @@ - - - - - - - + + + + + + + - - - + + + @@ -3381,106 +3381,106 @@ - - - + + + - + - + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + - - + + - - - - + + + + @@ -3489,35 +3489,35 @@ - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + - - + + @@ -3530,71 +3530,71 @@ - + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - + + + - - - + + + - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + @@ -3618,203 +3618,203 @@ - + - - + + - + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + - - - - - + + + + + - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + - - - - - + + + + + - - - + + + - + - - - + + + - - - - - - - - - - - + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -3838,53 +3838,53 @@ - + - + - - - + + + - - - - - - - - - - - - - - + + + + + + + + + + + + + + - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + - - - - - - + + + + + + @@ -3901,7 +3901,7 @@ - + @@ -4694,422 +4694,429 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - + - - + - + - - + - - + - + + + + + - - - - - - + + + + + - + + - - + + - - + + - - - - + + + - - - + + + - - - - + + + + + - - + + - - - + + + - - + + - - + + - - - - - - - - - + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + - + + + + - + - - - - - + + + + + + - - + - - - - - - - - - - + + + + + + + + + - - + + + + + + + + + + @@ -5233,79 +5240,79 @@ - + - - - + + + - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -5320,18 +5327,18 @@ - - - - - + + + + + - - - - - - + + + + + + @@ -5432,227 +5439,227 @@ - - - - - - - - - - - + + + + + + + + + + + - - - - - - - - + + + + + + + + - - + + - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + - - - - - + + + + + - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - + + + + + + + - - - + + + - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + - - - - - - + + + + + + - - - + + + - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + - - - - - + + + + + - - - + + + - - - - + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -5748,21 +5755,21 @@ - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + @@ -5915,7 +5922,7 @@ - + @@ -6173,7 +6180,7 @@ - + @@ -6185,48 +6192,48 @@ - - + + - - - - + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -6361,25 +6368,25 @@ - - + + - - - - - - - - - + + + + + + + + + - - - - + + + + @@ -6649,9 +6656,9 @@ - - - + + + @@ -6668,9 +6675,9 @@ - - - + + + @@ -6680,69 +6687,69 @@ - - - - - - - - - - - - - + + + + + + + + + + + + + - - - + + + - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + - - + + - - - - - - - - - - - - - + + + + + + + + + + + + + - + @@ -6760,16 +6767,16 @@ - - - + + + - + - - - + + + @@ -6833,7 +6840,7 @@ - + @@ -6845,11 +6852,11 @@ - - + + - + @@ -6905,16 +6912,16 @@ - - + + - - - - + + + + @@ -6929,20 +6936,20 @@ - - - - + + + + - + - + - + - - - + + + @@ -6953,19 +6960,19 @@ - - - - - - - - - - - - - + + + + + + + + + + + + + @@ -6978,45 +6985,45 @@ - - - - - - - - + + + + + + + + - - + + - + - - + + - - - + + + - + @@ -7031,12 +7038,12 @@ - - - - + + + + - + @@ -7049,11 +7056,11 @@ - - - - - + + + + + @@ -7111,9 +7118,9 @@ - - - + + + @@ -7121,14 +7128,14 @@ - - + + - + @@ -7236,132 +7243,132 @@ - + - - - + + + - - + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - + - - - + + + - - - - - - - - - - - + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -7404,74 +7411,74 @@ - + - - - - - - - - - - - - - - + + + + + + + + + + + + + + - - - - - - - + + + + + + + - - + + - + - - - + + + - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + - - + + @@ -7481,25 +7488,25 @@ - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + @@ -7522,83 +7529,83 @@ - + - - - + + + - - - - - - - - - - - - + + + + + + + + + + + + - + - - - + + + - + - - - - - - + + + + + + - - - - + + + + - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + - + - - - + + + @@ -7630,10 +7637,10 @@ - + - - + + @@ -7682,43 +7689,43 @@ - + - + - - - - - - - - - - - - + + + + + + + + + + + + - - - - - - - + + + + + + + - - - - - - - - - - + + + + + + + + + + @@ -7780,13 +7787,13 @@ - + - - - - - + + + + + @@ -7837,27 +7844,27 @@ - + - - - - - - - - + + + + + + + + - + - - - - - + + + + + @@ -7894,7 +7901,7 @@ - + @@ -7985,11 +7992,11 @@ - + - - - + + + @@ -8000,8 +8007,8 @@ - - + + @@ -8043,41 +8050,41 @@ - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + - + - - - - - - - + + + + + + + - - - + + + @@ -8121,7 +8128,7 @@ - + @@ -8228,13 +8235,13 @@ - - - - - - - + + + + + + + @@ -8243,44 +8250,44 @@ - - - - - - - + + + + + + + - + - - - - + + + + - - + + - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + @@ -8292,32 +8299,32 @@ - + - - - - - - - - - - - - - - + + + + + + + + + + + + + + - - - - - - - - - + + + + + + + + + @@ -8367,16 +8374,16 @@ - + - - - - + + + + - - - + + + @@ -8426,50 +8433,50 @@ - + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -8780,14 +8787,14 @@ - - - - - - - - + + + + + + + + @@ -8795,7 +8802,7 @@ - + @@ -8836,32 +8843,32 @@ - + - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + @@ -8871,7 +8878,7 @@ - + @@ -9043,10 +9050,10 @@ - + - - + + @@ -9105,7 +9112,7 @@ - + @@ -9237,29 +9244,29 @@ - + - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + @@ -9268,7 +9275,7 @@ - + @@ -9326,18 +9333,18 @@ - - - - - - - - - - - - + + + + + + + + + + + + @@ -9348,22 +9355,22 @@ - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + @@ -9373,19 +9380,19 @@ - + - - - - - - - - + + + + + + + + @@ -9410,107 +9417,107 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -9526,62 +9533,62 @@ - - - + + + - - - - - - - + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -9615,53 +9622,53 @@ - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + - + - - - + + + - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + @@ -9674,37 +9681,37 @@ - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - + + + + + + + + + + + + @@ -9715,45 +9722,45 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - + + + + + @@ -9763,28 +9770,28 @@ - + - - + + - - + + - - - + + + - - - - - + + + + + @@ -9795,11 +9802,11 @@ - + - + @@ -9816,17 +9823,17 @@ - - - - - - - - - - - + + + + + + + + + + + @@ -9838,45 +9845,45 @@ - - - - - + + + + + - - + + - - - - - - - - - + + + + + + + + + - - - - - - - + + + + + + + - - + + @@ -9892,8 +9899,8 @@ - - + + @@ -9911,8 +9918,8 @@ - - + + @@ -9926,7 +9933,7 @@ - + @@ -9938,22 +9945,22 @@ - - + + - - + + - - - - - + + + + + @@ -9979,67 +9986,67 @@ - + - - - - - + + + + + - - + + - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + - - - - - - - - + + + + + + + + - - - - - - - - - + + + + + + + + + - - + + @@ -10599,14 +10606,14 @@ - + - - + + - - + + @@ -10616,11 +10623,11 @@ - - - - - + + + + + @@ -10644,16 +10651,16 @@ - - - - - - - - - - + + + + + + + + + + @@ -10662,16 +10669,16 @@ - - - - - - - - - - + + + + + + + + + + @@ -10692,10 +10699,10 @@ - - - - + + + + @@ -10705,22 +10712,22 @@ - - + + - + - + - + - - + + @@ -10730,40 +10737,40 @@ - - + + - - + + - - - - - - - - + + + + + + + + - + - - - - - - - - - + + + + + + + + + - - - + + + @@ -10772,47 +10779,47 @@ - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + @@ -10822,11 +10829,11 @@ - + - + @@ -10847,14 +10854,14 @@ - - - - - - - - + + + + + + + + @@ -10863,7 +10870,7 @@ - + @@ -10928,6 +10935,6 @@ - + From 8e754302f278ba0437740e4104746832c4d00850 Mon Sep 17 00:00:00 2001 From: edwh Date: Tue, 21 Jul 2026 14:33:26 +0100 Subject: [PATCH 20/34] Make filtering move the map, not just shorten the list Filtering changed only the list, so the map kept every pin and the count kept claiming every group - you could be told there were 243 groups while looking at a list of 20. Now the filter applies to every group we know about, and the map draws only the matches, so the pins and the rows always agree. Because the filter is applied to all groups rather than only those in view, a search can find a group that isn't currently on screen; the map frames the matches so you actually get taken to it, rather than being told there are no results when what you asked for is just off the edge. The map is asked to reframe explicitly when a filter changes, rather than watching the list of groups: that list gets a new identity every time rows are hydrated, which would drag the map away from wherever the user had panned to. The predicate lives in one place and is used by both the map and the list, so they can't drift apart. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB --- resources/js/components/GroupMap.test.js | 38 ++++++++++++++ resources/js/components/GroupMap.vue | 49 ++++++++++++++++++- .../js/components/GroupMapAndList.test.js | 47 ++++++++++++++++++ resources/js/components/GroupMapAndList.vue | 39 ++++++++++++--- resources/js/components/GroupsTable.vue | 30 ++++++------ resources/js/misc/groupFilter.js | 26 ++++++++++ resources/js/misc/groupFilter.test.js | 41 ++++++++++++++++ 7 files changed, 247 insertions(+), 23 deletions(-) create mode 100644 resources/js/misc/groupFilter.js create mode 100644 resources/js/misc/groupFilter.test.js diff --git a/resources/js/components/GroupMap.test.js b/resources/js/components/GroupMap.test.js index f50e33d16d..6fd72cfdbb 100644 --- a/resources/js/components/GroupMap.test.js +++ b/resources/js/components/GroupMap.test.js @@ -248,6 +248,44 @@ describe('GroupMap place search preload', () => { }) }) +// Searching for a group that isn't in the current view should take you to it, +// not tell you there are no results. Framing is asked for explicitly rather +// than watching the group list, which changes whenever rows are hydrated and +// would otherwise yank the map away from wherever the user had panned to. +describe('GroupMap reframing on request', () => { + const groups = [ + { id: 1, location: { lat: 51.5, lng: -0.1 } }, + { id: 2, location: { lat: 55.9, lng: -3.2 } }, + ] + + test('frames the groups it is showing when the request changes', async () => { + const wrapper = mountMap(WORLD, groups, { frameRequest: 0 }) + const map = fakeMap() + wrapper.vm.mapObject = map + wrapper.vm.$refs.map = { mapObject: map } + map.fitBounds.mockClear() + + await wrapper.setProps({ frameRequest: 1 }) + + expect(map.fitBounds).toHaveBeenCalledTimes(1) + const bounds = map.fitBounds.mock.calls[0][0] + expect(bounds.contains([51.5, -0.1])).toBe(true) + expect(bounds.contains([55.9, -3.2])).toBe(true) + }) + + test('does not move the map when there is nothing to frame', async () => { + const wrapper = mountMap(WORLD, groups, { frameRequest: 0, groupids: [] }) + const map = fakeMap() + wrapper.vm.mapObject = map + wrapper.vm.$refs.map = { mapObject: map } + map.fitBounds.mockClear() + + await wrapper.setProps({ frameRequest: 1 }) + + expect(map.fitBounds).not.toHaveBeenCalled() + }) +}) + describe('GroupMap markers', () => { test('only renders markers for groups with coordinates', () => { const wrapper = mountMap(WORLD, [ diff --git a/resources/js/components/GroupMap.vue b/resources/js/components/GroupMap.vue index fdedfdfd5f..8e71cf1b4b 100644 --- a/resources/js/components/GroupMap.vue +++ b/resources/js/components/GroupMap.vue @@ -57,6 +57,13 @@ export default { required: false, default: () => [], }, + // The groups the map should draw. Null means "everything we know about"; + // the filter bar narrows it, so searching moves the map too. + groupids: { + type: Array, + required: false, + default: null, + }, hover: { type: Number, required: false, @@ -76,6 +83,15 @@ export default { type: Number, required: false, default: null, + }, + // Bumped by the parent when the user changes a filter, to ask the map to + // frame what it is now showing. Watching the group list instead would move + // the map every time rows are hydrated, yanking it away from wherever the + // user had panned to. + frameRequest: { + type: Number, + required: false, + default: 0, } }, data() { @@ -102,7 +118,11 @@ export default { } }, allGroups() { - const groups = this.$store.getters['groups/list'] + let groups = this.$store.getters['groups/list'] + + if (this.groupids !== null) { + groups = groups.filter((g) => this.groupids.includes(g.id || g.idgroups)) + } if (!this.network) { return groups @@ -161,6 +181,9 @@ export default { this.destroyed = true }, watch: { + frameRequest() { + this.frameShownGroups() + }, allGroups: { handler(newVal, oldVal) { // oldVal is undefined on the first (immediate) run. @@ -302,6 +325,30 @@ export default { this.$emit('update:moved', true) this.idle() }, + frameShownGroups() { + // A filter is an explicit request to see something, so take the map there + // even if the user has panned somewhere else. + const bounds = this.boundsOf(this.mappableGroups) + + if (this.mapObject && bounds) { + this.bounds = bounds + this.mapObject.fitBounds(bounds) + } + }, + boundsOf(groups) { + const bounds = new L.LatLngBounds() + + groups.forEach((group) => { + const lat = +(group.location && group.location.lat != null ? group.location.lat : group.lat) + const lng = +(group.location && group.location.lng != null ? group.location.lng : group.lng) + + if (!isNaN(lat) && !isNaN(lng)) { + bounds.extend(new L.LatLng(lat, lng)) + } + }) + + return bounds.isValid() ? bounds.pad(0.1) : null + }, zoomToGroups() { try { // Only zoom once the map has a real size. If it's still 0x0 (created in a diff --git a/resources/js/components/GroupMapAndList.test.js b/resources/js/components/GroupMapAndList.test.js index d3b4dc31de..da1e8dec0d 100644 --- a/resources/js/components/GroupMapAndList.test.js +++ b/resources/js/components/GroupMapAndList.test.js @@ -42,6 +42,8 @@ const groupMapStub = { yourLat: { type: Number, default: null }, yourLng: { type: Number, default: null }, hover: { type: Number, default: null }, + groupids: { type: Array, default: null }, + frameRequest: { type: Number, default: 0 }, }, template: '
', } @@ -126,6 +128,51 @@ test('passes the map centre through to the table', async () => { expect(wrapper.findComponent(groupsTableStub).props('centre')).toEqual({ lat: 54.19, lng: -3.09 }) }) +// Filtering used to change only the list, so the map still showed every pin and +// the count still claimed every group. Searching or filtering has to move the +// map too, or the two disagree about what you're looking at. +describe('filters drive the map', () => { + test('the map only gets the groups that match the filter', async () => { + const wrapper = await makeWrapper({ showFilters: true }) + + wrapper.findComponent({ name: 'GroupsTable' }).vm.$emit('update:filters', { name: 'Alpha' }) + await wrapper.vm.$nextTick() + + expect(wrapper.findComponent(groupMapStub).props('groupids')).toEqual([1]) + }) + + test('the list is narrowed to the filtered groups even when the map has more in view', async () => { + const wrapper = await makeWrapper({ showFilters: true }) + wrapper.vm.groupsChanged([1, 2]) + + wrapper.findComponent({ name: 'GroupsTable' }).vm.$emit('update:filters', { name: 'Beta' }) + await wrapper.vm.$nextTick() + + expect(wrapper.vm.effectiveGroupIds).toEqual([2]) + }) + + test('asks the map to reframe, so a search for a group out of view goes to it', async () => { + const wrapper = await makeWrapper({ showFilters: true }) + const before = wrapper.findComponent(groupMapStub).props('frameRequest') + + wrapper.findComponent({ name: 'GroupsTable' }).vm.$emit('update:filters', { name: 'Beta' }) + await wrapper.vm.$nextTick() + + expect(wrapper.findComponent(groupMapStub).props('frameRequest')).toBeGreaterThan(before) + }) + + test('clearing the filter puts every group back on the map', async () => { + const wrapper = await makeWrapper({ showFilters: true }) + + wrapper.findComponent({ name: 'GroupsTable' }).vm.$emit('update:filters', { name: 'Alpha' }) + await wrapper.vm.$nextTick() + wrapper.findComponent({ name: 'GroupsTable' }).vm.$emit('update:filters', { name: null }) + await wrapper.vm.$nextTick() + + expect(wrapper.findComponent(groupMapStub).props('groupids')).toEqual([1, 2]) + }) +}) + describe('effectiveGroupIds', () => { test('falls back to the full (network-filtered) list before the map has reported', async () => { const wrapper = await makeWrapper({ network: 5 }) diff --git a/resources/js/components/GroupMapAndList.vue b/resources/js/components/GroupMapAndList.vue index 7a5d94e9d5..9eeb3fc6d6 100644 --- a/resources/js/components/GroupMapAndList.vue +++ b/resources/js/components/GroupMapAndList.vue @@ -18,6 +18,8 @@ :your-lat="yourLat" :your-lng="yourLng" :hover="hover" + :groupids="matchingGroupIds" + :frame-request="frameRequest" @update:hover="hover = $event" @update:centre="centre = $event" @groups="groupsChanged($event)" @@ -29,6 +31,7 @@ :hover.sync="hover" :centre="centre" :search="showFilters" + @update:filters="filtersChanged" :all-group-tags="availableTags" :show-tags="canManageTags" /> @@ -38,6 +41,7 @@ diff --git a/resources/js/components/GroupsTable.vue b/resources/js/components/GroupsTable.vue index d7718ac2c2..44b6221f9b 100644 --- a/resources/js/components/GroupsTable.vue +++ b/resources/js/components/GroupsTable.vue @@ -99,6 +99,7 @@ import images from '../mixins/images' import moment from 'moment' import GroupsTableFilters from './GroupsTableFilters.vue' import GroupArchivedBadge from "./GroupArchivedBadge.vue"; +import { matchesFilters } from '../misc/groupFilter' import InfiniteLoading from 'vue-infinite-loading' @@ -193,23 +194,12 @@ export default { items() { return this.groups.filter((g) => this.groupids.includes(g.id)) }, + activeFilters() { + return { name: this.searchName, tags: this.searchTags } + }, filteredItems() { - let items = this.items - - if (this.searchName) { - const name = this.searchName.toLowerCase() - items = items.filter(g => g.name && g.name.toLowerCase().includes(name)) - } - - if (this.searchTags && this.searchTags.length) { - const tagIds = this.searchTags.map(t => t.id) - items = items.filter(g => { - const groupTags = g.group_tags_full || [] - return tagIds.every(id => groupTags.some(t => t.id === id)) - }) - } - - return items + // The same predicate the map uses, so the pins and the rows can't disagree. + return this.items.filter(g => matchesFilters(g, this.activeFilters)) }, itemsToShow() { // Sort before slicing, so the first page is the first groups in order @@ -239,6 +229,14 @@ export default { }, }, watch: { + activeFilters: { + handler(newVal) { + // Tell the map, so filtering moves the pins rather than just shortening + // the list underneath them. + this.$emit('update:filters', newVal) + }, + deep: true + }, itemsToShow: { immediate: true, handler(newVal) { diff --git a/resources/js/misc/groupFilter.js b/resources/js/misc/groupFilter.js new file mode 100644 index 0000000000..55d585503e --- /dev/null +++ b/resources/js/misc/groupFilter.js @@ -0,0 +1,26 @@ +// The map and the list must agree on what a filter means: the list shows what +// the map shows, so if they disagree you get a count that doesn't match the +// pins, or pins for groups that aren't listed. +export function matchesFilters(group, filters) { + if (!filters) { + return true + } + + if (filters.name) { + const name = filters.name.toLowerCase() + + if (!group.name || !group.name.toLowerCase().includes(name)) { + return false + } + } + + if (filters.tags && filters.tags.length) { + const groupTags = group.group_tags_full || [] + + // Every selected tag must be present, so choosing more tags narrows the + // results rather than widening them. + return filters.tags.every(t => groupTags.some(gt => gt.id === t.id)) + } + + return true +} diff --git a/resources/js/misc/groupFilter.test.js b/resources/js/misc/groupFilter.test.js new file mode 100644 index 0000000000..ff9072b25d --- /dev/null +++ b/resources/js/misc/groupFilter.test.js @@ -0,0 +1,41 @@ +import { matchesFilters } from './groupFilter' + +const group = { + id: 1, + name: 'Hackney Fixing Factory', + group_tags_full: [{ id: 1, name: 'Lille' }, { id: 2, name: 'Repair Cafe' }], +} + +test('everything matches when nothing is filtered', () => { + expect(matchesFilters(group, null)).toBe(true) + expect(matchesFilters(group, {})).toBe(true) +}) + +test('matches part of a name, ignoring case', () => { + expect(matchesFilters(group, { name: 'hackney' })).toBe(true) + expect(matchesFilters(group, { name: 'Fixing' })).toBe(true) + expect(matchesFilters(group, { name: 'Ulverston' })).toBe(false) +}) + +test('a group with no name matches nothing', () => { + expect(matchesFilters({ id: 2 }, { name: 'anything' })).toBe(false) +}) + +// Choosing more tags should narrow the results, not widen them. +test('requires every selected tag, not just one of them', () => { + expect(matchesFilters(group, { tags: [{ id: 1 }] })).toBe(true) + expect(matchesFilters(group, { tags: [{ id: 1 }, { id: 2 }] })).toBe(true) + expect(matchesFilters(group, { tags: [{ id: 1 }, { id: 99 }] })).toBe(false) +}) + +test('a group with no tags matches only an empty tag filter', () => { + const untagged = { id: 3, name: 'Untagged' } + expect(matchesFilters(untagged, { tags: [] })).toBe(true) + expect(matchesFilters(untagged, { tags: [{ id: 1 }] })).toBe(false) +}) + +test('name and tags both have to match', () => { + expect(matchesFilters(group, { name: 'hackney', tags: [{ id: 1 }] })).toBe(true) + expect(matchesFilters(group, { name: 'ulverston', tags: [{ id: 1 }] })).toBe(false) + expect(matchesFilters(group, { name: 'hackney', tags: [{ id: 99 }] })).toBe(false) +}) From 43e93dd5167f2cf016274043531df64bfc4d3da8 Mon Sep 17 00:00:00 2001 From: edwh Date: Tue, 21 Jul 2026 14:36:21 +0100 Subject: [PATCH 21/34] Cluster the pins on the groups map A large network is an unreadable mass of overlapping pins without clustering. Uses supercluster, matching the approach and version already proven in the Freegle codebase: same options, the same bypass below a handful of markers (supercluster can quietly return fewer points than it was given, which doesn't show among thousands but is obvious when there are only a few), and clicking a cluster flies to the zoom at which it breaks apart, so a click always reveals something. Which clusters exist depends on the zoom and what's in view, and Leaflet reports both imperatively rather than reactively. This map already counts map idles for its own purposes, so reading that counter is what makes the clusters recompute as the map moves; Freegle had to add a counter of its own for this. The bubble is a fixed-size circle with a black border. Its class replaces Leaflet's .leaflet-div-icon rather than adding to it, or Leaflet's own white box and grey border would show behind the circle. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB --- package-lock.json | 18 +++- package.json | 1 + resources/global/css/_global.scss | 30 ++++++ resources/js/components/GroupMap.test.js | 85 +++++++++++++++++ resources/js/components/GroupMap.vue | 114 ++++++++++++++++++++++- 5 files changed, 246 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index d6eff8aa56..03b877c4c4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,5 +1,5 @@ { - "name": "www", + "name": "restarters.net", "lockfileVersion": 3, "requires": true, "packages": { @@ -42,6 +42,7 @@ "select2": "~4.0", "slick-carousel": "^1.8.1", "sortablejs": "^1.7.0", + "supercluster": "^7.1.5", "tempusdominus-bootstrap-4": "^5.39.2", "text-clipper": "^2.1.0", "tinysort": "^3.2.8", @@ -13969,6 +13970,12 @@ "node": ">=8" } }, + "node_modules/kdbush": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/kdbush/-/kdbush-3.0.0.tgz", + "integrity": "sha512-hRkd6/XW4HTsA9vjVpY9tuXJYLSlelnkTmVFu4M9/7MIYQtFcHpbugAU7UbOfjOiVSVYl2fqgBuJ32JUmRo5Ew==", + "license": "ISC" + }, "node_modules/keyv": { "version": "4.5.4", "resolved": "https://registry.npmjs.org/keyv/-/keyv-4.5.4.tgz", @@ -21165,6 +21172,15 @@ "postcss": "^8.2.15" } }, + "node_modules/supercluster": { + "version": "7.1.5", + "resolved": "https://registry.npmjs.org/supercluster/-/supercluster-7.1.5.tgz", + "integrity": "sha512-EulshI3pGUM66o6ZdH3ReiFcvHpM3vAigyK+vcxdjpJyEbIIrtbmBdY23mGgnI24uXiGFvrGq9Gkum/8U7vJWg==", + "license": "ISC", + "dependencies": { + "kdbush": "^3.0.0" + } + }, "node_modules/supports-color": { "version": "7.2.0", "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-7.2.0.tgz", diff --git a/package.json b/package.json index 7bee43d1ad..8d44a6e616 100644 --- a/package.json +++ b/package.json @@ -91,6 +91,7 @@ "select2": "~4.0", "slick-carousel": "^1.8.1", "sortablejs": "^1.7.0", + "supercluster": "^7.1.5", "tempusdominus-bootstrap-4": "^5.39.2", "text-clipper": "^2.1.0", "tinysort": "^3.2.8", diff --git a/resources/global/css/_global.scss b/resources/global/css/_global.scss index 2e00aff11c..774cc034fc 100644 --- a/resources/global/css/_global.scss +++ b/resources/global/css/_global.scss @@ -168,4 +168,34 @@ h2 { .group-marker-hover { filter: hue-rotate(153deg) saturate(1.5); +} + +// Cluster bubble on the groups map. This class replaces Leaflet's own +// .leaflet-div-icon, which would otherwise draw a white box with a grey border +// behind the circle. +.group-cluster { + width: 46px; + height: 46px; + border-radius: 50%; + border: 5px solid #000; + background-color: #fff; + color: #000; + text-align: center; + font-weight: bold; + cursor: pointer; +} + +.group-cluster__count { + display: flex; + align-items: center; + justify-content: center; + height: 100%; + font-size: 16px; + line-height: 1; +} + +// The bubble is a fixed size whatever the count, so shrink four-figure numbers +// rather than letting them spill outside the circle. +.group-cluster__count--wide { + font-size: 12px; } \ No newline at end of file diff --git a/resources/js/components/GroupMap.test.js b/resources/js/components/GroupMap.test.js index 6fd72cfdbb..0159392a93 100644 --- a/resources/js/components/GroupMap.test.js +++ b/resources/js/components/GroupMap.test.js @@ -35,6 +35,7 @@ function fakeMap(size = { x: 688, y: 400 }) { invalidateSize: jest.fn(), fitBounds: jest.fn(), flyToBounds: jest.fn(), + flyTo: jest.fn(), getSize: () => size, getBounds: () => L.latLngBounds([[50, -1], [52, 1]]), getZoom: () => 5, @@ -286,6 +287,90 @@ describe('GroupMap reframing on request', () => { }) }) +// Without clustering a large network is an unreadable mass of overlapping pins. +describe('GroupMap clustering', () => { + // All within the fake map's bounds ([[50,-1],[52,1]]), close enough together + // to cluster at its zoom. + const many = Array.from({ length: 20 }, (_, i) => ({ + id: i + 1, + location: { lat: 51 + i * 0.001, lng: 0 + i * 0.001 }, + })) + + function mountWithMap(groups) { + const wrapper = mountMap(WORLD, groups) + const map = fakeMap() + wrapper.vm.mapObject = map + wrapper.vm.$refs.map = { mapObject: map } + return { wrapper, map } + } + + test('gathers overlapping groups into a single cluster', () => { + const { wrapper } = mountWithMap(many) + + const clusters = wrapper.vm.clusters + expect(clusters.length).toBeLessThan(many.length) + expect(clusters.some(c => c.properties.cluster)).toBe(true) + }) + + // Supercluster can quietly drop points when there are few of them, which is + // obvious at high zoom. Below the threshold, show the groups themselves. + test('shows every group individually when there are only a few', () => { + const few = many.slice(0, 3) + const { wrapper } = mountWithMap(few) + + const clusters = wrapper.vm.clusters + expect(clusters).toHaveLength(3) + expect(clusters.every(c => !c.properties.cluster)).toBe(true) + expect(clusters.map(c => c.properties.groupId).sort()).toEqual([1, 2, 3]) + }) + + // A four-figure count won't fit at the normal size in a fixed-size bubble. + test('shrinks the text for counts that would overflow the circle', () => { + const { wrapper } = mountWithMap(many) + const cluster = wrapper.vm.clusters.find(c => c.properties.cluster) + + expect(wrapper.vm.clusterIcon(cluster).options.html) + .not.toContain('group-cluster__count--wide') + + const big = { properties: { ...cluster.properties, point_count: 1234 } } + expect(wrapper.vm.clusterIcon(big).options.html) + .toContain('group-cluster__count--wide') + }) + + test('the cluster marker shows how many groups are in it', () => { + const { wrapper } = mountWithMap(many) + const cluster = wrapper.vm.clusters.find(c => c.properties.cluster) + + expect(wrapper.vm.clusterIcon(cluster).options.html) + .toContain(String(cluster.properties.point_count)) + }) + + test('clicking a cluster zooms in far enough to break it up', () => { + const { wrapper, map } = mountWithMap(many) + const cluster = wrapper.vm.clusters.find(c => c.properties.cluster) + + wrapper.vm.clusterClick(cluster) + + expect(map.flyTo).toHaveBeenCalled() + const [, zoom] = map.flyTo.mock.calls[0] + expect(zoom).toBeGreaterThan(map.getZoom()) + expect(zoom).toBeLessThanOrEqual(wrapper.vm.maxZoom) + }) + + // Leaflet's own .leaflet-div-icon would otherwise draw a white box with a grey + // border behind the circle. + test('the cluster icon replaces Leaflet default styling with its own class', () => { + const { wrapper } = mountWithMap(many) + const cluster = wrapper.vm.clusters.find(c => c.properties.cluster) + + const options = wrapper.vm.clusterIcon(cluster).options + expect(options.className).toBe('group-cluster') + // Anchored at its middle, so the circle sits over the point it represents. + expect(options.iconSize).toEqual([46, 46]) + expect(options.iconAnchor).toEqual([23, 23]) + }) +}) + describe('GroupMap markers', () => { test('only renders markers for groups with coordinates', () => { const wrapper = mountMap(WORLD, [ diff --git a/resources/js/components/GroupMap.vue b/resources/js/components/GroupMap.vue index 8e71cf1b4b..c418a08bff 100644 --- a/resources/js/components/GroupMap.vue +++ b/resources/js/components/GroupMap.vue @@ -13,7 +13,23 @@ @moveend="idle" @dragend="dragEnd" > - +
@@ -26,6 +42,7 @@ import { Photon } from 'leaflet-control-geocoder/src/geocoders/photon' // what keeps it hidden until a search actually fails (and styles the button). import 'leaflet-control-geocoder/dist/Control.Geocoder.css' import GroupMarker from './GroupMarker.vue' +import Supercluster from 'supercluster' export default { components: { @@ -92,6 +109,14 @@ export default { type: Number, required: false, default: 0, + }, + // Below this many groups, draw them all rather than clustering. Supercluster + // can quietly return fewer points than it was given, which doesn't matter + // among thousands but is obvious when there are only a handful. + minCluster: { + type: Number, + required: false, + default: 10, } }, data() { @@ -140,6 +165,66 @@ export default { return lat != null && lng != null && !isNaN(+lat) && !isNaN(+lng) }) }, + clusterPoints() { + return this.mappableGroups.map((g) => ({ + type: 'Feature', + id: g.id, + properties: { groupId: g.id, cluster: false }, + geometry: { + type: 'Point', + coordinates: [ + +(g.location && g.location.lng != null ? g.location.lng : g.lng), + +(g.location && g.location.lat != null ? g.location.lat : g.lat), + ], + }, + })) + }, + clusterIndex() { + // The index is immutable, so it has to be rebuilt whenever the points + // change rather than updated in place. + const index = new Supercluster({ + radius: 60, + maxZoom: this.maxZoom, + minZoom: this.minZoom, + }) + + index.load(this.clusterPoints) + + return index + }, + clusters() { + // Which clusters exist depends on the zoom and what's in view, but Leaflet + // reports both imperatively, so reading mapIdle here is what makes this + // recompute as the map moves. + this.mapIdle + + if (!this.mapObject || !this.clusterPoints.length) { + return [] + } + + if (this.clusterPoints.length < this.minCluster) { + return this.clusterPoints + } + + try { + const bounds = this.mapObject.getBounds() + + if (!bounds) { + return this.clusterPoints + } + + return this.clusterIndex.getClusters([ + bounds.getWest(), + bounds.getSouth(), + bounds.getEast(), + bounds.getNorth(), + ], Math.round(this.mapObject.getZoom())) + } catch (e) { + // Map state races (no bounds mid-transition) shouldn't lose the markers. + console.error('Error clustering groups', e) + return this.clusterPoints + } + }, hasLocation() { // The groups page sends the inverted world box [[90,180],[-90,-180]] when // the user has no location set; a real bounding box always has @@ -325,6 +410,33 @@ export default { this.$emit('update:moved', true) this.idle() }, + clusterIcon(cluster) { + const count = cluster.properties.point_count + const wide = count >= 1000 ? ' group-cluster__count--wide' : '' + + return L.divIcon({ + html: '
' + count + '
', + // Replaces Leaflet's .leaflet-div-icon, which would otherwise draw a + // white box with a grey border behind the circle. + className: 'group-cluster', + iconSize: [46, 46], + iconAnchor: [23, 23], + }) + }, + clusterClick(cluster) { + // Zoom to where this cluster breaks apart, so a click always reveals + // something rather than appearing to do nothing. + const zoom = Math.min( + this.clusterIndex.getClusterExpansionZoom(cluster.properties.cluster_id), + this.maxZoom + ) + + this.moved = true + this.mapObject.flyTo( + [cluster.geometry.coordinates[1], cluster.geometry.coordinates[0]], + zoom + ) + }, frameShownGroups() { // A filter is an explicit request to see something, so take the map there // even if the user has panned somewhere else. From d077bfea639a5b9835952359b06fa79753675703 Mon Sep 17 00:00:00 2001 From: edwh Date: Tue, 21 Jul 2026 14:38:47 +0100 Subject: [PATCH 22/34] Import supercluster's prebuilt bundle, and stop tracking coverage output Importing the package root resolves to the ESM source in the browser build but to the UMD bundle under Jest, so the tests would exercise different code from the thing that ships. Naming the bundle explicitly makes both the same, which is also how Freegle imports it. tests/clover.xml is regenerated by every test run and was picked up by an over-broad add. It's coverage output, not source. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB --- .gitignore | 3 + resources/js/components/GroupMap.vue | 6 +- tests/clover.xml | 10940 ------------------------- 3 files changed, 8 insertions(+), 10941 deletions(-) delete mode 100644 tests/clover.xml diff --git a/.gitignore b/.gitignore index e2459d1265..2e7256eadb 100644 --- a/.gitignore +++ b/.gitignore @@ -38,3 +38,6 @@ restarters-anonymised.sql /public/repair-data*.csv /uploads/*.jpg .mcp.json + +# PHPUnit coverage output, regenerated on every test run. +tests/clover.xml diff --git a/resources/js/components/GroupMap.vue b/resources/js/components/GroupMap.vue index c418a08bff..331bc367c6 100644 --- a/resources/js/components/GroupMap.vue +++ b/resources/js/components/GroupMap.vue @@ -42,7 +42,11 @@ import { Photon } from 'leaflet-control-geocoder/src/geocoders/photon' // what keeps it hidden until a search actually fails (and styles the button). import 'leaflet-control-geocoder/dist/Control.Geocoder.css' import GroupMarker from './GroupMarker.vue' -import Supercluster from 'supercluster' +// The prebuilt bundle rather than the package root: the root resolves to the +// ESM source in the browser build but to the UMD bundle under Jest, so tests +// would exercise different code from production. Freegle imports it this way +// for the same reason. +import Supercluster from 'supercluster/dist/supercluster' export default { components: { diff --git a/tests/clover.xml b/tests/clover.xml deleted file mode 100644 index cb81ed310c..0000000000 --- a/tests/clover.xml +++ /dev/null @@ -1,10940 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - From 3640890b1552d4c0f00b68a689f87503f0474756 Mon Sep 17 00:00:00 2001 From: edwh Date: Tue, 21 Jul 2026 14:46:46 +0100 Subject: [PATCH 23/34] Wait for the typing to stop before moving the map A filter change fires per keystroke, and every one of them was reframing the map, so typing a name lurched the viewport around under the user. Only the viewport move waits. The list and the pins still follow every keystroke: they just get shorter, which is what someone typing a name expects to see. The pending move is cancelled if the component goes away, so it can't wake up and touch something that has been destroyed. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB --- .../js/components/GroupMapAndList.test.js | 64 +++++++++++++++++-- resources/js/components/GroupMapAndList.vue | 25 +++++++- 2 files changed, 81 insertions(+), 8 deletions(-) diff --git a/resources/js/components/GroupMapAndList.test.js b/resources/js/components/GroupMapAndList.test.js index da1e8dec0d..465c3fcf12 100644 --- a/resources/js/components/GroupMapAndList.test.js +++ b/resources/js/components/GroupMapAndList.test.js @@ -151,25 +151,75 @@ describe('filters drive the map', () => { expect(wrapper.vm.effectiveGroupIds).toEqual([2]) }) - test('asks the map to reframe, so a search for a group out of view goes to it', async () => { + test('clearing the filter puts every group back on the map', async () => { + const wrapper = await makeWrapper({ showFilters: true }) + + wrapper.findComponent({ name: 'GroupsTable' }).vm.$emit('update:filters', { name: 'Alpha' }) + await wrapper.vm.$nextTick() + wrapper.findComponent({ name: 'GroupsTable' }).vm.$emit('update:filters', { name: null }) + await wrapper.vm.$nextTick() + + expect(wrapper.findComponent(groupMapStub).props('groupids')).toEqual([1, 2]) + }) +}) + +// Typing a name fires a filter change per keystroke. Narrowing the list and the +// pins each time is cheap and expected, but moving the viewport each time makes +// the map lurch around under the user while they are still typing. +describe('reframing waits for the typing to stop', () => { + beforeEach(() => jest.useFakeTimers()) + afterEach(() => jest.useRealTimers()) + + function type(wrapper, name) { + wrapper.findComponent({ name: 'GroupsTable' }).vm.$emit('update:filters', { name }) + } + + test('does not move the map between keystrokes', async () => { const wrapper = await makeWrapper({ showFilters: true }) const before = wrapper.findComponent(groupMapStub).props('frameRequest') - wrapper.findComponent({ name: 'GroupsTable' }).vm.$emit('update:filters', { name: 'Beta' }) + type(wrapper, 'B') + type(wrapper, 'Be') + type(wrapper, 'Bet') await wrapper.vm.$nextTick() - expect(wrapper.findComponent(groupMapStub).props('frameRequest')).toBeGreaterThan(before) + expect(wrapper.findComponent(groupMapStub).props('frameRequest')).toBe(before) }) - test('clearing the filter puts every group back on the map', async () => { + // This is also what takes you to a group that isn't currently on screen, + // rather than reporting no results for something that is just off the edge. + test('moves the map once, after the typing stops', async () => { const wrapper = await makeWrapper({ showFilters: true }) + const before = wrapper.findComponent(groupMapStub).props('frameRequest') - wrapper.findComponent({ name: 'GroupsTable' }).vm.$emit('update:filters', { name: 'Alpha' }) + type(wrapper, 'B') + type(wrapper, 'Be') + type(wrapper, 'Beta') + jest.runAllTimers() await wrapper.vm.$nextTick() - wrapper.findComponent({ name: 'GroupsTable' }).vm.$emit('update:filters', { name: null }) + + expect(wrapper.findComponent(groupMapStub).props('frameRequest')).toBe(before + 1) + }) + + // Only the viewport move waits: the list and the pins should keep up with + // what's being typed. + test('narrows the list and the pins straight away', async () => { + const wrapper = await makeWrapper({ showFilters: true }) + + type(wrapper, 'Beta') await wrapper.vm.$nextTick() - expect(wrapper.findComponent(groupMapStub).props('groupids')).toEqual([1, 2]) + expect(wrapper.findComponent(groupMapStub).props('groupids')).toEqual([2]) + expect(wrapper.vm.effectiveGroupIds).toEqual([2]) + }) + + test('a pending reframe does not fire after the component goes away', async () => { + const wrapper = await makeWrapper({ showFilters: true }) + + type(wrapper, 'Beta') + wrapper.destroy() + + expect(() => jest.runAllTimers()).not.toThrow() }) }) diff --git a/resources/js/components/GroupMapAndList.vue b/resources/js/components/GroupMapAndList.vue index 9eeb3fc6d6..59f145fbc1 100644 --- a/resources/js/components/GroupMapAndList.vue +++ b/resources/js/components/GroupMapAndList.vue @@ -42,6 +42,10 @@ import {MAX_MAP_ZOOM, MIN_MAP_ZOOM} from "../constants"; import GroupMap from "./GroupMap.vue"; import { matchesFilters } from '../misc/groupFilter' + +// Long enough to sit through normal typing, short enough that the map follows +// promptly once you stop. +const REFRAME_DEBOUNCE_MS = 500 import GroupsTable from "./GroupsTable.vue"; import VIcon from 'vue-awesome/components/Icon' @@ -121,6 +125,7 @@ export default { filters: null, // Bumped when a filter changes, to ask the map to frame the matches. frameRequest: 0, + frameTimer: null, mapready: false, bounds: null, hover: null, @@ -152,6 +157,13 @@ export default { return this.groupidsInBounds.filter(id => matching.includes(id)) }, }, + beforeDestroy() { + // Don't wake up and touch a component that has gone away. + if (this.frameTimer) { + clearTimeout(this.frameTimer) + this.frameTimer = null + } + }, async mounted() { // Wrap to avoid an unhandled async rejection breaking Vue 2's // scheduler — see notes in GroupsRequiringModeration.vue. @@ -170,8 +182,19 @@ export default { this.groupidsInBounds = groupids }, filtersChanged(filters) { + // The list and the pins follow immediately - they just get shorter. Moving + // the viewport waits until the typing stops, or the map lurches around + // under the user on every keystroke. this.filters = filters - this.frameRequest++ + + if (this.frameTimer) { + clearTimeout(this.frameTimer) + } + + this.frameTimer = setTimeout(() => { + this.frameTimer = null + this.frameRequest++ + }, REFRAME_DEBOUNCE_MS) }, }, } From 300b651008f57f5ad6c19b009a36e5662221ecba Mon Sep 17 00:00:00 2001 From: edwh Date: Tue, 21 Jul 2026 18:40:21 +0100 Subject: [PATCH 24/34] Fix the empty map on network pages, and the labels for sharing The network pages showed no groups at all. A group's networks arrive in two shapes - the names index that draws the map sends plain ids, while the summary API sends objects - and the scoping check only understood the objects. It had been harmless while it only chose the list's contents before the map reported, but once it started deciding which groups the map draws it emptied the network pages entirely. Both the map and the list now share one check that understands either shape. Labels, per feedback: "Other groups" becomes "Find a group"; the count says how many groups are in this area and suggests searching and zooming rather than only zooming out; and the empty state points at finding a group near you instead of following your nearest one. The mobile tab still read "Nearest", which contradicts all of that, so it follows the existing short-label convention and becomes "Find". fr and fr-BE updated to match. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB --- lang/en/groups.php | 8 ++--- lang/fr-BE/groups.php | 8 ++--- lang/fr/groups.php | 8 ++--- resources/js/components/GroupMap.vue | 8 ++--- .../js/components/GroupMapAndList.test.js | 33 ++++++++++++++++--- resources/js/components/GroupMapAndList.vue | 8 ++--- resources/js/misc/groupFilter.js | 11 +++++++ resources/js/misc/groupFilter.test.js | 26 ++++++++++++++- 8 files changed, 81 insertions(+), 29 deletions(-) diff --git a/lang/en/groups.php b/lang/en/groups.php index c2aad0e79f..0c6f94696a 100644 --- a/lang/en/groups.php +++ b/lang/en/groups.php @@ -12,7 +12,7 @@ 'create_groups' => 'Add a new group', 'create_group' => 'Create group', 'groups_title1' => 'Your Groups', - 'groups_title2' => 'Other groups', + 'groups_title2' => 'Find a group', 'groups_name' => 'Name', 'groups_name_of' => 'Name of group', 'groups_about_group' => 'Tell us about your group', @@ -105,12 +105,12 @@ 'no_unpowered_stats' => 'At the moment, these stats are only displayed for powered items. We hope to include unpowered items soon.', 'create_groups_mobile2' => 'Add new', 'groups_title1_mobile' => 'Yours', - 'groups_title2_mobile' => 'Nearest', + 'groups_title2_mobile' => 'Find', 'join_group_button_mobile' => 'Follow', - 'no_groups_mine' => 'If you can\'t see any here yet, why not follow your nearest group to hear about their upcoming repair events?', + 'no_groups_mine' => 'If you can\'t see any here yet, why not find a group near you?', 'no_groups_nearest_no_location' => '

You do not currently have a town/city set. You can set one in your profile.

You can also view all groups.

', 'no_groups_nearest_with_location' => '

There are no groups within 50 km of your location. You can see all groups here. Or why not start your own? Learn what running your own repair event involves.

', - 'group_count' => 'There is :count group. Zoom out to see more.|There are :count groups. Zoom out to see more.', + 'group_count' => 'There is :count group in this area. Search and zoom to find more.|There are :count groups in this area. Search and zoom to find more.', 'search_name_placeholder' => 'Search by name', 'search_location_placeholder' => 'Search by location', 'search_country_placeholder' => 'Filter by country', diff --git a/lang/fr-BE/groups.php b/lang/fr-BE/groups.php index c6b7567ae0..42976ec95b 100644 --- a/lang/fr-BE/groups.php +++ b/lang/fr-BE/groups.php @@ -8,7 +8,7 @@ 'create_groups' => 'Créer nouveau Repair Café', 'create_group' => 'Créer un nouveau Repair Café', 'groups_title1' => 'Vos Repair Cafés', - 'groups_title2' => 'Autres Repair Cafés', + 'groups_title2' => 'Trouver un Repair Café', 'groups_name' => 'Nom', 'groups_name_of' => 'Nom du Repair Café', 'groups_about_group' => 'Parlez-nous de votre Repair Café', @@ -106,8 +106,8 @@ 'volunteers_invited' => 'Bénévoles invités', 'create_groups_mobile2' => 'Ajouter nouveau', 'groups_title1_mobile' => 'Le vôtre', - 'groups_title2_mobile' => 'Le plus proche', - 'group_count' => 'Il y a :count Repair Café. Dézoomez pour en voir plus.|Il y a :count Repair Cafés. Dézoomez pour en voir plus.', + 'groups_title2_mobile' => 'Trouver', + 'group_count' => 'Il y a :count Repair Café dans cette zone. Cherchez et zoomez pour en trouver d\'autres.|Il y a :count Repair Cafés dans cette zone. Cherchez et zoomez pour en trouver d\'autres.', 'hide_filters' => 'Cacher les filtres', 'join_group_button_mobile' => 'Suivre', 'leave_group_button' => 'Ne plus suivre ce Repair Café', @@ -115,7 +115,7 @@ 'leave_group_confirm' => 'Veuillez confirmer que vous ne voulez plus suivre ce Repair Café', 'now_following' => 'Vous suivez maintenant :name!', 'now_unfollowed' => 'Vous ne suivez maintenant plus :name!', - 'no_groups_mine' => 'Si vous ne pouvez encore en voir aucun ici, pourquoi ne pas suivre le Repair Café le plus proche pour être informé de ses futurs événements?', + 'no_groups_mine' => 'Si vous ne pouvez encore en voir aucun ici, pourquoi ne pas trouver un Repair Café près de chez vous?', 'no_groups_nearest_no_location' => '

Vous n\'avez pas défini de village/ville. Vous pouvez en ajouter un.e dans votre profil.

Vous pouvez aussi voir tous les Repair Cafés.

', 'no_groups_nearest_with_location' => '

Il n\'y a apparemment pas encore de Repair Cafés listé proche de chez vous.

Voulez-vous créer ou ajouter un Repair Café? Regardez comment faire dans nos ressources.

', 'no_unpowered_stats' => 'Pour l\'instant, ces statistiques sont seulement affichées pour les appareils électriques. Nous espérons pouvoir inclure les appareils non-électriques sous peu.', diff --git a/lang/fr/groups.php b/lang/fr/groups.php index 4eef9e69b1..aecb8052fd 100644 --- a/lang/fr/groups.php +++ b/lang/fr/groups.php @@ -8,7 +8,7 @@ 'create_groups' => 'Créer nouveau Repair Café', 'create_group' => 'Créer un nouveau Repair Café', 'groups_title1' => 'Vos Repair Cafés', - 'groups_title2' => 'Autres Repair Cafés', + 'groups_title2' => 'Trouver un Repair Café', 'groups_name' => 'Nom', 'groups_name_of' => 'Nom du Repair Café', 'groups_about_group' => 'Parlez-nous de votre Repair Café', @@ -106,8 +106,8 @@ 'volunteers_invited' => 'Bénévoles invités', 'create_groups_mobile2' => 'Ajouter nouveau', 'groups_title1_mobile' => 'Le vôtre', - 'groups_title2_mobile' => 'Le plus proche', - 'group_count' => 'Il y a :count Repair Café. Dézoomez pour en voir plus.|Il y a :count Repair Cafés. Dézoomez pour en voir plus.', + 'groups_title2_mobile' => 'Trouver', + 'group_count' => 'Il y a :count Repair Café dans cette zone. Cherchez et zoomez pour en trouver d\'autres.|Il y a :count Repair Cafés dans cette zone. Cherchez et zoomez pour en trouver d\'autres.', 'hide_filters' => 'Cacher les filtres', 'join_group_button_mobile' => 'Suivre', 'leave_group_button' => 'Ne plus suivre ce Repair Café', @@ -115,7 +115,7 @@ 'leave_group_confirm' => 'Veuillez confirmer que vous ne voulez plus suivre ce Repair Café', 'now_following' => 'Vous suivez maintenant :name!', 'now_unfollowed' => 'Vous ne suivez maintenant plus :name!', - 'no_groups_mine' => 'Si vous ne pouvez encore en voir aucun ici, pourquoi ne pas suivre le Repair Café le plus proche pour être informé de ses futurs événements?', + 'no_groups_mine' => 'Si vous ne pouvez encore en voir aucun ici, pourquoi ne pas trouver un Repair Café près de chez vous?', 'no_groups_nearest_no_location' => '

Vous n\'avez pas défini de village/ville. Vous pouvez en ajouter un.e dans votre profil.

Vous pouvez aussi voir tous les Repair Cafés.

', 'no_groups_nearest_with_location' => '

Il n\'y a apparemment pas encore de Repair Cafés listé proche de chez vous.

Voulez-vous créer ou ajouter un Repair Café? Regardez comment faire dans nos ressources.

', 'no_unpowered_stats' => 'Pour l\'instant, ces statistiques sont seulement affichées pour les appareils électriques. Nous espérons pouvoir inclure les appareils non-électriques sous peu.', diff --git a/resources/js/components/GroupMap.vue b/resources/js/components/GroupMap.vue index 331bc367c6..09a15ec5ff 100644 --- a/resources/js/components/GroupMap.vue +++ b/resources/js/components/GroupMap.vue @@ -47,6 +47,7 @@ import GroupMarker from './GroupMarker.vue' // would exercise different code from production. Freegle imports it this way // for the same reason. import Supercluster from 'supercluster/dist/supercluster' +import { inNetwork } from '../misc/groupFilter' export default { components: { @@ -153,12 +154,7 @@ export default { groups = groups.filter((g) => this.groupids.includes(g.id || g.idgroups)) } - if (!this.network) { - return groups - } - - // Networks may be summary objects ([{id}]) or plain ids (the names index). - return groups.filter((g) => (g.networks || []).some((n) => (n && n.id !== undefined ? n.id : n) === this.network)) + return groups.filter((g) => inNetwork(g, this.network)) }, mappableGroups() { // A group with no geocode would put a marker at null island (0,0) — diff --git a/resources/js/components/GroupMapAndList.test.js b/resources/js/components/GroupMapAndList.test.js index 465c3fcf12..527f5e31e6 100644 --- a/resources/js/components/GroupMapAndList.test.js +++ b/resources/js/components/GroupMapAndList.test.js @@ -16,13 +16,20 @@ const GROUPS = [ { id: 2, name: 'Beta', networks: [{ id: 6, name: 'N6' }] }, ] -function makeStore() { +// What the names index actually sends, and what the map is drawn from: the +// store maps network_ids straight through, so these are plain integers. +const INDEX_GROUPS = [ + { id: 1, name: 'Alpha', networks: [5] }, + { id: 2, name: 'Beta', networks: [6] }, +] + +function makeStore(groups = GROUPS) { return new Vuex.Store({ modules: { groups: { namespaced: true, getters: { - list: () => GROUPS, + list: () => groups, }, actions: { list: () => Promise.resolve(), @@ -60,10 +67,10 @@ const groupsTableStub = { template: '
', } -async function makeWrapper(props = {}) { +async function makeWrapper(props = {}, groups = GROUPS) { const wrapper = mount(GroupMapAndList, { localVue, - store: makeStore(), + store: makeStore(groups), propsData: { initialBounds: [[90, 180], [-90, -180]], ...props, @@ -223,6 +230,24 @@ describe('reframing waits for the typing to stop', () => { }) }) +// Regression: the network pages showed an empty map and an empty list, because +// the scoping check only understood the summary API's {id} objects while the +// map is drawn from the names index, which sends plain ids. +describe('network scoping', () => { + test('finds the network\'s groups when networks are plain ids', async () => { + const wrapper = await makeWrapper({ network: 5 }, INDEX_GROUPS) + + expect(wrapper.vm.matchingGroupIds).toEqual([1]) + expect(wrapper.findComponent(groupMapStub).props('groupids')).toEqual([1]) + }) + + test('still finds them when networks are objects', async () => { + const wrapper = await makeWrapper({ network: 5 }, GROUPS) + + expect(wrapper.vm.matchingGroupIds).toEqual([1]) + }) +}) + describe('effectiveGroupIds', () => { test('falls back to the full (network-filtered) list before the map has reported', async () => { const wrapper = await makeWrapper({ network: 5 }) diff --git a/resources/js/components/GroupMapAndList.vue b/resources/js/components/GroupMapAndList.vue index 59f145fbc1..d97cf5a862 100644 --- a/resources/js/components/GroupMapAndList.vue +++ b/resources/js/components/GroupMapAndList.vue @@ -41,7 +41,7 @@