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/app/Http/Controllers/API/GroupController.php b/app/Http/Controllers/API/GroupController.php index 94a79844e4..d2e6941bfd 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), * ) * ) * ) @@ -269,20 +275,40 @@ 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: 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) { $ret[] = [ 'id' => $group->idgroups, 'name' => $group->name, + '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')->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 ]; } @@ -292,6 +318,99 @@ 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. Default false.", + * required=false, + * in="query", + * @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\Parameter( + * 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="string" + * ) + * ), + * @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'], + '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 + // each group lazy-loads its relations and the call scales O(N). + $query = Group::with(['networks', 'groupImage.image', 'group_tags']); + + if ($request->get('archived', 'false') !== 'true') { + $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 [ + '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 deb7f86eb8..a28eaabcba 100644 --- a/app/Http/Controllers/GroupController.php +++ b/app/Http/Controllers/GroupController.php @@ -32,11 +32,9 @@ use FixometerFile; use Illuminate\Database\QueryException; use Illuminate\Http\Request; -use Illuminate\Support\Arr; use Illuminate\Support\Facades\Log; use Notification; use Spatie\ValidationRules\Rules\Delimited; -use Carbon\Carbon; class GroupController extends Controller { @@ -60,11 +58,10 @@ private function indexVariations($tab, $network) } else { $all_group_tags = collect([]); } - $networks = Network::all(); // 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,15 +72,54 @@ 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); + + 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) { + $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, ]); } @@ -105,7 +141,9 @@ public function nearby() public function network($id) { - return $this->indexVariations('all', $id); + // Retired: network coordinators now see their groups on the network + // page itself (map + list). Kept as a redirect for old links. + return redirect('/networks/' . $id); } public function create(Request $request) @@ -481,68 +519,6 @@ public function delete($id): RedirectResponse } } - public static function expandGroups($groups, $your_groupids, $nearby_groupids) - { - $ret = []; - $user = Auth::user(); - - if ($groups) { - foreach ($groups as $group) { - $group_image = $group->groupImage; - - $event = $group->nextUpcomingParty; - - // We want to return the distance from our own location. - $distance = null; - $grouplat = $group->latitude; - $grouplng = $group->longitude; - $userlat = $user->latitude; - $userlng = $user->longitude; - - if ($grouplat !== null && $grouplng !== null && $userlat !== null && $userlng !== null) { - if ($grouplat == $userlat && $grouplng == $userlng) { - $distance = 0; - } else { - $distance = 6371 * acos( cos(deg2rad($userlat)) * cos(deg2rad($grouplat)) * cos(deg2rad($grouplng) - - deg2rad($userlng)) + sin(deg2rad($userlat) ) * sin(deg2rad($grouplat))); - } - } - - $ret[] = [ - 'idgroups' => $group->idgroups, - 'name' => $group->name, - 'image' => (is_object($group_image) && is_object($group_image->image)) ? - asset('uploads/mid_'.$group_image->image->path) : null, - 'location' => [ - 'location' => rtrim($group->location), - 'country' => Fixometer::getCountryFromCountryCode($group->country_code), - 'country_code' => $group->country_code, - 'distance' => $distance, - ], - 'next_event' => $event ? $event->event_date_local : null, - 'all_restarters_count' => $group->all_restarters_count, - 'all_hosts_count' => $group->all_hosts_count, - 'all_confirmed_restarters_count' => $group->all_confirmed_restarters_count, - 'all_confirmed_hosts_count' => $group->all_confirmed_hosts_count, - 'networks' => \Illuminate\Support\Arr::pluck($group->networks, 'id'), - 'group_tags' => $group->group_tags->pluck('id'), - 'group_tags_full' => $group->group_tags->map(function($tag) { - return [ - 'id' => $tag->id, - 'name' => $tag->tag_name, - 'network_id' => $tag->network_id, - ]; - }), - '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 - ]; - } - } - - return $ret; - } - public static function stats($id, $format = 'row') { $group = Group::where('idgroups', $id)->first(); diff --git a/app/Http/Resources/GroupSummary.php b/app/Http/Resources/GroupSummary.php index b4af5cf306..cacd66e331 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( @@ -50,6 +51,17 @@ * ) * ), * @OA\Property( + * property="group_tags_full", + * title="group_tags_full", + * description="Tags on this group. Only present on calls which load them, e.g. the summary list.", + * type="array", + * @OA\Items( + * @OA\Property(property="id", type="integer"), + * @OA\Property(property="name", type="string"), + * @OA\Property(property="network_id", type="integer", nullable=true), + * ) + * ), + * @OA\Property( * property="updated_at", * title="updated_at", * description="The last change to this group. This includes changes which affect the stats.", @@ -63,6 +75,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.", @@ -91,32 +115,74 @@ public function toArray(Request $request): array '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), + // 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. + 'group_tags_full' => $this->whenLoaded('group_tags', function () { + return $this->resource->group_tags->map(function ($tag) { + return [ + 'id' => $tag->id, + 'name' => $tag->tag_name, + 'network_id' => $tag->network_id, + ]; + }); + }), 'updated_at' => Carbon::parse($this->updated_at)->toIso8601String(), 'archived_at' => $this->archived_at ? Carbon::parse($this->archived_at)->toIso8601String() : null, '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. + // + // Only approved events count, matching Group::getNextUpcomingEvent() which the group's own page uses. + // 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 { + $future = \App\Party::future()->where('approved', true)->get(); + + // 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 + ]; + } - 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 - ]; + Cache::put('future_approved_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/app/Party.php b/app/Party.php index ac9ab31e9a..512a0cf7d2 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 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/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/docker/preview-startup.sh b/docker/preview-startup.sh index 0524e1ed95..927617406d 100755 --- a/docker/preview-startup.sh +++ b/docker/preview-startup.sh @@ -53,6 +53,10 @@ cat > /tmp/warming/index.html <<'HTML' + + Preview warming up diff --git a/resources/js/components/GroupMap.test.js b/resources/js/components/GroupMap.test.js new file mode 100644 index 0000000000..bb2fb6696a --- /dev/null +++ b/resources/js/components/GroupMap.test.js @@ -0,0 +1,537 @@ +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(), + flyTo: jest.fn(), + getSize: () => size, + getBounds: () => L.latLngBounds([[50, -1], [52, 1]]), + getZoom: () => 5, + getCenter: () => ({ lat: 0, lng: 0 }), + } +} + +function mountMap(initialBounds, groups, props = {}) { + return shallowMount(GroupMap, { + localVue, + store: makeStore(groups), + propsData: { initialBounds, ...props }, + }) +} + +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) + }) + + // A group with no geocode has null lat/lng; +null is 0, so it used to be + // framed as if it sat at null island (0,0), dragging the view out to sea. + test('ignores groups without coordinates when framing', () => { + const wrapper = mountMap(WORLD, [...groups, { id: 4, location: { lat: null, lng: null } }]) + const map = fakeMap({ x: 688, y: 400 }) + 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([0, 0])).toBe(false) + }) +}) + +describe('GroupMap options', () => { + // Regression: dragging was `!!window?.L?.Browser?.mobile`, i.e. enabled + // ONLY on mobile — desktop users could not pan the map at all. + test('allows dragging the map on desktop', () => { + expect(mountMap(WORLD, []).vm.mapOptions.dragging).toBe(true) + }) + + // User feedback: the wheel scrolled the page instead of zooming the map, + // and people couldn't work out how to zoom in/out. + test('zooms with the mouse wheel', () => { + expect(mountMap(WORLD, []).vm.mapOptions.scrollWheelZoom).toBe(true) + }) + + test('does not set gestureHandling (the plugin is not installed)', () => { + expect('gestureHandling' in mountMap(WORLD, []).vm.mapOptions).toBe(false) + }) +}) + +// 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' }) + 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() + }) +}) + +// 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() + }) +}) + +// 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 group-cluster--medium') + // 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]) + }) +}) + +// User feedback on the PR 887 preview (Stratford): zoom 14 was too shallow to +// read street names, clustering stayed active right up to max zoom so a +// cluster of co-located groups could never be broken apart, and the 60px +// cluster radius left too many small bubbles on screen at once. +describe('GroupMap street-level zoom and identical locations', () => { + // Twelve groups at the exact same venue - above the minCluster threshold, + // so the clustering path (not the draw-them-all shortcut) is exercised. + const colocated = Array.from({ length: 12 }, (_, i) => ({ + id: i + 1, + location: { lat: 51.5417, lng: -0.0035 }, + })) + + test('allows zooming to street level (max zoom 18) by default', () => { + expect(mountMap(WORLD, []).vm.maxZoom).toBe(18) + }) + + test('uses a wide cluster radius (120px) so fewer, larger bubbles show', () => { + const wrapper = mountMap(WORLD, []) + expect(wrapper.vm.clusterIndex.options.radius).toBe(120) + }) + + test('nudges each subsequent duplicate at a location by 0.00015 degrees', () => { + const groups = [ + { id: 1, location: { lat: 51.5, lng: -0.1 } }, + { id: 2, location: { lat: 51.5, lng: -0.1 } }, + ] + const wrapper = mountMap(WORLD, groups) + + const [first, second] = wrapper.vm.clusterPoints + expect(first.geometry.coordinates).toEqual([-0.1, 51.5]) + expect(second.geometry.coordinates[0]).toBeCloseTo(-0.09985, 10) + expect(second.geometry.coordinates[1]).toBeCloseTo(51.50015, 10) + + // Never mutate the store's own objects - the offset would corrupt the + // group's real coordinates and accumulate on every recompute (a bug + // Freegle hit with this exact approach, per its ClusterMarker comment). + expect(groups[1].location).toEqual({ lat: 51.5, lng: -0.1 }) + }) + + test('shows individual pins, not a cluster, at max zoom', () => { + const wrapper = mountMap(WORLD, colocated) + const map = fakeMap() + map.getZoom = () => wrapper.vm.maxZoom + map.getBounds = () => L.latLngBounds([[51.5, -0.1], [51.6, 0.1]]) + wrapper.vm.mapObject = map + wrapper.vm.$refs.map = { mapObject: map } + + const clusters = wrapper.vm.clusters + expect(clusters).toHaveLength(12) + expect(clusters.every((c) => !c.properties.cluster)).toBe(true) + }) + + // User feedback: cluster bubbles should visually indicate how many groups + // they hold - bigger and more saturated for larger clusters. + test('scales the cluster bubble and its colour tier with the group count', () => { + const wrapper = mountMap(WORLD, []) + const iconFor = (count) => wrapper.vm.clusterIcon({ properties: { cluster: true, point_count: count } }).options + + const small = iconFor(3) + expect(small.iconSize).toEqual([36, 36]) + expect(small.className).toBe('group-cluster group-cluster--small') + + const medium = iconFor(20) + expect(medium.iconSize).toEqual([46, 46]) + expect(medium.className).toBe('group-cluster group-cluster--medium') + + const large = iconFor(150) + expect(large.iconSize).toEqual([56, 56]) + expect(large.className).toBe('group-cluster group-cluster--large') + expect(large.iconAnchor).toEqual([28, 28]) + }) + + test('clicking a cluster of co-located groups flies to max zoom, where it splits', () => { + const wrapper = mountMap(WORLD, colocated) + const map = fakeMap() + map.getZoom = () => 17 + map.getBounds = () => L.latLngBounds([[51.5, -0.1], [51.6, 0.1]]) + wrapper.vm.mapObject = map + wrapper.vm.$refs.map = { mapObject: map } + + const cluster = wrapper.vm.clusters.find((c) => c.properties.cluster) + expect(cluster).toBeTruthy() + wrapper.vm.clusterClick(cluster) + + const [, zoom] = map.flyTo.mock.calls[0] + expect(zoom).toBe(wrapper.vm.maxZoom) + }) +}) + +describe('GroupMap place search', () => { + // Photon returns a London in England, another in Ontario, and more in + // Kentucky, Ohio, Arkansas and California. Without the state and the country + // every one of them renders as an identical bare "London". + test('labels results with enough to tell same-named places apart', () => { + const props = mountMap(WORLD, []).vm.placeNameProperties + + expect(props).toContain('state') + expect(props).toContain('country') + // Still leads with the specific part of the name. + expect(props[0]).toBe('name') + }) + + // flyToBounds arcs out and back: going from Aberdeen to London it pulls out + // two zoom levels below the destination over about three seconds. Once a + // place has been picked from the dropdown, the journey isn't worth watching. + test('cuts straight to a searched place instead of flying out and back', () => { + const wrapper = mountMap(WORLD, []) + const map = fakeMap() + wrapper.vm.mapObject = map + wrapper.vm.$refs.map = { mapObject: map } + + const bbox = L.latLngBounds([[51.28, -0.51], [51.69, 0.33]]) + wrapper.vm.goToPlace(bbox) + + expect(map.fitBounds).toHaveBeenCalledWith(bbox) + expect(map.flyToBounds).not.toHaveBeenCalled() + }) + + // The distance column in the list below anchors to the searched place, so + // the parent needs to know where the search landed. + test('tells the parent where a search landed', () => { + const wrapper = mountMap(WORLD, []) + const map = fakeMap() + wrapper.vm.mapObject = map + wrapper.vm.$refs.map = { mapObject: map } + + const bbox = L.latLngBounds([[51.28, -0.51], [51.69, 0.33]]) + wrapper.vm.goToPlace(bbox) + + const [[point]] = wrapper.emitted().searched + expect(point.lat).toBeCloseTo(51.485, 2) + expect(point.lng).toBeCloseTo(-0.09, 2) + }) + + test('counts a search as having moved the map, so it stops reframing itself', () => { + const wrapper = mountMap(WORLD, []) + const map = fakeMap() + wrapper.vm.mapObject = map + wrapper.vm.$refs.map = { mapObject: map } + + wrapper.vm.goToPlace(L.latLngBounds([[51.28, -0.51], [51.69, 0.33]])) + + expect(wrapper.vm.moved).toBe(true) + }) +}) + +describe('GroupMap markers', () => { + test('only renders markers for groups with coordinates', () => { + const wrapper = mountMap(WORLD, [ + { id: 1, location: { lat: 51.5, lng: -0.1 } }, + { id: 2, location: { lat: null, lng: null } }, + { id: 3, lat: 53.4, lng: -2.2 }, + { id: 4 }, + ]) + + expect(wrapper.vm.mappableGroups.map(g => g.id)).toEqual([1, 3]) + }) +}) diff --git a/resources/js/components/GroupMap.vue b/resources/js/components/GroupMap.vue new file mode 100644 index 0000000000..f820afc388 --- /dev/null +++ b/resources/js/components/GroupMap.vue @@ -0,0 +1,661 @@ + + + diff --git a/resources/js/components/GroupMapAndList.test.js b/resources/js/components/GroupMapAndList.test.js new file mode 100644 index 0000000000..3b89623232 --- /dev/null +++ b/resources/js/components/GroupMapAndList.test.js @@ -0,0 +1,310 @@ +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 GroupMapAndList from './GroupMapAndList.vue' +import { indexGroup, hydratedGroup } from '../testFixtures/groups' + +const localVue = createLocalVue() +localVue.use(Vuex) +localVue.mixin(LangMixin) + +// The shape the map is actually drawn from - see testFixtures/groups.js. +const GROUPS = [ + indexGroup({ id: 1, name: 'Alpha', networks: [5] }), + indexGroup({ id: 2, name: 'Beta', networks: [6] }), +] + +// The summary shape, which the same components see once rows are hydrated. +const HYDRATED_GROUPS = [ + hydratedGroup({ id: 1, name: 'Alpha', networks: [{ id: 5 }] }), + hydratedGroup({ id: 2, name: 'Beta', networks: [{ id: 6 }] }), +] + +function makeStore(groups = GROUPS) { + return new Vuex.Store({ + modules: { + groups: { + namespaced: true, + getters: { + list: () => groups, + }, + actions: { + list: () => Promise.resolve(), + }, + }, + }, + }) +} + +const groupMapStub = { + name: 'GroupMap', + props: { + initialBounds: { type: Array }, + 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 }, + groupids: { type: Array, default: null }, + frameRequest: { type: Number, default: 0 }, + }, + template: '
', +} + +const groupsTableStub = { + name: 'GroupsTable', + props: { + groupids: { type: Array }, + search: { type: Boolean, default: false }, + allGroupTags: { type: Array, default: null }, + showTags: { type: Boolean, default: false }, + centre: { type: Object, default: null }, + referencePoint: { type: Object, default: null }, + }, + template: '
', +} + +async function makeWrapper(props = {}, groups = GROUPS) { + const wrapper = mount(GroupMapAndList, { + localVue, + store: makeStore(groups), + propsData: { + initialBounds: [[90, 180], [-90, -180]], + ...props, + }, + stubs: { + GroupMap: groupMapStub, + GroupsTable: groupsTableStub, + 'v-icon': true, + }, + }) + + // mounted() awaits the groups/list dispatch before clearing `loading`. + await wrapper.vm.$nextTick() + await wrapper.vm.$nextTick() + await wrapper.vm.$nextTick() + + return wrapper +} + +test('forwards network and filter context to the map and the table', async () => { + const tags = [{ id: 1, tag_name: 'Foo' }] + const wrapper = await makeWrapper({ + network: 5, + showFilters: true, + availableTags: tags, + canManageTags: true, + }) + + expect(wrapper.findComponent(groupMapStub).props('network')).toBe(5) + + const table = wrapper.findComponent(groupsTableStub) + expect(table.props('search')).toBe(true) + expect(table.props('allGroupTags')).toEqual(tags) + 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') +}) + +// 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) +}) + +// The list is ordered by distance from the middle of the map, so it has to know +// where that is, and follow it as the map moves. +test('passes the map centre through to the table', async () => { + const wrapper = await makeWrapper() + + wrapper.findComponent({ name: 'GroupMap' }).vm.$emit('update:centre', { lat: 54.19, lng: -3.09 }) + await wrapper.vm.$nextTick() + + 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('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') + + type(wrapper, 'B') + type(wrapper, 'Be') + type(wrapper, 'Bet') + await wrapper.vm.$nextTick() + + expect(wrapper.findComponent(groupMapStub).props('frameRequest')).toBe(before) + }) + + // 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') + + type(wrapper, 'B') + type(wrapper, 'Be') + type(wrapper, 'Beta') + jest.runAllTimers() + await wrapper.vm.$nextTick() + + 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([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() + }) +}) + +// 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 }, 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 }, HYDRATED_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 }) + expect(wrapper.vm.effectiveGroupIds).toEqual([1]) + }) + + // Regression: an empty array was treated the same as "no report yet", so + // panning to an area with no groups listed EVERY group under the map. + test('shows an empty list when the map reports no groups in view', async () => { + const wrapper = await makeWrapper() + wrapper.vm.groupsChanged([]) + await wrapper.vm.$nextTick() + expect(wrapper.vm.effectiveGroupIds).toEqual([]) + }) + + test('shows exactly the groups the map reports in view', async () => { + const wrapper = await makeWrapper() + wrapper.vm.groupsChanged([2]) + await wrapper.vm.$nextTick() + expect(wrapper.vm.effectiveGroupIds).toEqual([2]) + }) +}) + +// 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) + await wrapper.vm.$nextTick() + expect(wrapper.vm.hover).toBe(42) + + wrapper.findComponent({ name: 'GroupMap' }).vm.$emit('update:hover', null) + await wrapper.vm.$nextTick() + expect(wrapper.vm.hover).toBe(null) +}) + +// User feedback: the list's distance column anchors to your own location - +// or, once you search for a place, to that place. +describe('GroupMapAndList distance reference point', () => { + test('anchors to your own location by default', async () => { + const wrapper = await makeWrapper({ yourLat: 51.5, yourLng: -0.1 }) + expect(wrapper.findComponent(groupsTableStub).props('referencePoint')).toEqual({ lat: 51.5, lng: -0.1 }) + }) + + test('has no reference point when the user has no location', async () => { + const wrapper = await makeWrapper() + expect(wrapper.findComponent(groupsTableStub).props('referencePoint')).toBeNull() + }) + + test('re-anchors to the searched place after a place search', async () => { + const wrapper = await makeWrapper({ yourLat: 51.5, yourLng: -0.1 }) + + wrapper.findComponent(groupMapStub).vm.$emit('searched', { lat: 53.48, lng: -2.24 }) + await wrapper.vm.$nextTick() + + expect(wrapper.findComponent(groupsTableStub).props('referencePoint')).toEqual({ lat: 53.48, lng: -2.24 }) + }) +}) diff --git a/resources/js/components/GroupMapAndList.vue b/resources/js/components/GroupMapAndList.vue new file mode 100644 index 0000000000..782dbd48b2 --- /dev/null +++ b/resources/js/components/GroupMapAndList.vue @@ -0,0 +1,225 @@ + + + diff --git a/resources/js/components/GroupMarker.test.js b/resources/js/components/GroupMarker.test.js new file mode 100644 index 0000000000..ef1a018626 --- /dev/null +++ b/resources/js/components/GroupMarker.test.js @@ -0,0 +1,116 @@ +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 GroupMarker from './GroupMarker.vue' + +// GroupMarker uses the global `L` for L.icon. +global.L = L + +const localVue = createLocalVue() +localVue.use(Vuex) +localVue.mixin(LangMixin) + +function makeStore() { + return new Vuex.Store({ + modules: { + groups: { + namespaced: true, + getters: { + get: () => () => ({ id: 1, name: 'Test Group', location: { lat: 51.5, lng: -0.1 } }), + }, + }, + }, + }) +} + +function mountMarker(props = {}) { + return shallowMount(GroupMarker, { + localVue, + store: makeStore(), + propsData: { id: 1, ...props }, + }) +} + +// User feedback: the pin should share the cluster bubble's restart style - +// black border, white inner - rather than the stock blue Leaflet teardrop. +// An inline-SVG divIcon rather than an image, so the states can recolour the +// fill directly instead of hue-rotating a blue PNG. +describe('GroupMarker icon', () => { + test('draws the restart-style pin: a black-bordered white teardrop SVG', () => { + const options = mountMarker().vm.icon.options + expect(options.html).toContain(' { + const options = mountMarker().vm.icon.options + expect(options.iconSize).toEqual([30, 42]) + expect(options.iconAnchor).toEqual([15, 42]) + // Measured from the anchor (the tip), so the tooltip clears the pin head. + expect(options.tooltipAnchor).toEqual([0, -42]) + }) + + test('recolours via a CSS class: green for groups you follow, red on hover', () => { + expect(mountMarker().vm.icon.options.className).toBe('group-pin') + expect(mountMarker({ highlight: true }).vm.icon.options.className).toBe('group-pin group-pin--yours') + // Hover wins over highlight, matching the previous priority. + expect(mountMarker({ hover: true, highlight: true }).vm.icon.options.className).toBe('group-pin group-pin--hover') + }) +}) + +// Groups at the exact same venue are separated by a tiny display-only nudge +// (GroupMap's clusterPoints). The marker resolves its position from the +// store, so the nudged position must be passable from outside - otherwise +// both pins would still draw on the same spot. +describe('GroupMarker position override', () => { + test('draws at the store position by default', () => { + const vm = mountMarker().vm + expect([vm.lat, vm.lng]).toEqual([51.5, -0.1]) + }) + + test('draws at the passed lat-lng when one is given', () => { + const vm = mountMarker({ latLng: [51.50015, -0.09985] }).vm + expect([vm.lat, vm.lng]).toEqual([51.50015, -0.09985]) + }) +}) + +// 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() + + const tooltip = wrapper.find('l-tooltip') + expect(tooltip.exists()).toBe(true) + expect(tooltip.text()).toContain('Test Group') + }) +}) + +// 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() + + wrapper.vm.markerHover(true) + expect(wrapper.emitted('update:hover').pop()).toEqual([1]) + + wrapper.vm.markerHover(false) + expect(wrapper.emitted('update:hover').pop()).toEqual([null]) + }) + + test('marker hover also turns its own pin red', () => { + const wrapper = mountMarker() + wrapper.vm.markerHover(true) + expect(wrapper.vm.icon.options.className).toBe('group-pin group-pin--hover') + }) +}) diff --git a/resources/js/components/GroupMarker.vue b/resources/js/components/GroupMarker.vue new file mode 100644 index 0000000000..e9cd0e1427 --- /dev/null +++ b/resources/js/components/GroupMarker.vue @@ -0,0 +1,132 @@ + + + diff --git a/resources/js/components/GroupsPage.test.js b/resources/js/components/GroupsPage.test.js new file mode 100644 index 0000000000..bc630e170b --- /dev/null +++ b/resources/js/components/GroupsPage.test.js @@ -0,0 +1,142 @@ +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 }, + allGroupTags: { type: Array, default: null }, + showTags: { type: Boolean, default: false }, + }, + template: '
', +} + +const groupMapStub = { + name: 'GroupMapAndList', + props: { + 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 }, + availableTags: { type: Array, default: () => [] }, + }, + template: '
', +} + +function makeWrapper(props = {}) { + return mount(GroupsPage, { + localVue, + store: makeStore(), + mixins: [LangMixin], + propsData: { + yourGroups: [1, 2], + nearbyGroups: [], + 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 / allGroupTags to the inner GroupsTable so tag badges render on the "your groups" tab', async () => { + const allGroupTags = [{ id: 1, tag_name: 'Foo' }, { id: 2, tag_name: 'Bar' }] + const wrapper = makeWrapper({ showTags: true, allGroupTags }) + await flushTabs(wrapper) + + const table = wrapper.findComponent(groupsTableStub) + expect(table.exists()).toBe(true) + expect(table.props('showTags')).toBe(true) + 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') +}) + +const WORLD_BOUNDS = [[90, 180], [-90, -180]] + +test('forwards network + filter context to the map list on a network view', async () => { + const wrapper = makeWrapper({ + tab: 'other', + network: 99, + nearbyGroups: WORLD_BOUNDS, + showTags: true, + }) + await flushTabs(wrapper) + + const map = wrapper.findComponent(groupMapStub) + expect(map.exists()).toBe(true) + // Regression: /group/network/{id} showed ALL groups because the network + // prop stopped at GroupsPage. + expect(map.props('network')).toBe(99) + expect(map.props('showFilters')).toBe(true) + expect(map.props('canManageTags')).toBe(true) + expect(map.props('availableTags')).toEqual([{ id: 1, tag_name: 'Foo' }]) +}) + +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('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) +}) + +// Regression: both tabs were plain `lazy`, which destroys content when the tab +// is hidden — every switch back to Other Groups threw the map away and +// re-fetched everything. +test('keeps the map mounted when switching back to Your Groups', async () => { + const wrapper = makeWrapper({ tab: 'other', nearbyGroups: WORLD_BOUNDS }) + await flushTabs(wrapper) + expect(wrapper.find('.stub-map').exists()).toBe(true) + + wrapper.vm.currentTab = 0 + await flushTabs(wrapper) + expect(wrapper.find('.stub-map').exists()).toBe(true) +}) diff --git a/resources/js/components/GroupsPage.vue b/resources/js/components/GroupsPage.vue index 4dd8ebd953..1f42d3473d 100644 --- a/resources/js/components/GroupsPage.vue +++ b/resources/js/components/GroupsPage.vue @@ -27,32 +27,36 @@
- + +
-

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

