Moving alt_text to attachment name.

This commit is contained in:
Dessalines 2024-02-28 23:35:09 -05:00
parent c5a98c5a3f
commit c24542c7c1
10 changed files with 1123 additions and 2222 deletions

View file

@ -19,16 +19,16 @@
"api-test-image": "jest -i image.spec.ts"
},
"devDependencies": {
"@types/jest": "^29.5.11",
"@types/node": "^20.10.4",
"@typescript-eslint/eslint-plugin": "^6.14.0",
"@typescript-eslint/parser": "^6.14.0",
"@types/jest": "^29.5.12",
"@types/node": "^20.11.22",
"@typescript-eslint/eslint-plugin": "^7.1.0",
"@typescript-eslint/parser": "^7.1.0",
"download-file-sync": "^1.0.4",
"eslint": "^8.55.0",
"eslint": "^8.57.0",
"eslint-plugin-prettier": "^5.0.1",
"jest": "^29.5.0",
"lemmy-js-client": "0.19.3-alpha.2",
"prettier": "^3.1.1",
"lemmy-js-client": "0.19.4-alpha.6",
"prettier": "^3.2.5",
"ts-jest": "^29.1.0",
"typescript": "^5.3.3"
}

File diff suppressed because it is too large Load diff

View file

@ -203,4 +203,7 @@ test("No image proxying if setting is disabled", async () => {
gammaPost.post!.post.url?.startsWith("http://127.0.0.1:8551/pictrs/image/"),
).toBeTruthy();
expect(gammaPost.post!.post.body).toBe("![](http://example.com/image2.png)");
// Make sure the alt text got federated
expect(post.post_view.post.alt_text).toBe(gammaPost.post!.post.alt_text);
});

View file

