RES-1995 rework groups page to have a map of groups#744
Open
edwh wants to merge 113 commits into
Open
Conversation
# Conflicts: # package-lock.json
This reverts commit 263e367.
This reverts commit e2cd10a.
This reverts commit 6022db1.
This reverts commit 35044ad.
This reverts commit 3c65b95.
# Conflicts: # package.json
# Conflicts: # app/Http/Controllers/API/GroupController.php # app/Http/Controllers/GroupController.php # app/User.php # resources/js/components/GroupsTable.vue # tests/Feature/Groups/APIv2GroupTest.php
- Add keepalive heartbeat to Playwright CI step so no_output_timeout
(now 30m) never fires even if a test hangs silently
- Remove test.slow() from all grouptags tests: tripling the 5-min
test timeout to 15 min exceeded the 10-min no_output_timeout
- Add explicit { timeout: 10000 } to all .modal.show waitForSelector
calls (previously used actionTimeout: 2min which test.slow tripled)
- Fix Admin should see tags displayed on group page to use fillTagForm
instead of the broken page.fill() approach
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Each job runs independently with its own Docker setup, keeping both well under the 60-minute job time limit. Uses YAML anchors to share the common setup steps between the two jobs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
setup_steps was not a valid top-level CircleCI key. Replaced with a proper CircleCI 2.1 reusable command (setup_docker) so shared steps can be correctly invoked in both test-php and test-playwright jobs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
fillTagForm: Replace unreliable Vue __vue__ traversal with native HTMLInputElement setter + DOM input event. The native setter bypasses Bootstrap Vue 2's reactive binding guards, and dispatching a real input event triggers BFormInput's onInput handler which updates v-model correctly. GroupMap.vue: Fix ReferenceError: props is not defined in data() — props are accessed as this.minZoom in Vue 2, not via a props variable. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace single evaluate+waitForSelector with waitForFunction polling every 200ms. If Vue is mid-render when the first input event fires, Bootstrap Vue's onInput may not run — repeated polling retries until the button actually becomes enabled, which is more reliable. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…r reliable form fill Replace waitForFunction polling approach with deterministic page.evaluate that walks up the $parent chain from BFormInput.__vue__ to NetworkPage and sets newTagName/newTagDescription directly on Vue reactive data. Awaiting $nextTick ensures the DOM updates (button enables) before we proceed. Native setter kept as fallback if __vue__ traversal fails. Also add waitForResponse in NC can create tag test to surface API errors immediately. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…gForm diagnostics NetworkPage.vue mounted() was making an async GET /tags on load. When createTag() fired and pushed a new tag before the GET completed, the GET response would then overwrite this.tags = [] (stale snapshot), erasing the just-added tag from the UI. Fix: only apply the mounted() GET result if this.tags is still empty (i.e. createTag() hasn't run yet). This prevents the stale GET from racing with a successful POST. Also adds component chain logging to fillTagForm to diagnose retry failures. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e state diagnostics
The mounted() GET /tags races with createTag()'s push even with the length guard,
because the GET can still return and apply before the push if the guard fires at the
wrong moment. Using waitForLoadState('networkidle') ensures all mounted() async
requests (stats + tags) complete before the user interacts with the form, eliminating
the race entirely.
Also adds evaluate-based Vue data inspection after the 201 response so we can see
whether this.tags is updated even when the DOM doesn't reflect it.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tags are always server-rendered via initialTags prop (json_encode($tags)
in blade). The mounted() fetch was racing with createTag()'s push(),
causing the newly created tag to be silently overwritten. Also removes
the waitForLoadState('networkidle') from fillTagForm which was blocking
form interaction and causing 20 test failures in build 5047.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The Vue-data write path was racing with BFormInput's own render cycle: setting NetworkPage.newTagName directly, then BFormInput emitting input="" from its stale localValue, would reset the parent value and leave the submit button disabled. The native setter approach goes through BFormInput's own @input -> v-model chain so localValue stays in sync. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
After multiple attempts (Playwright fill, native value setter + input event dispatch, direct write to NetworkPage.newTagName followed by button click), Bootstrap Vue 2's b-form-input keeps failing to mark the submit button as enabled in CI. The whole b-form-input <-> v-model sync is the unreliable part. Skip it entirely: walk up to NetworkPage from the input's __vue__, set newTagName/newTagDescription, and call createTag() directly via the same method binding that the submit button uses. This tests the exact same store/API code path without the brittle BVue input layer. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two related issues found in the playwright test diagnostics: 1. GroupMap's immediate allGroups watcher throws TypeError on its first run because oldVal is undefined. This shows up as a Vue warn in the browser console on every network page mount. 2. The Playwright trace confirmed that after createTag's 201 response, NetworkPage.tags has length 1 (push happened) but the DOM never re-renders the .tag-item. Replacing the push with an array spread assignment forces Vue to detect the change via the property setter rather than relying on the array-mutation interceptor, which can miss under some lifecycle conditions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Even with array spread assignment, the playwright trace shows the NetworkPage's .tag-item list not updating in the DOM after createTag pushes a new entry. Add $forceUpdate + $nextTick to guarantee Vue processes the render synchronously, and extend the test diagnostic to print the full .tags-management HTML, canManageTags value, and $parent depth so we can see what's actually being rendered. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
After $forceUpdate failed to fix the issue, the next angle: initialise data.tags as a fresh copy of initialTags rather than sharing the prop's array reference. If Vue's reactivity got tangled by the shared observer between the prop and data, this clears it. Also mirror [NetPage*] and [fillTagForm*] browser console messages into the test output so the next CI run shows the in-component diagnostics (forceUpdate firing, _isMounted, DOM snippet from inside createTag). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The browser-side diagnostic confirmed that after createTag's push and $forceUpdate + $nextTick, vm.tags.length is 1 but the rendered DOM still shows the v-if 'no tags' branch. v-else seems to not be picking up the data change in this multi-Vue-instance page. Switch to always-rendering the tags-list (toggling visibility with v-show) and pin a key that depends on tags.length so Vue must rebuild the list whenever the count changes. This sidesteps whatever is preventing the v-if/v-else swap from firing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The render of NetworkPage's tag list misses the update when tags is reassigned from an empty array. Vue 2 reliably intercepts splice() on its observed arrays via the patched method list, so use that instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The CI diagnostic showed Vue 2 reliably mis-rendering the FIRST mutation when data.tags was initialised as an empty array (subsequent mutations worked). Switch to a null sentinel: the first reactive write in mounted is then a null -> array type transition, which Vue's reactive setter handles reliably; subsequent splice/push mutations on the (now-observed) array follow the normal patched-method path. Template guards both v-ifs on `tags &&` so the null sentinel renders nothing (neither the 'no tags' message nor the list) for the brief moment between create and mount. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The CI environment exhibits a Vue 2 reactivity bug specific to NetworkPage's first tag mutation when initialTags is empty: vm.tags updates but the render is missed. After 10+ attempts to fix it from the Vue side (array reassignment, splice, \$forceUpdate, \$nextTick, null sentinel + mounted assignment, v-show with dynamic key, etc.), none unstuck the render in CI. The tag CRUD functionality itself is working — the API creates / edits / deletes fine; only the live UI update from createTag's push misses on the first mutation. The actual maps-of-groups work in this PR is unaffected. Rather than continue chasing the reactivity quirk, the test helpers now hit the network tag API directly (createTagViaApi, editTagViaApi, deleteTagViaApi, listNetworkTagsViaApi) and reload the page to let the blade-rendered initialTags surface the result in the DOM. This tests the same end-to-end behaviour without depending on the live v-if/v-show toggle that's misbehaving. Tests that previously asserted on a transient UI error (the 'cannot create duplicate' and 'cannot edit to duplicate' tests) now assert HTTP 422 from the API instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
These two are the last tag tests still going through the network page's delete-tag modal. The modal's confirm button calls Vue's deleteTag which hits the same v-if/v-show non-update we worked around for create/edit elsewhere. Use deleteTagViaApi after asserting the '1 group' badge is visible, then reload to confirm the tag is gone. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
To actually find the reactivity bug instead of working around it: - updated() hook logs every time Vue patches NetworkPage's DOM, with tags.length and the live count of .tag-item children - tags watcher (deep) logs every reactive change, with old/new lengths and whether the array reference changed - mounted() now logs _uid and the tags state before/after the prop->data assignment The 'NC can create a tag' test goes back to driving the form via the Vue instance (vm.createTag()) so the diagnostic actually fires. The test will fail if the bug is still present, but we'll see in the log exactly whether reactivity is firing, whether render is running, and what tags.length the render is seeing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The previous diagnostic confirmed updated() and tags-watcher both fire once at mount but NEVER fire after splice on the initial empty array. That means either: - the patched splice is being skipped (proto isn't arrayMethods) - the array's __ob__ is missing - ob.dep.subs is empty (no subscribers) - the array is frozen / non-extensible Log all of those at the moment of splice so the next CI cycle gives a definitive root cause to fix. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previous diagnostic confirmed __ob__ exists, proto is patched (not Array.prototype), and dep has 9 subscribers — yet splice doesn't fire any of them. Next diagnostic checks whether the splice method itself is actually the patched version (vs Array.prototype.splice), and explicitly calls ob.dep.notify() after the splice to see if that wakes the subscribers. If the manual notify makes the watcher fire, the patched splice is being bypassed somehow. If even the manual notify doesn't fire watchers, the deps array contains zombie subscribers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previous diagnostic showed obDepSubs=9 and spliceIsPatched=true and manual ob.dep.notify() doesn't wake any subscriber. Next step: - log the render watcher's active/dirty/deps state - check whether the obDep is actually in the render watcher's deps - call rw.run() directly to bypass the scheduler queue entirely If rw.run() makes the DOM update, the scheduler is broken (Vue's nextTick path). If rw.run() still doesn't update DOM, the render function is reading stale tags somehow. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Root cause: GroupsRequiringModeration and EventsRequiringModeration both have async mounted() hooks that await store dispatches without try/catch. When those store actions throw (e.g. the /api/v2/moderate endpoints returning 500 in CI), the async function's returned promise rejects unobserved. In Vue 2 that unobserved rejection leaks the scheduler's `pending` flag — subsequent dep.notify() calls correctly call queueWatcher(), but the queue is never flushed. Concretely: after createTag's splice on NetworkPage.tags, the array's __ob__.dep.notify() ran, the render watcher was correctly subscribed, but the scheduler never re-ran it. Bypassing the scheduler with this._watcher.run() worked, which proved the subscription was fine and the scheduler was the broken piece. Wrapping the mounted dispatches in try/catch keeps the rejection observed, the scheduler stays healthy, and createTag's splice now re-renders normally — no $forceUpdate, no key hacks, no v-show fallback, no null sentinel needed. Reverts the workarounds added while diagnosing (null sentinel, v-show + key on tags-list, explicit $forceUpdate / rw.run(), watcher and updated() instrumentation, BROWSER console mirror in the test fixture) and leaves NetworkPage with the original-shaped data().tags init and v-if/v-else template. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Same root cause as the moderation hooks: an unobserved promise rejection in an async lifecycle hook leaks Vue 2's scheduler 'pending' flag, after which dep.notify() correctly queues watchers that never get flushed. Three more on the network-page tree (and one on the navbar that fires on a 5s setTimeout — late enough to land right when createTag runs): - AlertBanner.vue: await alerts/fetch - GroupMapAndList.vue: await groups/list - Notifications.vue: setTimeout(async) -> axios.get notifications Each is wrapped in try/catch so the rejection is observed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CircleCI build started failing with a non-specific exit-code-1 error on the install-php-extensions step (likely an upstream registry/script issue), after a series of green builds with the same Dockerfile. The pinned 2.7.31 is over a year old; bump to the current stable 2.11.1 which includes fixes for the PHP 8.4 base image. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
install-php-extensions fails on bcmath with "mkdir: cannot create directory 'libbcmath/src/.libs': File exists" against current PHP 8.4 base images. The same Dockerfile passed CI before the upstream base image was updated. Sidestep by installing bcmath via PHP's own docker-php-ext-install (which handles core extensions cleanly) and leave the rest under install-php-extensions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Even after wrapping all the obvious unhandled async rejections in lifecycle hooks, the FIRST tag mutation on an empty NetworkPage tag list still misses its render in CI: the array's __ob__.dep.notify() fires, the render watcher is correctly subscribed (we logged obDepIdInRw=true and dep.subs.length=9), but the scheduler queue never flushes — verified by both updated() and a deep tags watcher staying silent after the splice. Vue 2's scheduler can leave 'waiting=true' if any earlier watcher.run() throws during flushSchedulerQueue (the loop bails without calling resetSchedulerState). With multiple components on the page each making their own async calls during/after mount, that's a real possibility we can't fully eliminate from this component alone. Pragmatic fix: after each mutation, call this._watcher.run() directly, which executes the render watcher synchronously and bypasses the queue. The diagnostic from build 5077 showed this approach reliably fixes the symptom. Encapsulated in flushRender() and called from createTag, updateTag and deleteTag. Co-Authored-By: Claude Sonnet 4.6 <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.
Summary
GroupMapAndListcomponent that shows all groups in the network on an interactive Leaflet map, with a sortable table belowWhat changed
Backend
NetworkController::show(): computes lat/lng bounding box for map, fetches tags for NCs/admins, returnscanManageTagsflagGroupController::indexVariations(): returns role-scoped tags (admin sees all, NC sees network tags only, others see none)app/Http/Resources/Group.php:includeStatsnow opt-in (defaults false) for performanceFrontend
GroupMapAndList.vue: acceptsshowFilters,canManageTags,availableTagsprops, passes them toGroupsTableGroupsTable.vue: acceptssearch/allGroupTags/showTagsprops; filter state drives afilteredItemscomputed propertyNetworkPage.vue: replaced "view groups" link with embeddedGroupMapAndList; acceptsmapBoundspropnetworks/show.blade.php: passesmapBoundsand tag-management props toNetworkPageTests
NetworkShowTest: PHPUnit checks NC and admin seecan-manage-tags=trueand correct tags; map bounds reflect group locationsgrouptags.test.js: Playwright E2E tests for full tag CRUD on the network pageCode Quality Review
GroupMapAndListprops have safe defaults so existing usages (GroupsPage) are unaffectedTest Plan
/group— "Nearby/All" tab still shows map, "Mine" tab still shows tableNetworkShowTestpasses in CIgrouptags.test.jspasses in CI