[HOLD for payment 2023-06-13] [$1000] In the assign task flow, Value for description not changing

[kavimuru]

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

---

Action Performed:

1. Click on Assign Task in global create menu.
2. Add title and description.
3. Click on the next button.
4. On Confirm Task page, click on the Description
5. Change the value for description
6. Click on Next. You will go to Confirm Task menu
7. Click on back chevron button near heading
8. Description is not updated

Expected Result:

Description should be updated

Actual Result:

Description is not updated

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 / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.17.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
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-05-20.at.1.20.02.PM.mov

Recording.733.mp4

Expensify/Expensify Issue URL:
Issue reported by: @BhuvaneshPatil
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1684554959081519

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0159a2a911d29c5676
• Upwork Job ID: 1661123057838981120
• Last Price Increase: 2023-05-30

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[greg-schroeder]

I reproduced this. Applying External as I don't see any exact dupes

Potentially related #19343

[melvin-bot]

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

[melvin-bot]

Current assignee @greg-schroeder is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @tgolen (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[akinwale]

Proposal

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

In the assign task flow, the value for description on in the initial page is not updated if it is changed on the details page and the user navigates back to the initial page.

What is the root cause of that problem?

The taskDescription input field in the form on the initial page is not being updated with the description value stored in Onyx.

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

Update the NewTaskDetailsPage to retrieve the updated value from Onyx when the page loads. This can be achieved in a few steps:

1. Add task: { key: ONYXKEYS.TASK, }} to the withOnyx HOC in the compose call.
2. Update propTypes and defaultProps as required for props.task.description. ...task: { description: '', },
3. Add a state variable to track changes made to the description from the text input.

const [taskDescription, setTaskDescription] = useState(props.task.description);

4. Add a useEffect call to update the state with the description from Onyx if the current value is different.

useEffect(() => {
    if (taskDescription !== props.task.description) {
        setTaskDescription(props.task.description);
    }
}, [props.task.description]);

5. Update the TextInput with inputID taskDescription like so:

<TextInput
    inputID="taskDescription"
    defaultValue={props.task.description}
    label={props.translate('newTaskPage.descriptionOptional')}
    value={taskDescription}
    onChangeText={text => setTaskDescription(text)}
/>

What alternative solutions did you explore? (Optional)

None.

[dukenv0307]

Proposal

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

In the assign task flow, value for title and description not changing

What is the root cause of that problem?

In NewTaskDetailPage, we don't have any effect to refresh state when we goBack and don't subscribe task from Onyx to update title and input field

App/src/pages/tasks/NewTaskDetailsPage.js

Lines 78 to 91 in 7a42755

	<View style={styles.mb5}>
	<TextInput
	ref={(el) => (inputRef.current = el)}
	inputID="taskTitle"
	label={props.translate('newTaskPage.title')}
	/>
	</View>
	<View style={styles.mb5}>
	<TextInput
	inputID="taskDescription"
	defaultValue=""
	label={props.translate('newTaskPage.descriptionOptional')}
	/>
	</View>

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

In NewTaskDetailPage we could:

1. Subscribe task from Onyx with key ONYXKEYS.TASK and update propTypes and defaultProps corresponding
2. Create title and description state to update title and description field
3. Create a useEffect with depenences is props.task to update title and description state
4. Add value prop for title field value={title} and description field value={description}
5. Add onValueChange for each field to update state when input change. If we don't add this, we cannot edit title and description after go back this page

What alternative solutions did you explore? (Optional)

NA

Result

Screencast.from.25-05-2023.14.38.55.webm

[greg-schroeder]

@abdulrahuman5196 thoughts on the proposals above?

[tienifr]

Proposal

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

The title and description are not updated in AssignTask page

What is the root cause of that problem?

In

https://github.com/Expensify/App/blob/main/src/pages/tasks/NewTaskPage.js

We're using Form.js to render the form, it's data is only updated when user changes value manually. That why when we update title or description from other forms and go back, this form still shows stale data

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

• add new field shouldSync in task
• When we call setTitleValue or setDescriptionValue function, update shouldSync to true
• In NewTaskDetailPage subscribe task from onyx, if shouldSync is true -> update inputValues and reset shouldSync to false

    useEffect(()=>{
        if(props.task.shouldSync){
            formRef.current.setState({
                inputValues:{
                    taskTitle:props.task.title,
                    taskDescription: props.task.description
                }
            })
            TaskUtils.setShouldSync(false)
        }
    },[props.task.shouldSync])

Result

Screen.Recording.2023-05-26.at.17.11.31.mov

[BhuvaneshPatil]

Proposal

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

Title and Description value in Assign Task form not updating, after we change there values in next pages.

What is the root cause of that problem?

There are two possible reasons for these -

• The NewTaskPage components does not take any prop from onyx's task. That's why updating/re-rendering is not happening when those values are changed outside of that component.
  

  App/src/pages/tasks/NewTaskDetailsPage.js

  Lines 26 to 28 in 0070968

  	const defaultProps = {
  	betas: [],
  	};

• The Form component used in above mentioned component doesn't take any default values as of now, and if we check component tree it's not unmounted even if we go to another page i.e. Confirm Task. So the values that are present in both input fields are only particular to that component only.
  

  App/src/pages/tasks/NewTaskDetailsPage.js

  Lines 70 to 77 in 0070968

  	<Form
  	formID={ONYXKEYS.FORMS.NEW_TASK_FORM}
  	submitButtonText={props.translate('common.next')}
  	style={[styles.mh5, styles.mt5, styles.flexGrow1]}
  	validate={(values) => validate(values)}
  	onSubmit={(values) => onSubmit(values)}
  	enabledWhenOffline
  	>

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

There are few changes that we need to make -

1. Add task to props, that will subscribe to the task from onyx.
2. The current implementation of Form component uses draftValues as props, they are used to initialize the state.inputValues, we can use them for our use case.
   

   App/src/components/Form.js

   Lines 94 to 99 in 0070968

   	this.state = {
   	errors: {},
   	inputValues: {
   	...props.draftValues,
   	},
   	};

3. To make sure we are updating the values for the Form component, we can modify - componentDidUpdate() in Form component like following -

    componentDidUpdate(prevProps) {
      if (!_.isEqual(prevProps.draftValues, this.props.draftValues)) {
          this.setState((pre) => ({...pre, inputValues: {...pre.inputValues, ...this.props.draftValues}}));
      }
        if (prevProps.preferredLocale === this.props.preferredLocale) {
            return;
        }

        // Update the error messages if the language changes
        this.validate(this.state.inputValues);
    }

4. We can pass the values for draftValues as props in Form component.
5. As we don't want to save the input values from draft, we won't pass shouldSaveDraft prop in form inputs.

What alternative solutions did you explore? (Optional)

To avoid any regressions, we can add two props to the Form component.

1. intialValues - a key-value pair/object of [inputId]:<inital_value>
2. shouldInitializeWithValues - If this prop is passed we can conditionally initialize the form input values.

If we implement this approach, in future if such issue occurs this can be used.

Looking forward to your feedbacks.

Description.Error.Solving.-.Made.with.Clipchamp.mp4

[abdulrahuman5196]

@akinwale's proposal here #19460 (comment) looks good and works well. Root cause is also correct.
And as to note its not just description affected, task title is also affected by same issue. We should fix both here since its on the same page and @akinwale's proposal could be applied the same way for title as well.

🎀👀🎀
C+ Reviewed

~~cc: @tgolen ~~

[dukenv0307]

@abdulrahuman5196 Can you check my proposal here? My proposal mentioned both description and title bug and @akinwale proposal it's unnecessary to use defaultValue if use value props and onChangeText should replace by onValueChange when TextInput is wrapped by Form

[abdulrahuman5196]

@dukenv0307 Thank you for the proposal and reaching out. Just to note I did go through all the proposal's at this time before approving.
Both the proposals are almost same. I agree, Your proposal included further concerns of title also needs change and others as mentioned here #19460 (comment).
But IMO, these are minor improvements which could have easily solved during PR phase. So I have approved the proposal which came first since both are almost same.

[BhuvaneshPatil]

@abdulrahuman5196 can you please provide feedback and any improvements to my proposal.
The reason behind following the approach is we are not using state to store the values anywhere in the code when Form component is used.

[abdulrahuman5196]

@BhuvaneshPatil Thank you reaching out.
The first part of using task from Onyx is same in all proposals.
The second problem is that Form with TextInput currently won't change the value if the defaultValue is changed. This is a known issue and already have a conversation here - https://github.com/Expensify/App/pull/15672/files#r1139341904 (The conversation is not solved yet).
But I am not aligned to use draftValues to solve this problem, since it's purpose is to store draft values in form and show it again. IMO using the value prop of TextInput directly like the accepted proposal is more apt in this case.

[tienifr]

@abdulrahuman5196 What's about my proposal #19460 (comment). I don't think adding a new value and onChangeText is a good idea, we already have the logic to render value and handle onInputChange in Form.js, I also wonder if user's typing, the task is updated by pusher(update task from other devices), so the input will be changed (That is the reason we have to create the inputValues state in Form.js instead of using onyx value, and just only update inputValues when users change input). That why I just want to update the input value when setTitleValue or setDescriptionValue is executed.

[abdulrahuman5196]

Hi @tienifr , Thank you for the proposal. But I am not aligned with the idea of adding a new field of shouldSync just to solve this issue.

I don't think adding a new value and onChangeText is a good idea, we already have the logic to render value and handle onInputChange in Form.js

I don't think its forbidden to use value or onChangeText(onValueChange in Form) in Forms, infact Forms expose these params - https://github.com/Expensify/App/blob/main/contributingGuides/FORMS.md. But in most cases you won't need it since Form has a default behaviour and its handled internally by Form. But for our current issue we need more than the default behaviour.

I also wonder if user's typing, the task is updated by pusher

I don't think we should worry more about this. These are just draft values stored in onyx.

[melvin-bot]

Current assignee @abdulrahuman5196 is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

Triggered auto assignment to @aldo-expensify (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[tgolen]

Unfortunately, I need to reassign this one due to going on extended leave.

[greg-schroeder]

@aldo-expensify thanks for stepping in for Tim! can you take a look at the C+ reviewed proposal here and let us know if we're good to proceed?

[aldo-expensify]

I had a look at the issue + @akinwale 's proposal and it does look good to me.

Thanks @abdulrahuman5196 for the good handling of the issue and answering all questions about other proposals :)

[melvin-bot]

📣 @akinwale You have been assigned to this job by @aldo-expensify!
Please apply to this job in Upwork 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 📖

[akinwale]

@aldo-expensify @abdulrahuman5196 I have applied to the Upwork job and the PR is ready for review. Thanks.

[aldo-expensify]

Thanks!

[greg-schroeder]

C+ review complete, looks like PR is waiting on @aldo-expensify for a final review

[greg-schroeder]

Merged, awaiting deploy to prod

[melvin-bot]

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

[melvin-bot]

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

• fix: set initial task title and description values from Onyx if present #19908

If no regressions arise, payment will be issued on 2023-06-13. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter - @BhuvaneshPatil
• Contributor that fixed the issue - @akinwale
• Contributor+ that helped on the issue and/or PR - @abdulrahuman5196

https://www.upwork.com/jobs/~0159a2a911d29c5676

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@abdulrahuman5196] The PR that introduced the bug has been identified. Link to the PR:
• [@abdulrahuman5196] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@abdulrahuman5196] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@abdulrahuman5196] Determine if we should create a regression test for this bug.
• [@abdulrahuman5196] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@greg-schroeder] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[abdulrahuman5196]

BugZero Checklist

The PR that introduced the bug has been identified. Link to the PR:
The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

Not a regression. It is a miss during the original tasks implementation.

Determine if we should create a regression test for this bug.

yes

If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

1. Click on Assign Task in global create menu.
2. Add title and description.
3. Click on the next button.
4. On Confirm Task page, click on the Description
5. Change the value for description
6. Click on Next. You will go to Confirm Task menu
7. Do steps 4 to 7 again for title
8. Click on back chevron button near heading
9. Title and Description is should have been updated to new value

[greg-schroeder]

Offers sent to @BhuvaneshPatil @akinwale @abdulrahuman5196

[abdulrahuman5196]

Accepted @greg-schroeder

[akinwale]

Offers sent to @BhuvaneshPatil @akinwale @abdulrahuman5196

Accepted, thanks!

[BhuvaneshPatil]

Accepted

[greg-schroeder]

Paid, regression test update filed: https://github.com/Expensify/Expensify/issues/291643