RES-1995: rework groups page to have a map of groups (re-opened after revert)#862
Open
edwh wants to merge 10 commits into
Open
RES-1995: rework groups page to have a map of groups (re-opened after revert)#862edwh wants to merge 10 commits into
edwh wants to merge 10 commits into
Conversation
PR #744 changed GroupsTable's prop API from `groups` (an array of group objects) to `groupids` (an array of group ids), but the migration of GroupsRequiringModeration was left half-finished: the template had a literal 'TODO' string visible to admins / NCs and the original `<GroupsTable :groups="groups" approve />` was commented out next to it. Render the table with `:groupids="groupIds" :approve="true"` and add a computed `groupIds` derived from the moderation store (which uses `idgroups`, but fall back to `id` for safety). Tests cover: - renders GroupsTable (not literal TODO) when groups exist - renders nothing when there are no groups - filters by the networks prop Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…le comment Three follow-ups to the GroupsRequiringModeration fix: 1. GroupsPage forwards showTags / networks / allGroupTags through to its inner GroupsTable on the 'Your groups' tab so tag badges actually render there (they didn't — the props were received from the blade but dropped on the floor). Also fixes a `your-area="yourArea"` literal-string bug (now `:your-area="yourArea"`). Removes three genuinely dead props (yourLat, yourLng, userId) and the matching blade attributes. Resolves the TODO at GroupsPage.vue:118. Added GroupsPage.test.js with 2 cases. 2. groups/fetch action now de-dups concurrent in-flight requests for the same (id, includeStats) key — GroupsPage's mounted() loop fires one fetch per yourGroup in parallel, so without this the same group could be in flight twice. Implemented as a module-scope Map; entry is cleared in a `finally` so a rejection doesn't poison the slot. Resolves the TODO at groups.js:227. Added groups.test.js with 5 cases (concurrent same-id, sequential, concurrent different-ids, error-then-retry, different includeStats). 3. Removed the stale 'See TODO below to chase the underlying reactivity bug separately' comment in grouptags.test.js — the reactivity bug WAS chased (unhandled async lifecycle rejections leaking Vue 2's scheduler `pending` flag) and the fix shipped in this PR (try/catch on async mounted hooks + flushRender() helper). Test infra: jest.config.json gets resources/js/store as a third root, plus moduleNameMapper stubs for leaflet-control-geocoder and vue-awesome so component tests can mount GroupsPage without resolving its transitive Leaflet/Icon deps. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…revert; keep PR content) # Conflicts: # resources/js/components/GroupsPage.vue # resources/js/components/GroupsRequiringModeration.vue # resources/js/store/groups.js # tests/Integration/grouptags.test.js
edwh
force-pushed
the
RES-1995_map_of_groups
branch
from
4b3b587 to
4043de1
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| <!-- Styles --> | ||
| @vite(['resources/sass/app.scss', 'resources/global/css/app.scss']) | ||
| <link rel="stylesheet" href="https://unpkg.com/leaflet-control-geocoder/dist/Control.Geocoder.css" type="text/css" /> |
| <!-- Styles --> | ||
| @vite(['resources/sass/app.scss', 'resources/global/css/app.scss']) | ||
| <link rel="stylesheet" href="https://unpkg.com/leaflet-control-geocoder/dist/Control.Geocoder.css" type="text/css" /> | ||
| <link rel="stylesheet" href="//unpkg.com/leaflet-gesture-handling/dist/leaflet-gesture-handling.min.css" type="text/css"> |
The userId prop was removed from GroupsPage.vue and its blade view as genuinely dead code while clearing PR #744 review TODOs. Update the test contract to match.
GroupInfoModal.groupImage guarded on group.image but built the URL from group.group_image, which the Group API resource never sets — so any group with an image rendered /uploads/mid_undefined. Use group.image (the back-filled groups.image column the resource exposes). Found by adversarial review of PR #862.
The `hosts` and `restarters` columns rendered their text labels ("Hosts" /
"Restarters") instead of icons, while every other column showed an icon. The
b-table header-override slots were named `head(all_confirmed_hosts_count)` /
`head(all_confirmed_restarters_count)`, but the field keys are `hosts` /
`restarters`, so the slots never matched and b-table fell back to the labels.
Rename the slots to `head(hosts)` / `head(restarters)`.
Test (GroupsTable.test.js): asserts the host/restarter column headers render
the user / volunteer icons rather than text.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When GroupMap is created inside a hidden tab (the groups page renders the map tab eagerly), its Leaflet container is 0x0. On switching to the tab the container resizes but nothing told Leaflet to re-measure, so tiles never filled the now-visible area and most of the map showed as grey. Observe the container with a ResizeObserver and call `invalidateSize()` (then re-run `idle()` to recompute in-bounds groups) when it changes size. Test (GroupMap.test.js): firing the resize observer calls invalidateSize(). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The groups table filter bar (shown on the network page) referenced groups.search_name_placeholder, search_location_placeholder, search_country_placeholder, search_tags_placeholder, show_filters and hide_filters, none of which existed in the lang files, so the UI rendered the raw keys. Add them for en, fr and fr-BE. Test (GroupFilterTranslationsTest.php): asserts the keys exist and are non-empty in all three locales. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… flyToBounds The groups-page map rendered grey for users with no location set. Two causes: 1. GroupController::index sends the inverted whole-world box [[90,180],[-90,-180]] when the user has no location, so the map had no real area to frame and centred on (0,0) — open ocean, hence grey. Frame all groups instead when there's no location (new `hasLocation` computed detects the world box). 2. zoomToGroups used flyToBounds, whose animation fights the :bounds.sync binding and leaves the map mid-animation so tiles for the final view never settle (also grey, and the zoom visibly oscillated). Use fitBounds for the initial framing instead. With this, tiles cover 100% of the viewport. Also harden the off-screen case (the map tab is rendered eagerly): - Add a size-guard so zoomToGroups won't frame while the map is still 0x0 (created in a hidden/transitioning tab) — it would otherwise centre on null island and never retry. - refreshSize() now resolves mapObject from the l-map ref if @ready hasn't fired yet, and re-frames the groups once the container is visible. - Mark the map tab in GroupsPage `lazy` so it isn't created off-screen at all. Tests: hasLocation detection, the size-guard, and framing all groups via fitBounds (not flyToBounds) when there's no location. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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
2 participants
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.
Re-opens replacement for #744, which was merged but then reverted on master (commit 78282c1) because the Laravel 10 → 13 upgrade was bundled in unintentionally.
This PR contains the same code as the original #744 plus fixes for the incomplete items that surfaced during review:
groups.imagecolumn with back-fillFollow-up fixes added on this branch (TDD throughout)
GroupsRequiringModerationwas stubbed with a literalTODOstring visible on the Groups page (commit da1b055). Restored the<GroupsTable :groupids="groupIds" approve />call with agroupIdscomputed derived from the moderation store. 3 Jest tests.GroupsPagewas receivingshowTags/networks/allGroupTagsfrom the blade but never forwarding them to its innerGroupsTable, so tag badges didn't render on the "Your groups" tab. Also fixed ayour-area="yourArea"literal-string binding bug. Removed three genuinely-unused props (yourLat,yourLng,userId) from the component and the matching blade attributes. 2 Jest tests. (commit d9c127b)groups/fetchaction now de-dups concurrent in-flight requests for the same(id, includeStats)key — GroupsPage'smounted()loop dispatches one fetch peryourGroupin parallel, so without this the same group could be in flight twice. Module-scope Map with afinally-cleared entry so rejections don't poison the slot. 5 Jest tests. (commit d9c127b)tests/Integration/grouptags.test.jsreferring to chasing the Vue reactivity bug — fix shipped, comment updated. (commit d9c127b)A single empty commit (9beecf1) was added at the start so GitHub would allow opening this PR from the branch (the original commits are still in develop's history via the merge ancestry, so GitHub otherwise reports 'no commits between').
Database / migration note
Production already has the
groups.imagecolumn from the original deploy of #744. The revert removed the migration file but the column and itsmigrationstable row remain in prod. When this PR is re-merged the file comes back, Laravel sees the migration as already-ran (because the row is still there), skips it, and the back-filled data is preserved. No rollback needed.References