Skip to content

Commit 049db21

Browse files
fix(server): name-based event PR titles and tidier batch PR bodies (#3275)
Event submissions previously produced misleading PRs: the title used the opaque `event_<hash>` key, the body linked it to a 404 `/view/` page, labelled every event an "addition" even when it replaced an existing one, and embedded the poster full-bleed. Events now render by their human name, distinguishing "add" from "update", in an `event | image` table with a thumbnail-sized poster. Per-edit `### Edit N` headers (which auto-linked `#N` to issues) are replaced by `---` separators under a small `<sub>` label. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent a1e4b96 commit 049db21

9 files changed

Lines changed: 270 additions & 70 deletions

File tree

server/src/routes/feedback/batch_processor/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ pub async fn update_batch_pr_metadata(
7676

7777
// Append the new edit's description
7878
let updated_description = if current_description.is_empty() {
79-
format!("## Batched Edits\n\n### Edit #{edit_count}\n{new_edit_description}")
79+
format!("<sub>Batched edits</sub>\n\n{new_edit_description}")
8080
} else {
81-
format!("{current_description}\n\n---\n\n### Edit #{edit_count}\n{new_edit_description}")
81+
format!("{current_description}\n\n---\n\n{new_edit_description}")
8282
};
8383

8484
let github = GitHub::default();

server/src/routes/feedback/proposed_edits/addition/building.rs

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ use std::path::Path;
44
use serde::{Deserialize, Serialize};
55

66
use super::super::coordinate::Coordinate;
7-
use super::AppliableAddition;
87
use super::areatree::{AreatreeKind, format_line, insert_under};
98
use super::validation::{AdditionError, AdditionVariant, CollisionSource, RepoSnapshot};
9+
use super::{AppliableAddition, AppliedAddition};
1010

1111
const MAX_NAME_LEN: usize = 200;
1212

@@ -136,7 +136,7 @@ impl AppliableAddition for NewBuilding {
136136
Ok(())
137137
}
138138

139-
fn apply(&self, _key: &str, base_dir: &Path, _branch: &str) -> anyhow::Result<String> {
139+
fn apply(&self, _key: &str, base_dir: &Path, _branch: &str) -> anyhow::Result<AppliedAddition> {
140140
let areatree_path = base_dir
141141
.join("data")
142142
.join("processors")
@@ -167,14 +167,16 @@ impl AppliableAddition for NewBuilding {
167167
}
168168
}
169169

170-
Ok(self.coords.fenced_geojson_feature(&serde_json::json!({
171-
"kind": "new-building",
172-
"id": effective_id,
173-
"name": self.name,
174-
"node_kind": self.kind,
175-
"parent_id": self.parent_id,
176-
"building_prefixes": self.building_prefixes,
177-
})))
170+
Ok(AppliedAddition::created(
171+
self.coords.fenced_geojson_feature(&serde_json::json!({
172+
"kind": "new-building",
173+
"id": effective_id,
174+
"name": self.name,
175+
"node_kind": self.kind,
176+
"parent_id": self.parent_id,
177+
"building_prefixes": self.building_prefixes,
178+
})),
179+
))
178180
}
179181

180182
fn kind_label(&self) -> &'static str {
@@ -302,7 +304,8 @@ mod tests {
302304

303305
let summary = sample_building()
304306
.apply("0103", dir.path(), "branch")
305-
.unwrap();
307+
.unwrap()
308+
.summary;
306309
assert_snapshot!(summary, @r#"
307310
```geojson
308311
{

server/src/routes/feedback/proposed_edits/addition/event.rs

Lines changed: 43 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ use serde::Deserialize;
1010
use super::super::AppliableEdit as _;
1111
use super::super::coordinate::Coordinate;
1212
use super::super::image::Image;
13-
use super::AppliableAddition;
1413
use super::validation::{AdditionError, RepoSnapshot};
14+
use super::{AppliableAddition, AppliedAddition};
1515

1616
const MAX_NAME_LEN: usize = 200;
1717
/// The photo marker renders a 256×256 thumb crop, so smaller uploads would be upscaled.
@@ -239,29 +239,34 @@ impl AppliableAddition for NewEvent {
239239
Ok(())
240240
}
241241

242-
fn apply(&self, key: &str, base_dir: &Path, branch: &str) -> anyhow::Result<String> {
242+
fn apply(&self, key: &str, base_dir: &Path, branch: &str) -> anyhow::Result<AppliedAddition> {
243243
let csv_path = events_csv_path(base_dir);
244244
let raw = fs::read(&csv_path)?;
245245
// Content-addressed key, so always the first slot.
246246
let image = format!("/cdn/thumb/{key}_0.webp");
247247
// Identity = key: an existing row under this key makes the addition an update.
248-
let (verb, image_md) = if let Some(row) = matching_row_range(&raw, key)? {
249-
let image_md = self.image.replace(key, base_dir, branch)?;
248+
let replaced = if let Some(row) = matching_row_range(&raw, key)? {
249+
self.image.replace(key, base_dir, branch)?;
250250
self.replace_events_row(&image, &csv_path, &raw, row)?;
251-
("update", image_md)
251+
true
252252
} else {
253-
let image_md = self.image.apply(key, base_dir, branch)?;
253+
self.image.apply(key, base_dir, branch)?;
254254
self.append_events_row(&image, &csv_path)?;
255-
("new", image_md)
255+
false
256256
};
257257

258-
Ok(format!(
259-
"{verb} event `{name}` ({starts_at} - {ends_at}, org `{org}`)\n\n{image_md}",
260-
name = self.name,
261-
starts_at = format_de(&self.starts_at)?,
262-
ends_at = format_de(&self.ends_at)?,
263-
org = self.organising_org_id,
264-
))
258+
let verb = if replaced { "update" } else { "new" };
259+
Ok(AppliedAddition {
260+
summary: format!(
261+
"{verb} event \"{name}\" ({starts_at} - {ends_at}, org `{org}`)",
262+
name = self.name,
263+
starts_at = format_de(&self.starts_at)?,
264+
ends_at = format_de(&self.ends_at)?,
265+
org = self.organising_org_id,
266+
),
267+
image_url: Some(Image::raw_lg_url(key, branch)?),
268+
replaced,
269+
})
265270
}
266271

267272
fn kind_label(&self) -> &'static str {
@@ -514,14 +519,20 @@ mod tests {
514519
)
515520
.unwrap();
516521

517-
let summary = sample_event()
522+
let applied = sample_event()
518523
.apply("event_9d02ddd940c43f87", dir.path(), "branch")
519524
.unwrap();
520-
assert_snapshot!(summary, @r"
521-
update event `GARNIX Festival` (10.6.26 16:00 - 12.6.26 23:00, org `51897`)
522-
523-
![image showing event_9d02ddd940c43f87](https://raw.githubusercontent.com/TUM-Dev/NavigaTUM/refs/heads/branch/data/sources/img/lg/event_9d02ddd940c43f87_0.webp)
524-
");
525+
assert!(applied.replaced);
526+
assert_eq!(
527+
applied.summary,
528+
"update event \"GARNIX Festival\" (10.6.26 16:00 - 12.6.26 23:00, org `51897`)"
529+
);
530+
assert_eq!(
531+
applied.image_url.as_deref(),
532+
Some(
533+
"https://raw.githubusercontent.com/TUM-Dev/NavigaTUM/refs/heads/branch/data/sources/img/lg/event_9d02ddd940c43f87_0.webp"
534+
)
535+
);
525536

526537
let csv = fs::read_to_string(sources.join("events.csv")).unwrap();
527538
assert_snapshot!(csv, @r#"
@@ -609,14 +620,20 @@ mod tests {
609620
)
610621
.unwrap();
611622

612-
let summary = sample_event()
623+
let applied = sample_event()
613624
.apply("event_9d02ddd940c43f87", dir.path(), "branch")
614625
.unwrap();
615-
assert_snapshot!(summary, @r"
616-
new event `GARNIX Festival` (10.6.26 16:00 - 12.6.26 23:00, org `51897`)
617-
618-
![image showing event_9d02ddd940c43f87](https://raw.githubusercontent.com/TUM-Dev/NavigaTUM/refs/heads/branch/data/sources/img/lg/event_9d02ddd940c43f87_0.webp)
619-
");
626+
assert!(!applied.replaced);
627+
assert_eq!(
628+
applied.summary,
629+
"new event \"GARNIX Festival\" (10.6.26 16:00 - 12.6.26 23:00, org `51897`)"
630+
);
631+
assert_eq!(
632+
applied.image_url.as_deref(),
633+
Some(
634+
"https://raw.githubusercontent.com/TUM-Dev/NavigaTUM/refs/heads/branch/data/sources/img/lg/event_9d02ddd940c43f87_0.webp"
635+
)
636+
);
620637

621638
let csv = fs::read_to_string(sources.join("events.csv")).unwrap();
622639
assert_snapshot!(csv, @r#"

server/src/routes/feedback/proposed_edits/addition/mod.rs

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,31 @@ pub enum Addition {
2828

2929
pub trait AppliableAddition {
3030
fn validate(&self, key: &str, snap: &RepoSnapshot) -> Result<(), AdditionError>;
31-
fn apply(&self, key: &str, base_dir: &Path, branch: &str) -> anyhow::Result<String>;
31+
fn apply(&self, key: &str, base_dir: &Path, branch: &str) -> anyhow::Result<AppliedAddition>;
3232
fn kind_label(&self) -> &'static str;
3333
}
3434

35+
/// The result of applying an [`Addition`], as the PR description renders it.
36+
pub struct AppliedAddition {
37+
/// One-line human summary for the PR body.
38+
pub summary: String,
39+
/// Image to show beside the summary (events only).
40+
pub image_url: Option<String>,
41+
/// Whether an existing entry was replaced rather than created (events only).
42+
pub replaced: bool,
43+
}
44+
45+
impl AppliedAddition {
46+
/// A new addition with no image.
47+
pub fn created(summary: String) -> Self {
48+
Self {
49+
summary,
50+
image_url: None,
51+
replaced: false,
52+
}
53+
}
54+
}
55+
3556
impl Addition {
3657
fn as_appliable(&self) -> &dyn AppliableAddition {
3758
match self {
@@ -46,13 +67,26 @@ impl Addition {
4667
self.as_appliable().validate(key, snap)
4768
}
4869

49-
pub fn apply(&self, key: &str, base_dir: &Path, branch: &str) -> anyhow::Result<String> {
70+
pub fn apply(
71+
&self,
72+
key: &str,
73+
base_dir: &Path,
74+
branch: &str,
75+
) -> anyhow::Result<AppliedAddition> {
5076
self.as_appliable().apply(key, base_dir, branch)
5177
}
5278

5379
pub fn kind_label(&self) -> &'static str {
5480
self.as_appliable().kind_label()
5581
}
82+
83+
/// The event's name, or `None` for non-event additions.
84+
pub fn event_name(&self) -> Option<&str> {
85+
match self {
86+
Self::Event(e) => Some(&e.name),
87+
_ => None,
88+
}
89+
}
5690
}
5791

5892
#[cfg(test)]

server/src/routes/feedback/proposed_edits/addition/poi.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ use std::path::Path;
55
use serde::{Deserialize, Serialize};
66

77
use super::super::coordinate::Coordinate;
8-
use super::AppliableAddition;
98
use super::validation::{AdditionError, CollisionSource, RepoSnapshot};
9+
use super::{AppliableAddition, AppliedAddition};
1010

1111
const MAX_NAME_LEN: usize = 200;
1212
const MAX_KEY_LEN: usize = 64;
@@ -122,7 +122,7 @@ impl AppliableAddition for NewPoi {
122122
Ok(())
123123
}
124124

125-
fn apply(&self, key: &str, base_dir: &Path, _branch: &str) -> anyhow::Result<String> {
125+
fn apply(&self, key: &str, base_dir: &Path, _branch: &str) -> anyhow::Result<AppliedAddition> {
126126
let yaml_path = base_dir.join("data").join("sources").join("21_pois.yaml");
127127
let raw = fs::read_to_string(&yaml_path).unwrap_or_default();
128128
let mut map: BTreeMap<String, serde_yaml::Value> = if raw.trim().is_empty() {
@@ -147,12 +147,12 @@ impl AppliableAddition for NewPoi {
147147
let out = serde_yaml::to_string(&map)?;
148148
fs::write(&yaml_path, out)?;
149149

150-
Ok(format!(
150+
Ok(AppliedAddition::created(format!(
151151
"new POI `{key}` ({name}, usage `{usage}`, parent `{parent}`)",
152152
name = self.name,
153153
usage = self.usage_name,
154154
parent = self.parent,
155-
))
155+
)))
156156
}
157157

158158
fn kind_label(&self) -> &'static str {
@@ -263,7 +263,8 @@ mod tests {
263263

264264
let summary = sample_poi()
265265
.apply("validierungsautomat-99", dir.path(), "branch")
266-
.unwrap();
266+
.unwrap()
267+
.summary;
267268
assert_snapshot!(
268269
summary,
269270
@"new POI `validierungsautomat-99` (Validierungsautomat 99, usage `Validierungsautomat`, parent `5101.EG.917`)"

server/src/routes/feedback/proposed_edits/addition/room.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ use std::path::Path;
55
use serde::{Deserialize, Serialize};
66

77
use super::super::coordinate::Coordinate;
8-
use super::AppliableAddition;
98
use super::areatree::AreatreeKind;
109
use super::validation::{AdditionError, AdditionVariant, CollisionSource, RepoSnapshot};
10+
use super::{AppliableAddition, AppliedAddition};
1111

1212
#[derive(Debug, Deserialize, Serialize, Clone, utoipa::ToSchema)]
1313
pub struct RoomLink {
@@ -156,7 +156,7 @@ impl AppliableAddition for NewRoom {
156156
Ok(())
157157
}
158158

159-
fn apply(&self, key: &str, base_dir: &Path, _branch: &str) -> anyhow::Result<String> {
159+
fn apply(&self, key: &str, base_dir: &Path, _branch: &str) -> anyhow::Result<AppliedAddition> {
160160
let yaml_path = base_dir
161161
.join("data")
162162
.join("sources")
@@ -185,13 +185,13 @@ impl AppliableAddition for NewRoom {
185185

186186
self.coords.apply_to_csv(key, base_dir)?;
187187

188-
Ok(format!(
188+
Ok(AppliedAddition::created(format!(
189189
"new room `{key}` ({alt}, arch_name `{arch}`, usage_id {uid}) @ {coords:?}",
190190
alt = self.alt_name,
191191
arch = self.arch_name,
192192
uid = self.usage_id,
193193
coords = self.coords,
194-
))
194+
)))
195195
}
196196

197197
fn kind_label(&self) -> &'static str {
@@ -331,7 +331,8 @@ mod tests {
331331
let dir = setup_apply_dir();
332332
let summary = sample_room()
333333
.apply("5117.EG.103", dir.path(), "branch")
334-
.unwrap();
334+
.unwrap()
335+
.summary;
335336
assert_snapshot!(
336337
summary,
337338
@"new room `5117.EG.103` (Testraum, arch_name `EG103@5117`, usage_id 12) @ Coordinate { lat: 48.262, lon: 11.668 }"

0 commit comments

Comments
 (0)