-
@@ -60,34 +64,17 @@
- - - -
\ No newline at end of file diff --git a/resources/js/components/GroupsTableFilters.test.js b/resources/js/components/GroupsTableFilters.test.js new file mode 100644 index 0000000000..f6d776a5da --- /dev/null +++ b/resources/js/components/GroupsTableFilters.test.js @@ -0,0 +1,68 @@ +import Vue from "vue" +import { BootstrapVue } from 'bootstrap-vue' +Vue.use(BootstrapVue) + +import { shallowMount, createLocalVue } from '@vue/test-utils' +import LangMixin from 'resources/js/mixins/lang.js' +import GroupsTableFilters from './GroupsTableFilters.vue' + +const localVue = createLocalVue() +localVue.mixin(LangMixin) + +const multiselectStub = { + name: 'multiselect', + props: { placeholder: { type: String, default: '' } }, + template: '
', +} + +function mountFilters(props = {}) { + return shallowMount(GroupsTableFilters, { + localVue, + propsData: { groups: [], ...props }, + stubs: { multiselect: multiselectStub }, + }) +} + +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('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 30c794b894..a4ab7f543e 100644 --- a/resources/js/components/GroupsTableFilters.vue +++ b/resources/js/components/GroupsTableFilters.vue @@ -23,63 +23,11 @@ :selectedLabel="__('partials.remove')" open-direction="bottom" /> - - -
\ No newline at end of file + diff --git a/resources/js/components/NetworkPage.test.js b/resources/js/components/NetworkPage.test.js new file mode 100644 index 0000000000..64e7a29d4b --- /dev/null +++ b/resources/js/components/NetworkPage.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 NetworkPage from './NetworkPage.vue' + +const localVue = createLocalVue() +localVue.use(Vuex) +localVue.mixin(LangMixin) + +function makeStore() { + return new Vuex.Store({ + modules: { + groups: { + namespaced: true, + getters: { getModerate: () => ({}) }, + }, + events: { + namespaced: true, + getters: { getModerate: () => ({}) }, + }, + }, + }) +} + +const groupMapStub = { + name: 'GroupMapAndList', + props: { + initialBounds: { type: Array }, + network: { type: Number, default: null }, + showFilters: { type: Boolean, default: false }, + canManageTags: { type: Boolean, default: false }, + availableTags: { type: Array, default: () => [] }, + networks: { type: Array, default: null }, + }, + template: '
', +} + +function makeWrapper(props = {}) { + return mount(NetworkPage, { + localVue, + store: makeStore(), + propsData: { + network: { id: 5, name: 'Test Network', coordinators: [] }, + // Non-empty stats and tags so mounted() doesn't hit the network. + initialStats: { groups: 2 }, + initialTags: [{ id: 1, name: 'Solder', description: null, groups_count: 2 }], + canManageTags: true, + isLoggedIn: true, + ...props, + }, + stubs: { + GroupsRequiringModeration: true, + EventsRequiringModeration: true, + GroupMapAndList: groupMapStub, + }, + }) +} + +// The network page must show the map + list of the network's groups +// (previously it only linked out to /group/network/{id}). +test('embeds the group map and list, scoped to this network, with filters', () => { + const wrapper = makeWrapper() + + const map = wrapper.findComponent(groupMapStub) + expect(map.exists()).toBe(true) + expect(map.props('network')).toBe(5) + expect(map.props('showFilters')).toBe(true) + expect(map.props('canManageTags')).toBe(true) + // Start zoomed out to show every group in the network: an inverted world + // box makes GroupMap frame all (network-filtered) groups. + expect(map.props('initialBounds')).toEqual([[90, 180], [-90, -180]]) +}) + +test('passes network tags to the list in the shape the tag filter expects (tag_name)', () => { + const wrapper = makeWrapper() + + const tags = wrapper.findComponent(groupMapStub).props('availableTags') + expect(tags).toHaveLength(1) + expect(tags[0].id).toBe(1) + expect(tags[0].tag_name).toBe('Solder') + expect(tags[0].name).toBe('Solder') +}) + +test('does not render the removed groups_count/view_groups_link lang keys', () => { + const wrapper = makeWrapper() + + // These keys were removed from the lang files; rendering them would show + // the literal key strings to users. + expect(wrapper.text()).not.toContain('networks.show.groups_count') + expect(wrapper.text()).not.toContain('networks.show.view_groups_link') +}) + +test('does not offer the tag filter to users who cannot see tags', () => { + const wrapper = makeWrapper({ canManageTags: false, initialTags: [] }) + + const map = wrapper.findComponent(groupMapStub) + expect(map.props('canManageTags')).toBe(false) + expect(map.props('availableTags')).toEqual([]) +}) diff --git a/resources/js/components/NetworkPage.vue b/resources/js/components/NetworkPage.vue index 0e0caf36f8..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') }} @@ -76,13 +75,17 @@
{{ __('networks.show.none') }}
- +

