From 9beecf167f529b66e1aa5b556d2d9009231bf3f8 Mon Sep 17 00:00:00 2001 From: edwh Date: Wed, 27 May 2026 12:12:22 +0100 Subject: [PATCH 1/9] Re-open after revert of #744 merge (78282c1c1d) From da1b0555524e2ec1ca03ea651f77ef1e108d5673 Mon Sep 17 00:00:00 2001 From: edwh Date: Wed, 27 May 2026 13:43:48 +0100 Subject: [PATCH 2/9] Restore GroupsTable in GroupsRequiringModeration (was stubbed with TODO) 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 `` 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 --- .../GroupsRequiringModeration.test.js | 103 ++++++++++++++++++ .../components/GroupsRequiringModeration.vue | 8 +- 2 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 resources/js/components/GroupsRequiringModeration.test.js diff --git a/resources/js/components/GroupsRequiringModeration.test.js b/resources/js/components/GroupsRequiringModeration.test.js new file mode 100644 index 0000000000..6d03b3b790 --- /dev/null +++ b/resources/js/components/GroupsRequiringModeration.test.js @@ -0,0 +1,103 @@ +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 GroupsRequiringModeration from './GroupsRequiringModeration.vue' + +const localVue = createLocalVue() +localVue.use(Vuex) + +function makeStore(moderate) { + return new Vuex.Store({ + modules: { + groups: { + namespaced: true, + getters: { + getModerate: () => moderate || {}, + }, + actions: { + getModerationRequired: () => Promise.resolve(), + } + } + } + }) +} + +async function flush(wrapper) { + // mounted() awaits a store dispatch then calls $nextTick to flip `loaded`. + // Wait long enough for both to settle. + await new Promise(resolve => setTimeout(resolve, 0)) + await wrapper.vm.$nextTick() + await wrapper.vm.$nextTick() +} + +const groupsTableStub = { + name: 'GroupsTable', + // Declare approve as Boolean so Vue applies its boolean prop coercion + // (presence-without-value becomes true) — matches the real component. + props: { groupids: { type: Array }, approve: { type: Boolean, default: false } }, + template: '
' +} + +test('renders GroupsTable (not literal TODO) when there are groups awaiting moderation', async () => { + const store = makeStore({ + 1: { idgroups: 1, name: 'G1', networks: [] }, + 2: { idgroups: 2, name: 'G2', networks: [] }, + }) + const wrapper = mount(GroupsRequiringModeration, { + localVue, + store, + mixins: [LangMixin], + stubs: { GroupsTable: groupsTableStub }, + }) + + await flush(wrapper) + + // Must not display the literal "TODO" placeholder + expect(wrapper.text()).not.toContain('TODO') + + // Must render a GroupsTable with the moderation group ids and approve flag + const stub = wrapper.findComponent(groupsTableStub) + expect(stub.exists()).toBe(true) + expect(stub.props('groupids')).toEqual([1, 2]) + expect(stub.props('approve')).toBe(true) +}) + +test('renders nothing when there are no groups to moderate', async () => { + const store = makeStore({}) + const wrapper = mount(GroupsRequiringModeration, { + localVue, + store, + mixins: [LangMixin], + stubs: { GroupsTable: groupsTableStub }, + }) + + await flush(wrapper) + + expect(wrapper.findComponent(groupsTableStub).exists()).toBe(false) + expect(wrapper.text()).not.toContain('TODO') +}) + +test('filters groups by the networks prop when supplied', async () => { + const store = makeStore({ + 1: { idgroups: 1, name: 'In', networks: [{ id: 10 }] }, + 2: { idgroups: 2, name: 'Out', networks: [{ id: 99 }] }, + 3: { idgroups: 3, name: 'Also in', networks: [{ id: 10 }, { id: 20 }] }, + }) + const wrapper = mount(GroupsRequiringModeration, { + localVue, + store, + mixins: [LangMixin], + propsData: { networks: [10] }, + stubs: { GroupsTable: groupsTableStub }, + }) + + await flush(wrapper) + + const stub = wrapper.findComponent(groupsTableStub) + expect(stub.exists()).toBe(true) + expect(stub.props('groupids').sort()).toEqual([1, 3]) +}) diff --git a/resources/js/components/GroupsRequiringModeration.vue b/resources/js/components/GroupsRequiringModeration.vue index 5bd055ced9..935dc3303d 100644 --- a/resources/js/components/GroupsRequiringModeration.vue +++ b/resources/js/components/GroupsRequiringModeration.vue @@ -1,8 +1,7 @@ @@ -46,6 +45,11 @@ export default { return ret }, + groupIds() { + // GroupsTable's `groupids` prop is the ids it should render — the + // moderate store uses { idgroups, id } (via newToOld), so accept either. + return this.groups.map(g => g.idgroups || g.id) + }, }, async mounted() { try { From d9c127b0433d9bbce2be622967257dc8b9194856 Mon Sep 17 00:00:00 2001 From: edwh Date: Wed, 27 May 2026 13:56:06 +0100 Subject: [PATCH 3/9] Clear the remaining PR #744 TODOs: GroupsPage props, fetch dedup, stale comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- jest.config.json | 7 +- resources/js/components/GroupsPage.test.js | 82 +++++++++++++++++ resources/js/components/GroupsPage.vue | 21 +---- resources/js/store/groups.js | 43 ++++++--- resources/js/store/groups.test.js | 98 +++++++++++++++++++++ resources/views/group/index.blade.php | 3 - tests/Integration/grouptags.test.js | 18 ++-- tests/__mocks__/leaflet-control-geocoder.js | 10 +++ tests/__mocks__/vue-awesome.js | 4 + 9 files changed, 243 insertions(+), 43 deletions(-) create mode 100644 resources/js/components/GroupsPage.test.js create mode 100644 resources/js/store/groups.test.js create mode 100644 tests/__mocks__/leaflet-control-geocoder.js create mode 100644 tests/__mocks__/vue-awesome.js 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/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 cdb7990513..3249e493be 100644 --- a/resources/js/components/GroupsPage.vue +++ b/resources/js/components/GroupsPage.vue @@ -31,7 +31,10 @@ class="mt-3" :tab="currentTab" @nearest="currentTab = 1" - your-area="yourArea" + :your-area="yourArea" + :networks="networks" + :all-group-tags="allGroupTags" + :show-tags="showTags" />
@@ -86,21 +89,6 @@ export default { required: false, default: null }, - yourLat: { - type: String, - required: false, - default: null - }, - yourLng: { - type: String, - required: false, - default: null - }, - userId: { - type: Number, - required: false, - default: null - }, canCreate: { type: Boolean, required: false, @@ -115,7 +103,6 @@ export default { type: Array, required: true }, - // TODO Check whether all these parameters are now used or can be removed allGroupTags: { type: Array, required: true diff --git a/resources/js/store/groups.js b/resources/js/store/groups.js index 13cb37ec85..ea4b628b62 100644 --- a/resources/js/store/groups.js +++ b/resources/js/store/groups.js @@ -31,6 +31,10 @@ function getLocale() { return el.innerText.trim() } +// Module-scoped map of in-flight `groups/fetch` promises, keyed by +// `${id}|${includeStats}`. Used by the action to de-dup concurrent callers. +const inFlight = new Map() + export default { namespaced: true, state: { @@ -224,21 +228,38 @@ export default { return id }, async fetch({ rootGetters, commit }, params) { - // TODO Handle fetching case. - try { - let url = '/api/v2/groups/' + params.id + '?api_token=' + rootGetters['auth/apiToken'] + '&locale=' + getLocale() + // De-dup concurrent fetches for the same (id, includeStats) so callers + // like GroupsPage's mounted loop don't fire N parallel requests when + // the user has many groups. The cache key includes includeStats because + // the two responses have different shapes. + const key = params.id + '|' + (params.hasOwnProperty('includeStats') ? String(params.includeStats) : '') + if (inFlight.has(key)) { + return inFlight.get(key) + } - if (params.hasOwnProperty('includeStats')) { - url += '&includeStats=' + params.includeStats - } + const request = (async () => { + try { + let url = '/api/v2/groups/' + params.id + '?api_token=' + rootGetters['auth/apiToken'] + '&locale=' + getLocale() + + if (params.hasOwnProperty('includeStats')) { + url += '&includeStats=' + params.includeStats + } - let ret = await axios.get(url) + let ret = await axios.get(url) - commit('set', ret.data.data) + commit('set', ret.data.data) - return ret.data.data - } catch (e) { - console.error("Group fetch failed", e) + return ret.data.data + } catch (e) { + console.error("Group fetch failed", e) + } + })() + + inFlight.set(key, request) + try { + return await request + } finally { + inFlight.delete(key) } } }, diff --git a/resources/js/store/groups.test.js b/resources/js/store/groups.test.js new file mode 100644 index 0000000000..b36ef199c3 --- /dev/null +++ b/resources/js/store/groups.test.js @@ -0,0 +1,98 @@ +// Tests in-flight de-duplication of the groups/fetch action. +// +// The action is called eagerly by GroupsPage (one dispatch per yourGroups id) +// and again whenever a user navigates to a group page. Without de-dup, the +// same id can be in flight twice concurrently. + +jest.mock('axios', () => ({ + __esModule: true, + default: { + get: jest.fn(), + post: jest.fn(), + patch: jest.fn(), + delete: jest.fn(), + }, +})) +import axios from 'axios' + +import groups from './groups' + +// store/groups.js reads locale via document.getElementById('language-current').innerText. +// jsdom doesn't populate innerText reliably, so set the property explicitly. +beforeEach(() => { + document.body.innerHTML = '
' + const el = document.getElementById('language-current') + Object.defineProperty(el, 'innerText', { value: 'en', configurable: true }) + axios.get.mockReset() +}) + +function commit() {} +const rootGetters = { 'auth/apiToken': 'TEST' } + +function deferred() { + let resolve, reject + const promise = new Promise((res, rej) => { resolve = res; reject = rej }) + return { promise, resolve, reject } +} + +test('two concurrent fetches for the same group share a single in-flight request', async () => { + const d = deferred() + axios.get.mockReturnValueOnce(d.promise) + + const a = groups.actions.fetch({ rootGetters, commit }, { id: 42 }) + const b = groups.actions.fetch({ rootGetters, commit }, { id: 42 }) + + expect(axios.get).toHaveBeenCalledTimes(1) + + d.resolve({ data: { data: { id: 42, name: 'G' } } }) + await Promise.all([a, b]) + + expect(axios.get).toHaveBeenCalledTimes(1) +}) + +test('a new fetch after the previous one settled hits the network again', async () => { + axios.get.mockResolvedValueOnce({ data: { data: { id: 7, name: 'G7' } } }) + await groups.actions.fetch({ rootGetters, commit }, { id: 7 }) + expect(axios.get).toHaveBeenCalledTimes(1) + + axios.get.mockResolvedValueOnce({ data: { data: { id: 7, name: 'G7 again' } } }) + await groups.actions.fetch({ rootGetters, commit }, { id: 7 }) + expect(axios.get).toHaveBeenCalledTimes(2) +}) + +test('concurrent fetches for different groups each get their own request', async () => { + const d1 = deferred() + const d2 = deferred() + axios.get.mockReturnValueOnce(d1.promise).mockReturnValueOnce(d2.promise) + + const a = groups.actions.fetch({ rootGetters, commit }, { id: 1 }) + const b = groups.actions.fetch({ rootGetters, commit }, { id: 2 }) + + expect(axios.get).toHaveBeenCalledTimes(2) + + d1.resolve({ data: { data: { id: 1 } } }) + d2.resolve({ data: { data: { id: 2 } } }) + await Promise.all([a, b]) +}) + +test('a fetch that throws clears its in-flight slot so retries can run', async () => { + axios.get.mockRejectedValueOnce(new Error('boom')) + await groups.actions.fetch({ rootGetters, commit }, { id: 9 }) + + // The previous fetch is no longer in flight, so a new one re-hits the network. + axios.get.mockResolvedValueOnce({ data: { data: { id: 9 } } }) + await groups.actions.fetch({ rootGetters, commit }, { id: 9 }) + + expect(axios.get).toHaveBeenCalledTimes(2) +}) + +test('fetches with the same id but different includeStats are independent requests', async () => { + axios.get.mockResolvedValue({ data: { data: { id: 3 } } }) + + await groups.actions.fetch({ rootGetters, commit }, { id: 3, includeStats: false }) + await groups.actions.fetch({ rootGetters, commit }, { id: 3, includeStats: true }) + + expect(axios.get).toHaveBeenCalledTimes(2) + expect(axios.get.mock.calls[0][0]).toContain('includeStats=false') + expect(axios.get.mock.calls[1][0]).toContain('includeStats=true') +}) diff --git a/resources/views/group/index.blade.php b/resources/views/group/index.blade.php index 726c69539a..8a576f6715 100644 --- a/resources/views/group/index.blade.php +++ b/resources/views/group/index.blade.php @@ -62,10 +62,7 @@ :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 }}" - your-lng="{{ $your_lng }}" :can-create="{{ $can_create ? 'true' : 'false' }}" - :user-id="{{ $myid }}" tab="{{ $tab }}" :network="{{ $network ? $network : 'null' }}" :networks="{{ json_encode($networks, JSON_INVALID_UTF8_IGNORE) }}" diff --git a/tests/Integration/grouptags.test.js b/tests/Integration/grouptags.test.js index 0eeb5f59e6..ca112d3a50 100644 --- a/tests/Integration/grouptags.test.js +++ b/tests/Integration/grouptags.test.js @@ -37,16 +37,14 @@ async function getGroupId(page, baseURL) { return group.id } -// Helper that creates a tag against the network's API and reloads the network -// page so the new tag is in `initialTags` from the blade template. -// -// We could (and originally did) drive the live Vue form, but Vue 2's render of -// NetworkPage doesn't reliably re-render the .tag-item list after the FIRST -// mutation from an empty `tags` array — the data is updated (verified via -// $parent walk) but the v-if/v-show DOM doesn't reflect it. Subsequent -// mutations from a non-empty starting state render correctly. We sidestep -// that quirk by hitting the API and reloading. (See TODO below to chase the -// underlying reactivity bug separately.) +// Helpers that drive the network tag CRUD via the API directly. Tests assert +// the resulting state after a page reload so they don't depend on Vue's +// reactive list update at all — that path is already exercised end-to-end by +// the NetworkPage component itself (with the flushRender() helper after each +// mutation) and unit-tested separately. Keeping these tests at the API+reload +// level makes them robust against the Vue 2 scheduler quirks we hit +// originally (unhandled async lifecycle rejections leaking the `pending` +// flag, fixed in this PR). async function getApiTokenFromPage(page) { const token = await page.evaluate(() => { let host = document.querySelector('.create-tag .tag-name-input') || diff --git a/tests/__mocks__/leaflet-control-geocoder.js b/tests/__mocks__/leaflet-control-geocoder.js new file mode 100644 index 0000000000..cc8882da7c --- /dev/null +++ b/tests/__mocks__/leaflet-control-geocoder.js @@ -0,0 +1,10 @@ +// Stub for the leaflet-control-geocoder CommonJS subpath imports used by +// GroupMap.vue. The real package is a runtime-only dependency; we never +// exercise its behaviour from Jest, so any plain class is fine. +class Stub { + constructor() {} + on() { return this } + addTo() { return this } + setQuery() {} +} +module.exports = { Geocoder: Stub, Photon: Stub, default: Stub } diff --git a/tests/__mocks__/vue-awesome.js b/tests/__mocks__/vue-awesome.js new file mode 100644 index 0000000000..8926fb88fb --- /dev/null +++ b/tests/__mocks__/vue-awesome.js @@ -0,0 +1,4 @@ +// Stub for vue-awesome's Icon component (ES module export trips up babel-jest's +// CJS transformer when imported transitively from a Vue component under test). +module.exports = { name: 'fa-icon', template: '' } +module.exports.default = module.exports From a6e73945dfe2478c51ae7a858567381195870d65 Mon Sep 17 00:00:00 2001 From: edwh Date: Sat, 30 May 2026 22:11:57 +0100 Subject: [PATCH 4/9] Drop :user-id assertion from BasicTest 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. --- tests/Feature/Groups/BasicTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Feature/Groups/BasicTest.php b/tests/Feature/Groups/BasicTest.php index b859e83937..5d59d79c4e 100644 --- a/tests/Feature/Groups/BasicTest.php +++ b/tests/Feature/Groups/BasicTest.php @@ -39,7 +39,6 @@ public function testPageLoads($url, $tab): void // Can't assert on all-group-tags dev systems might have varying info. 'your-area' => 'London', ':can-create' => 'true', - ':user-id' => $user->id, '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}]', From 81d8eb285b87b6bcc1524cc5e5c27fc69cb0bac5 Mon Sep 17 00:00:00 2001 From: edwh Date: Sun, 31 May 2026 08:10:27 +0100 Subject: [PATCH 5/9] Fix broken group image in map modal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- resources/js/components/GroupInfoModal.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/js/components/GroupInfoModal.vue b/resources/js/components/GroupInfoModal.vue index 24f1e158d2..833b86c5ac 100644 --- a/resources/js/components/GroupInfoModal.vue +++ b/resources/js/components/GroupInfoModal.vue @@ -58,7 +58,7 @@ export default { return this.group && this.group.next_event ? new moment(this.group.next_event.start).format('ddd Do MMM YYYY') : null }, groupImage() { - return this.group && this.group.image ? ('/uploads/mid_' + this.group.group_image) : DEFAULT_PROFILE + return this.group && this.group.image ? ('/uploads/mid_' + this.group.image) : DEFAULT_PROFILE }, }, data: function() { From 19e84be3bd46702b01cd15879e757231491cddb4 Mon Sep 17 00:00:00 2001 From: edwh Date: Tue, 2 Jun 2026 17:42:52 +0100 Subject: [PATCH 6/9] Fix groups table host/restarter headers showing text not icons 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 --- resources/js/components/GroupsTable.test.js | 70 +++++++++++++++++++++ resources/js/components/GroupsTable.vue | 4 +- 2 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 resources/js/components/GroupsTable.test.js diff --git a/resources/js/components/GroupsTable.test.js b/resources/js/components/GroupsTable.test.js new file mode 100644 index 0000000000..f9e46d5434 --- /dev/null +++ b/resources/js/components/GroupsTable.test.js @@ -0,0 +1,70 @@ +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 GroupsTable from './GroupsTable.vue' + +const localVue = createLocalVue() +localVue.use(Vuex) +localVue.mixin(LangMixin) + +function makeStore(groups) { + const list = {} + groups.forEach(g => { list[g.id] = g }) + return new Vuex.Store({ + modules: { + groups: { + namespaced: true, + getters: { + list: () => Object.values(list), + get: () => id => list[id], + }, + actions: { + fetch: () => Promise.resolve(), + }, + }, + }, + }) +} + +const group = { + id: 1, + name: 'Test Group', + hosts: 5, + restarters: 12, + location: { location: 'Townsville', country: 'United Kingdom', distance: null }, + image: null, +} + +function mountTable() { + return mount(GroupsTable, { + localVue, + store: makeStore([group]), + propsData: { groupids: [1] }, + stubs: { + GroupsTableFilters: true, + ConfirmModal: true, + GroupArchivedBadge: true, + InfiniteLoading: true, + }, + }) +} + +describe('GroupsTable column headers', () => { + // The hosts/restarters columns should show icons in the header, like the + // location and next-event columns do. Regression: the header-slot names did + // not match the field keys, so b-table fell back to the text labels + // "Hosts" / "Restarters". + test('renders the hosts column header as the user icon, not text', () => { + const thead = mountTable().find('thead').html() + expect(thead).toContain('user_ico') + }) + + test('renders the restarters column header as the volunteer icon, not text', () => { + const thead = mountTable().find('thead').html() + expect(thead).toContain('volunteer_ico-thick') + }) +}) diff --git a/resources/js/components/GroupsTable.vue b/resources/js/components/GroupsTable.vue index 105a29176e..1c6f388dbd 100644 --- a/resources/js/components/GroupsTable.vue +++ b/resources/js/components/GroupsTable.vue @@ -71,10 +71,10 @@ {{ data.item.location.country }}
-