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.

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

[$500] Missing fields still shows error message even after selecting required fields #34945

Closed
6 tasks done
kavimuru opened this issue Jan 23, 2024 · 11 comments
Closed
6 tasks done
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@kavimuru
Copy link

kavimuru commented Jan 23, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 1.4.30-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @luacmartins
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1705808628895589

Action Performed:

Precondition : Have some violation rules for the expenses in the OD

  1. Create an expense report
  2. Open the receipt to view the details
  3. enter the required fields values (in this case category and tag fields)

Expected Result:

Error message should go away

Actual Result:

Error message still appears

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01724bf6068307f162
  • Upwork Job ID: 1749663634886561792
  • Last Price Increase: 2024-01-23
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 23, 2024
Copy link

melvin-bot bot commented Jan 23, 2024

Triggered auto assignment to @abekkala (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@kavimuru kavimuru added the External Added to denote the issue can be worked on by a contributor label Jan 23, 2024
@melvin-bot melvin-bot bot changed the title Missing fields still shows error message even after selecting required fields [$500] Missing fields still shows error message even after selecting required fields Jan 23, 2024
Copy link

melvin-bot bot commented Jan 23, 2024

Job added to Upwork: https://www.upwork.com/jobs/~01724bf6068307f162

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 23, 2024
Copy link

melvin-bot bot commented Jan 23, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav (External)

@wildan-m
Copy link
Contributor

wildan-m commented Jan 23, 2024

How can we add a violation rule in OD? This is how OD workspace setting looks in my account

Screenshot 2024-01-23 at 12 51 30

@wildan-m
Copy link
Contributor

wildan-m commented Jan 23, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

Missing fields still shows error message even after selecting required fields

What is the root cause of that problem?

We are not adding category/tag value to the error props

What changes do you think we should make in order to solve the problem?

We can add corresponding value to error props:

                    {shouldShowCategories && (
                        <MenuItemWithTopDescription
                            shouldShowRightIcon={!props.isReadOnly}
                            title={props.iouCategory}
                            description={translate('common.category')}
                            numberOfLinesTitle={2}
                            onPress={() => {
                                if (props.isEditingSplitBill) {
                                    Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(props.reportID, props.reportActionID, CONST.EDIT_REQUEST_FIELD.CATEGORY));
                                    return;
                                }
                                Navigation.navigate(ROUTES.MONEY_REQUEST_CATEGORY.getRoute(props.iouType, props.reportID));
                            }}
                            style={[styles.moneyRequestMenuItem]}
                            titleStyle={styles.flex1}
                            disabled={didConfirm}
                            interactive={!props.isReadOnly}
                            rightLabel={canUseViolations && Boolean(props.policy.requiresCategory) ? translate('common.required') : ''}
                            brickRoadIndicator={shouldDisplayFieldError && !props.iouCategory && canUseViolations && Boolean(props.policy.requiresCategory)? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''}
                            error={(shouldDisplayFieldError && !props.iouCategory && canUseViolations && Boolean(props.policy.requiresCategory)) ? translate('common.error.fieldRequired') : ''}
                        />
                    )}
                    {shouldShowTags && (
                        <MenuItemWithTopDescription
                            shouldShowRightIcon={!props.isReadOnly}
                            title={props.iouTag}
                            description={policyTagListName}
                            numberOfLinesTitle={2}
                            onPress={() => {
                                if (props.isEditingSplitBill) {
                                    Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(props.reportID, props.reportActionID, CONST.EDIT_REQUEST_FIELD.TAG));
                                    return;
                                }
                                Navigation.navigate(ROUTES.MONEY_REQUEST_TAG.getRoute(props.iouType, props.reportID));
                            }}
                            style={[styles.moneyRequestMenuItem]}
                            disabled={didConfirm}
                            interactive={!props.isReadOnly}
                            rightLabel={canUseViolations && Boolean(props.policy.requiresTag) ? translate('common.required') : ''}
                            brickRoadIndicator={shouldDisplayFieldError && !props.iouTag && canUseViolations && Boolean(props.policy.requiresTag)? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''}
                            error={(shouldDisplayFieldError && !props.iouTag && canUseViolations && Boolean(props.policy.requiresTag)) ? translate('common.error.fieldRequired') : ''}
                        />
                    )}

or PolicyUtils.getCleanedTagName(iouTag) for tag

Apply it for MoneyTemporaryForRefactorRequestConfirmationList and MoneyRequestConfirmationList files

What alternative solutions did you explore? (Optional)

@situchan
Copy link
Contributor

cc: @cead22 as violation related

@wildan-m
Copy link
Contributor

Proposal Updated

Mention the related files, add PolicyUtils.getCleanedTagName for tag

@wildan-m
Copy link
Contributor

Proposal Updated

Fix props, not rightLabel, but error props

@dukenv0307
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

Error message still appears

What is the root cause of that problem?

When we edit the category or tag field we don't clear the transaction violation for this field

What changes do you think we should make in order to solve the problem?

When we update any field we should clear the transaction violation message of this field in optimistic data and maybe BE also needs to update as well.

What alternative solutions did you explore? (Optional)

NA

@paultsimura
Copy link
Contributor

This is basically a dupe of #34541, and if it is decided to be fixed Externally, my proposal is the first in the queue 👀

@luacmartins luacmartins self-assigned this Jan 23, 2024
@luacmartins
Copy link
Contributor

Thanks for pointing that out @paultsimura! I think we can close this issue in favor of #34541. Thanks for the proposals everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests

8 participants