Skip to content

Commit 37b3fd9

Browse files
S8M1TS8M1T
committed
refactor: address PR review feedback
- Simplify settings.json endpoint to return URL-encoded string directly - Use HTTP status codes for error handling (200/500) - Update button text to show '(bringing preferences)' when transferring - Revert unnecessary variable renaming (response, data) - Remove confusing comments per maintainer feedback
1 parent d72e6e8 commit 37b3fd9

File tree

2 files changed

+25
-64
lines changed

2 files changed

+25
-64
lines changed

‎src/settings.rs‎

Lines changed: 15 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -300,63 +300,25 @@ pub async fn encoded_restore(req: Request<Body>) -> Result<Response<Body>, Strin
300300

301301
// Get current user settings as JSON for API consumption
302302
pub async fn get_json(req: Request<Body>) -> Result<Response<Body>, String> {
303-
// Create preferences - this should be safe but we wrap it for completeness
304303
let prefs = Preferences::new(&req);
305304

306-
// Try to encode preferences, but provide fallback if it fails
307-
let url_encoded = match prefs.to_urlencoded() {
308-
Ok(encoded) => Some(encoded),
309-
Err(e) => {
310-
eprintln!("Warning: Failed to encode preferences for settings transfer: {e}");
311-
None
312-
}
313-
};
314-
315-
// Create a response structure with encoded preferences for easy transfer
316-
let settings_data = serde_json::json!({
317-
"url_encoded": url_encoded,
318-
"success": url_encoded.is_some(),
319-
"preferences": {
320-
"theme": prefs.theme,
321-
"front_page": prefs.front_page,
322-
"layout": prefs.layout,
323-
"wide": prefs.wide,
324-
"comment_sort": prefs.comment_sort,
325-
"post_sort": prefs.post_sort,
326-
"blur_spoiler": prefs.blur_spoiler,
327-
"show_nsfw": prefs.show_nsfw,
328-
"blur_nsfw": prefs.blur_nsfw,
329-
"use_hls": prefs.use_hls,
330-
"hide_hls_notification": prefs.hide_hls_notification,
331-
"autoplay_videos": prefs.autoplay_videos,
332-
"hide_sidebar_and_summary": prefs.hide_sidebar_and_summary,
333-
"fixed_navbar": prefs.fixed_navbar,
334-
"hide_awards": prefs.hide_awards,
335-
"hide_score": prefs.hide_score,
336-
"disable_visit_reddit_confirmation": prefs.disable_visit_reddit_confirmation,
337-
"video_quality": prefs.video_quality,
338-
"remove_default_feeds": prefs.remove_default_feeds,
339-
"subscriptions": prefs.subscriptions,
340-
"filters": prefs.filters
305+
// Try to encode preferences and return directly or error with HTTP status
306+
match prefs.to_urlencoded() {
307+
Ok(encoded) => {
308+
Response::builder()
309+
.status(200)
310+
.header("content-type", "text/plain")
311+
.header("cache-control", "no-cache, no-store, must-revalidate")
312+
.body(encoded.into())
313+
.map_err(|e| format!("Failed to build response: {e}"))
341314
}
342-
});
343-
344-
let body = match serde_json::to_string(&settings_data) {
345-
Ok(json) => json,
346315
Err(e) => {
347-
eprintln!("Error serializing settings to JSON: {e}");
348-
return Response::builder()
316+
eprintln!("Warning: Failed to encode preferences for settings transfer: {e}");
317+
Response::builder()
349318
.status(500)
350-
.header("content-type", "application/json")
351-
.body(r#"{"error": "Failed to serialize settings", "success": false}"#.into())
352-
.map_err(|e| format!("Failed to build error response: {e}"));
319+
.header("content-type", "text/plain")
320+
.body("Failed to encode preferences".into())
321+
.map_err(|e| format!("Failed to build error response: {e}"))
353322
}
354-
};
355-
356-
Response::builder()
357-
.status(200)
358-
.header("content-type", "application/json")
359-
.header("cache-control", "no-cache, no-store, must-revalidate")
360-
.body(body.into())
361-
.map_err(|e| format!("Failed to build response: {e}"))
323+
}
362324
}

‎static/check_update.js‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,25 +40,24 @@ async function checkInstanceUpdateStatus() {
4040
async function checkOtherInstances() {
4141
try {
4242
// Fetch list of available instances
43-
const instancesResponse = await fetch('/instances.json');
44-
const instancesData = await instancesResponse.json();
45-
const randomInstance = instancesData.instances[Math.floor(Math.random() * instancesData.instances.length)];
43+
const response = await fetch('/instances.json');
44+
const data = await response.json();
45+
const randomInstance = data.instances[Math.floor(Math.random() * data.instances.length)];
4646
const instanceUrl = randomInstance.url;
4747

4848
// Fetch current user settings to transfer them to the new instance
4949
let targetUrl = instanceUrl + window.location.pathname;
50+
let text = "Visit Random Instance";
5051

5152
try {
5253
const settingsResponse = await fetch('/settings.json');
5354
if (settingsResponse.ok) {
54-
const settingsData = await settingsResponse.json();
55-
// Check if settings were successfully encoded and are available for transfer
56-
if (settingsData.success && settingsData.url_encoded) {
57-
targetUrl = instanceUrl + '/settings/restore/?' + settingsData.url_encoded + '&redirect=' + encodeURIComponent(window.location.pathname.substring(1));
58-
} else if (settingsData.error) {
59-
console.warn('Settings server error:', settingsData.error, '- visiting random instance without settings transfer');
55+
const urlEncoded = await settingsResponse.text();
56+
if (urlEncoded && urlEncoded.trim()) {
57+
targetUrl = instanceUrl + '/settings/restore/?' + urlEncoded + '&redirect=' + encodeURIComponent(window.location.pathname.substring(1));
58+
text += " (bringing preferences)";
6059
} else {
61-
console.warn('Settings encoding failed - visiting random instance without settings transfer');
60+
console.warn('Settings encoding returned empty - visiting random instance without settings transfer');
6261
}
6362
} else {
6463
console.warn('Could not fetch user settings (HTTP', settingsResponse.status + ') - visiting random instance without settings transfer');
@@ -70,7 +69,7 @@ async function checkOtherInstances() {
7069

7170
// Set the href of the <a> tag to the instance URL with path and settings included
7271
document.getElementById('random-instance').href = targetUrl;
73-
document.getElementById('random-instance').innerText = "Visit Random Instance";
72+
document.getElementById('random-instance').innerText = text;
7473
} catch (error) {
7574
console.error('Error fetching instances:', error);
7675
document.getElementById('update-status').innerText = '⚠️ Error checking other instances: ' + error;

0 commit comments

Comments
 (0)