Saving a group changes only the fields sent

app/Http/Controllers/API/GroupController.php

@@ -1060,25 +1060,43 @@ public function updateGroupv2(Request $request, $idGroup): JsonResponse {
 
         $old_zone = $group->timezone;
 
-        $data = [
+        // Only update the fields the request sends.  A partial update (e.g. the archive action, or an API client
+        // setting one field) must leave the rest of the group alone rather than blanking it.
+        $fields = [
             'name' => $name,
             'website' => $website,
-            'location' => $location,
-            'latitude' => $latitude,
-            'longitude' => $longitude,
-            'country_code' => $country,
             'free_text' => $description,
             'timezone' => $timezone,
             'phone' => $phone,
             'network_data' => $network_data,
             'email' => $email,
         ];
+        $requestKeys = [
+            'free_text' => 'description',
+        ];
+
+        $data = [];
+        foreach ($fields as $column => $value) {
+            if ($request->has($requestKeys[$column] ?? $column)) {
+                $data[$column] = $value;
+            }
+        }
+
+        if ($request->has('location')) {
+            // The coordinates and country come from geocoding the location.
+            $data['location'] = $location;
+            $data['latitude'] = $latitude;
+            $data['longitude'] = $longitude;
+            $data['country_code'] = $country;
+        }
 
         if ($user->hasRole('Administrator') || ($user->hasRole('NetworkCoordinator') && $isCoordinatorForGroup)) {
             // Got permission to update these.
-            $data['area'] = $area;
-            $data['postcode'] = $postcode;
-            $data['archived_at'] = $archived_at;
+            foreach (['area' => $area, 'postcode' => $postcode, 'archived_at' => $archived_at] as $column => $value) {
+                if ($request->has($column)) {
+                    $data[$column] = $value;
+                }
+            }
         }
 
         if (isset($_FILES) && !empty($_FILES)) {
@@ -1158,7 +1176,7 @@ public function updateGroupv2(Request $request, $idGroup): JsonResponse {
             }
         }
 
-        if ($timezone != $old_zone) {
+        if (array_key_exists('timezone', $data) && $timezone != $old_zone) {
             // The timezone of the group has changed.  Update the zone of any future events.  This happens
             // sometimes when a group is created and events are created before the group is approved (and therefore
             // before the admin has a chance to set the zone on the group.

resources/js/components/GroupActions.test.js

@@ -0,0 +1,49 @@
+import Vue from 'vue'
+import Vuex from 'vuex'
+import { BootstrapVue } from 'bootstrap-vue'
+import { createLocalVue, shallowMount } from '@vue/test-utils'
+import LangMixin from 'resources/js/mixins/lang.js'
+import GroupActions from './GroupActions.vue'
+
+const localVue = createLocalVue()
+localVue.use(Vuex)
+Vue.use(BootstrapVue)
+
+function makeStore (edit, fetch) {
+  return new Vuex.Store({
+    modules: {
+      groups: {
+        namespaced: true,
+        getters: {
+          // The store's copy of a group, including object-valued fields.
+          get: () => (id) => ({
+            idgroups: id,
+            name: 'Test Group',
+            free_text: 'About us',
+            network_data: { foo: 'bar' },
+            networks: [{ id: 1 }],
+          }),
+        },
+        actions: { edit, fetch },
+      },
+    },
+  })
+}
+
+test('archiving sends only the group id and archive date', async () => {
+  const edit = jest.fn()
+  const wrapper = shallowMount(GroupActions, {
+    localVue,
+    store: makeStore(edit, jest.fn()),
+    mixins: [LangMixin],
+    propsData: { idgroups: 42, canPerformArchive: true },
+  })
+
+  await wrapper.vm.archiveConfirmed()
+
+  expect(edit).toHaveBeenCalledTimes(1)
+  const payload = edit.mock.calls[0][1]
+  expect(Object.keys(payload).sort()).toEqual(['archived_at', 'id'])
+  expect(payload.id).toBe(42)
+  expect(isNaN(Date.parse(payload.archived_at))).toBe(false)
+})

resources/js/components/GroupActions.vue

@@ -126,15 +126,12 @@ export default {
       form.submit()
     },
     async archiveConfirmed() {
-      const group = this.group
-      group.id = this.idgroups
-      group.archived_at = (new Date()).toISOString()
-      group.description = group.free_text
-
-      // Make sure we don't stomp on the networks.
-      delete group.networks
-
-      await this.$store.dispatch('groups/edit', group)
+      // Send only the archive date: the API updates just the fields it's given.  Sending the whole group back
+      // posted object fields such as network_data as the string "[object Object]".
+      await this.$store.dispatch('groups/edit', {
+        id: this.idgroups,
+        archived_at: (new Date()).toISOString()
+      })
 
       await this.$store.dispatch('groups/fetch', {
         id: this.idgroups

tests/Feature/Groups/GroupEditTest.php

@@ -92,6 +92,76 @@
         ]);
     }
 
+    /** @test */
+    public function partial_update_only_changes_the_fields_sent(): void {
+        $group = Group::factory()->create([
+            'name' => 'Partial Group',
+            'location' => 'Hackney, London',
+            'latitude' => 51.545,
+            'longitude' => -0.0553,
+            'country_code' => 'GB',
+            'free_text' => 'About us',
+            'website' => 'https://example.org',
+            'timezone' => 'Europe/London',
+            'email' => 'group@example.org',
+            'postcode' => 'E8 1AA',
+            'area' => 'London',
+            'network_data' => ['foo' => 'bar'],
+        ]);
+        $event = \App\Party::factory()->create([
+            'group' => $group->idgroups,
+            'event_start_utc' => Carbon::now()->addWeek()->toIso8601String(),
+            'event_end_utc' => Carbon::now()->addWeek()->addHours(2)->toIso8601String(),
+            'timezone' => 'Europe/London',
+        ]);
+
+        $host = User::factory()->host()->create();
+        $group->addVolunteer($host);
+        $group->makeMemberAHost($host);
+        $this->actingAs($host);
+
+        $this->patch('/api/v2/groups/' . $group->idgroups, ['phone' => '999'])->assertSuccessful();
+
+        $group->refresh();
+        $this->assertEquals('999', $group->phone);
+        $this->assertEquals('Partial Group', $group->name);
+        $this->assertEquals('Hackney, London', $group->location);
+        $this->assertEquals(51.545, $group->latitude);
+        $this->assertEquals(-0.0553, $group->longitude);
+        $this->assertEquals('GB', $group->country_code);
+        $this->assertEquals('About us', $group->free_text);
+        $this->assertEquals('https://example.org', $group->website);
+        $this->assertEquals('Europe/London', $group->timezone);
+        $this->assertEquals('group@example.org', $group->email);
+        $this->assertEquals('E8 1AA', $group->postcode);
+        $this->assertEquals('London', $group->area);
+        $this->assertEquals(['foo' => 'bar'], $group->network_data);
+
+        // Not sending the timezone isn't a timezone change: future events keep theirs.
+        $this->assertEquals('Europe/London', $event->fresh()->timezone);
+    }
+
+    /** @test */
+    public function archiving_with_only_the_date_keeps_the_rest_of_the_group(): void {
+        $group = Group::factory()->create([
+            'name' => 'Archive Me',
+            'network_data' => ['foo' => 'bar'],
+        ]);
+
+        $admin = User::factory()->administrator()->create();
+        $this->actingAs($admin);
+
+        // What the Archive group action sends.
+        $this->patch('/api/v2/groups/' . $group->idgroups, [
+            'archived_at' => Carbon::now()->toIso8601String(),
+        ])->assertSuccessful();
+
+        $group->refresh();
+        $this->assertNotNull($group->archived_at);
+        $this->assertEquals('Archive Me', $group->name);
+        $this->assertEquals(['foo' => 'bar'], $group->network_data);
+    }
+
     /** @test */
     public function image_upload(): void {
         Storage::fake('avatars');