Allow empty string to clear URL-type DB fields.

api_tests/src/user.spec.ts

@@ -21,6 +21,7 @@ import {
   fetchFunction,
   alphaImage,
   unfollows,
+  saveUserSettingsBio,
 } from "./shared";
 import { LemmyHttp, SaveUserSettings, UploadImage } from "lemmy-js-client";
 import { GetPosts } from "lemmy-js-client/dist/types/GetPosts";
@@ -198,4 +199,14 @@ test("Set a new avatar, old avatar is deleted", async () => {
   // make sure only the new avatar is kept
   const listMediaRes3 = await alphaImage.listMedia();
   expect(listMediaRes3.images.length).toBe(1);
+
+  // Now try to save a user settings, with the icon missing,
+  // and make sure it doesn't clear the data, or delete the image
+  await saveUserSettingsBio(alpha);
+  let site = await getSite(alpha);
+  expect(site.my_user?.local_user_view.person.avatar).toBe(upload2.url);
+
+  // make sure only the new avatar is kept
+  const listMediaRes4 = await alphaImage.listMedia();
+  expect(listMediaRes4.images.length).toBe(1);
 });

crates/api/src/community/ban.rs

@@ -43,7 +43,10 @@ pub async fn ban_from_community(
     &mut context.pool(),
   )
   .await?;
