RES-1995: map of groups (Laravel 10)#887
Open
edwh wants to merge 29 commits into
Open
Conversation
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 <noreply@anthropic.com>
…ity) SonarCloud flagged the raw `require $path` in GroupFilterTranslationsTest as a reliability bug (require -> require_once). require_once would be incorrect here: if the lang file was already included elsewhere it returns true rather than the translations array. Use Filesystem::getRequire() instead — returns the array, keeps the include inside framework code, and needs no app container (this is a pure PHPUnit test). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review against the RES-1995 discussion found regressions in the map rework
and the missing network-page half of the feature. All fixed test-first
(Jest 42->78 tests; new GroupSummaryApiTest server-side).
Restored functionality:
- NetworkPage embeds the map + list of the network's groups (replaces the
groups_count/view_groups_link box whose lang keys were deleted; those keys
would have rendered literally). Starts zoomed out to frame the network.
- /group/network/{id} filters by network again: GroupsPage now forwards the
network prop to GroupMapAndList. NetworkTest regains a regression guard.
- Filter bar reachable again (name/tag/location/country/network) on map
lists; network dropdown hidden on network-scoped views; searchNetwork is
actually applied; missing toggleFilters method added (mobile toggle threw).
- Groups-requiring-moderation rows render again: moderation groups are
backfilled into the list store GroupsTable reads from.
Bug fixes:
- Your Groups tab showed "Follow group" for followed groups (yourGroups not
passed); unfollow now flips the button immediately.
- Group logos 404'd: v2 APIs return bare paths, now prefixed /uploads/mid_.
- Name-column sort threw (old nested group_name shape); next_event sort
compared the object not its start; list now sorts before slicing so the
first page is genuinely alphabetical.
- Panning to an empty area listed ALL groups (empty-in-view == "no report
yet"); now distinguished.
- Map dragging was mobile-only (!!L.Browser.mobile) - desktop couldn't pan;
dropped gestureHandling (plugin not installed).
- Followed/hover pins referenced marker-icon-green/red.png which don't ship;
recoloured via CSS hue-rotate classes instead. Hover state now seeds at
marker mount. Ungeocoded groups no longer drop markers at (0,0).
- Tab switches no longer destroy/refetch the map (lazy until first shown).
- /api/v2/groups/summary: honours the validated-but-ignored archived param,
eager-loads networks/groupImage.image/group_tags (was O(N) queries), and
returns group_tags_full (whenLoaded) so tag badges/filters work from API
data. Store requests archived=true for UI parity with the old page.
- Removed dead expandGroups()/imports, inert :interactive attr, dead
computeds/listeners.
Verified: Jest 78/78; PHPUnit Groups+Networks+Unit 86 tests/969 assertions
green locally (full suite on CI); npm run production clean; browser-validated
(markers, modal, tab persistence, network scoping, filters, mobile toggle).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
Follow-up: bug fixes + network page map/listA review of this PR against the RES-1995 discussion history turned up a set of regressions and the missing network-page half of the feature. All fixed here, test-first (each fix has a failing test committed with it). SummaryRestored functionality
Bug fixes
Code quality review
Verified
Future improvements / known edges
Test plan
|
The dead nearestGroups computed removed in cf581b3 was its last reference, and testCheckTranslations (CI) rejects unused keys. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Preview: https://restarters-pr-887.fly.dev✅ Ready - restore and migrations succeeded.
|
- Hovering a map pin now highlights the matching list row (the reverse
of row-hover -> red pin): GroupMarker emits update:hover, which flows
through GroupMap into the shared hover state and back down to
GroupsTable as a row class
- The group modal's logo and name link to the group page, same as the
Go to group button
- /group/network/{id} is retired: coordinators see their groups on the
network page itself (map + list), so the route now redirects to
/networks/{id} and the View groups menu item is gone. Old links keep
working via the redirect
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TzY1phBjyT3HogpeS53zon
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TzY1phBjyT3HogpeS53zon
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TzY1phBjyT3HogpeS53zon
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TzY1phBjyT3HogpeS53zon
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TzY1phBjyT3HogpeS53zon
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TzY1phBjyT3HogpeS53zon
…oups_l10 # Conflicts: # lang/de/groups.php # lang/it/groups.php # lang/ne/groups.php # lang/nl-BE/groups.php # lang/no/groups.php
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TzY1phBjyT3HogpeS53zon
edwh
added a commit
that referenced
this pull request
Jul 16, 2026
edwh
added a commit
that referenced
this pull request
Jul 16, 2026
edwh
added a commit
that referenced
this pull request
Jul 16, 2026
edwh
added a commit
that referenced
this pull request
Jul 16, 2026
Built by the phase-de-client workflow: /profile/edit/[id] five+ tabs against the folded PR-868 API (Uppy/tus photo, delete-account flow), public /profile/[id] (graceful until D2 endpoint), /user/all admin list, generic AdminCrudTable + five reference-data pages (PR-863 API), /networks pages with the group-tags permission matrix, cookie policy, onboarding modal, notifications dropdown, locale switcher, and the guest landing page (preserving the legacy h2 contract string). Plural parity spec made self-adapting after #887's networks.php rework removed the corpus's only three-form string.
edwh
added a commit
that referenced
this pull request
Jul 16, 2026
notifications.js moves to global/js/misc (canonical, survives cutover) with a temporary annotated copy at the old path — vite's define global->window shim (for select2) mangles any import path containing the token 'global' from resources/js, and that tree dies at F anyway. box-shadows moves into global/css/helpers; _cards.scss gets a local copy of the legacy fixometer variables instead of reaching into resources/sass. Legacy vite build verified green (also surfaced + fixed stale container deps from the #887 merge: leaflet-control-geocoder).
edwh
added a commit
that referenced
this pull request
Jul 17, 2026
/group/map as its own route/tab: markers for every non-archived group from the names index (clustered, leaflet.markercluster), visible rows hydrated via GET /api/v2/groups/summary?ids= chunked at the API's 200-id cap - the split-fetch architecture from PR #887. Photon place search (leaflet-control-geocoder), CARTO tiles, row<->marker hover linking in GroupsTable, and pure geometry helpers split into a composable for unit testing. Deviations from legacy documented in GroupMap.vue (no initial user-area bounds - no v2 source; marker popup links to /group/view instead of the modal). 53 new vitest cases plus a groupmap e2e spec. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LzRkdRa6w1P3ZXKH5pGDh5
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB
Searching by place is what the map's own "Search for a place..." box is for, and country is a coarser version of the same question, so both go. A network filter only means anything to someone who already knows the networks, and the network pages scope their own lists - that goes too, along with the prop chain and the Network::all() that fed it. Name and tags are left, tags still only for those who can see them. The host and restarter counts have never helped anyone pick a group, and the follow button on every row invites following groups you haven't read about; that belongs on the group's own page. Moderation reuses this table and does need a per-row action, so that column stays when approving. The list is now ordered by distance from the middle of the map, nearest first, rather than alphabetically - someone looking at a map wants what's in front of them. Away from the map there's no centre to measure from, so it stays alphabetical there. Groups we can't place sort last. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB
The network pages shipped empty because component tests invented their own
idea of a group: they said networks were [{id}] objects, while the map is
drawn from the names index, which sends plain ids. The store test had the
shape right all along - nothing connected the two, so the components were
tested against a group that never occurs.
There is now one definition of a group in the store, and the store test feeds
the matching API response in and asserts the whole entry comes back out with
nothing overridden. Change the mapping and that test fails, which forces the
fixture to be updated, which moves every component test using it. Verified by
drifting the fixture and watching the store test fail.
Both live shapes are covered, because both are live at once: the index shape
the map draws from, and the summary shape the same components see once the
visible rows are hydrated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB
Measured on the preview, all three reported against it. The list looked stuck because the row count was frozen: it started at three rows and only ever grew by the infinite scroll, so zooming out from 12 groups in view to 120 still showed three. It now starts a fresh page whenever the map reports a different set of groups, compared by contents rather than identity so that hydration re-running the computed doesn't throw away someone's scrolling. The page is 25 rows instead of 3, and loads a page at a time rather than a row: with 230 groups in view the old size needed about 76 scrolls. Searching for London offered seven identical-looking results - England, Ontario, Kentucky, Ohio, Arkansas, California - because the label was built without the state or the country, which are the only things that tell them apart. Both are now included. Picking a place flew there with Leaflet's arcing animation, which drops two zoom levels below the destination and takes about three seconds on a long journey. It cuts straight there instead. That also makes the handler testable, which it wasn't while it lived inside the control's callback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB
"There are 0 groups in this area. Search and zoom to find more." counts nothing and offers no way out. When the area is empty, suggest finding a group near you instead, linking to the nearby view. fr and fr-BE match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194jGdUWidcpiC3RaZneBHB
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reworks the groups page to show a map of groups. Split out from #862, which bundled this feature with a Laravel 10→13 upgrade — this PR is the map feature alone, on the current Laravel 10 develop. The L13 upgrade is a separate PR.
What's here
GroupMap,GroupMarker,GroupMapAndList,GroupInfoModalGroupsPage/GroupsTable/GroupsRequiringModerationreworked for the map + filter bar/api/v2/groups/summaryendpoint (+lat/lngon the groups list,GroupSummaryresource)store/groups.js: summary fetch + dedupleaflet-control-geocoderJS depstoreroot + leaflet/vue-awesome mocks; new component + store testsNetworkTest::network_page: dropped the obsolete server-rendered"networks":[id]assertion (groups are now API-loaded)Stays on Laravel 10
No composer / framework / Docker / phpunit changes — verified
laravel/frameworkis still^10.48.Verified locally
🤖 Generated with Claude Code