diff --git a/services/frontend/tests/e2e/committee-save.spec.ts b/services/frontend/tests/e2e/committee-save.spec.ts new file mode 100644 index 000000000..531a181f0 --- /dev/null +++ b/services/frontend/tests/e2e/committee-save.spec.ts @@ -0,0 +1,53 @@ +import {expect, test} from "./test" +import {installApiMocks, loginAsBoard} from "./mocks" + +/** + * Saving a committee whose member list has not arrived yet. + * + * The manager fetches its users after the form is already open. The picker is given a member's + * id and cannot name them until that list lands, and a save in the meantime has to reach the + * api all the same — the id is on the model, and the reader changed a name, not a member. + * + * A system test found this as `updates committee name and description` failing with a submit + * that produced no request at all: the form judged itself invalid because the picker had + * reported the member it could not yet name as no member. + */ +const COMMITTEE = { + id: 900, + name: "Events Committee", + description: "Runs the events, and has done for years.", + version: 3, + members: [{userId: 1, role: "Chair"}], +} + +test.describe("the committee manager", () => { + test("saves a name change while the member list is still on its way", async ({page}) => { + await installApiMocks(page, {committees: [COMMITTEE]}) + await loginAsBoard(page.context()) + + // Later routes win, so this one holds the user list back without touching the rest. + await page.route("**/users?**", async route => { + if (route.request().method() !== "GET") return route.fallback() + await new Promise(resolve => setTimeout(resolve, 3000)) + return route.fallback() + }) + + const saved = page.waitForRequest( + request => request.method() === "PUT" && /\/committees\/900$/.test(new URL(request.url()).pathname), + ) + + await page.goto("/committees/manage") + await page.getByTestId("committee-edit-btn-900").click() + await page.getByLabel("Committee name").fill("Events Committee Renamed") + await page.getByLabel("Description").fill("A description long enough to satisfy the rule.") + + await page.getByTestId("committee-form-submit-btn").click() + + const request = await saved + expect(JSON.parse(request.postData() ?? "{}")).toMatchObject({ + name: "Events Committee Renamed", + description: "A description long enough to satisfy the rule.", + members: [{userId: 1, role: "Chair"}], + }) + }) +}) diff --git a/services/frontend/tests/e2e/mocks.ts b/services/frontend/tests/e2e/mocks.ts index fb360ed31..adab2e7da 100644 --- a/services/frontend/tests/e2e/mocks.ts +++ b/services/frontend/tests/e2e/mocks.ts @@ -496,7 +496,7 @@ export async function installApiMocks(page: Page, fixtures: Fixtures = {}) { ] const baseCommittees = fixtures.committees ?? [ - {id: 900, name: "Events Committee"}, + {id: 900, name: "Events Committee", description: "Runs the events.", version: 0, members: []}, ] const baseBlogs = fixtures.blogs ?? [ @@ -896,6 +896,12 @@ export async function installApiMocks(page: Page, fixtures: Fixtures = {}) { if (method === "GET" && path === "/committees") { return fulfillJson(route, baseCommittees) } + if (method === "PUT" && /^\/committees\/\d+$/.test(path)) { + const id = Number(path.split("/").at(-1)) + const body = JSON.parse(request.postData() ?? "{}") as Record + const stored = baseCommittees.find((candidate) => Number(candidate.id) === id) ?? {id} + return fulfillJson(route, {...stored, ...body, id, version: Number(body.version ?? 0) + 1}) + } if (method === "GET" && path === "/committeeMembers/committees") { return fulfillJson(route, baseCommittees) } diff --git a/services/frontend/tests/unit/components/form/fields/UserSelect.test.ts b/services/frontend/tests/unit/components/form/fields/UserSelect.test.ts index cea975b0f..be5f10adc 100644 --- a/services/frontend/tests/unit/components/form/fields/UserSelect.test.ts +++ b/services/frontend/tests/unit/components/form/fields/UserSelect.test.ts @@ -98,3 +98,43 @@ describe("UserSelect", () => { expect(mockFindUsers).not.toHaveBeenCalled() }) }) + +describe("a member the list cannot account for yet", () => { + /** + * The parent's value survives a list that does not contain it. + * + * `users` is one page of an unbounded table, fetched after this field mounts, so a member + * off that page cannot be resolved to a user yet. Emitting `undefined` there does not blank + * a field — it clears the id out of the form, the picked-a-user rule then refuses the save, + * and the submit sends no request at all. Which is the whole bug, three times over. + */ + it("does not clear an id it simply cannot resolve", async () => { + const wrapper = mountSelect([], 4242) + await wrapper.vm.$nextTick() + + const cleared = (wrapper.emitted("update:modelValue") ?? []).filter(([id]) => id === undefined) + expect(cleared).toHaveLength(0) + }) + + it("resolves it once the list arrives, without having lost it", async () => { + const wrapper = mountSelect([], alice.id) + await wrapper.vm.$nextTick() + + await wrapper.setProps({users: [alice]}) + await wrapper.vm.$nextTick() + + expect(selected(wrapper)).toMatchObject({id: alice.id}) + expect((wrapper.emitted("update:modelValue") ?? []).filter(([id]) => id === undefined)).toHaveLength(0) + }) + + /** A field the reader can actually see is a field the reader can actually clear. */ + it("still reports a user clearing a value that was resolved", async () => { + const wrapper = mountSelect([alice], alice.id) + await wrapper.vm.$nextTick() + + await wrapper.findComponent({name: "VAutocomplete"}).vm.$emit("update:modelValue", undefined) + await wrapper.vm.$nextTick() + + expect((wrapper.emitted("update:modelValue") ?? []).some(([id]) => id === undefined)).toBe(true) + }) +}) diff --git a/tests/system/src/test/kotlin/net/blueshell/api/system/frontend/helper/CommitteeFormHelper.kt b/tests/system/src/test/kotlin/net/blueshell/api/system/frontend/helper/CommitteeFormHelper.kt index c0e59f832..2c3a89f97 100644 --- a/tests/system/src/test/kotlin/net/blueshell/api/system/frontend/helper/CommitteeFormHelper.kt +++ b/tests/system/src/test/kotlin/net/blueshell/api/system/frontend/helper/CommitteeFormHelper.kt @@ -1,11 +1,15 @@ package net.blueshell.api.system.frontend.helper +import com.microsoft.playwright.Locator import com.microsoft.playwright.Page import com.microsoft.playwright.options.AriaRole import net.blueshell.systemtests.HttpFailureLog import com.microsoft.playwright.assertions.PlaywrightAssertions.assertThat as assertPw object CommitteeFormHelper { + /** The picker's 250ms settle, plus one api round trip, plus room for a machine under load. */ + private const val OPTION_TIMEOUT_MS = 20_000.0 + fun fillCommittee(page: Page, name: String, description: String) { val nameField = page.getByLabel("Committee name") val descriptionField = page.getByLabel("Description") @@ -36,16 +40,35 @@ object CommitteeFormHelper { // loads users asynchronously after mount — on a fresh create // form the autocomplete can be empty when this helper runs. // Waiting for the option locks the binding without polling. + // + // Given longer than the default: the picker settles for 250ms before it asks the api + // at all, and the answer is a round trip to a container that may be serving this + // query for the first time. Five seconds covers that on an idle machine and not on a + // loaded one, which is the whole of why this test failed in CI and never here. page.getByRole( AriaRole.OPTION, Page.GetByRoleOptions().setName(fullName).setExact(false), - ).first().click() + ).first().click(Locator.ClickOptions().setTimeout(OPTION_TIMEOUT_MS)) } fun removeFirstMember(page: Page) { TestIdLocatorHelper.byTestIdPrefix(page, "committee-form-remove-member-btn-").click() } + /** + * What the form is refusing to save on, in its own words. + * + * A submit that sends no request at all has been refused by a rule rather than by the api, + * and the rule that refused is on the page as a message under its field. Read only when a + * save has already failed to arrive, so that the failure says which field rather than only + * that nothing was sent. + */ + fun refusals(page: Page): List = + page.locator("[data-testid=committee-form] .v-messages__message") + .allTextContents() + .map { it.trim() } + .filter { it.isNotEmpty() } + fun submit(page: Page) { val submitBtn = TestIdLocatorHelper.byTestId(page, "committee-form-submit-btn") submitBtn.waitFor() diff --git a/tests/system/src/test/kotlin/net/blueshell/api/system/frontend/management/CommitteeManagerPageSystemTest.kt b/tests/system/src/test/kotlin/net/blueshell/api/system/frontend/management/CommitteeManagerPageSystemTest.kt index eaf94828b..7672e01dc 100644 --- a/tests/system/src/test/kotlin/net/blueshell/api/system/frontend/management/CommitteeManagerPageSystemTest.kt +++ b/tests/system/src/test/kotlin/net/blueshell/api/system/frontend/management/CommitteeManagerPageSystemTest.kt @@ -161,9 +161,16 @@ class CommitteeManagerPageSystemTest : PlaywrightTestBase() { // that is what is waited for. CommitteeFormHelper.submit(page) - pollFor("committee metadata updated") { - val refreshed = TestHelper.findCommittee(committeeId) - refreshed != null && refreshed.name == updatedName && refreshed.description == updatedDescription + // A save that never reaches the api has been refused by a rule on the form, and the rule + // that refused says so under its own field. Without it the failure can only report that + // nothing was sent, which is where this one sat for two runs. + try { + pollFor("committee metadata updated") { + val refreshed = TestHelper.findCommittee(committeeId) + refreshed != null && refreshed.name == updatedName && refreshed.description == updatedDescription + } + } catch (refused: AssertionError) { + throw AssertionError("${refused.message} form refused with ${CommitteeFormHelper.refusals(page)}", refused) } } private fun pollForCommitteeByName(name: String): TestHelper.CommitteeRow =