-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(core): add localization support for unified error messages #5624
Conversation
} | ||
|
||
impl UnifiedTranslations { | ||
pub async fn find( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pub async fn find( | |
pub async fn find_by_unified_code_unified_message_locale( |
) | ||
.await | ||
} | ||
pub async fn retrieve_translation( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would prefer is this was moved to the db
module, you can only keep the unique database queries here.
.map(|item| item.translation) | ||
} | ||
|
||
pub async fn update( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please indicate what fields are used to update / delete: update_by_field1_field2
etc.
You also don't need translation
to search for the record, right?
Default, | ||
)] | ||
#[diesel(table_name = unified_translations)] | ||
pub struct UnifiedTranslationsUpdateInternal { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add modified_at
as a mandatory field in UnifiedTranslationsUpdateInternal
.
"Translation missing for unified_code - {}, unified_message - {:?}, locale - {:?}", | ||
unified_code, | ||
unified_message, | ||
locale |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either use Debug
impl for all or Display
impl for all.
&self, | ||
_translation: storage::UnifiedTranslationsNew, | ||
) -> CustomResult<storage::UnifiedTranslations, errors::StorageError> { | ||
Err(errors::StorageError::MockDbError)? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a task item in #172.
-- Your SQL goes here | ||
-- Your SQL goes here | ||
-- Tables |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Remove these comments.
CREATE TABLE IF NOT EXISTS unified_translations ( | ||
unified_code VARCHAR(255) NOT NULL, | ||
unified_message VARCHAR(1024) NOT NULL, | ||
locale VARCHAR(255) NOT NULL DEFAULT 'en', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure providing a default from database side is a good idea, better to populate the default value from the application, if we absolutely need to.
crates/diesel_models/src/schema.rs
Outdated
#[max_length = 1024] | ||
translation -> Varchar, | ||
created_at -> Timestamp, | ||
last_modified -> Timestamp, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
last_modified -> Timestamp, | |
last_modified_at -> Timestamp, |
|
||
#[derive(Debug)] | ||
pub struct UnifiedTranslationsUpdate { | ||
pub unified_code: Option<String>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would need not update unified_code, unified_message and locale. Can we remove it from here is not required?
@@ -0,0 +1,2 @@ | |||
-- This file should undo anything in `up.sql` | |||
DROP TABLE unified_translations; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DROP TABLE unified_translations; | |
DROP TABLE IF EXISTS unified_translations; |
Type of Change
Description
Add localization support for unifed error messages.
Implementation:
Additional Changes
Motivation and Context
How did you test it?
step1 : Run data migrations after deployment . Make sure all unified codes and unified messages have their translations in all available languages.
step2: create a payment link with adyen as connector. Can vary the locale in headers .
step3 : create failed payment with random card and expiry. error code and error message should display in preferred locale.
step4 : payments retrieve (pass locale in headers)
Checklist
cargo +nightly fmt --all
cargo clippy