@ -200,11 +200,13 @@ export async function createPost(
body = randomString(10),
// use example.com for consistent title and embed description
name: string = randomString(5),
alt_text = randomString(10),
): Promise<PostResponse> {
let form: CreatePost = {
name,
url,
body,
alt_text,
community_id,
};
return api.createPost(form);

View file

@ -35,7 +35,13 @@ use lemmy_utils::{
spawn_try_task,
utils::{
slurs::check_slurs,
validation::{check_url_scheme, clean_url_params, is_valid_body_field, is_valid_post_title},
validation::{
check_url_scheme,
clean_url_params,
is_valid_alt_text_field,
is_valid_body_field,
is_valid_post_title,
},
},
};
use tracing::Instrument;
@ -56,7 +62,7 @@ pub async fn create_post(
check_slurs(&data.name, &slur_regex)?;
let body = process_markdown_opt(&data.body, &slur_regex, &context).await?;
let alt_text = process_markdown_opt(&data.alt_text, &slur_regex, &context).await?;
let alt_text = &data.alt_text;
let data_url = data.url.as_ref();
let url = data_url.map(clean_url_params); // TODO no good way to handle a "clear"
@ -64,7 +70,7 @@ pub async fn create_post(
is_valid_post_title(&data.name)?;
is_valid_body_field(&body, true)?;
is_valid_body_field(&alt_text, false)?;
is_valid_alt_text_field(alt_text)?;
check_url_scheme(&url)?;
check_url_scheme(&custom_thumbnail)?;
@ -128,7 +134,7 @@ pub async fn create_post(
.name(data.name.trim().to_string())
.url(url)
.body(body)
.alt_text(alt_text)
.alt_text(alt_text.clone())
.community_id(data.community_id)
.creator_id(local_user_view.person.id)
.nsfw(data.nsfw)

View file

@ -27,7 +27,13 @@ use lemmy_utils::{
error::{LemmyError, LemmyErrorExt, LemmyErrorType},
utils::{
slurs::check_slurs_opt,
validation::{check_url_scheme, clean_url_params, is_valid_body_field, is_valid_post_title},
validation::{
check_url_scheme,
clean_url_params,
is_valid_alt_text_field,
is_valid_body_field,
is_valid_post_title,
},
},
};
use std::ops::Deref;
@ -48,14 +54,14 @@ pub async fn update_post(
let slur_regex = local_site_to_slur_regex(&local_site);
check_slurs_opt(&data.name, &slur_regex)?;
let body = process_markdown_opt(&data.body, &slur_regex, &context).await?;
let alt_text = process_markdown_opt(&data.alt_text, &slur_regex, &context).await?;
let alt_text = &data.alt_text;
if let Some(name) = &data.name {
is_valid_post_title(name)?;
}
is_valid_body_field(&body, true)?;
is_valid_body_field(&alt_text, false)?;
is_valid_alt_text_field(alt_text)?;
check_url_scheme(&url)?;
check_url_scheme(&custom_thumbnail)?;
@ -118,7 +124,7 @@ pub async fn update_post(
name: data.name.clone(),
url,
body: diesel_option_overwrite(body),
alt_text: diesel_option_overwrite(alt_text),
alt_text: diesel_option_overwrite(alt_text.clone()),
nsfw: data.nsfw,
embed_title,
embed_description,

View file

@ -115,7 +115,13 @@ impl Object for ApubPost {
let attachment = self
.url
.clone()
.map(|url| Attachment::new(url.into(), self.url_content_type.clone()))
.map(|url| {
Attachment::new(
url.into(),
self.url_content_type.clone(),
self.alt_text.clone(),
)
})
.into_iter()
.collect();
@ -127,7 +133,6 @@ impl Object for ApubPost {
cc: vec![],
name: Some(self.name.clone()),
content: self.body.as_ref().map(|b| markdown_to_html(b)),
alt_text: self.alt_text.as_ref().map(|b| markdown_to_html(b)),
media_type: Some(MediaTypeMarkdownOrHtml::Html),
source: self.body.clone().map(Source::new),
attachment,
@ -204,10 +209,12 @@ impl Object for ApubPost {
// read existing, local post if any (for generating mod log)
let old_post = page.id.dereference_local(context).await;
let first_attachment = page.attachment.first();
let form = if !page.is_mod_action(context).await? {
let first_attachment = page.attachment.into_iter().map(Attachment::url).next();
let first_attachment_url = first_attachment.cloned().map(Attachment::url);
let url = if first_attachment.is_some() {
first_attachment
first_attachment_url
} else if page.kind == PageType::Video {
// we cant display videos directly, so insert a link to external video page
Some(page.id.inner().clone())
@ -216,6 +223,7 @@ impl Object for ApubPost {
};
check_url_scheme(&url)?;
let alt_text = first_attachment.cloned().and_then(Attachment::alt_text);
let local_site = LocalSite::read(&mut context.pool()).await.ok();
let allow_sensitive = local_site_opt_to_sensitive(&local_site);
let page_is_sensitive = page.sensitive.unwrap_or(false);
@ -235,7 +243,6 @@ impl Object for ApubPost {
let body = read_from_string_or_source_opt(&page.content, &page.media_type, &page.source);
let body = process_markdown_opt(&body, slur_regex, context).await?;
let alt_text = process_markdown_opt(&page.alt_text, slur_regex, context).await?;
let language_id =
LanguageTag::to_language_id_single(page.language, &mut context.pool()).await?;

View file

@ -52,7 +52,6 @@ pub struct Page {
#[serde(deserialize_with = "deserialize_one_or_many", default)]
pub(crate) cc: Vec<Url>,
pub(crate) content: Option<String>,
pub(crate) alt_text: Option<String>,
pub(crate) media_type: Option<MediaTypeMarkdownOrHtml>,
#[serde(deserialize_with = "deserialize_skip_error", default)]
pub(crate) source: Option<Source>,
@ -75,6 +74,8 @@ pub(crate) struct Link {
href: Url,
media_type: Option<String>,
r#type: LinkType,
/// Used for alt_text
name: Option<String>,
}
#[derive(Clone, Debug, Deserialize, Serialize)]
@ -83,6 +84,8 @@ pub(crate) struct Image {
#[serde(rename = "type")]
kind: ImageType,
url: Url,
/// Used for alt_text
name: Option<String>,
}
#[derive(Clone, Debug, Deserialize, Serialize)]
@ -91,6 +94,8 @@ pub(crate) struct Document {
#[serde(rename = "type")]
kind: DocumentType,
url: Url,
/// Used for alt_text
name: Option<String>,
}
#[derive(Clone, Debug, Deserialize, Serialize)]
@ -112,6 +117,17 @@ impl Attachment {
Attachment::Document(d) => d.url,
}
}
pub(crate) fn alt_text(self) -> Option<String> {
match self {
// url as sent by Lemmy (new)
Attachment::Link(l) => l.name,
// image sent by lotide
Attachment::Image(i) => i.name,
// sent by mobilizon
Attachment::Document(d) => d.name,
}
}
}
#[derive(Clone, Debug, Deserialize, Serialize)]
@ -169,18 +185,20 @@ impl Page {
impl Attachment {
/// Creates new attachment for a given link and mime type.
pub(crate) fn new(url: Url, media_type: Option<String>) -> Attachment {
pub(crate) fn new(url: Url, media_type: Option<String>, name: Option<String>) -> Attachment {
let is_image = media_type.clone().unwrap_or_default().starts_with("image");
if is_image {
Attachment::Image(Image {
kind: Default::default(),
url,
name,
})
} else {
Attachment::Link(Link {
href: url,
media_type,
r#type: Default::default(),
name,
})
}
}

View file

@ -101,6 +101,7 @@ pub enum LemmyErrorType {
InvalidPostTitle,
InvalidBodyField,
BioLengthOverflow,
AltTextLengthOverflow,
MissingTotpToken,
MissingTotpSecret,
IncorrectTotpToken,

View file

@ -19,6 +19,7 @@ const ALLOWED_POST_URL_SCHEMES: [&str; 3] = ["http", "https", "magnet"];
const BODY_MAX_LENGTH: usize = 10000;
const POST_BODY_MAX_LENGTH: usize = 50000;
const BIO_MAX_LENGTH: usize = 300;
const ALT_TEXT_MAX_LENGTH: usize = 300;
const SITE_NAME_MAX_LENGTH: usize = 20;
const SITE_NAME_MIN_LENGTH: usize = 1;
const SITE_DESCRIPTION_MAX_LENGTH: usize = 150;
@ -172,6 +173,18 @@ pub fn is_valid_bio_field(bio: &str) -> LemmyResult<()> {
max_length_check(bio, BIO_MAX_LENGTH, LemmyErrorType::BioLengthOverflow)
}
pub fn is_valid_alt_text_field(alt_text: &Option<String>) -> LemmyResult<()> {
if let Some(alt_text) = alt_text {
max_length_check(
alt_text,
ALT_TEXT_MAX_LENGTH,
LemmyErrorType::AltTextLengthOverflow,
)
} else {
Ok(())
}
}
/// Checks the site name length, the limit as defined in the DB.
pub fn site_name_length_check(name: &str) -> LemmyResult<()> {
min_length_check(name, SITE_NAME_MIN_LENGTH, LemmyErrorType::SiteNameRequired)?;