From 17bcd7645bd548658d313b0c38b11e01c2778947 Mon Sep 17 00:00:00 2001 From: Valentin Tolmer Date: Wed, 2 Oct 2024 20:40:06 +0200 Subject: [PATCH] app: Clean up code, don't error on admin empty email --- app/src/components/create_user.rs | 88 ++++++----------- app/src/components/user_details.rs | 17 +++- app/src/components/user_details_form.rs | 125 ++++++------------------ app/src/infra/form_utils.rs | 64 ++++++++++++ app/src/infra/mod.rs | 1 + server/src/infra/graphql/mutation.rs | 1 + 6 files changed, 144 insertions(+), 152 deletions(-) create mode 100644 app/src/infra/form_utils.rs diff --git a/app/src/components/create_user.rs b/app/src/components/create_user.rs index eded954..abc8948 100644 --- a/app/src/components/create_user.rs +++ b/app/src/components/create_user.rs @@ -11,16 +11,15 @@ use crate::{ infra::{ api::HostService, common_component::{CommonComponent, CommonComponentParts}, + form_utils::{read_all_form_attributes, AttributeValue, GraphQlAttributeSchema}, schema::AttributeType, }, }; -use anyhow::{anyhow, ensure, Result}; +use anyhow::{ensure, Result}; use gloo_console::log; use graphql_client::GraphQLQuery; use lldap_auth::{opaque, registration}; -use validator::validate_email; use validator_derive::Validate; -use web_sys::{FormData, HtmlFormElement}; use yew::prelude::*; use yew_form_derive::Model; use yew_router::{prelude::History, scope_ext::RouterScopeExt}; @@ -40,6 +39,17 @@ pub type Attribute = get_user_attributes_schema::GetUserAttributesSchemaSchemaUs convert_attribute_type!(get_user_attributes_schema::AttributeType); +impl From<&Attribute> for GraphQlAttributeSchema { + fn from(attr: &Attribute) -> Self { + Self { + name: attr.name.clone(), + is_list: attr.is_list, + is_readonly: attr.is_readonly, + is_editable: attr.is_editable, + } + } +} + #[derive(GraphQLQuery)] #[graphql( schema_path = "../schema.graphql", @@ -108,37 +118,24 @@ impl CommonComponent for CreateUserForm { Msg::SubmitForm => { ensure!(self.form.validate(), "Check the form for errors"); - let form = self.form_ref.cast::().unwrap(); - let form_data = FormData::new_with_form(&form) - .map_err(|e| anyhow!("Failed to get FormData: {:#?}", e.as_string()))?; - let all_values = get_values_from_form_data( - self.attributes_schema - .iter() - .flatten() - .filter(|attr| !attr.is_readonly) - .collect(), - &form_data, + let all_values = read_all_form_attributes( + self.attributes_schema.iter().flatten(), + &self.form_ref, + true, + true, )?; - { - let email_values = &all_values - .iter() - .find(|(name, _)| name == "mail") - .ok_or_else(|| anyhow!("Email is required"))? - .1; - ensure!(email_values.len() == 1, "Email is required"); - ensure!(validate_email(&email_values[0]), "Email is not valid"); - } - let attributes = if all_values.is_empty() { - None - } else { - Some( - all_values - .into_iter() - .filter(|(_, value)| !value.is_empty()) - .map(|(name, value)| create_user::AttributeValueInput { name, value }) - .collect(), - ) - }; + let attributes = Some( + all_values + .into_iter() + .filter(|a| !a.values.is_empty()) + .map( + |AttributeValue { name, values }| create_user::AttributeValueInput { + name, + value: values, + }, + ) + .collect(), + ); let model = self.form.model(); let req = create_user::Variables { @@ -322,28 +319,3 @@ pub fn get_custom_attribute_input(attribute_schema: &Attribute) -> Html { } } } - -type AttributeValue = (String, Vec); - -fn get_values_from_form_data( - schema: Vec<&Attribute>, - form: &FormData, -) -> Result> { - schema - .into_iter() - .map(|attr| -> Result { - let val = form - .get_all(attr.name.as_str()) - .iter() - .map(|js_val| js_val.as_string().unwrap_or_default()) - .filter(|val| !val.is_empty()) - .collect::>(); - ensure!( - val.len() <= 1 || attr.is_list, - "Multiple values supplied for non-list attribute {}", - attr.name - ); - Ok((attr.name.clone(), val)) - }) - .collect() -} diff --git a/app/src/components/user_details.rs b/app/src/components/user_details.rs index 431646a..7e2e97b 100644 --- a/app/src/components/user_details.rs +++ b/app/src/components/user_details.rs @@ -6,7 +6,10 @@ use crate::{ user_details_form::UserDetailsForm, }, convert_attribute_type, - infra::common_component::{CommonComponent, CommonComponentParts}, + infra::{ + common_component::{CommonComponent, CommonComponentParts}, + form_utils::GraphQlAttributeSchema, + }, }; use anyhow::{bail, Error, Result}; use graphql_client::GraphQLQuery; @@ -29,6 +32,17 @@ pub type AttributeType = get_user_details::AttributeType; convert_attribute_type!(AttributeType); +impl From<&AttributeSchema> for GraphQlAttributeSchema { + fn from(attr: &AttributeSchema) -> Self { + Self { + name: attr.name.clone(), + is_list: attr.is_list, + is_readonly: attr.is_readonly, + is_editable: attr.is_editable, + } + } +} + pub struct UserDetails { common: CommonComponentParts, /// The user info. If none, the error is in `error`. If `error` is None, then we haven't @@ -219,6 +233,7 @@ impl Component for UserDetails { user={u.clone()} user_attributes_schema={schema.clone()} is_admin={ctx.props().is_admin} + is_edited_user_admin={u.groups.iter().any(|g| g.display_name == "lldap_admin")} /> {self.view_group_memberships(ctx, u)} {self.view_add_group_button(ctx, u)} diff --git a/app/src/components/user_details_form.rs b/app/src/components/user_details_form.rs index 318608c..6f9778b 100644 --- a/app/src/components/user_details_form.rs +++ b/app/src/components/user_details_form.rs @@ -9,25 +9,13 @@ use crate::{ }, infra::{ common_component::{CommonComponent, CommonComponentParts}, + form_utils::{read_all_form_attributes, AttributeValue}, schema::AttributeType, }, }; -use anyhow::{anyhow, bail, ensure, Ok, Result}; -use gloo_console::log; +use anyhow::{Ok, Result}; use graphql_client::GraphQLQuery; -use validator::HasLen; -use validator_derive::Validate; -use web_sys::{FormData, HtmlFormElement}; use yew::prelude::*; -use yew_form_derive::Model; - -/// The fields of the form, with the editable details and the constraints. -#[derive(Model, Validate, PartialEq, Eq, Clone)] -pub struct UserModel { - #[validate(email)] - email: String, - display_name: String, -} /// The GraphQL query sent to the server to update the user details. #[derive(GraphQLQuery)] @@ -43,7 +31,6 @@ pub struct UpdateUser; /// A [yew::Component] to display the user details, with a form allowing to edit them. pub struct UserDetailsForm { common: CommonComponentParts, - form: yew_form::Form, /// True if we just successfully updated the user, to display a success message. just_updated: bool, user: User, @@ -65,6 +52,7 @@ pub struct Props { pub user: User, pub user_attributes_schema: Vec, pub is_admin: bool, + pub is_edited_user_admin: bool, } impl CommonComponent for UserDetailsForm { @@ -76,7 +64,11 @@ impl CommonComponent for UserDetailsForm { match msg { Msg::Update => Ok(true), Msg::SubmitClicked => self.submit_user_update_form(ctx), - Msg::UserUpdated(response) => self.user_update_finished(response), + Msg::UserUpdated(Err(e)) => Err(e), + Msg::UserUpdated(Result::Ok(_)) => { + self.just_updated = true; + Ok(true) + } } } @@ -90,13 +82,8 @@ impl Component for UserDetailsForm { type Properties = Props; fn create(ctx: &Context) -> Self { - let model = UserModel { - email: ctx.props().user.email.clone(), - display_name: ctx.props().user.display_name.clone(), - }; Self { common: CommonComponentParts::::create(), - form: yew_form::Form::new(model), just_updated: false, user: ctx.props().user.clone(), form_ref: NodeRef::default(), @@ -168,59 +155,29 @@ impl Component for UserDetailsForm { } } -type AttributeValue = (String, Vec); - -fn get_values_from_form_data( - schema: Vec<&AttributeSchema>, - form: &FormData, -) -> Result> { - schema - .into_iter() - .map(|attr| -> Result { - let val = form - .get_all(attr.name.as_str()) - .iter() - .map(|js_val| js_val.as_string().unwrap_or_default()) - .filter(|val| !val.is_empty()) - .collect::>(); - ensure!( - val.length() <= 1 || attr.is_list, - "Multiple values supplied for non-list attribute {}", - attr.name - ); - Ok((attr.name.clone(), val)) - }) - .collect() -} - fn get_custom_attribute_input( attribute_schema: &AttributeSchema, user_attributes: &[Attribute], ) -> Html { + let values = user_attributes + .iter() + .find(|a| a.name == attribute_schema.name) + .map(|attribute| attribute.value.clone()) + .unwrap_or_default(); if attribute_schema.is_list { - let values = user_attributes - .iter() - .find(|a| a.name == attribute_schema.name) - .map(|attribute| attribute.value.clone()) - .unwrap_or_default(); html! { ::into(attribute_schema.attribute_type.clone())} values={values} /> } } else { - let value = user_attributes - .iter() - .find(|a| a.name == attribute_schema.name) - .and_then(|attribute| attribute.value.first().cloned()) - .unwrap_or_default(); html! { ::into(attribute_schema.attribute_type.clone())} - value={value} + value={values.first().cloned().unwrap_or_default()} /> } } @@ -244,9 +201,6 @@ fn get_custom_attribute_static( impl UserDetailsForm { fn submit_user_update_form(&mut self, ctx: &Context) -> Result { - if !self.form.validate() { - bail!("Invalid inputs"); - } // TODO: Handle unloaded files. // if let Some(JsFile { // file: Some(_), @@ -255,36 +209,25 @@ impl UserDetailsForm { // { // bail!("Image file hasn't finished loading, try again"); // } - let form = self.form_ref.cast::().unwrap(); - let form_data = FormData::new_with_form(&form) - .map_err(|e| anyhow!("Failed to get FormData: {:#?}", e.as_string()))?; - let mut all_values = get_values_from_form_data( - ctx.props() - .user_attributes_schema - .iter() - .filter(|attr| (ctx.props().is_admin && !attr.is_readonly) || attr.is_editable) - .collect(), - &form_data, + let mut all_values = read_all_form_attributes( + ctx.props().user_attributes_schema.iter(), + &self.form_ref, + ctx.props().is_admin, + !ctx.props().is_edited_user_admin, )?; let base_attributes = &self.user.attributes; - log!(format!( - "base_attributes: {:#?}\nall_values: {:#?}", - base_attributes, all_values - )); - all_values.retain(|(name, val)| { - let name = name.clone(); + all_values.retain(|a| { let base_val = base_attributes .iter() - .find(|base_val| base_val.name == name); - let new_values = val.clone(); + .find(|base_val| base_val.name == a.name); base_val - .map(|v| v.value != new_values) - .unwrap_or(!new_values.is_empty()) + .map(|v| v.value != a.values) + .unwrap_or(!a.values.is_empty()) }); let remove_attributes: Option> = if all_values.is_empty() { None } else { - Some(all_values.iter().map(|(name, _)| name.clone()).collect()) + Some(all_values.iter().map(|a| a.name.clone()).collect()) }; let insert_attributes: Option> = if remove_attributes.is_none() { @@ -293,8 +236,13 @@ impl UserDetailsForm { Some( all_values .into_iter() - .filter(|(_, value)| !value.is_empty()) - .map(|(name, value)| update_user::AttributeValueInput { name, value }) + .filter(|a| !a.values.is_empty()) + .map( + |AttributeValue { name, values }| update_user::AttributeValueInput { + name, + value: values, + }, + ) .collect(), ) }; @@ -324,13 +272,4 @@ impl UserDetailsForm { ); Ok(false) } - - fn user_update_finished(&mut self, r: Result) -> Result { - r?; - let model = self.form.model(); - self.user.email = model.email; - self.user.display_name = model.display_name; - self.just_updated = true; - Ok(true) - } } diff --git a/app/src/infra/form_utils.rs b/app/src/infra/form_utils.rs new file mode 100644 index 0000000..2e506e7 --- /dev/null +++ b/app/src/infra/form_utils.rs @@ -0,0 +1,64 @@ +use anyhow::{anyhow, ensure, Result}; +use validator::validate_email; +use web_sys::{FormData, HtmlFormElement}; +use yew::NodeRef; + +#[derive(Debug)] +pub struct AttributeValue { + pub name: String, + pub values: Vec, +} + +pub struct GraphQlAttributeSchema { + pub name: String, + pub is_list: bool, + pub is_readonly: bool, + pub is_editable: bool, +} + +fn validate_attributes(all_values: &[AttributeValue], email_is_required: bool) -> Result<()> { + let maybe_email_values = all_values.iter().find(|a| a.name == "mail"); + if email_is_required || maybe_email_values.is_some() { + let email_values = &maybe_email_values + .ok_or_else(|| anyhow!("Email is required"))? + .values; + ensure!(email_values.len() == 1, "Email is required"); + ensure!(validate_email(&email_values[0]), "Email is not valid"); + } + Ok(()) +} + +pub fn read_all_form_attributes( + schema: impl IntoIterator>, + form_ref: &NodeRef, + is_admin: bool, + email_is_required: bool, +) -> Result> { + let form = form_ref.cast::().unwrap(); + let form_data = FormData::new_with_form(&form) + .map_err(|e| anyhow!("Failed to get FormData: {:#?}", e.as_string()))?; + let all_values = schema + .into_iter() + .map(Into::::into) + .filter(|attr| !attr.is_readonly && (is_admin || attr.is_editable)) + .map(|attr| -> Result { + let val = form_data + .get_all(attr.name.as_str()) + .iter() + .map(|js_val| js_val.as_string().unwrap_or_default()) + .filter(|val| !val.is_empty()) + .collect::>(); + ensure!( + val.len() <= 1 || attr.is_list, + "Multiple values supplied for non-list attribute {}", + attr.name + ); + Ok(AttributeValue { + name: attr.name.clone(), + values: val, + }) + }) + .collect::>>()?; + validate_attributes(&all_values, email_is_required)?; + Ok(all_values) +} diff --git a/app/src/infra/mod.rs b/app/src/infra/mod.rs index 056d4c4..dc0d8a4 100644 --- a/app/src/infra/mod.rs +++ b/app/src/infra/mod.rs @@ -1,6 +1,7 @@ pub mod api; pub mod common_component; pub mod cookies; +pub mod form_utils; pub mod functional; pub mod graphql; pub mod modal; diff --git a/server/src/infra/graphql/mutation.rs b/server/src/infra/graphql/mutation.rs index b7ca740..d7bcaf9 100644 --- a/server/src/infra/graphql/mutation.rs +++ b/server/src/infra/graphql/mutation.rs @@ -277,6 +277,7 @@ impl Mutation { .remove_attributes .unwrap_or_default() .into_iter() + .filter(|attr| attr != "mail") // mail can be sent when editing an admin user .map(Into::into) .collect(), insert_attributes,