-  is_valid_body_field(&data.reason, false)?;
+
+  if let Some(reason) = &data.reason {
+    is_valid_body_field(reason, false)?;
+  }
 
   let community_user_ban_form = CommunityPersonBanForm {
     community_id: data.community_id,

crates/api/src/local_user/ban_person.rs

@@ -31,7 +31,9 @@ pub async fn ban_from_site(
   // Make sure user is an admin
   is_admin(&local_user_view)?;
 
-  is_valid_body_field(&data.reason, false)?;
+  if let Some(reason) = &data.reason {
+    is_valid_body_field(reason, false)?;
+  }
 
   let expires = check_expire_time(data.expires)?;
 

crates/api/src/local_user/save_settings.rs

@@ -21,7 +21,7 @@ use lemmy_db_schema::{
     person::{Person, PersonUpdateForm},
   },
   traits::Crud,
-  utils::diesel_option_overwrite,
+  utils::{diesel_string_update, diesel_url_update},
 };
 use lemmy_db_views::structs::{LocalUserView, SiteView};
 use lemmy_utils::{
@@ -42,18 +42,22 @@ pub async fn save_user_settings(
 
   let slur_regex = local_site_to_slur_regex(&site_view.local_site);
   let url_blocklist = get_url_blocklist(&context).await?;
-  let bio = diesel_option_overwrite(
-    process_markdown_opt(&data.bio, &slur_regex, &url_blocklist, &context).await?,
+  let bio = diesel_string_update(
+    &process_markdown_opt(&data.bio, &slur_regex, &url_blocklist, &context).await?,
   );
-  replace_image(&data.avatar, &local_user_view.person.avatar, &context).await?;
-  replace_image(&data.banner, &local_user_view.person.banner, &context).await?;
 
-  let avatar = proxy_image_link_opt_api(&data.avatar, &context).await?;
-  let banner = proxy_image_link_opt_api(&data.banner, &context).await?;
-  let display_name = diesel_option_overwrite(data.display_name.clone());
-  let matrix_user_id = diesel_option_overwrite(data.matrix_user_id.clone());
+  let avatar = diesel_url_update(&data.avatar)?;
+  replace_image(&avatar, &local_user_view.person.avatar, &context).await?;
+  let avatar = proxy_image_link_opt_api(avatar, &context).await?;
+
+  let banner = diesel_url_update(&data.banner)?;
+  replace_image(&banner, &local_user_view.person.banner, &context).await?;
+  let banner = proxy_image_link_opt_api(banner, &context).await?;
+
+  let display_name = diesel_string_update(&data.display_name);
+  let matrix_user_id = diesel_string_update(&data.matrix_user_id.clone());
   let email_deref = data.email.as_deref().map(str::to_lowercase);
-  let email = diesel_option_overwrite(email_deref.clone());
+  let email = diesel_string_update(&email_deref.clone());
 
   if let Some(Some(email)) = &email {
     let previous_email = local_user_view.local_user.email.clone().unwrap_or_default();

crates/api/src/post/get_link_metadata.rs

@@ -4,14 +4,19 @@ use lemmy_api_common::{
   post::{GetSiteMetadata, GetSiteMetadataResponse},
   request::fetch_link_metadata,
 };
-use lemmy_utils::error::LemmyResult;
+use lemmy_utils::{
+  error::{LemmyErrorExt, LemmyResult},
+  LemmyErrorType,
+};
+use url::Url;
 
 #[tracing::instrument(skip(context))]
 pub async fn get_link_metadata(
   data: Query<GetSiteMetadata>,
   context: Data<LemmyContext>,
 ) -> LemmyResult<Json<GetSiteMetadataResponse>> {
-  let metadata = fetch_link_metadata(&data.url, &context).await?;
+  let url = Url::parse(&data.url).with_lemmy_type(LemmyErrorType::InvalidUrl)?;
+  let metadata = fetch_link_metadata(&url, &context).await?;
 
   Ok(Json(GetSiteMetadataResponse { metadata }))
 }

crates/api/src/site/registration_applications/approve.rs

@@ -10,7 +10,7 @@ use lemmy_db_schema::{
     registration_application::{RegistrationApplication, RegistrationApplicationUpdateForm},
   },
   traits::Crud,
-  utils::diesel_option_overwrite,
+  utils::diesel_string_update,
 };
 use lemmy_db_views::structs::{LocalUserView, RegistrationApplicationView};
 use lemmy_utils::{error::LemmyResult, LemmyErrorType};
@@ -26,7 +26,7 @@ pub async fn approve_registration_application(
   is_admin(&local_user_view)?;
 
   // Update the registration with reason, admin_id
-  let deny_reason = diesel_option_overwrite(data.deny_reason.clone());
+  let deny_reason = diesel_string_update(&data.deny_reason.clone());
   let app_form = RegistrationApplicationUpdateForm {
     admin_id: Some(Some(local_user_view.person.id)),
     deny_reason,

crates/api_common/src/post.rs

@@ -10,7 +10,6 @@ use serde::{Deserialize, Serialize};
 use serde_with::skip_serializing_none;
 #[cfg(feature = "full")]
 use ts_rs::TS;
-use url::Url;
 
 #[skip_serializing_none]
 #[derive(Debug, Serialize, Deserialize, Clone, Default, PartialEq, Eq, Hash)]
@@ -20,8 +19,7 @@ use url::Url;
 pub struct CreatePost {
   pub name: String,
   pub community_id: CommunityId,
-  #[cfg_attr(feature = "full", ts(type = "string"))]
-  pub url: Option<Url>,
+  pub url: Option<String>,
   /// An optional body for the post in markdown.
   pub body: Option<String>,
   /// An optional alt_text, usable for image posts.
@@ -30,9 +28,8 @@ pub struct CreatePost {
   pub honeypot: Option<String>,
   pub nsfw: Option<bool>,
   pub language_id: Option<LanguageId>,
-  #[cfg_attr(feature = "full", ts(type = "string"))]
   /// Instead of fetching a thumbnail, use a custom one.
-  pub custom_thumbnail: Option<Url>,
+  pub custom_thumbnail: Option<String>,
 }
 
 #[derive(Debug, Serialize, Deserialize, Clone)]
@@ -114,17 +111,15 @@ pub struct CreatePostLike {
 pub struct EditPost {
   pub post_id: PostId,
   pub name: Option<String>,
-  #[cfg_attr(feature = "full", ts(type = "string"))]
-  pub url: Option<Url>,
+  pub url: Option<String>,
   /// An optional body for the post in markdown.
   pub body: Option<String>,
   /// An optional alt_text, usable for image posts.
   pub alt_text: Option<String>,
   pub nsfw: Option<bool>,
   pub language_id: Option<LanguageId>,
-  #[cfg_attr(feature = "full", ts(type = "string"))]
   /// Instead of fetching a thumbnail, use a custom one.
-  pub custom_thumbnail: Option<Url>,
+  pub custom_thumbnail: Option<String>,
 }
 
 #[derive(Debug, Serialize, Deserialize, Clone, Copy, Default, PartialEq, Eq, Hash)]
@@ -249,8 +244,7 @@ pub struct ListPostReportsResponse {
 #[cfg_attr(feature = "full", ts(export))]
 /// Get metadata for a given site.
 pub struct GetSiteMetadata {
-  #[cfg_attr(feature = "full", ts(type = "string"))]
-  pub url: Url,
+  pub url: String,
 }
 
 #[derive(Debug, Serialize, Deserialize, Clone)]

crates/api_common/src/request.rs

@@ -340,15 +340,15 @@ async fn is_image_content_type(client: &ClientWithMiddleware, url: &Url) -> Lemm
 
 /// When adding a new avatar, banner or similar image, delete the old one.
 pub async fn replace_image(
-  new_image: &Option<String>,
+  new_image: &Option<Option<DbUrl>>,
   old_image: &Option<DbUrl>,
   context: &Data<LemmyContext>,
 ) -> LemmyResult<()> {
-  if let (Some(new_image), Some(old_image)) = (new_image, old_image) {
+  if let (Some(Some(new_image)), Some(old_image)) = (new_image, old_image) {
     // Note: Oftentimes front ends will include the current image in the form.
     // In this case, deleting `old_image` would also be deletion of `new_image`,
     // so the deletion must be skipped for the image to be kept.
-    if new_image != old_image.as_str() {
+    if new_image != old_image {
       // Ignore errors because image may be stored externally.
       let image = LocalImage::delete_by_url(&mut context.pool(), old_image)
         .await

crates/api_common/src/utils.rs

@@ -1004,26 +1004,25 @@ pub(crate) async fn proxy_image_link(link: Url, context: &LemmyContext) -> Lemmy
 }
 
 pub async fn proxy_image_link_opt_api(
-  link: &Option<String>,
+  link: Option<Option<DbUrl>>,
   context: &LemmyContext,
 ) -> LemmyResult<Option<Option<DbUrl>>> {
-  proxy_image_link_api(link, context).await.map(Some)
+  if let Some(Some(link)) = link {
+    proxy_image_link(link.into(), context)
+      .await
+      .map(Some)
+      .map(Some)
+  } else {
+    Ok(link)
+  }
 }
 
 pub async fn proxy_image_link_api(
-  link: &Option<String>,
+  link: Option<DbUrl>,
   context: &LemmyContext,
 ) -> LemmyResult<Option<DbUrl>> {
-  let link: Option<DbUrl> = match link.as_ref().map(String::as_str) {
-    // An empty string is an erase
-    Some("") => None,
-    Some(str_url) => Url::parse(str_url)
-      .map(|u| Some(u.into()))
-      .with_lemmy_type(LemmyErrorType::InvalidUrl)?,
-    None => None,
-  };
-  if let Some(l) = link {
-    proxy_image_link(l.into(), context).await.map(Some)
+  if let Some(link) = link {
+    proxy_image_link(link.into(), context).await.map(Some)
   } else {
     Ok(link)
   }
@@ -1130,29 +1129,4 @@ mod tests {
         .is_ok()
     );
   }
-
-  #[tokio::test]
-  #[serial]
-  async fn test_diesel_option_overwrite_to_url() {
-    let context = LemmyContext::init_test_context().await;
-
-    assert!(matches!(
-      proxy_image_link_api(&None, &context).await,
-      Ok(None)
-    ));
-    assert!(matches!(
-      proxy_image_link_opt_api(&Some(String::new()), &context).await,
-      Ok(Some(None))
-    ));
-    assert!(
-      proxy_image_link_opt_api(&Some("invalid_url".to_string()), &context)
-        .await
-        .is_err()
-    );
-    let example_url = "https://lemmy-alpha/image.png";
-    assert!(matches!(
-      proxy_image_link_opt_api(&Some(example_url.to_string()), &context).await,
-      Ok(Some(Some(url))) if url == Url::parse(example_url).unwrap().into()
-    ));
-  }
 }

crates/api_crud/src/comment/create.rs

@@ -47,7 +47,7 @@ pub async fn create_comment(
   let slur_regex = local_site_to_slur_regex(&local_site);
   let url_blocklist = get_url_blocklist(&context).await?;
   let content = process_markdown(&data.content, &slur_regex, &url_blocklist, &context).await?;
-  is_valid_body_field(&Some(content.clone()), false)?;
+  is_valid_body_field(&content, false)?;
 
   // Check for a community ban
   let post_id = data.post_id;

crates/api_crud/src/comment/update.rs

@@ -63,7 +63,9 @@ pub async fn update_comment(
   let slur_regex = local_site_to_slur_regex(&local_site);
   let url_blocklist = get_url_blocklist(&context).await?;
   let content = process_markdown_opt(&data.content, &slur_regex, &url_blocklist, &context).await?;
-  is_valid_body_field(&content, false)?;
+  if let Some(content) = &content {
+    is_valid_body_field(content, false)?;
+  }
 
   let comment_id = data.comment_id;
   let form = CommentUpdateForm {

crates/api_crud/src/community/create.rs

@@ -30,6 +30,7 @@ use lemmy_db_schema::{
     },
   },
   traits::{ApubActor, Crud, Followable, Joinable},
+  utils::diesel_url_create,
 };
 use lemmy_db_views::structs::{LocalUserView, SiteView};
 use lemmy_utils::{
@@ -61,11 +62,18 @@ pub async fn create_community(
   check_slurs(&data.title, &slur_regex)?;
   let description =
     process_markdown_opt(&data.description, &slur_regex, &url_blocklist, &context).await?;
-  let icon = proxy_image_link_api(&data.icon, &context).await?;
-  let banner = proxy_image_link_api(&data.banner, &context).await?;
+
+  let icon = diesel_url_create(&data.icon)?;
+  let icon = proxy_image_link_api(icon, &context).await?;
+
+  let banner = diesel_url_create(&data.banner)?;
+  let banner = proxy_image_link_api(banner, &context).await?;
 
   is_valid_actor_name(&data.name, local_site.actor_name_max_length as usize)?;
-  is_valid_body_field(&data.description, false)?;
+
+  if let Some(desc) = &data.description {
+    is_valid_body_field(desc, false)?;
+  }
 
   // Double check for duplicate community actor_ids
   let community_actor_id = generate_local_apub_endpoint(

crates/api_crud/src/community/update.rs

@@ -21,7 +21,7 @@ use lemmy_db_schema::{
     local_site::LocalSite,
   },
   traits::Crud,
-  utils::{diesel_option_overwrite, naive_now},
+  utils::{diesel_string_update, diesel_url_update, naive_now},
 };
 use lemmy_db_views::structs::LocalUserView;
 use lemmy_utils::{
@@ -40,18 +40,26 @@ pub async fn update_community(
   let slur_regex = local_site_to_slur_regex(&local_site);
   let url_blocklist = get_url_blocklist(&context).await?;
   check_slurs_opt(&data.title, &slur_regex)?;
-  let description =
-    process_markdown_opt(&data.description, &slur_regex, &url_blocklist, &context).await?;
-  is_valid_body_field(&data.description, false)?;
+
+  let description = diesel_string_update(
+    &process_markdown_opt(&data.description, &slur_regex, &url_blocklist, &context).await?,
+  );
+
+  if let Some(Some(desc)) = &description {
+    is_valid_body_field(desc, false)?;
+  }
+
   let old_community = Community::read(&mut context.pool(), data.community_id)
     .await?
     .ok_or(LemmyErrorType::CouldntFindCommunity)?;
-  replace_image(&data.icon, &old_community.icon, &context).await?;
-  replace_image(&data.banner, &old_community.banner, &context).await?;
 
-  let description = diesel_option_overwrite(description);
-  let icon = proxy_image_link_opt_api(&data.icon, &context).await?;
-  let banner = proxy_image_link_opt_api(&data.banner, &context).await?;
+  let icon = diesel_url_update(&data.icon)?;
+  replace_image(&icon, &old_community.icon, &context).await?;
+  let icon = proxy_image_link_opt_api(icon, &context).await?;
+
+  let banner = diesel_url_update(&data.banner)?;
+  replace_image(&banner, &old_community.banner, &context).await?;
+  let banner = proxy_image_link_opt_api(banner, &context).await?;
 
   // Verify its a mod (only mods can edit it)
   check_community_mod_action(