fix(site): validate group name before submitting to the backend #16115
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
The changes look great! I’d suggest adding a test to ensure the name validation always works correctly by including at least one story in site/src/pages/GroupsPage/CreateGroupPageView.stories.tsx to cover the invalid state.
export const InvalidName: Story = {
play: async ({ canvasElement }) => {
const user = userEvent.setup();
const input = await body.findByLabelText("Name");
await user.type(input, "$om3 !nv@lid Name");
},
};For more details about the FE codebase, you can refer to this guide.
c592357 to
913d8a0
Compare
Thanks @BrunoQuaresma. I've added a story for each component, as you suggested. |
Change-Id: I6ffc2d571b2f6c6387ef143e0b0e74cf4196a551 Signed-off-by: Thomas Kosiewski <tk@coder.com>
913d8a0 to
83d95f4
Compare
You can always review and approve the stories yourself on Chromatic - they’re part of the commit checks. However, if you’d like a second opinion or an extra set of eyes on them, feel free to ping someone from the @coder/ts group. |
| const body = within(canvasElement.ownerDocument.body); | ||
| const input = await body.findByLabelText("Name"); | ||
| await user.type(input, "$om3 !nv@lid Name"); | ||
| input.blur(); |
There was a problem hiding this comment.
@BrunoQuaresma I'm not sure if I'm missing something, but are these tests incomplete?
We're checking that the interactive elements exist and that the user can fill them out incorrectly, but then it looks like we're not asserting that anything happens in response to that? I would expect something like asserting that some kind of validation message appears
There was a problem hiding this comment.
chromatic will snapshot the final state, which will include the error message
There was a problem hiding this comment.
Okay, that's good to know. I guess my worry is that without an expect-style assertion, somebody could accidentally accept the stories and miss that the validation message is no longer on screen
I'd prefer to force the test to fail if there's a good chance something is wrong, but maybe I subscribe too much to paranoia-driven development
Fixes #13354
This PR adds the missing form validation for names when creating groups.
Change-Id: I6ffc2d571b2f6c6387ef143e0b0e74cf4196a551