Page MenuHomePhabricator
Create Task
Maniphest T369494

Regression: Form validation fails on submit regardless of validity
Closed, ResolvedPublicBUG REPORT
Actions

Description

Steps to replicate the issue (include links if applicable):

What happens?:

  • Validation errors are shown for every field
  • The form does not submit

What should have happened instead?:

  • No validation errors are shown
  • The form submits successfully

Software version (on Special:Version page; skip for WMF-hosted wikis like Wikipedia):

Other information (browser name/version, screenshots, etc.):

image.png (131×508 px, 18 KB)

Related Objects

Event Timeline

TheresNoTime created this task.Mon, Jul 8, 10:16 AM
Restricted Application added a project: Community-Tech. · View Herald TranscriptMon, Jul 8, 10:16 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
TheresNoTime triaged this task as High priority.EditedMon, Jul 8, 10:18 AM
TheresNoTime edited projects, added Community-Tech (July 1-12 2024 (Corsac Fox)); removed Community-Tech.
TheresNoTime added subscribers: MusikAnimal, Samwilson.

I'm currently thinking it was caused by part of a change introduced in Load wish data alongside Vue, rework props for T367917: Prepopulate wishlist intake form instead of loading it afterwards, or some of the work for T368539: Edits to wish description not being saved correctly

TheresNoTime renamed this task from Regression: Form validation fails on submit regardless of validity to Regression: Form validation fails on submit regardless of validity.Mon, Jul 8, 10:21 AM
TheresNoTime added a comment.EditedMon, Jul 8, 12:32 PM

this.validateForm() is firing before some/all promises are completed? this.wishtitle.length (and others) in validateForm are returning 0 when called in

handleSubmit() {
	this.formDisabled = true;
	Promise.all( preSubmitPromises ).then( () => {
		// @todo Handle this nicer?
		if ( !this.validateForm() ) {
			this.formDisabled = false;
			return;
		}

That'll be because it's this.wish.title (:

TheresNoTime claimed this task.Mon, Jul 8, 1:42 PM
TheresNoTime moved this task from Ready for dev to In development on the Community-Tech (July 1-12 2024 (Corsac Fox)) board.
CodeReviewBot added a project: Patch-For-Review.Mon, Jul 8, 1:44 PM

samtar opened https://gitlab.wikimedia.org/repos/commtech/wishlist-intake/-/merge_requests/157

Use Wish props

TheresNoTime moved this task from In development to Feedback and review on the Community-Tech (July 1-12 2024 (Corsac Fox)) board.Mon, Jul 8, 1:50 PM
CodeReviewBot added a comment.Mon, Jul 8, 5:28 PM

musikanimal merged https://gitlab.wikimedia.org/repos/commtech/wishlist-intake/-/merge_requests/157

Use Wish props

CodeReviewBot added a comment.Mon, Jul 8, 6:00 PM

musikanimal opened https://gitlab.wikimedia.org/repos/commtech/wishlist-intake/-/merge_requests/159

db: rename problem field to description

CodeReviewBot added a comment.Mon, Jul 8, 6:09 PM

samtar merged https://gitlab.wikimedia.org/repos/commtech/wishlist-intake/-/merge_requests/159

db: rename problem field to description

CodeReviewBot added a comment.Mon, Jul 8, 9:51 PM

musikanimal opened https://gitlab.wikimedia.org/repos/commtech/wishlist-intake/-/merge_requests/160

WishlistIntake: fix more issues follow botched merge commit

MusikAnimal added a comment.EditedMon, Jul 8, 9:55 PM

I've attached two more MRs to this task. The task description is accurate as to the initial issue, but the actual problem was a botched merge commit that was part of !104. The other two MRs I believe fix the remaining issues.

CodeReviewBot added a comment.Tue, Jul 9, 12:06 AM

musikanimal merged https://gitlab.wikimedia.org/repos/commtech/wishlist-intake/-/merge_requests/160

WishlistIntake: fix more issues follow botched merge commit

TheresNoTime moved this task from Feedback and review to QA on the Community-Tech (July 1-12 2024 (Corsac Fox)) board.Tue, Jul 9, 11:53 AM
TheresNoTime removed a project: Patch-For-Review.
MusikAnimal closed this task as Resolved.Tue, Jul 9, 6:48 PM
MusikAnimal moved this task from QA to Done on the Community-Tech (July 1-12 2024 (Corsac Fox)) board.

QA here is basically "make sure nothing broke", so instead I'm going to call this resolved and point QA to T367917 which is the work that caused these regressions. All issues mentioned in this task have been fixed, which essentially puts us to where T367917 was supposed to be originally.

MusikAnimal mentioned this in T367917: Prepopulate wishlist intake form instead of loading it afterwards.Tue, Jul 9, 6:49 PM

Also noting for the record we have a decently well-rounded Selenium test now, so assuming developers remember to run it (because it doesn't run in CI, yet), regressions like this should never happen again :)

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits