Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix: 'Link sent!' message is not dynamically updated when changing language preference #19480

Merged
merged 3 commits into from
Jun 9, 2023
Merged

Fix: 'Link sent!' message is not dynamically updated when changing language preference #19480

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
41d0246
dynamic translate for unlink sent link
Pujan92 May 23, 2023
163dedd
Merge remote-tracking branch 'upstream/main' into fix/18827
Pujan92 May 24, 2023
1843c73
Merge branch 'main' into fix/18827
Pujan92 May 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 1 addition & 41 deletions src/libs/actions/Session/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,45 +165,6 @@ function resendValidateCode(login = credentials.login) {
API.write('RequestNewValidateCode', {email: login}, {optimisticData, successData, failureData});
}

/**
* Request a new validate / magic code for user to sign in automatically with the link
*
* @param {String} [login]
*/
function resendLinkWithValidateCode(login = credentials.login) {
cristipaval marked this conversation as resolved.
Show resolved Hide resolved
const optimisticData = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: ONYXKEYS.ACCOUNT,
value: {
isLoading: true,
message: null,
},
},
];
const successData = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: ONYXKEYS.ACCOUNT,
value: {
isLoading: false,
message: Localize.translateLocal('validateCodeModal.successfulNewCodeRequest'),
},
},
];
const failureData = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: ONYXKEYS.ACCOUNT,
value: {
isLoading: false,
message: null,
},
},
];
API.write('RequestNewValidateCode', {email: login}, {optimisticData, successData, failureData});
}

cristipaval marked this conversation as resolved.
Show resolved Hide resolved
/**
* Checks the API to see if an account exists for the given login
*
Expand Down Expand Up @@ -717,7 +678,7 @@ function requestUnlinkValidationLink() {
key: ONYXKEYS.ACCOUNT,
value: {
isLoading: false,
message: Localize.translateLocal('unlinkLoginForm.linkSent'),
message: 'unlinkLoginForm.linkSent',
Copy link
Member

Choose a reason for hiding this comment

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

This might not work always. What if we receive an error from the backend from this API? In that case, the page will crash due to the wrong translate key.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@parasharrajat for the error we are not using the prop message but we are using the errors(props.account.errors), so it won't create an issue.

App/src/pages/signin/UnlinkLoginForm.js

Lines 72 to 77 in 34ccc82

{!_.isEmpty(props.account.errors) && (
<DotIndicatorMessage
style={[styles.mb5]}
type="error"
messages={props.account.errors}
/>

Copy link
Member

Choose a reason for hiding this comment

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

I meant to say a message in general. Error case is fine, but the message key is also used to show success response, etc. I am not sure if this is completely safe to change. A proper solution will be to use a different key than the message for translation purposes like messageKey. So we can use this key to show a translated message and let the message be completely backend message. Though, this will add the overhead of managing both properties.

I am not saying that we should do that in this PR but a better implementation.

For this, are you sure that we are safe to change the message key here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is safe to change in this case bcoz for particular this page we aren't using any backend response of account.message for success and on link press we are cleaning these props within optimisticData.

Copy link
Member

Choose a reason for hiding this comment

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

OK. Thanks.

},
},
];
Expand Down Expand Up @@ -872,7 +833,6 @@ export {
signOutAndRedirectToSignIn,
resendValidationLink,
resendValidateCode,
resendLinkWithValidateCode,
resetPassword,
resendResetPassword,
requestUnlinkValidationLink,
Expand Down
2 changes: 1 addition & 1 deletion src/pages/signin/UnlinkLoginForm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ const UnlinkLoginForm = (props) => {
<DotIndicatorMessage
style={[styles.mb5, styles.flex0]}
type="success"
messages={{0: props.account.message}}
messages={{0: props.translate(props.account.message)}}
/>
)}
{!_.isEmpty(props.account.errors) && (
Expand Down