{{ __('networks.general.groups') }}

-
- {{ __('networks.show.groups_count', { count: stats.groups || 0, name: network.name }) }} - {{ __('networks.show.view_groups_link') }} -
+
@@ -171,10 +174,11 @@ import axios from 'axios' import GroupsRequiringModeration from './GroupsRequiringModeration.vue' import EventsRequiringModeration from './EventsRequiringModeration.vue' +import GroupMapAndList from './GroupMapAndList.vue' import images from '../mixins/images' export default { - components: { GroupsRequiringModeration, EventsRequiringModeration }, + components: { GroupsRequiringModeration, EventsRequiringModeration, GroupMapAndList }, mixins: [images], props: { network: { @@ -230,6 +234,16 @@ export default { } }, computed: { + worldBounds() { + // The inverted whole-world box: GroupMap treats it as "no location", so + // it frames all the (network-filtered) groups instead. + return [[90, 180], [-90, -180]] + }, + groupFilterTags() { + // The network tags API uses `name`; the group tag filter multiselect + // labels by `tag_name`. Provide both. + return this.tags.map(t => ({ ...t, tag_name: t.name })) + }, truncatedDescription() { if (!this.network.description) return '' const stripped = this.network.description.replace(/<[^>]*>/g, '') @@ -430,12 +444,6 @@ export default { } } -.groups-section { - .groups-info { - background: $white; - } -} - .tags-management { .tag-item { background: $brand-grey; diff --git a/resources/js/constants.js b/resources/js/constants.js index 6e4519d314..ad0a847f7e 100644 --- a/resources/js/constants.js +++ b/resources/js/constants.js @@ -60,4 +60,9 @@ export const UNKNOWN_STRINGS = [ 'no brand', 'no model', 'none' -] \ No newline at end of file +] + +export const MIN_MAP_ZOOM = 1 +// 14 was too shallow to read street names or tell close-together pins apart +// (user feedback on the groups map); CARTO's raster tiles serve up to z18. +export const MAX_MAP_ZOOM = 18 \ No newline at end of file diff --git a/resources/js/misc/groupFilter.js b/resources/js/misc/groupFilter.js new file mode 100644 index 0000000000..60d75a9361 --- /dev/null +++ b/resources/js/misc/groupFilter.js @@ -0,0 +1,37 @@ +// A group's networks arrive in two shapes: the names index that draws the map +// sends plain ids, while the summary API sends objects. Understanding only one +// of them leaves the network pages with an empty map and an empty list. +export function inNetwork(group, networkId) { + if (!networkId) { + return true + } + + return (group.networks || []).some(n => (n && n.id !== undefined ? n.id : n) === networkId) +} + +// 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..f38f94d3a3 --- /dev/null +++ b/resources/js/misc/groupFilter.test.js @@ -0,0 +1,65 @@ +import { matchesFilters, inNetwork } from './groupFilter' + +// A group's networks arrive in two shapes: the names index that draws the map +// sends plain ids, while the summary API sends objects. Matching only one of +// them empties the network pages. +describe('inNetwork', () => { + test('matches plain ids, as sent by the names index', () => { + expect(inNetwork({ networks: [5, 6] }, 5)).toBe(true) + expect(inNetwork({ networks: [6] }, 5)).toBe(false) + }) + + test('matches objects, as sent by the summary API', () => { + expect(inNetwork({ networks: [{ id: 5 }, { id: 6 }] }, 5)).toBe(true) + expect(inNetwork({ networks: [{ id: 6 }] }, 5)).toBe(false) + }) + + test('a group in no networks matches nothing', () => { + expect(inNetwork({}, 5)).toBe(false) + expect(inNetwork({ networks: [] }, 5)).toBe(false) + }) + + test('everything is in scope when no network is asked for', () => { + expect(inNetwork({ networks: [] }, null)).toBe(true) + }) +}) + +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) +}) diff --git a/resources/js/misc/placeSearchGeocoder.js b/resources/js/misc/placeSearchGeocoder.js new file mode 100644 index 0000000000..16cd1eed49 --- /dev/null +++ b/resources/js/misc/placeSearchGeocoder.js @@ -0,0 +1,55 @@ +// The map's place-search geocoder. Photon never ranks administrative +// boundaries into a plain query's results - searching "Haringey" (a London +// borough) returned only streets and bus stops while "Muswell Hill" (a +// suburb, which Photon treats as a place) worked (user feedback, +// 2026-08-20). So the search runs twice: once filtered to Photon's place +// layers (district/city/county/state - boroughs are `layer=district`), once +// unfiltered, and the dropdown lists the places first. + +// Places first, then everything else, deduped by display name (the +// place-layer copy wins - it carries the boundary's full extent, so +// selecting it frames the whole area). Capped at 10 like Photon's own +// default result list. +export function mergePlaceSearchResults(places, general) { + const seen = {} + + return places.concat(general) + .filter((result) => { + if (seen[result.name]) { + return false + } + seen[result.name] = true + return true + }) + .slice(0, 10) +} + +// Wraps two leaflet-control-geocoder v1 geocoders (callback API) into one. +// A failure on either side degrades to the other's results rather than +// failing the search. +export function buildPlaceSearchGeocoder({ places, general }) { + const collect = (geocoder, query, context) => + new Promise((resolve) => { + try { + geocoder.geocode(query, (results) => resolve(results || []), context) + } catch (e) { + resolve([]) + } + }) + + return { + geocode(query, cb, context) { + Promise.all([collect(places, query, context), collect(general, query, context)]).then(([p, g]) => { + cb.call(context, mergePlaceSearchResults(p, g)) + }) + }, + suggest(query, cb, context) { + return this.geocode(query, cb, context) + }, + } +} + +// Photon's layers that represent named places rather than addresses or +// venues. `district` covers both suburbs and London-borough-style +// administrative areas. +export const PLACE_LAYERS = ['district', 'city', 'county', 'state'] diff --git a/resources/js/misc/placeSearchGeocoder.test.js b/resources/js/misc/placeSearchGeocoder.test.js new file mode 100644 index 0000000000..c4859f6924 --- /dev/null +++ b/resources/js/misc/placeSearchGeocoder.test.js @@ -0,0 +1,65 @@ +import { mergePlaceSearchResults, buildPlaceSearchGeocoder } from './placeSearchGeocoder' + +// User feedback: searching "Haringey" (a borough) found nothing while +// "Muswell Hill" (a suburb) worked. Photon never ranks administrative +// boundaries into a plain query's results, so the search runs twice - once +// filtered to place layers, once unfiltered - and shows the places first. +describe('placeSearchGeocoder', () => { + const r = (name) => ({ name, center: { lat: 0, lng: 0 } }) + + describe('mergePlaceSearchResults', () => { + test('puts place-layer results ahead of general ones', () => { + const merged = mergePlaceSearchResults([r('London Borough of Haringey')], [r('Haringey Park')]) + expect(merged.map((x) => x.name)).toEqual(['London Borough of Haringey', 'Haringey Park']) + }) + + test('drops duplicates by name, keeping the place-layer copy', () => { + const place = r('Muswell Hill') + const merged = mergePlaceSearchResults([place], [r('Muswell Hill'), r('Muswell Hill Library')]) + expect(merged.map((x) => x.name)).toEqual(['Muswell Hill', 'Muswell Hill Library']) + expect(merged[0]).toBe(place) + }) + + test('caps the merged list at 10', () => { + const many = Array.from({ length: 8 }, (_, i) => r(`P${i}`)) + const more = Array.from({ length: 8 }, (_, i) => r(`G${i}`)) + expect(mergePlaceSearchResults(many, more)).toHaveLength(10) + }) + }) + + describe('buildPlaceSearchGeocoder (v1 callback API)', () => { + const cbGeocoder = (results) => ({ + geocode: jest.fn((query, cb, context) => cb.call(context, results)), + }) + + test('queries both geocoders and calls back with places first', (done) => { + const places = cbGeocoder([r('The Borough')]) + const general = cbGeocoder([r('A Street')]) + + buildPlaceSearchGeocoder({ places, general }).geocode('borough', function (results) { + expect(results.map((x) => x.name)).toEqual(['The Borough', 'A Street']) + done() + }) + }) + + test('still returns the other list when one side throws', (done) => { + const places = { geocode: jest.fn(() => { throw new Error('boom') }) } + const general = cbGeocoder([r('A Street')]) + + buildPlaceSearchGeocoder({ places, general }).geocode('q', function (results) { + expect(results.map((x) => x.name)).toEqual(['A Street']) + done() + }) + }) + + test('suggest() is the same search', (done) => { + const places = cbGeocoder([]) + const general = cbGeocoder([r('X')]) + + buildPlaceSearchGeocoder({ places, general }).suggest('q', function (results) { + expect(results.map((x) => x.name)).toEqual(['X']) + done() + }) + }) + }) +}) diff --git a/resources/js/store/groups.js b/resources/js/store/groups.js index e467948f0c..b27ed7328b 100644 --- a/resources/js/store/groups.js +++ b/resources/js/store/groups.js @@ -10,6 +10,8 @@ function newToOld(e) { // new API completely, we can then migrate the Vue components to use the new field names and retire this function. // Similar code in event and device store. let ret = { + // Keep both id styles: components (e.g. GroupsTable) match rows on `id`. + id: e.id, idgroups: e.id, name: e.name, location: e.location, @@ -31,12 +33,20 @@ 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: { // 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: {}, @@ -62,8 +72,17 @@ 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.idgroups, params) + Vue.set(state.list, params.id || params.idgroups, params) }, setList(state, params) { let list = {} @@ -123,23 +142,84 @@ export default { commit('set', group) } }, - async getModerationRequired({commit, rootGetters}, params) { + async getModerationRequired({commit, rootGetters, state}, params) { const apiToken = rootGetters['auth/apiToken'] let ret = await axios.get('/api/v2/moderate/groups?api_token=' + apiToken + '&locale=' + getLocale()) if (ret && ret.data) { commit('setModerate', ret.data) + + // GroupsTable renders its rows from the list store, so moderation + // groups must be there too. Don't clobber a richer entry (e.g. from + // the summary fetch). + ret.data.forEach(e => { + if (!state.list[e.id]) { + commit('set', newToOld(e)) + } + }) } }, - async list({commit}) { - let ret = await axios.get('/api/v2/groups/names?locale=' + getLocale()) - if (ret && ret.data) { + async list({commit}, params) { + // 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() @@ -213,14 +293,38 @@ export default { return id }, async fetch({ rootGetters, commit }, params) { - try { - let ret = await axios.get('/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) + } - commit('set', ret.data.data) + const request = (async () => { + try { + let url = '/api/v2/groups/' + params.id + '?api_token=' + rootGetters['auth/apiToken'] + '&locale=' + getLocale() - return ret.data.data - } catch (e) { - console.error("Group fetch failed", e) + if (params.hasOwnProperty('includeStats')) { + url += '&includeStats=' + params.includeStats + } + + let ret = await axios.get(url) + + commit('set', ret.data.data) + + 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..95c60a99cc --- /dev/null +++ b/resources/js/store/groups.test.js @@ -0,0 +1,222 @@ +// 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' +import { INDEX_API_ENTRY, INDEX_STORE_ENTRY } from '../testFixtures/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') +}) + +test('the details list fetch asks for archived groups (shown with a badge, as the old page did)', async () => { + axios.get.mockResolvedValueOnce({ data: { data: [] } }) + + await groups.actions.list({ commit }, { details: true }) + + expect(axios.get.mock.calls[0][0]).toContain('includeArchived=true') +}) + +// GroupsTable renders its rows from the groups/list store; groups that only +// exist in the moderate store never showed up, so the "groups requiring +// moderation" section rendered an empty table. +describe('getModerationRequired', () => { + const Vue = require('vue') + const Vuex = require('vuex') + Vue.use(Vuex) + + function makeRealStore() { + return new Vuex.Store({ + modules: { + groups: { + ...groups, + state: { list: {}, moderate: {}, tags: {}, stats: {} }, + }, + auth: { + namespaced: true, + getters: { apiToken: () => 'TEST' }, + }, + }, + }) + } + + test('moderation groups become renderable by GroupsTable (in the list store, with an id)', async () => { + axios.get.mockResolvedValueOnce({ + data: [{ id: 7, name: 'Mod Group', location: 'Somewhere', country: 'UK', networks: [] }], + }) + const store = makeRealStore() + + await store.dispatch('groups/getModerationRequired') + + expect(store.getters['groups/getModerate'][7]).toBeTruthy() + const listed = store.getters['groups/list'].find(g => g.id === 7) + expect(listed).toBeTruthy() + expect(listed.idgroups).toBe(7) + }) + + test('does not clobber a richer entry already in the list store', async () => { + const store = makeRealStore() + store.commit('groups/set', { id: 7, name: 'Rich', location: { lat: 1, lng: 2 } }) + axios.get.mockResolvedValueOnce({ + data: [{ id: 7, name: 'Mod Group', networks: [] }], + }) + + await store.dispatch('groups/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: [INDEX_API_ENTRY] } + }) + 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] + + // The whole entry, with nothing overridden: this is what ties the fixture + // that component tests build on to what the store really produces. If this + // fails, the mapping changed - update the fixture, not the assertion. + expect(g).toEqual(INDEX_STORE_ENTRY) +}) diff --git a/resources/js/testFixtures/groups.js b/resources/js/testFixtures/groups.js new file mode 100644 index 0000000000..8057067792 --- /dev/null +++ b/resources/js/testFixtures/groups.js @@ -0,0 +1,70 @@ +// One definition of what a group looks like once it is in the store, so that +// component tests exercise the shape the store actually produces rather than +// one someone imagined while writing the test. +// +// Inventing a group inline in a component test is what let the network pages +// ship empty: those fixtures said networks were [{id}] objects, while the map +// is drawn from the names index, which sends plain ids. +// +// INDEX_API_ENTRY and INDEX_STORE_ENTRY are a matched pair - the response the +// names index sends, and exactly what the store turns it into. +// store/groups.test.js feeds the first in and asserts the whole of the second +// comes out, with nothing overridden, so a change to the mapping fails there +// and this file has to be updated with it. + +export const INDEX_API_ENTRY = { + id: 7, + name: 'G', + lat: 51, + lng: 0, + country: 'United Kingdom', + network_ids: [3], + tag_ids: [9], + archived_at: null, +} + +export const INDEX_STORE_ENTRY = { + id: 7, + name: 'G', + // The index puts coordinates at the top level and inside location. + lat: 51, + lng: 0, + location: { + location: null, + country: 'United Kingdom', + lat: 51, + lng: 0, + }, + // Plain ids, not objects. + networks: [3], + // Tag ids wrapped as objects; the names arrive with hydration. + group_tags_full: [{ id: 9 }], + archived_at: null, +} + +// A group as the names index leaves it: enough to draw the map and run the +// client-side filters, with no image, location text, counts or next event. +export function indexGroup(overrides = {}) { + return { ...INDEX_STORE_ENTRY, ...overrides } +} + +// A group after hydrate() has filled in the rest from the summary API. Note +// that the summary API sends networks as objects, unlike the index - both +// shapes are live at once, because the table hydrates only its visible rows. +export function hydratedGroup(overrides = {}) { + return { + ...INDEX_STORE_ENTRY, + image: null, + location: { + location: 'Townsville', + country: 'United Kingdom', + lat: 51, + lng: 0, + distance: null, + }, + networks: [{ id: 3 }], + next_event: null, + summary: true, + ...overrides, + } +} diff --git a/resources/views/group/index.blade.php b/resources/views/group/index.blade.php index c5f7b94e9f..2fe1adb739 100644 --- a/resources/views/group/index.blade.php +++ b/resources/views/group/index.blade.php @@ -21,8 +21,6 @@ @endif diff --git a/routes/api.php b/routes/api.php index 5564ad0dcc..5c6c7f97f5 100644 --- a/routes/api.php +++ b/routes/api.php @@ -72,6 +72,7 @@ Route::prefix('v2')->group(function() { Route::middleware(\App\Http\Middleware\APISetLocale::class)->group(function() { Route::prefix('/groups')->group(function() { + Route::get('/summary', [API\GroupController::class, 'listSummaryv2']); Route::get('/names', [API\GroupController::class, 'listNamesv2']); Route::get('/tags', [API\GroupController::class, 'listTagsv2']); Route::get('{id}/events', [API\GroupController::class, 'getEventsForGroupv2']); diff --git a/routes/web.php b/routes/web.php index 78480bd7de..5f81d1bae1 100644 --- a/routes/web.php +++ b/routes/web.php @@ -360,6 +360,7 @@ Route::get('/all', [GroupController::class, 'all']); Route::get('/mine', [GroupController::class, 'mine']); Route::get('/nearby', [GroupController::class, 'nearby']); + Route::get('/other', [GroupController::class, 'nearby']); Route::get('/network/{id}', [GroupController::class, 'network']); Route::post('/delete/{id}', [GroupController::class, 'delete']); }); diff --git a/tests/Feature/Groups/BasicTest.php b/tests/Feature/Groups/BasicTest.php index fc6d969caa..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; @@ -39,26 +38,21 @@ 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}]', ':show-tags' => 'false', ], ]); - - $groups = json_decode($props[1][':all-groups'], true); - $this->assertEquals($group->idgroups, $groups[0]['idgroups']); - $this->assertEquals(0, $groups[0]['location']['distance']); } - public function tabProvider(): array { + public static function tabProvider(): array { return [ ['', 'mine'], - ['/all', 'all'], + ['/all', 'other'], ['/mine', 'mine'], - ['/nearby','nearby'], + ['/nearby','other'], + ['/other', 'other'], ]; } } diff --git a/tests/Feature/Groups/GroupSummaryApiTest.php b/tests/Feature/Groups/GroupSummaryApiTest.php new file mode 100644 index 0000000000..3cc8b06b26 --- /dev/null +++ b/tests/Feature/Groups/GroupSummaryApiTest.php @@ -0,0 +1,146 @@ +create(['name' => 'Active Summary Group']); + $archived = Group::factory()->create([ + 'name' => 'Archived Summary Group', + 'archived_at' => Carbon::now(), + ]); + + // The endpoint validates an `archived` parameter; it must also honour it. + $response = $this->get('/api/v2/groups/summary'); + $response->assertSuccessful(); + $ids = collect($response->json('data'))->pluck('id'); + $this->assertTrue($ids->contains($active->idgroups)); + $this->assertFalse($ids->contains($archived->idgroups)); + + $response = $this->get('/api/v2/groups/summary?archived=true'); + $response->assertSuccessful(); + $ids = collect($response->json('data'))->pluck('id'); + $this->assertTrue($ids->contains($active->idgroups)); + $this->assertTrue($ids->contains($archived->idgroups)); + } + + public function testIncludesGroupTagsForBadgesAndFiltering(): void + { + $group = Group::factory()->create(); + $tag = GroupTags::factory()->create(); + $group->addTag($tag); + + $response = $this->get('/api/v2/groups/summary'); + $response->assertSuccessful(); + + $summary = collect($response->json('data'))->firstWhere('id', $group->idgroups); + $this->assertNotNull($summary); + $this->assertArrayHasKey('group_tags_full', $summary); + $this->assertEquals($tag->id, $summary['group_tags_full'][0]['id']); + $this->assertEquals($tag->tag_name, $summary['group_tags_full'][0]['name']); + } + + public function testQueryCountDoesNotScaleWithGroupCount(): void + { + Group::factory()->count(3)->create(); + + // Warm up (first request may have extra overhead). + $this->get('/api/v2/groups/summary?includeNextEvent=true&includeCounts=true'); + + DB::enableQueryLog(); + $this->get('/api/v2/groups/summary?includeNextEvent=true&includeCounts=true')->assertSuccessful(); + $queriesForFew = count(DB::getQueryLog()); + DB::disableQueryLog(); + DB::flushQueryLog(); + + Group::factory()->count(6)->create(); + + DB::enableQueryLog(); + $this->get('/api/v2/groups/summary?includeNextEvent=true&includeCounts=true')->assertSuccessful(); + $queriesForMany = count(DB::getQueryLog()); + DB::disableQueryLog(); + DB::flushQueryLog(); + + // Tripling the groups must not grow the queries: relations should be + // eager-loaded, not fetched per group. + $this->assertLessThan( + $queriesForFew * 1.5, + $queriesForMany, + "Summary endpoint queries scale with group count: $queriesForFew -> $queriesForMany" + ); + } + + public function testIdsParamHydratesOnlyThoseGroups(): void + { + $network = Network::factory()->create(); + $tag = GroupTags::factory()->create(); + $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' => $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_approved_events'); + + $response = $this->get('/api/v2/groups/summary?ids=' . $a->idgroups . ',' . $b->idgroups + . '&includeNextEvent=true&includeCounts=true&archived=true'); + $response->assertSuccessful(); + $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']); + } + + public function testIdsParamRejectsMoreThanTwoHundred(): void + { + $this->expectException(\Illuminate\Validation\ValidationException::class); + $this->get('/api/v2/groups/summary?ids=' . implode(',', range(1, 201))); + } + + public function testNamesIndexCarriesFilterFields(): void + { + $network = Network::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/names?includeArchived=true'); + $response->assertSuccessful(); + $g = collect($response->json('data'))->firstWhere('id', $group->idgroups); + $this->assertNotNull($g); + + // 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']); + } +} diff --git a/tests/Feature/Groups/GroupViewTest.php b/tests/Feature/Groups/GroupViewTest.php index 6d6bbd2254..3df7417db8 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; @@ -182,26 +183,205 @@ public function testGroupIndexNextEventIsEagerLoaded(): void 'event_end_utc' => $nextWeek->addHours(2)->toIso8601String(), ]); - // Load the group index and check both groups appear with a next_event. - $response = $this->get('/group'); + // Groups are fetched via API (not server-rendered) — test that next_event is returned. + $response = $this->get('/api/v2/groups/summary?includeNextEvent=true'); $response->assertSuccessful(); - $props = $this->getVueProperties($response); - $allGroupsJson = null; - foreach ($props as $prop) { - if (isset($prop[':all-groups'])) { - $allGroupsJson = $prop[':all-groups']; - break; - } + $groups = $response->json('data'); + $group1 = collect($groups)->firstWhere('id', $id1); + $group2 = collect($groups)->firstWhere('id', $id2); + + $this->assertNotNull($group1, "Group Alpha (id=$id1) not found in API response"); + $this->assertNotNull($group2, "Group Beta (id=$id2) not found in API response"); + $this->assertNotNull($group1['next_event'], 'Group Alpha should have a next_event'); + $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); + + // 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_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($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 + // 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(), + ]); } - $this->assertNotNull($allGroupsJson, 'Could not find :all-groups prop. Props found: ' . json_encode(array_map('array_keys', $props))); - $groups = json_decode($allGroupsJson, true); - $group1 = collect($groups)->firstWhere('idgroups', $id1); - $group2 = collect($groups)->firstWhere('idgroups', $id2); + $starts = Party::future()->forGroup($id)->get()->pluck('event_start_utc')->map(function ($d) { + return Carbon::parse($d)->timestamp; + })->all(); - $this->assertNotNull($group1['next_event'], 'Group 1 should have a next_event'); - $this->assertNotNull($group2['next_event'], 'Group 2 should have a next_event'); + $sorted = $starts; + sort($sorted); + $this->assertEquals($sorted, $starts, 'Party::future() should return the soonest event first'); } public function testGroupIndexQueryCountScalesWithO1NotN(): void diff --git a/tests/Feature/Networks/NetworkTest.php b/tests/Feature/Networks/NetworkTest.php index 416d96117d..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,10 +281,21 @@ public function network_page(): void ]); $response->assertRedirect(); - // Group should now show on network page and in encoded list of networks for a group. + // /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->assertSee($group->name); - $response->assertSee('"networks":[' . $network->id . ']', false); + $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="{"id":' . $network->id, false); + + // And the group must be in the API data the page will fetch. + $response = $this->get('/api/v2/groups/summary'); + $summary = collect($response->json('data'))->firstWhere('id', $group->idgroups); + $this->assertNotNull($summary); + $this->assertEquals($network->id, collect($summary['networks'])->first()['id']); // All networks list visible to admin. $this->loginAsTestUser(Role::ADMINISTRATOR); diff --git a/tests/Unit/GroupFilterTranslationsTest.php b/tests/Unit/GroupFilterTranslationsTest.php new file mode 100644 index 0000000000..88cac8937a --- /dev/null +++ b/tests/Unit/GroupFilterTranslationsTest.php @@ -0,0 +1,48 @@ +assertFileExists($path, "Missing lang file for locale '$locale'"); + + // Read via the filesystem helper rather than require(): the lang + // file may already have been included elsewhere, in which case + // require_once would return true instead of the translations array. + $translations = (new Filesystem())->getRequire($path); + + foreach ($keys as $key) { + $this->assertArrayHasKey( + $key, + $translations, + "Missing translation key 'groups.$key' for locale '$locale'" + ); + $this->assertNotSame('', trim((string) $translations[$key]), "Empty 'groups.$key' for locale '$locale'"); + } + } + } +} 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