Skip to content

fix(ui): improve i18n toast and duplicate slug error handling #7254

Draft
anonatul wants to merge 3 commits intolouislam:masterfrom
anonatul:fix/ui-ux-i18n-toast
Draft

fix(ui): improve i18n toast and duplicate slug error handling #7254
anonatul wants to merge 3 commits intolouislam:masterfrom
anonatul:fix/ui-ux-i18n-toast

Conversation

@anonatul
Copy link
Copy Markdown

@anonatul anonatul commented Apr 8, 2026

Summary

In this pull request, the following changes are made:

  • Replaced a hardcoded success toast in src/pages/Details.vue with i18n key testNotificationRequested.

  • Added testNotificationRequested to src/lang/en.json so it is translatable.

  • Improved duplicate slug error handling in src/pages/AddStatusPage.vue by checking common DB error messages (unique constraint, duplicate entry, already exists) and showing a friendly localized message.

  • Relates to #issue-number

  • Resolves #issue-number

Please follow this checklist to avoid unnecessary back and forth (click to expand)
  • ⚠️ If there are Breaking change (a fix or feature that alters existing functionality in a way that could cause issues) I have called them out
  • 🧠 I have disclosed any use of LLMs/AI in this contribution and reviewed all generated content.
    I understand that I am responsible for and able to explain every line of code I submit.
  • 🔍 Any UI changes adhere to visual style of this project.
  • 🛠️ I have self-reviewed and self-tested my code to ensure it works as expected.
  • 📝 I have commented my code, especially in hard-to-understand areas (e.g., using JSDoc for methods).
  • 🤖 I added or updated automated tests where appropriate.
  • 📄 Documentation updates are included (if applicable).
  • 🧰 Dependency updates are listed and explained.
  • ⚠️ CI passes and is green.

AI/LLM Usage Disclosure

  • I wrote and finalized the code changes myself.
  • I manually reviewed, tested, and can explain every change.

Screenshots for Visual Changes

No visual layout change (toast text/error handling only), so screenshots are not applicable.

@anonatul anonatul marked this pull request as ready for review April 8, 2026 14:43
Copilot AI review requested due to automatic review settings April 8, 2026 14:43
@github-actions github-actions bot added the pr:needs review this PR needs a review by maintainers or other community members label Apr 8, 2026
Copy link
Copy Markdown
Contributor

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 improves user-facing messaging in the Vue frontend by ensuring a “test notification requested” success toast is translatable via i18n and by broadening duplicate-slug detection for status page creation errors across common database backends.

Changes:

  • Replaced a hardcoded “test notification requested” toast in Details.vue with an i18n key.
  • Added the new testNotificationRequested entry to en.json.
  • Expanded duplicate-slug error matching in AddStatusPage.vue to handle common “unique/duplicate/already exists” DB message variants.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/pages/Details.vue Switches toast to use an i18n key for the “test notification requested” message.
src/pages/AddStatusPage.vue Improves duplicate slug detection by matching additional common DB error message patterns.
src/lang/en.json Adds the testNotificationRequested translation string.

Comment on lines +96 to +101
const msg = res.msg.toLowerCase();
if (
msg.includes("unique constraint") ||
msg.includes("duplicate entry") ||
msg.includes("already exists")
) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

how did you come up with these 3 strigns, where are they coming from?

This beind different computes for me, but I don't understand why 3.. we only have 2 possible DB backends

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yes actually those 3 strings are not 3 DB backends they’re 3 possible error message fragments that can be returned in res.msg for a unique or duplicate violation

Copy link
Copy Markdown
Collaborator

@CommanderStorm CommanderStorm Apr 9, 2026

Choose a reason for hiding this comment

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

Which error messages would this be and how can I reproduce them?

To explain why I am asking this:
The reproducing is important to me to ensure that we don't accidentally merge code which was hallucinated. I am saying that this is, we just have to be a bit more carfull as a large-ish OSS project. -> https://github.com/louislam/uptime-kuma/pulls?q=is%3Apr+label%3Aai-slop+is%3Aclosed

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

i get it.. here unique constraint covers SQLite's default error message. duplicate entry handles MariaDB/MySQL, which throws ER_DUP_ENTRY. already exists is a catch-all for anything generic coming out of the ORM.

To reproduce it: i spin up Kuma with MariaDB instead of the default SQLite, create a status Page with a slug like test-slug, then try to create another one with the same slug. without this fix, MariaDB users just get a raw database error instead of the friendly validation message, because the old code only checked for SQLite's exact error string

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

So if the sqlite error is when running sqlite and the mariadb error is for mariadb, how is the third error created?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

the third one isn't tied to any specific DB engine.. "already exists" is there because res.msg doesn't always contain the raw driver error

if you'd rather only match known raw DB strings, happy to drop it and keep just the sqllite and mariadb patterns

i added it mainly to avoid regressions when the backend abstracts the message

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

under which case can you trigger this? Why exactly did you add this?

I am fine with it if there is a reason

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

i added it defensively because res.msg isn't a stable API. depending on the DB, driver, or how the ORM wraps things,

the UI could end up receiving a normalized message like "status page already exists" or "record already exists" instead of the raw sqlite or mariadb text and in that case we'd still want to show the friendly toast rather than nothing

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

please only add things that you:

  • have tested
  • actually fix a problem, add a feature or make the code better

Random "savety" checks don't do that

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

my apologies

@CommanderStorm CommanderStorm marked this pull request as draft April 11, 2026 03:56
@CommanderStorm CommanderStorm added pr:please address review comments this PR needs a bit more work to be mergable and removed pr:needs review this PR needs a review by maintainers or other community members labels Apr 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:please address review comments this PR needs a bit more work to be mergable

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants