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

[CP Staging] Fix crash for unknown action type when getting quick action title #39149

Merged
merged 5 commits into from
Mar 29, 2024
Merged

[CP Staging] Fix crash for unknown action type when getting quick action title #39149

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
57477ef
Fix crash for unknown action type when getting quick action
parasharrajat Mar 28, 2024
2e5c1a6
Add more actions
parasharrajat Mar 28, 2024
1fc252b
Update translations
parasharrajat Mar 29, 2024
1deb99a
Merge branch 'main' of github.com:Expensify/App into parasharrajat/qu…
parasharrajat Mar 29, 2024
e43acd0
QuickAction is optional prop
parasharrajat Mar 29, 2024
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
7 changes: 6 additions & 1 deletion src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,11 @@ function FloatingActionButtonAndPopover(props) {
return [];
}, [props.personalDetails, props.session.accountID, quickActionReport]);

const quickActionTitle = useMemo(() => {
const titleKey = getQuickActionTitle(props.quickAction.action);
return titleKey ? translate(titleKey) : '';
Copy link
Member Author

Choose a reason for hiding this comment

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

Now the title will be empty for unknown actions so should we return something instead of ''.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually the crash only occurs in dev enviornment

App/src/libs/Localize/index.ts

Lines 161 to 169 in f5cf3fb

if (Config.IS_IN_PRODUCTION || Config.IS_IN_STAGING) {
const phraseString: string = Array.isArray(phraseKey) ? phraseKey.join('.') : phraseKey;
Log.alert(`${phraseString} was not found in the en locale`);
if (userEmail.includes(CONST.EMAIL.EXPENSIFY_EMAIL_DOMAIN)) {
return CONST.MISSING_TRANSLATION;
}
return phraseString;
}
throw new Error(`${phraseKey} was not found in the default language`);

so i beleive we should not suppress it as this may result in unnoticed missing title, which otherwise will be caught if theres a crash

Copy link
Member Author

Choose a reason for hiding this comment

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

We are not suppressing localization errors but correcting the logic for title so that only when there is a title we translate it. I also added new keys for action.

Copy link
Contributor

@ishpaul777 ishpaul777 Mar 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

actually what i meant is if app is crashed when title key is not found in translation that should be the ideal behviour in dev enviornment (opinion), that will help us quickly spot the error in dev enviornment for future newly added actions, otherwise this can be missed.

Copy link
Contributor

Choose a reason for hiding this comment

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

lets leave it like this, my comment ^ is optional btw

}, [props.quickAction.action, translate]);

const navigateToQuickAction = () => {
switch (props.quickAction.action) {
case CONST.QUICK_ACTIONS.REQUEST_MANUAL:
Expand Down Expand Up @@ -339,7 +344,7 @@ function FloatingActionButtonAndPopover(props) {
? [
{
icon: getQuickActionIcon(props.quickAction.action),
text: translate(getQuickActionTitle(props.quickAction.action)),
text: quickActionTitle,
label: translate('quickAction.shortcut'),
isLabelHoverable: false,
floatRightAvatars: quickActionAvatars,
Expand Down
Loading