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

[HOLD for payment 2024-11-13] [$250] Report fields - Initial value field shows disabled list value but it is not shown in the list #48429

Closed
6 tasks done
IuliiaHerets opened this issue Sep 2, 2024 · 71 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Sep 2, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: v9.0.27-0
Reproducible in staging?: Y
Reproducible in production?: Y
Issue reported by: Applause Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to workspace settings > Report fields.
  3. Click Add field > Click Type > Select List.
  4. Click List values.
  5. Add a few list values.
  6. Return to the previous page.
  7. Click Initial value > Select any list value as default list value.
  8. Click List values.
  9. Disable all the list values.
  10. Save the list type report field.
  11. Click on the added list type report field.
  12. Click List values.
  13. Add a new list value.
  14. Return to previous page.
  15. Note that Initial value field shows the disabled list value, but when opening the page, the disabled list value is absent.

Expected Result:

The Initial value field should not display the disabled list value (disabled in Step 9) because the list value is disabled.
The field should be empty while clickable.

Actual Result:

The Initial value field shows the disabled list value, but it is not shown in the list in Initial value page.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6590738_1725293979256.20240903_000954.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021831343278141946652
  • Upwork Job ID: 1831343278141946652
  • Last Price Increase: 2024-10-22
  • Automatic offers:
    • akinwale | Reviewer | 104573420
    • abzokhattab | Contributor | 104573428
Issue OwnerCurrent Issue Owner: @CortneyOfstad
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 2, 2024
Copy link

melvin-bot bot commented Sep 2, 2024

Triggered auto assignment to @dylanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@IuliiaHerets
Copy link
Author

@dylanexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@etCoderDysto
Copy link
Contributor

etCoderDysto commented Sep 2, 2024

Edited by proposal-police: This proposal was edited at 2024-09-02 20:15:33 UTC.

Proposal

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

Report fields - Initial value field shows disabled list value but it is not shown in the list

What is the root cause of that problem?

We are not checking if the default value (reportField.defaultValue) is disabled before returning reportField.defaultValue in getReportFieldInitialValue

function getReportFieldInitialValue(reportField: PolicyReportField | null): string {
if (!reportField) {
return '';
}
if (reportField.type === CONST.REPORT_FIELD_TYPES.LIST) {

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

We should check if the default value is disabled first, and return empty string if it is disabled

function getReportFieldInitialValue(reportField: PolicyReportField | null): string {
    const indexOfDefaultValue = reportField?.values.indexOf(reportField?.defaultValue);
    const defalutValueDisabled = indexOfDefaultValue && reportField?.disabledOptions[indexOfDefaultValue];
    if (defalutValueDisabled) {
        return '';
    }

Note: We can move the suggested change into this if condition here, to make the change only to list report field and not text and date report fields.

What alternative solutions did you explore? (Optional)

@etCoderDysto
Copy link
Contributor

I am able to reproduce this.

@Nodebrute
Copy link
Contributor

Yes, I tested it again and the issue is reproducible.

@abzokhattab
Copy link
Contributor

abzokhattab commented Sep 2, 2024

Edited by proposal-police: This proposal was edited at 2024-09-02 20:43:53 UTC.

Proposal

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

In the report fields, the initial value field displays a disabled list value, but this value is not shown in the list.

What is the root cause of that problem?

  • When all list values are disabled, the InitialListValueSelector component is not rendered because the condition availableListValuesLength > 0 is not met:
    {inputValues[INPUT_IDS.TYPE] === CONST.REPORT_FIELD_TYPES.LIST && availableListValuesLength > 0 && (
    <InputWrapper
    InputComponent={InitialListValueSelector}
    inputID={INPUT_IDS.INITIAL_VALUE}
    label={translate('common.initialValue')}
    subtitle={translate('workspace.reportFields.listValuesInputSubtitle')}
    />
  • As a result, this useEffect inside the picker doesn't get the chance to set the current value of the initial value in the form to empty string because the component will not be rendered. and therefore keeps the staled value .. which is used when submitting the form

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

First solution:

  • A simple solution would be to hide the picker instead of preventing it from rendering.
  • This can be achieved by adding the prop isVisible=true to the InitialListValueSelector/index.tsx.Then, include this condition to return null if the value is false:
if (!isVisible) {
    return;
}
{inputValues[INPUT_IDS.TYPE] === CONST.REPORT_FIELD_TYPES.LIST && (
    <InputWrapper
        InputComponent={InitialListValueSelector}
        inputID={INPUT_IDS.INITIAL_VALUE}
        label={translate('common.initialValue')}
        isVisible={availableListValuesLength > 0}
        onInputChange={(value) => ReportField.setInitialCreateReportFieldsForm(value)}
        subtitle={translate('workspace.reportFields.listValuesInputSubtitle')}
    />
)}
  • This approach ensures that the useEffect will still execute even after all values are disabled.

Second solution:

another approach would be to show a placeholder or disable the menu item if the availableListValuesLength > 0 is false

Third solution:

Another approach would be to check if the default value is disabled when submitting the form. If it is, replace the default value with an empty string here.

    const submitForm = useCallback(
        (values: FormOnyxValues<typeof ONYXKEYS.FORMS.WORKSPACE_REPORT_FIELDS_FORM>) => {
            const initialValueIndex = formDraft?.[INPUT_IDS.LIST_VALUES].indexOf(values[INPUT_IDS.INITIAL_VALUE]) ?? -1;
            const initialValue = formDraft?.[INPUT_IDS.DISABLED_LIST_VALUES]?.[initialValueIndex] ? '' : values[INPUT_IDS.INITIAL_VALUE];
            ReportField.createReportField(policyID, {
                name: values[INPUT_IDS.NAME],
                type: values[INPUT_IDS.TYPE],
                initialValue,
            });
            Navigation.goBack();
        },
        [formDraft, policyID],
    );

or better would be to check if availableListValuesLength == 0 and assign the initial value to "" otherwise return the form's initial value

@dylanexpensify dylanexpensify added the External Added to denote the issue can be worked on by a contributor label Sep 4, 2024
Copy link

melvin-bot bot commented Sep 4, 2024

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

@melvin-bot melvin-bot bot changed the title Report fields - Initial value field shows disabled list value but it is not shown in the list [$250] Report fields - Initial value field shows disabled list value but it is not shown in the list Sep 4, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 4, 2024
Copy link

melvin-bot bot commented Sep 4, 2024

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

@dylanexpensify dylanexpensify changed the title [$250] Report fields - Initial value field shows disabled list value but it is not shown in the list [$50] Report fields - Initial value field shows disabled list value but it is not shown in the list Sep 4, 2024
Copy link

melvin-bot bot commented Sep 4, 2024

Upwork job price has been updated to $50

@hayes102
Copy link

hayes102 commented Sep 4, 2024

Proposal

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

In the report fields, the initial value field displays a disabled list value, but this value is not shown in the list.

What is the root cause of that problem?

inside the getReportFieldInitialValue function we dont check if the value is disabled or not

function getReportFieldInitialValue(reportField: PolicyReportField | null): string {
if (!reportField) {
return '';
}
if (reportField.type === CONST.REPORT_FIELD_TYPES.LIST) {

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

We should check if the default value is disabled, then return empty string

function getReportFieldInitialValue(reportField: PolicyReportField | null): string {
    if (!reportField) return ''; // we shouldnt remove this line as done in the first proposal 
const indexOfDefaultValue = reportField?.values?.indexOf(reportField.defaultValue) ?? -1;
const defaultValueDisabled = indexOfDefaultValue >= 0 && reportField?.disabledOptions?.[indexOfDefaultValue];
if (defaultValueDisabled) return ""

Copy link

melvin-bot bot commented Sep 4, 2024

📣 @hayes102! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@hayes102
Copy link

hayes102 commented Sep 4, 2024

Could we please update the price to match the other issues? This issue seems to be tricky to identify and address.. i believe it deserves a higher bounty, can u please confirm? @akinwale

I mean it seems that the company pays more for the simple upgrade version task. so can we please revise the price.

i am eager to get my first job

Copy link

melvin-bot bot commented Sep 9, 2024

@akinwale, @dylanexpensify Eep! 4 days overdue now. Issues have feelings too...

@melvin-bot melvin-bot bot added the Overdue label Sep 9, 2024
@dylanexpensify dylanexpensify changed the title [$50] Report fields - Initial value field shows disabled list value but it is not shown in the list [$250] Report fields - Initial value field shows disabled list value but it is not shown in the list Sep 10, 2024
Copy link

melvin-bot bot commented Sep 10, 2024

⚠️ This issue has had its price increased by 4x or more. Please review the issue and ensure the price is correct.

Copy link

melvin-bot bot commented Sep 10, 2024

Upwork job price has been updated to $250

@dylanexpensify
Copy link
Contributor

@akinwale can we review proposals? 🙏

Copy link

melvin-bot bot commented Sep 11, 2024

@akinwale, @dylanexpensify 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

@akinwale
Copy link
Contributor

We can move forward with @etCoderDysto's proposal here.

🎀👀🎀 C+ reviewed.

@abzokhattab
Copy link
Contributor

The issue is not assigned to me yet. The pr will be ready once assigned

Copy link

melvin-bot bot commented Oct 22, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@rezkiy37
Copy link
Contributor

rezkiy37 commented Oct 22, 2024

@abzokhattab are you going to implement your proposal or I can work on the issue?

The issue is not assigned to me yet. The pr will be ready once assigned

Bump @dylanexpensify

Copy link

melvin-bot bot commented Oct 22, 2024

@akinwale, @CortneyOfstad, @MonilBhavsar Still overdue 6 days?! Let's take care of this!

@CortneyOfstad
Copy link
Contributor

Sorry, was OoO so going to get this adjusted!

@abzokhattab assigning you now!

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 24, 2024
Copy link

melvin-bot bot commented Oct 24, 2024

📣 @akinwale 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

Copy link

melvin-bot bot commented Oct 24, 2024

📣 @abzokhattab 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@melvin-bot melvin-bot bot removed the Overdue label Oct 24, 2024
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Oct 27, 2024
@abzokhattab
Copy link
Contributor

PR is ready !

@CortneyOfstad
Copy link
Contributor

PR deployed to staging yesterday, so will keep an eye as to when it deploys to production!

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Nov 6, 2024
@melvin-bot melvin-bot bot changed the title [$250] Report fields - Initial value field shows disabled list value but it is not shown in the list [HOLD for payment 2024-11-13] [$250] Report fields - Initial value field shows disabled list value but it is not shown in the list Nov 6, 2024
Copy link

melvin-bot bot commented Nov 6, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Nov 6, 2024
Copy link

melvin-bot bot commented Nov 6, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.57-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-11-13. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Nov 6, 2024

@akinwale @CortneyOfstad The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Nov 12, 2024
@CortneyOfstad
Copy link
Contributor

Payment set for tomorrow!

@CortneyOfstad
Copy link
Contributor

Payment Summary

@akinwale — paid $250 via Upwork
@abzokhattab — paid $250 via Upwork

@github-project-automation github-project-automation bot moved this from Bugs and Follow Up Issues to Done in [#whatsnext] #expense Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
Archived in project
Development

No branches or pull requests