Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undocumented survey select widget bug #1866

Conversation

richardolsson
Copy link
Member

Description

This PR fixes an undocumented bug in the new survey page. This bug recently surfaced in the production Gen2 version of Zetkin, and since the code in #1756 was based on the logic from there, I guessed (correctly) that the bug would exist here as well.

The bug basically means that if a <select> is not interacted with, it will still be submitted (at least on Chrome) with an empty value, and that empty value will not be handled correctly on the server. Instead of removing it, or submitting it with empty options (options: []) it gets submitted with a null option (options: [null]).

Screenshots

None

Changes

  • Changes integration tests to not navigate (page.goto) until within the test cases, to allow for test cases to make additional mocks before navigating
  • Adds integration test that reproduces the bug
  • Adds unit tests for prepareSurveyApiSubmission() that reproduces the bug
  • Fixes the bug in prepareSurveyApiSubmission()

Notes to reviewer

None

Related issues

Undocumented

Copy link
Collaborator

@henrycatalinismith henrycatalinismith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💖

@richardolsson richardolsson merged commit 867db13 into issue-1620/next-v13-survey-page-with-app-router-and-server-action Mar 27, 2024
5 checks passed
@richardolsson richardolsson deleted the undocumented/survey-select-widget-bug branch March 27, 2024 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants