max / makenotwork
5 files changed,
+106 insertions,
-20 deletions
| @@ -14,17 +14,25 @@ pub struct ApiErrorMessage(pub String); | |||
| 14 | 14 | ||
| 15 | 15 | /// Payload carried by `AppError::Validation`. | |
| 16 | 16 | /// | |
| 17 | - | /// The `summary` is the user-visible banner shown on the generic error page. | |
| 18 | - | /// Per-field error rendering is not currently wired into any template; if | |
| 19 | - | /// you find yourself wanting it, build the form re-render path first. | |
| 17 | + | /// The `summary` is the user-visible banner. `field`, when set, names the form | |
| 18 | + | /// input the error belongs to so a re-rendered form can mark that input invalid | |
| 19 | + | /// (`aria-invalid`) and focus it, instead of only showing a top-level banner. | |
| 20 | + | /// The join wizard's account step is the reference wiring of this path; other | |
| 21 | + | /// non-HTMX form re-renders should follow it. | |
| 20 | 22 | #[derive(Debug, Clone, PartialEq, Eq)] | |
| 21 | 23 | pub struct ValidationError { | |
| 22 | 24 | pub summary: String, | |
| 25 | + | pub field: Option<String>, | |
| 23 | 26 | } | |
| 24 | 27 | ||
| 25 | 28 | impl ValidationError { | |
| 26 | 29 | pub fn new(summary: impl Into<String>) -> Self { | |
| 27 | - | Self { summary: summary.into() } | |
| 30 | + | Self { summary: summary.into(), field: None } | |
| 31 | + | } | |
| 32 | + | ||
| 33 | + | /// A validation error attributed to a specific form field. | |
| 34 | + | pub fn with_field(field: impl Into<String>, summary: impl Into<String>) -> Self { | |
| 35 | + | Self { summary: summary.into(), field: Some(field.into()) } | |
| 28 | 36 | } | |
| 29 | 37 | } | |
| 30 | 38 |
| @@ -48,6 +48,10 @@ pub async fn wizard_page( | |||
| 48 | 48 | csrf_token: get_csrf_token(&session).await, | |
| 49 | 49 | nav: build_step_nav(JOIN_STEPS, JOIN_LABELS, "account"), | |
| 50 | 50 | invite_code: query.invite, | |
| 51 | + | username: String::new(), | |
| 52 | + | email: String::new(), | |
| 53 | + | error: None, | |
| 54 | + | error_field: None, | |
| 51 | 55 | } | |
| 52 | 56 | .into_response() | |
| 53 | 57 | } | |
| @@ -70,8 +74,9 @@ pub async fn step_account_create( | |||
| 70 | 74 | Form(form): Form<AccountForm>, | |
| 71 | 75 | ) -> Result<Response> { | |
| 72 | 76 | let is_htmx = is_htmx_request(&headers); | |
| 77 | + | let csrf_token = get_csrf_token(&session).await; | |
| 73 | 78 | ||
| 74 | - | let return_error = |summary: &str| -> Result<Response> { | |
| 79 | + | let return_error = |field: Option<&str>, summary: &str| -> Result<Response> { | |
| 75 | 80 | if is_htmx { | |
| 76 | 81 | Ok(Html( | |
| 77 | 82 | LoginErrorTemplate { | |
| @@ -81,18 +86,30 @@ pub async fn step_account_create( | |||
| 81 | 86 | ) | |
| 82 | 87 | .into_response()) | |
| 83 | 88 | } else { | |
| 84 | - | Err(AppError::validation(summary.to_string())) | |
| 89 | + | // Non-HTMX (JS disabled): re-render the account step with the typed | |
| 90 | + | // username/email preserved and the offending field marked invalid, | |
| 91 | + | // instead of a generic 422 that drops everything the user entered. | |
| 92 | + | Ok(WizardJoinTemplate { | |
| 93 | + | csrf_token: csrf_token.clone(), | |
| 94 | + | nav: build_step_nav(JOIN_STEPS, JOIN_LABELS, "account"), | |
| 95 | + | invite_code: form.invite_code.clone(), | |
| 96 | + | username: form.username.clone(), | |
| 97 | + | email: form.email.clone(), | |
| 98 | + | error: Some(summary.to_string()), | |
| 99 | + | error_field: field.map(|f| f.to_string()), | |
| 100 | + | } | |
| 101 | + | .into_response()) | |
| 85 | 102 | } | |
| 86 | 103 | }; | |
| 87 | 104 | ||
| 88 | 105 | let username = match db::Username::new(&form.username) { | |
| 89 | 106 | Ok(u) => u, | |
| 90 | - | Err(e) => return return_error(&e.to_string()), | |
| 107 | + | Err(e) => return return_error(Some("username"), &e.to_string()), | |
| 91 | 108 | }; | |
| 92 | 109 | ||
| 93 | 110 | let email = match db::Email::new(&form.email) { | |
| 94 | 111 | Ok(e) => e, | |
| 95 | - | Err(_) => return return_error("Please enter a valid email address"), | |
| 112 | + | Err(_) => return return_error(Some("email"), "Please enter a valid email address"), | |
| 96 | 113 | }; | |
| 97 | 114 | ||
| 98 | 115 | // Check uniqueness | |
| @@ -103,19 +120,19 @@ pub async fn step_account_create( | |||
| 103 | 120 | .await? | |
| 104 | 121 | .is_some(); | |
| 105 | 122 | if username_taken && email_taken { | |
| 106 | - | return return_error("This username and email are already registered"); | |
| 123 | + | return return_error(Some("username"), "This username and email are already registered"); | |
| 107 | 124 | } else if username_taken { | |
| 108 | - | return return_error("This username is already taken"); | |
| 125 | + | return return_error(Some("username"), "This username is already taken"); | |
| 109 | 126 | } else if email_taken { | |
| 110 | - | return return_error("This email is already registered"); | |
| 127 | + | return return_error(Some("email"), "This email is already registered"); | |
| 111 | 128 | } | |
| 112 | 129 | ||
| 113 | 130 | let password_len = form.password.chars().count(); | |
| 114 | 131 | if password_len < 8 { | |
| 115 | - | return return_error("Password must be at least 8 characters"); | |
| 132 | + | return return_error(Some("password"), "Password must be at least 8 characters"); | |
| 116 | 133 | } | |
| 117 | 134 | if password_len > 128 { | |
| 118 | - | return return_error("Password must be 128 characters or fewer"); | |
| 135 | + | return return_error(Some("password"), "Password must be 128 characters or fewer"); | |
| 119 | 136 | } | |
| 120 | 137 | ||
| 121 | 138 | // Check for breached password (advisory only) | |
| @@ -145,14 +162,14 @@ pub async fn step_account_create( | |||
| 145 | 162 | if db_err.code().as_deref() == Some("23505") => | |
| 146 | 163 | { | |
| 147 | 164 | let constraint = db_err.constraint().unwrap_or(""); | |
| 148 | - | let msg = if constraint.contains("username") { | |
| 149 | - | "This username is no longer available" | |
| 165 | + | let (field, msg): (Option<&str>, &str) = if constraint.contains("username") { | |
| 166 | + | (Some("username"), "This username is no longer available") | |
| 150 | 167 | } else if constraint.contains("email") { | |
| 151 | - | "This email is already registered" | |
| 168 | + | (Some("email"), "This email is already registered") | |
| 152 | 169 | } else { | |
| 153 | - | "An account with these details already exists" | |
| 170 | + | (None, "An account with these details already exists") | |
| 154 | 171 | }; | |
| 155 | - | return return_error(msg); | |
| 172 | + | return return_error(field, msg); | |
| 156 | 173 | } | |
| 157 | 174 | Err(e) => return Err(e), | |
| 158 | 175 | }; |
| @@ -152,6 +152,13 @@ pub struct WizardJoinTemplate { | |||
| 152 | 152 | pub csrf_token: CsrfTokenOption, | |
| 153 | 153 | pub nav: Vec<super::StepNavItem>, | |
| 154 | 154 | pub invite_code: Option<String>, | |
| 155 | + | /// Preserved input + error on a non-HTMX account-step re-render (so a | |
| 156 | + | /// JS-disabled submit that fails validation doesn't drop the typed | |
| 157 | + | /// username/email). Empty / `None` on the initial render. | |
| 158 | + | pub username: String, | |
| 159 | + | pub email: String, | |
| 160 | + | pub error: Option<String>, | |
| 161 | + | pub error_field: Option<String>, | |
| 155 | 162 | } | |
| 156 | 163 | ||
| 157 | 164 | /// Step 1 partial: account creation (for back-nav reload). |
| @@ -25,12 +25,17 @@ | |||
| 25 | 25 | </div> | |
| 26 | 26 | {% endif %} | |
| 27 | 27 | ||
| 28 | + | {% if let Some(err) = error %} | |
| 29 | + | <div class="info-box info-box--error mb-4" role="alert">{{ err }}</div> | |
| 30 | + | {% endif %} | |
| 31 | + | ||
| 28 | 32 | <form hx-post="/join/step/account" | |
| 29 | 33 | hx-target="#wizard-step" hx-swap="innerHTML"> | |
| 30 | 34 | <div class="form-group"> | |
| 31 | 35 | <label for="wiz-username">Username</label> | |
| 32 | 36 | <input type="text" id="wiz-username" name="username" required | |
| 33 | - | placeholder="username" autocomplete="off" | |
| 37 | + | placeholder="username" autocomplete="off" value="{{ username }}" | |
| 38 | + | {% if error_field.as_deref() == Some("username") %}aria-invalid="true"{% endif %} | |
| 34 | 39 | hx-post="/api/validate/username" | |
| 35 | 40 | hx-trigger="keyup changed delay:500ms" | |
| 36 | 41 | hx-target="#username-status" | |
| @@ -43,7 +48,8 @@ | |||
| 43 | 48 | <div class="form-group"> | |
| 44 | 49 | <label for="wiz-email">Email</label> | |
| 45 | 50 | <input type="email" id="wiz-email" name="email" required | |
| 46 | - | placeholder="you@example.com" autocomplete="email"> | |
| 51 | + | placeholder="you@example.com" autocomplete="email" value="{{ email }}" | |
| 52 | + | {% if error_field.as_deref() == Some("email") %}aria-invalid="true"{% endif %}> | |
| 47 | 53 | <div class="hint">Used for account recovery and notifications</div> | |
| 48 | 54 | </div> | |
| 49 | 55 |
| @@ -1009,3 +1009,51 @@ async fn project_wizard_appearance_rejects_non_cdn_cover_url() { | |||
| 1009 | 1009 | .unwrap(); | |
| 1010 | 1010 | assert_eq!(stored.as_deref(), Some("https://cdn.makenot.work/projects/abc/cover.jpg")); | |
| 1011 | 1011 | } | |
| 1012 | + | ||
| 1013 | + | // ============================================================================= | |
| 1014 | + | // Join wizard — non-HTMX validation re-render preserves input (Run #22 UX LOW) | |
| 1015 | + | // ============================================================================= | |
| 1016 | + | ||
| 1017 | + | #[tokio::test] | |
| 1018 | + | async fn join_account_non_htmx_validation_preserves_input() { | |
| 1019 | + | // A non-HTMX (JS-disabled) account-step submit that fails validation must | |
| 1020 | + | // re-render the wizard with the typed username/email preserved and the | |
| 1021 | + | // offending field marked, not bounce to a generic error page that drops | |
| 1022 | + | // everything. post_form sends no HX-Request header, so this is that path. | |
| 1023 | + | let mut h = TestHarness::new().await; | |
| 1024 | + | ||
| 1025 | + | let resp = h | |
| 1026 | + | .client | |
| 1027 | + | .post_form( | |
| 1028 | + | "/join/step/account", | |
| 1029 | + | "username=keepme&email=not-an-email&password=testpassword123", | |
| 1030 | + | ) | |
| 1031 | + | .await; | |
| 1032 | + | ||
| 1033 | + | assert!( | |
| 1034 | + | resp.status.is_success(), | |
| 1035 | + | "invalid non-HTMX submit should re-render the form (200), got {}: {}", | |
| 1036 | + | resp.status, resp.text | |
| 1037 | + | ); | |
| 1038 | + | // The typed username survives the round-trip (value attribute). | |
| 1039 | + | assert!( | |
| 1040 | + | resp.text.contains("value=\"keepme\""), | |
| 1041 | + | "the typed username must be preserved in the re-render" | |
| 1042 | + | ); | |
| 1043 | + | // The email field is flagged invalid and an error is shown. | |
| 1044 | + | assert!( | |
| 1045 | + | resp.text.contains("aria-invalid=\"true\""), | |
| 1046 | + | "the offending field must be marked invalid" | |
| 1047 | + | ); | |
| 1048 | + | assert!( | |
| 1049 | + | resp.text.contains("valid email"), | |
| 1050 | + | "the validation message must be shown, got: {}", | |
| 1051 | + | resp.text | |
| 1052 | + | ); | |
| 1053 | + | // No account was created. | |
| 1054 | + | let count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM users WHERE username = 'keepme'") | |
| 1055 | + | .fetch_one(&h.db) | |
| 1056 | + | .await | |
| 1057 | + | .unwrap(); | |
| 1058 | + | assert_eq!(count, 0, "a failed validation must not create the account"); | |
| 1059 | + | } |