Skip to content

Conversation

@lkocman
Copy link
Collaborator

@lkocman lkocman commented Nov 7, 2025

Needed for prod, I don't want to kill db in middle of transaction :-)

@lkocman lkocman requested a review from Copilot November 7, 2025 15:56
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds internationalization (i18n) support to the openSUSE Kudos application using the gettext system with vue3-gettext. It also includes backend improvements for graceful shutdown handling.

Key changes:

  • Integration of vue3-gettext for frontend translations with dynamic locale loading
  • Scripts for extracting and compiling gettext translation files
  • Addition of Czech language support including a playful "Brněnské Hantec" dialect
  • Backend graceful shutdown implementation for proper cleanup on termination

Reviewed Changes

Copilot reviewed 12 out of 14 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
frontend/src/main.js Adds gettext plugin initialization with dynamic locale discovery from JSON files
frontend/src/components/LanguageToggle.vue New component for cycling through available languages
scripts/extract-gettext.js Script to extract translatable strings from Vue/JS files into POT format
scripts/compile-gettext.js Script to compile PO files into JSON format for runtime use
package.json Adds i18n-related dependencies and npm scripts for translation workflows
frontend/locale/*.json Language-specific translation files (Czech, English)
frontend/locale/messages.pot Generated POT template file with all extractable strings
frontend/locale/gettext.json Compiled translations in JSON format
frontend/public/language.svg Language toggle icon asset
backend/src/app.js Adds graceful shutdown handlers for SIGINT/SIGTERM signals

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

if (match) {
const code = match[1];
translations[code] = content.default || content;
const langName =
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The content.meta property access may fail if the JSON file structure is incorrect. The code assumes that locale JSON files have a meta property, but this could cause runtime errors if the property is missing or malformed.

Consider adding a safer fallback:

const langName =
  content.default?.meta?.LanguageName ||
  content.meta?.LanguageName ||
  new Intl.DisplayNames([code], { type: "language" }).of(code) ||
  code;
Suggested change
const langName =
const langName =
content.default?.meta?.LanguageName ||

Copilot uses AI. Check for mistakes.
const match = path.match(/locale\/([a-zA-Z_-]+)\.json$/);
if (match) {
const code = match[1];
translations[code] = content.default || content;
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The translations object is built from JSON files but vue3-gettext expects a specific structure. The current implementation assigns the entire content to translations[code], but based on the JSON files shown (e.g., cs.json, en.json), the structure appears to be { meta: {...}, [code]: {...} }.

This mismatch could cause translations to fail. Consider:

translations[code] = content[code] || content.default?.[code] || {};
Suggested change
translations[code] = content.default || content;
translations[code] = content[code] || content.default?.[code] || {};

Copilot uses AI. Check for mistakes.
const translations = {};

for (const [path, content] of Object.entries(localeModules)) {
const match = path.match(/locale\/([a-zA-Z_-]+)\.json$/);
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex pattern /locale\/([a-zA-Z_-]+)\.json$/ allows locale codes with underscores and hyphens, but BCP 47 language tags use hyphens (e.g., en-US), not underscores. While cs_HANTEC uses an underscore, this inconsistency could cause confusion.

Consider standardizing on either hyphens or underscores, and document the expected format.

Suggested change
const match = path.match(/locale\/([a-zA-Z_-]+)\.json$/);
// Only allow BCP 47 language tags (e.g., en-US, zh-Hant). Locale filenames must use hyphens, not underscores.
const match = path.match(/locale\/([a-zA-Z-]+)\.json$/);

Copilot uses AI. Check for mistakes.
Comment on lines +26 to +29
const langName =
content.meta?.LanguageName ||
new Intl.DisplayNames([code], { type: "language" }).of(code) ||
code;
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Intl.DisplayNames API may not be supported in all browsers, and could throw an error if the locale code is invalid. Add error handling:

let langName = code;
try {
  langName = content.meta?.LanguageName ||
    new Intl.DisplayNames([code], { type: "language" }).of(code) ||
    code;
} catch (e) {
  console.warn(`Could not get display name for locale ${code}:`, e);
}
availableLanguages[code] = langName;
Suggested change
const langName =
content.meta?.LanguageName ||
new Intl.DisplayNames([code], { type: "language" }).of(code) ||
code;
let langName = code;
try {
langName =
content.meta?.LanguageName ||
new Intl.DisplayNames([code], { type: "language" }).of(code) ||
code;
} catch (e) {
console.warn(`Could not get display name for locale ${code}:`, e);
}

Copilot uses AI. Check for mistakes.
Comment on lines +226 to +251
async function gracefulShutdown(signal) {
console.log(`\n🛑 Received ${signal}. Shutting down gracefully...`);

if (server) {
server.close(async (err) => {
if (err) {
console.error("❌ Error closing server:", err);
}
try {
console.log("💾 Closing Prisma connection...");
await prisma.$disconnect();
console.log("✅ Database disconnected cleanly.");
} catch (e) {
console.error("⚠️ Error disconnecting database:", e);
}

console.log("👋 Bye!");
process.exit(0);
});

// Optional safety timeout (10s)
setTimeout(() => {
console.warn("⏱ Forced exit after timeout.");
process.exit(1);
}, 10000).unref();
}
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The graceful shutdown implementation has a potential issue: if server.close() callback is never called (e.g., if there are hanging connections), the process won't exit until the 10-second timeout.

Consider tracking active connections and closing them explicitly, or reducing the timeout duration to 5 seconds for faster feedback during development.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant