[HOLD for payment 2022-11-07] [$250] BUG: Dev Console error when edit message reported by @aimane-chnaif

[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. Login with any account
2. Go to any chat
3. Long tap any message
4. Tap "Edit comment"

Expected Result:

No console errors

Actual Result:

Console error Warning: Failed prop type: The prop isComposerFullSize is marked as required in Composer, but its value is undefined.

Workaround:

unknown

Platform:

Where is this issue occurring?

• iOS
• Android

Version Number: 1.2.18-2
Reproducible in staging?: Need reproduction
Reproducible in production?: Need reproduction
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

console.error.mov

Expensify/Expensify Issue URL:
Issue reported by: @aimane-chnaif
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1666283934468759

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @lschurr (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[mallenexpensify]

@lschurr new BZ chore SO is here, we want to ensure BZ stays assigned to all issues so we can get to BugZero,

[melvin-bot]

@lschurr Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

@lschurr Whoops! This issue is 2 days overdue. Let's get this updated quick!

[lschurr]

Triaging this today.

[lschurr]

Tagging Eng since this is a dev issue that I can't reproduce.

[melvin-bot]

Triggered auto assignment to @dangrous (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[dangrous]

Am indeed seeing this on dev. Since its only a warning, though, I think we can ignore it for now as it's not user facing, so I'll close this. However, ccing @neil-marcellini since it looks like it's related to this PR - #9031 - in case there's a reason you can see to update, or it's a super quick fix?

I think it's talking about this Composer here

App/src/pages/home/report/ReportActionItemMessageEdit.js

Line 202 in 83676c1

<Composer

though a little hard to tell since the stack trace is wonky due to the simulator...

[mallenexpensify]

We are compensating for bug reports we fix for console errors but... I hadn't thought of examples where they're only on dev. Addressed internally https://expensify.slack.com/archives/C02NK2DQWUX/p1666661515587739?thread_ts=1663583691.520459&cid=C02NK2DQWUX

[aimane-chnaif]

This still happens in latest version (dev only).
isComposerFullSize is a flag to check if composer is expanded or collapsed.
As long as edit message composer is always collapsed (no case of expanded), the solution is quite simple:
Add isComposerFullSize={false} to

App/src/pages/home/report/ReportActionItemMessageEdit.js

Line 202 in 83676c1

<Composer

If we want to support expand in edit message composer in the future, we can update this flag accordingly.

[neil-marcellini]

Yes @aimane-chnaif has the solution. I think we could also make it optional and default to false.

[neil-marcellini]

We should keep this open and fix it even though it's just a prop type warning.

[aimane-chnaif]

Proposal

(if this GH can be exported to contributors)

Solution 1: (the case when isComposerFullSize is required prop)
Add isComposerFullSize={false} to

App/src/pages/home/report/ReportActionItemMessageEdit.js

Line 202 in 83676c1

<Composer

Solution 2: (the case when isComposerFullSize is optional prop)
https://github.com/Expensify/App/blob/main/src/components/Composer/index.android.js

    /** Whether the composer is full size */
-   isComposerFullSize: PropTypes.bool.isRequired,
+   isComposerFullSize: PropTypes.bool,

    /** General styles to apply to the text input */
    // eslint-disable-next-line react/forbid-prop-types
    style: PropTypes.any,

};

const defaultProps = {
    shouldClear: false,
    onClear: () => {},
    autoFocus: false,
    isDisabled: false,
    forwardedRef: null,
    selection: {
        start: 0,
        end: 0,
    },
    isFullComposerAvailable: false,
    setIsFullComposerAvailable: () => {},
+   isComposerFullSize: false,
    style: null,
};

Same applies to iOS (https://github.com/Expensify/App/blob/main/src/components/Composer/index.ios.js)

And I see that isComposerFullSize prop type is not defined in desktop/web (https://github.com/Expensify/App/blob/main/src/components/Composer/index.js)
We can add it to propTypes and defaultProps similar to above android code

[melvin-bot]

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

[melvin-bot]

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

[lschurr]

Job posted:
https://www.upwork.com/ab/applicants/1584959643548332032/job-details
https://www.upwork.com/jobs/~01a688c0ae4629126f

[eVoloshchak]

I agree with @aimane-chnaif's proposal (second solution), we should make isComposerFullSize an optional prop

🎀👀🎀 C+ reviewed!
cc: @dangrous

[dangrous]

Yep, that second proposal sounds great to me as well! I'll assign and lets go ahead with a PR.

[melvin-bot]

📣 @aimane-chnaif You have been assigned to this job by @dangrous!
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 📖

[dangrous]

@lschurr this is reviewed and approved. Do we need to do anything else on this issue/Upwork/etc. other than merge? This is my first one!

[lschurr]

Also my first one @dangrous - I asked here for clarification on next steps.

[melvin-bot]

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

• A regression test has been added or updated so that the same bug will not reach production again. Link to the updated test here:
• 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 #contributor-plus 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:
• Payment has been made to the issue reporter (if applicable)
• Payment has been made to the contributor that fixed the issue (if applicable)
• Payment has been made to the contributor+ that helped on the issue (if applicable)

[dangrous]

also @aimane-chnaif just wanted to share this info below in case you weren't aware how things work re: upwork, etc.!

I know there's a lot of information in our contributing guidelines, so some points to take note of:

1. Once your PR is merged, you can be hired for another issue. Once you've completed a few issues, we may start hiring you for more than one issue at a time.
2. Once your PR is deployed to our staging servers, it will undergo quality assurance (QA) testing. If we find that this doesn't work as expected or causes a regression, you'll be responsible for fixing it. Typically we would revert this PR and give you another chance to create a similar PR without causing a regression. (I don't imagine this will happen with this PR, but it's something to be aware of)
3. Once your PR is deployed to production, we start a 7-day timer. After it has been on production for 7 days without causing any regressions, then we pay out the Upwork job.

So it might take a while before you're paid for your work, but we typically post multiple new jobs every day, so there's plenty of opportunity. I hope you've had a positive experience contributing to this repo!

[aimane-chnaif]

@dangrous thanks for the info. I already know this process since I completed lots of jobs already :)
After deploying to production, @MelvinBot will automatically comment here so no need to worry

[dangrous]

@aimane-chnaif awesome! Sounds great.

[melvin-bot]

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

• add defaultProps of isComposerFullSize in Composer #12149

If no regressions arise, payment will be issued on 2022-11-07. 🎊

[lschurr]

Hi @aimane-chnaif - Could you please apply for the job in Upwork so that we can hire you? https://www.upwork.com/jobs/~01a688c0ae4629126f

[aimane-chnaif]

@lschurr applied with $500 since I am also a reporter 🙂

[lschurr]

Hi @eVoloshchak - could you apply to the job in Upwork so that I can pay you for C+?

[lschurr]

Hey @aimane-chnaif - Could you accept the offer in Upwork and I'll get that paid!

[aimane-chnaif]

@lschurr accepted thanks

[lschurr]

All set. This has been fixed and paid!