text:
Android - App crashed after tap the back button in offline mode

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

Action Performed:

Open the test app and log in with any account
Enable offline mode
Open any chat
Tap the Back button at the left top

Expected Result:
The user is returned to the main screen (LHN) after tapping the Back button at the chat view
Actual Result:
App crashed after tapping the back button at chat view in offline mode
Workaround:
Unknown
Platform:

Where is this issue occurring?

Android

Version Number: 1.1.96.0
Reproducible in staging?: Yes
Reproducible in production?: No
Email or phone of affected tester (no customers): any
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
https://user-images.githubusercontent.com/93399543/187978779-0c2d55a2-8dd0-420f-b7e0-fef0fef34155.mp4
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:
View all open jobs on GitHub

Repro'd this in dev on main with the Android emulator.

@mateusbra @AndrewGable @parasharrajat
Do any of you guys think this has anything to do with the react-native-svg upgrade here: https://github.com/Expensify/App/pull/10334? Other than the stack trace pictured above I can't find any useful log output.

I see this immediately before the crash:
09-02 09:43:40.127   497  6106 I system_server: oneway function results will be dropped but finished with status OK and parcel size 4
09-02 09:43:40.994  6696  6810 D ReactNativeJS: [info] [Onyx] set() called for key: reportDraftComment_7 - ""
09-02 09:43:41.418  1220  1246 I PeriodicStatsRunner: PeriodicStatsRunner.call():180 call()
09-02 09:43:41.418  1220  1246 I PeriodicStatsRunner: PeriodicStatsRunner.call():184 No submit PeriodicStats since input started.
09-02 09:43:42.982   497  6106 I system_server: oneway function results will be dropped but finished with status OK and parcel size 4
09-02 09:43:42.982   497  6106 I system_server: oneway function results will be dropped but finished with status OK and parcel size 4
09-02 09:43:43.116   497  3185 I system_server: oneway function results will be dropped but finished with status OK and parcel size 4
09-02 09:43:43.549  6696  6810 D ReactNativeJS: [info] Navigating to route - {"path":"/r/7"}
09-02 09:43:43.573  6696  6810 D ReactNativeJS: [info] [Onyx] set() called for key: session properties: loading,shouldShowComposeInput,authToken,accountID,email,encryptedAuthToken - ""
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: Exception in native call
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String java.lang.String.trim()' on a null object reference
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.horcrux.svg.SVGLength.<init>(SVGLength.java:37)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.horcrux.svg.SVGLength.from(SVGLength.java:107)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.horcrux.svg.LinearGradientView.setX1(LinearGradientView.java:50)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.horcrux.svg.RenderableViewManager$LinearGradientManager.setX1(RenderableViewManager.java:1806)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.horcrux.svg.RenderableViewManager$LinearGradientManager.setX1(RenderableViewManager.java:1770)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.viewmanagers.RNSVGLinearGradientManagerDelegate.setProperty(RNSVGLinearGradientManagerDelegate.java:59)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.uimanager.ViewManagerPropertyUpdater.updateProps(ViewManagerPropertyUpdater.java:46)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.uimanager.ViewManager.updateProperties(ViewManager.java:46)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.uimanager.NativeViewHierarchyManager.updateProperties(NativeViewHierarchyManager.java:142)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.uimanager.UIImplementation.synchronouslyUpdateViewOnUIThread(UIImplementation.java:305)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.uimanager.UIManagerModule.synchronouslyUpdateViewOnUIThread(UIManagerModule.java:400)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.animated.PropsAnimatedNode.restoreDefaultValues(PropsAnimatedNode.java:90)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.animated.NativeAnimatedNodesManager.restoreDefaultValues(NativeAnimatedNodesManager.java:470)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.animated.NativeAnimatedModule$20.execute(NativeAnimatedModule.java:866)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.animated.NativeAnimatedModule$ConcurrentOperationQueue.executeBatch(NativeAnimatedModule.java:147)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.animated.NativeAnimatedModule$2.execute(NativeAnimatedModule.java:324)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.uimanager.UIViewOperationQueue$UIBlockOperation.execute(UIViewOperationQueue.java:579)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.uimanager.UIViewOperationQueue$1.run(UIViewOperationQueue.java:915)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.uimanager.UIViewOperationQueue.flushPendingBatches(UIViewOperationQueue.java:1026)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.uimanager.UIViewOperationQueue.access$2600(UIViewOperationQueue.java:47)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.uimanager.UIViewOperationQueue$DispatchUIFrameCallback.doFrameGuarded(UIViewOperationQueue.java:1086)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.uimanager.GuardedFrameCallback.doFrame(GuardedFrameCallback.java:29)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.modules.core.ReactChoreographer$ReactChoreographerDispatcher.doFrame(ReactChoreographer.java:175)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.facebook.react.modules.core.ChoreographerCompat$FrameCallback$1.doFrame(ChoreographerCompat.java:85)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at android.view.Choreographer$CallbackRecord.run(Choreographer.java:970)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at android.view.Choreographer.doCallbacks(Choreographer.java:796)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at android.view.Choreographer.doFrame(Choreographer.java:727)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:957)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at android.os.Handler.handleCallback(Handler.java:938)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at android.os.Handler.dispatchMessage(Handler.java:99)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at android.os.Looper.loop(Looper.java:223)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at android.app.ActivityThread.main(ActivityThread.java:7656)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at java.lang.reflect.Method.invoke(Native Method)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
09-02 09:43:44.426  6696  6696 E unknown:ReactNative: 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)

at com.horcrux.svg.SVGLength.<init>(SVGLength.java:37)

Looks like SVG- Good find!

Yea, I can confirm that reverting the upgrade resolves the issue

@AndrewGable should we revert this upgrade for now?

Yes please!

Revert merged. Closing this issue.

authors: AndrewGable arosiclair kbecciv luacmartins


text:
App on ios emulator not detecting correct emulator to use

Context https://expensify.slack.com/archives/C03TQ48KC/p1666362417121579
Problem
Apparently when trying to run the ios app, it is not finding correctly the latest emulator I have installed and for some reason is looking for iPhone13, when I only have iPhone14 installed.
Error:
➜  App git:(main) npm run ios

> new.expensify@1.2.18-4 ios
> scripts/set-pusher-suffix.sh && npx react-native run-ios --port=8082

Using PUSHER_DEV_SUFFIX from ../Web-Expensify/_config.local.php
Found suffix: 65b74ba46c7c4edfaf0d7dc4b6c4caf6
Updating .env
warn Package react-native-safe-area-context contains invalid configuration: "dependency.platforms.android.cmakeListsPath" is not allowed. Please verify it's properly linked using "react-native config" command and contact the package maintainers about this.
warn Package react-native-flipper contains invalid configuration: "dependency.platforms.ios.project" is not allowed. Please verify it's properly linked using "react-native config" command and contact the package maintainers about this.
info Found Xcode workspace "NewExpensify.xcworkspace"
error No simulator available with name "iPhone 13".
info Run CLI with --verbose flag for more details.

My emulators:
info Run CLI with --verbose flag for more details.
➜  App git:(main) xcrun simctl list devices
== Devices ==
-- iOS 16.0 --
    iPhone SE (3rd generation) (1880E442-085D-45CA-B198-F59D19EC0919) (Shutdown)
    iPhone 14 (E3112A94-F1BA-4B56-9B0F-59B9E48E0546) (Shutdown)
    iPhone 14 Plus (E3537629-E979-4D64-A9D6-D0581584FE98) (Shutdown)
    iPhone 14 Pro (5B1AF5C8-23D9-43AC-8499-2A6B251513DB) (Shutdown)
    iPhone 14 Pro Max (5F4913AB-1821-4C74-8C09-9D5B40755356) (Shutdown)
    iPad (9th generation) (7761FF05-80C3-488A-9638-02AB93151AC8) (Shutdown)
    iPad Pro (11-inch) (3rd generation) (EB1FC844-8E82-48F8-9335-A4F3DE2DE162) (Shutdown)
    iPad Pro (12.9-inch) (5th generation) (53EF422D-533C-4D2A-BEF1-65051AEB9465) (Shutdown)
    iPad Air (5th generation) (3A8C0CCC-B5EF-4F42-93B5-4CCC85D0F3C8) (Shutdown)
    iPad mini (6th generation) (8B66D73F-F99F-4AD1-B354-563A36337D7A) (Shutdown)

Workaround: run npx react-native run-ios --simulator='iPhone 14' instead of npm run ios

This could be External right?

This happens because at node_modules/@react-native-community/cli-platform-ios/build/commands/runIOS/index.js the default is iPhone 13. You can set any emulator here

node_modules/@react-native-community/cli-platform-ios/build/commands/runIOS/index.js

Would that not be overwritten when someone runs npm install since it is in noce_modules? How can we make it default?

Just tried npm install and it didn't change back to the previous value.

Yeah that makes sense but I believe there's no other way to set the default simulator. The only other way to use specific simulator is to use --simulator flag but that's also a problem because we can't tell which emulator is available on the system.

Is this a problem still then? @iwiznia We need to get in the habit of creating issues and ensuring that people are assigned so that these issues don't end up hanging out for months. Accordingly, I'm going to assign you and you can either close / make sure someone is assigned.

This is still a problem I think since everyone should be on XCode 16 and the default one is iPhone 14 we should set that as default so we can update the package.json to use the --simulator flag and use the iPhone 14 right?

Yes, I think it makes sense since most people will be using the latest XCode.
We just have to use the line below
  "ios": "npx react-native run-ios --simulator=\"iPhone 14\"",
https://github.com/Expensify/App/blob/c638000bce780372f046886db9bc182135973d6a/package.json#L13

@parasharrajat No need to review this, I think Puneet's proposal is good, but it would be great if you could help to test the PR later.
@Puneet-here thank you, please raise a PR with these changes

npx react-native run-ios --port=8082 --simulator=\"iPhone 14\ I am not in favor of doing this. I use iphone 13 pro for testing. IMO, this is more a user side overhead. A user should specify the device they want to use.

I use iphone 13 pro for testing.

Why this specifically? It would be better to use the standard which is the default iPhone 14, but I am happy with your suggestion.
Although we would break this for everyone since they would need to update the env for that.  With setting iphone 14 most people should be fine as up to date xcode uses that

my Preference is to cover most devices. There were more iPhone 13 users than 14 so I used that device to test. But it is not specific. Sometimes, I have to test different models based on the problem.
I think if $DEVICE is not set --simulator=$DEVICE will be ignored and default will be used. But that needs to be tested first.

Alrighty, I am happy with that solution as well, the goal is for this to work out of the box.

OK, great. Let's see what works well on the PR.

We may not need to hardcode the simulator option. iPhone 14 is added as a preferred fallback simulator in https://github.com/react-native-community/cli/releases/tag/v9.1.2.
React Native Expensify
    "@react-native-community/cli": "^9.0.0",
    "@react-native-community/cli-platform-android": "^9.0.0",
    "@react-native-community/cli-platform-ios": "^9.0.0",

https://github.com/Expensify/react-native/blob/6e9d4bf1b41242d582c03be5b933b340a832aacf/package.json#L107-L109

React Native upstream
    "@react-native-community/cli": "9.2.1",
    "@react-native-community/cli-platform-android": "9.2.1",
    "@react-native-community/cli-platform-ios": "9.2.1",

https://github.com/facebook/react-native/blob/428feb2f76c55d6ae5914b019095cd835aa514b0/package.json#L112-L114
The latest version for @react-native-community/cli-platform-ios is v9.2.1. I believe, upgrading the package/packages to v9.1.2+ would fix the issue. Thanks!

Oh good shout @mdneyazahmad, let me have a look into where we are with RN updates

What version of RN is required for that?

@Puneet-here @parasharrajat lets do that, I chatted with @roryabraham and this should be fine, we just need to check if everything else works fine with the new RN version.

Just to be sure, we wanna use npx react-native run-ios --port=8082 --simulator=$DEVICE right? Or are we planning to upgrade RN version.

are we planning to upgrade RN version

I think.

Then I think we should assign @mdneyazahmad here since they researched and pointed out it.

@mountiny Can you please confirm what is the expected behavior now?

The expected behaviour is that if you are on up-to-date mac with up-to-date XCode, you can just run npm run ios and it works ™.
I agree that if updating react native (which we will do eitherway) will accomplish this, then we should do this.
in that case @mdneyazahmad would be the best solution, thanks for the message @Puneet-here
@mdneyazahmad would you be able to update our version of the RN for to the necessary version and test the App with this version? hopefully no breaking changes.

Updating the RN form is kind of a PITA and I havent had time for this yet. Since that takes couple of hours of work and this does not seem like as such a valuable reason to switch I would go ahead with using the original proposal from @Puneet-here.
I hope that make sense, we are trying to squash all the bug and this can be clean up once we eventually update the RN version for something more important I would say.
@Puneet-here Could you please implement the solution you have proposed which will solve this issue quickly and eventually this can be removed once RN will be updated?

Oh Dang, we have updated this already 😂
In that case this should be fixed now! I think we are good just close this issue then. Not sure if we would pay out anything since there was not really any testing involved here.
Thank you for help everyone though!

source: gharchive/issue

authors: JmillsExpensify Puneet-here iwiznia mdneyazahmad mountiny parasharrajat

repo: Expensify/App

license: MIT

license_type: permissive

license_source: github-api


text:
Request money/send money/split money to primaryLogin phone numbers are displaying '@expensify.sms' after the number in the message

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

Action Performed:

Open the app
Open search and search for random number or open any report of primaryLogin phone number
Request money/send money in the chat

OR

Open the app
Open group which has primaryLogin phone number user
Split any amount of bill with the group members

Expected Result:
Request money/send money/split bill message should only display phone number of user
Actual Result:
Request money/send money/split bill message displays '@expensify.sms' after phone number
Workaround:
unknown
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: v1.2.65-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:
https://user-images.githubusercontent.com/43996225/216863325-b8ae0864-f90f-43c2-90f4-f3012fff0769.mp4
https://user-images.githubusercontent.com/43996225/216863335-7df3211d-bb4e-4669-a159-5b8589982bab.mp4
Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1675432415830139
View all open jobs on GitHub

let me review this one today!

asking about this here - https://expensify.slack.com/archives/C049HHMV9SM/p1675968863469549?thread_ts=1675060631.104939&cid=C049HHMV9SM

Problem: you cannot start randomly chatting / requesting IOU / [insert other action here] to a phone number that does not yet exist in Expensify. This is intentional by design, due to SMS fraud.
Workaround: Ask the phone number to signup on Expensify.
Solution:

Do nothing for now
If we want when https://github.com/Expensify/Expensify/issues/242614 is done, we can revisit doing something

based on that discussion, i will close out this issue and we can reopen it once https://github.com/Expensify/Expensify/issues/242614 is done.

Hi @maddylewis , I don't have access to mentioned issue so can you let me know whether that issue is completed or not?

authors: dhanashree-sawant kavimuru maddylewis


text:
Bump Onfido lib in attempt to resolve Google Play 'non compliant SDK version' warning

Problem
We are risk of having our app delisted, or downranked in the Google Play store as the following warning has been ignored for some time. While we have been allowed to release for a while, there is no guarantee this will remain true forever.

We believe the problematic library is Onfido.
Solution
It appears that Onfido have finally responded to  and resolved this issue with release 12.2.2.
We should update the Onfido library to 12.2.2 or higher, perform any necessary migrations, and check to see if the Play Store warning is resolved. If this doesn't work then we need to confront Onfiido and ask them to investigate further. Here is the migration guide

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01c8a6d55c5eec6925
Upwork Job ID: 1622551282901274624
Last Price Increase: 2023-02-06

This is lower priority than a bunch of other OldDot issues

Not a priority yet

I can't prioritise this

This issue has not been updated in over 15 days.  eroding to Monthly issue.
P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

I didn't, as there are some additional steps required in the migration guide. but thanks for the reminder. Will add this to my list.

Okay, looked into this today.
The version they reported was soooo out of date and we have already resolved this by bumping the Onfido libraries in other PRs. Closing!

I also archived the message in Google Play

authors: Julesssss MelvinBot


text:
Chat - Multiline link structure changes if user adds emoji when editing

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

Action Performed:

Open app or go to staging.new.expensify.com
Navigate to any conversation
Send a message with a multiline hyperlink in the following format:
multilinex
Text
Click/Tap on the Edit icon to activate the edit message box
Edit the message and add any emoji inside message but do not edit the link

Expected Result:
Edited message still displays the hyperlink with the multiline format
Actual Result:
Multiline link structure changes if user adds emoji when editing
Workaround:
Unknown
Platforms:

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

[x] Android / native
[x] Android / Chrome
[x] iOS / native
[x] iOS / Safari
[x] MacOS / Chrome / Safari
[x] MacOS / Desktop

Version Number: 1.3.17.0
Reproducible in staging?: yes
Reproducible in production?: yes
If this was caught during regression testing, add the test name, ID and link from TestRail:
https://expensify.testrail.io/index.php?/tests/view/3467098
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
https://platform.applause.com/services/links/v1/external/dd338a69358f7772aacd3f4ed98a0cef9e9b8baf591b83be3a1e51a57fec5c89
https://platform.applause.com/services/links/v1/external/78260a7b242fea9b7dc94c34a678643252deedf770a6bd504159c0d08e5eeaf1
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:
View all open jobs on GitHub

Proposal
Please re-state the problem that we are trying to solve in this issue.
Multiline link structure changes if user adds emoji when editing
What is the root cause of that problem?
We're validating if the anchor text contains only emojis in the 'link' with rule in here
What changes do you think we should make in order to solve the problem?

We need to check if the anchor text g1 is multiline:
      if (g1.includes('/n') && g1.match(CONST.REG_EXP.EMOJIS) || !g1.trim()) {
          return match;
      }

What alternative solutions did you explore? (Optional)
NA
Result

As confirmed from this comment https://github.com/Expensify/App/issues/18386#issuecomment-1541043535, it's intentional not to render multiline link if the link text contains emoji.

Agreed, this is not a bug


text:
Concierge - Concierge icon is not displaying in popup

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

Issue found when executing PR https://github.com/Expensify/App/pull/20665
Action Performed:

Access staging.new.expensify.com
Sign into a valid account
Search for concierge
Hover on concierge name or avatar

Expected Result:
User should see the concierge icon along with the email address
Actual Result:
User only sees the email address and the icon is not displayed
Workaround:
Unknown
Platforms:

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

[ ] Android / native
[ ] Android / Chrome
[ ] iOS / native
[ ] iOS / Safari
[x] MacOS / Chrome / Safari
[ ] MacOS / Desktop

Version Number: 1.3.29.0
Reproducible in staging?: Yes
Reproducible in production?: No
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

https://github.com/Expensify/App/assets/78819774/f3e632aa-f4d5-421d-8501-bda60e4193d3
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:
View all open jobs on GitHub

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

Identify the pull request that introduced this issue and revert it.
Find someone who can quickly fix the issue.
Fix the issue yourself.

Agreed, should be handled in https://github.com/Expensify/App/issues/20822

Contrib has a fix up! ^

No longer a deploy blocker - fix was pushed to staging

authors: Beamanator OSBotify lanitochka17


text:
[$1000] Images blinks on click on Got it button

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

Action Performed:

Go to any chat
send images
go to Settings > Security > Two-factor authentication
complete 2FA process
click on Disable two-factor authentication
click on disable button
which will navigates to Disable two-factor
click on got it button

Expected Result:
images should not blinks
Actual Result:
images blinks on click on Got it button
Workaround:
Unknown
Platforms:

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

[ ] Android / native
[ ] Android / Chrome
[ ] iOS / native
[ ] iOS / Safari
[x] MacOS / Chrome / Safari
[ ] MacOS / Desktop

Version Number: 1.3.57-5
Reproducible in staging?: Yes
Reproducible in production?: Yes
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
https://github.com/Expensify/App/assets/38435837/4e336e02-c236-4e98-aeb6-8aeb04f25721
https://github.com/Expensify/App/assets/38435837/2871bd48-dc6f-4f2e-a996-9cc9ad779c20
Expensify/Expensify Issue URL:
Issue reported by: @gadhiyamanan
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1692429279843929
View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~011b882428c04f1629
Upwork Job ID: 1696199843097894912
Last Price Increase: 2023-08-28

Proposal
Please re-state the problem that we are trying to solve in this issue.
Images blinks on click on Got it button
What is the root cause of that problem?
Whenever we click on Disable Two factor auth we're calling this function https://github.com/Expensify/App/blob/c16995257c738761206ea07645e863ee717ecbc0/src/pages/settings/Security/TwoFactorAuth/Steps/EnabledStep.js#L50 which will do an API call for DisableTwoFactorAuth for the first time maybe because of security purposes backend is requesting re-auth by sending a 407 request.
Due to this 407 code we're calling Authenticate API call here in this function https://github.com/Expensify/App/blob/5b4dee82a3634d73b9543474c5f298f1153460f9/src/libs/Authentication.js#L55.
Once we have the response we're setting ONYXKEYS.SESSION value here in this function https://github.com/Expensify/App/blob/6673ba4eb3d0dacad6f63000b00c70950e6fb2c6/src/libs/actions/Session/updateSessionAuthTokens.js#L8.
So by clicking on Disable 2FA we're resetting SESSION value in onyx.
So why only Image flickering?
We use Image component to render the images which has subscribed to the SESSION key here in this block.
https://github.com/Expensify/App/blob/5b4dee82a3634d73b9543474c5f298f1153460f9/src/components/Image/index.js#L61-L68.
And in this snippet https://github.com/Expensify/App/blob/5b4dee82a3634d73b9543474c5f298f1153460f9/src/components/Image/index.js#L16 we're updating the RNImage's source prop whenever we see a change in SESSION.
So because of that API call SESSION is key getting updated, because of session key getting update RNImage is updating it's source which causing the reload for the Image.
What changes do you think we should make in order to solve the problem?
We can safely remove the session inside useMemo dependencies which keeps the image to previous version even the session token is changed.

Once image is rendered I don't think there is a need for re-rendering.

At any point user leaves and comes back to same report, Image component also would be re-rendered which ensures that SESSION values are updated and Image would also be having the secured image with latest auth token.

What alternative solutions did you explore? (Optional)
NA
Result
https://github.com/Expensify/App/assets/59088937/24e490ec-d12d-4391-b4c9-34d2511de59b

@cubuspl42 Please review my proposal when you get a chance. Thanks!

After giving it a thought, I also can't see a reason to reload images when the auth token changes. It's not like we expect the content to differ.
We can go with @b4s36t4 proposal.
C+ reviewed 🎀 👀 🎀

Melvin answers the call!

The PR that introduced the bug has been identified. Link to the PR:

It seems that the bug was present in the relevant components since they were introduced

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:

N/A

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:

No need for additional discussion

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

No need for a new regression test

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.

N/A

Summary of payments

urgency bonus: yes
Reporter: @gadhiyamanan $250 paid in Upwork
Contributor: @b4s36t4 $1500 paid in Upwork
Contributor+: @cubuspl42 $1500 paid in Upwork


text:
Create Example Migration PR and Outline Steps

cc @chrispader

ay ay, here i am 🏴☠😃

not overdue melvin

actually this is done

authors: chrispader grgia


text:
[HOLD #29691] [$125] Request Money - The text "Split bill with" is not translated into Spanish

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: 1.3.84.9
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: @ayazhussain79
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1697471385072909
Action Performed:
Action Performed:

Go to the group chat.
Click on the "+" and then click on Split bill.
Enter a specific amount and complete the flow.
Change the language to Spanish.
Click on the sent split bill and then click on "Show more"
Check the Merchant value.

Expected Result:
The text "Split bill with" should be translated into Spanish
Actual Result:
The text "Split bill with" is not translated into Spanish
Workaround:
Unknown
Platforms:

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

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

Screenshots/Videos

Android: Native

https://github.com/Expensify/App/assets/93399543/2bfebd77-2536-489b-bfd8-56f13cb783b4

Android: mWeb Chrome

https://github.com/Expensify/App/assets/93399543/5091ed17-efe1-4bfd-85c1-b86395a13805

iOS: Native

https://github.com/Expensify/App/assets/93399543/bd43ea3b-a21a-4fd2-8e2b-942d4e5f0c8a

iOS: mWeb Safari

https://github.com/Expensify/App/assets/93399543/640a8fe4-d69a-4bb6-ac50-a28cda5ba988

MacOS: Chrome / Safari

https://github.com/Expensify/App/assets/93399543/993dbcd9-f450-4a0a-8d84-6a8fc5fbecee
https://github.com/Expensify/App/assets/93399543/fd377986-0673-4e63-b19a-26b3d87f4e66

MacOS: Desktop

https://github.com/Expensify/App/assets/93399543/18a65f0d-cd64-4369-be68-f879338f63d9

View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~019e81ce43efa73a84
Upwork Job ID: 1714017029487788032
Last Price Increase: 2023-10-19

Proposal
Please re-state the problem that we are trying to solve in this issue.
Split bill details merchant is not translated
What is the root cause of that problem?
This is due to the split bill merchant created and saved using the user's preferred locale here
const splitTransaction = TransactionUtils.buildOptimisticTransaction(
        amount,
        currency,
        CONST.REPORT.SPLIT_REPORTID,
        comment,
        '',
        '',
        '',
        `${Localize.translateLocal('iou.splitBill')} ${Localize.translateLocal('common.with')} ${formattedParticipants} [${DateUtils.getDBTime().slice(0, 10)}]`,
        undefined,
        undefined,
        undefined,
        category,
        formattedParticipants,
    );

This means that whatever language was set during the creation of that transaction will be used from now as Merchant, anywhere.
What changes do you think we should make in order to solve the problem?
I suggest generating the Split bill merchant dynamically on the form the front end as the following.
In the SplitBillDetails component, we create the merchant string with
const merchant = ${props.translate('iou.splitBill')} ${props.translate('common.with')} ${arrayToString( _.map(participants, (participant) => participant.login || ''))}``
This will ensure the text is translated correctly, instead of relying on a hard-coded value.

What alternative solutions did you explore? (Optional)
N/A

Proposal from @abdel-h66 looks good to me https://github.com/Expensify/App/issues/29713#issuecomment-1765230901
🎀 👀 🎀 C+ reviewed

The mentioned issue focuses on modifying the value before the save. This one happens when no changes are made by the user and the merchant is automatically generated.

So is the merchant for a split transaction ALWAYS bill split with [participants]? Or can it be custom? We wouldn't want to override it here (which is what the proposal is, unless I'm misunderstanding) if it COULD be something else. But if it can't be, then @abdel-h66's idea works for me!

Good catch ! I just double checked. even when the user changes the merchant, that value is never taken into account. Maybe we could fix that too at the same time.

Good catch ! I just double checked. even when the user changes the merchant, that value is never taken into account. Maybe we could fix that too at the same time.

Cool. I'm going to bring this up internally for a quick discussion - your solution matches current expected behavior, which is great, but if we decide to allow custom merchants in the future, it's going to be hard to figure out what's going on. So I'm gonna see if I can get a sense of if that's ever on the roadmap or not. (To be clear - this is NOT an issue with your proposal, so don't worry! I think it just might be something people haven't thought about yet.)

@dangrous I think we can close this, we're deprecating the "Bill split with..." merchant name, we'll be using the default one "Request" or whatever the user has entered.

okay got it - will that update not have the same translation issues? If so, that's great!

ah good point, I think the issue will persist, so I guess we need to handle the translation when creating a manual request or split and translate Request into Spanish if needed. Since I'm working on allowing this for split, I'll include this change in the PR for both requests and splits. Sounds good?

Actually @dangrous, I think I'll leave this out of the scope of my PR, as we use the default merchant "Request" to also decide whether we should show the merchant or not https://github.com/Expensify/App/blob/18acd82f40153fc1e5091aea3217c532142e500e/src/components/ReportActionItem/MoneyRequestPreview.js#L171

Okay cool - we'll hold on your PR and then retest/reevaluate afterwards to figure out next steps!

Not overdue, on hold

The backend PR for #29691 has been merged, I need to figure out whether or not there's a front end fix needed as well. Once we know that, we can get this one going again and revamp proposals based on the changes!

Checking in on that held issue, we might be able to take another look at this shortly

Still waiting to hear whether or not #29691 is fully complete and we can take this off of hold

@dangrous will report back here today!

Looks like we're waiting on an App and a Web-E PR to finish #29691 and then we should be ready to move forward

Still on hold, looks like most of the PRs are merged so we should be able to get going shortly

Still waiting on linked issue

Not overdue, still on hold

I'm going to make this weekly while we're waiting.

Still waiting on held issue

Still on hold

Still on hold

Okay I believe we're off hold here, as #29691 has been deployed! We should retest and update any proposals based on the new changes cc @abdel-h66 since you were working on it (long ago!) before the hold.

I can't reproduce

Yeah I'm also not seeing this. I think we can close out - was this before/after the reporting bonus change, @kevinksullivan ? Let me know what you think (and if you could double check that you're unable to reproduce too, that would be awesome!)

bump when you have time to confirm we can close this - thanks @kevinksullivan!

Issue not reproducible during KI retests. (First week)

authors: DylanDylann abdel-h abdel-h66 dangrous kbecciv mvtglobally narefyev91 youssef-lr


text:
[ON HOLD for #25397][$500] No required field validation on no value select on new dropdowns like state (validation works on older dropdown)

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: 1.3.89.5
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: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1698055983660749
Action Performed:

Open the app
Open settings->workspaces->any workspace->bank account->connect manually
Fill in details in step 1 and continue
Open company type dropdown (older style dropdown), close without any value selection and focus on different field
Observe that required field validation is triggered
Try similarly on any other text fields in step 2 and observe that on focus out, required field validation will be triggered
Open state field and go back without selection (new dropdown)
Observe that no validation is triggered
Click on Save & continue and observe that now state field will show required field validation

Expected Result:
App should trigger required field validation when we focus / open any field and focus away
Actual Result:
App does not trigger required field validation when we open state fields in workspace connect bank account or any such new dropdown field throughout the app, close the field and focus on another field
Workaround:
Unknown
Platforms:

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

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

Screenshots/Videos

Android: Native

https://github.com/Expensify/App/assets/93399543/a44eaf1c-067d-4508-ba4c-e8f1ccda86d5

Android: mWeb Chrome

https://github.com/Expensify/App/assets/93399543/a48e2239-20e7-4c32-b9ae-247a28dcb913

iOS: Native

https://github.com/Expensify/App/assets/93399543/d5181f44-4319-48d8-8144-5e8af11e1149

iOS: mWeb Safari

https://github.com/Expensify/App/assets/93399543/9d963b87-34c2-4849-8075-dbc5add9d988

MacOS: Chrome / Safari

https://github.com/Expensify/App/assets/93399543/d031f821-bc94-418c-a741-b35479d02723
https://github.com/Expensify/App/assets/93399543/ee02b20e-499b-42ca-8128-272fe4a61db0

MacOS: Desktop

https://github.com/Expensify/App/assets/93399543/cfa478db-9a5e-4eeb-8f5b-d8978d592f40

View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0131d5a852872ad378
Upwork Job ID: 1716523176886263808
Last Price Increase: 2023-11-20

Proposal
Please re-state the problem that we are trying to solve in this issue.
Touched fields in the no not show the validation error
What is the root cause of that problem?
For this type of field, which is a MenuItem with a modal, does not follow the conventions of the normal input where we have blur and focus events. We don't have a way at the moment to set it to touched. And trigger the validation.
In src/components/Form.js the onblur is defined to run the following
 setTouchedInput(inputID);

if (props.shouldValidateOnBlur) {
    onValidate(inputValues, !hasServerError);
}

This even will never occur
What changes do you think we should make in order to solve the problem?
We should explicitly call the onBlur to trigger the validation during the closing of the modal.
By updating
    const hidePickerModal = () => {
        onBlur();
        setIsPickerVisible(false);
    };

What alternative solutions did you explore? (Optional)
N/A

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

No required field validation on no value select on new dropdowns like state (validation works on older dropdown)

What is the root cause of that problem?

With the StatePicker component, blurring from it does not trigger the onFocus callback function that we have handled in Form component like others input
https://github.com/Expensify/App/blob/1c85ebeb55ffe81bf0cfa64665b4dc6625b94683/src/components/Form.js#L349-L356
Because the setTouchedInput(inputID) is not called, so it will not display error.

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

We need to call setTouchedInput(inputID) manually with this type of input by:

In Form component, expose the formRef with method setTouchedInput like:

useImperativeHandle(ref, () => ({
    setTouchedInput
    },
  }));

In StatePicker component, we import the above ref, then once we click on MenuItem, call the setTouchedInput:

Also we should apply this logic to others similar inputs

What alternative solutions did you explore? (Optional)

NA

What are your thoughts on the above proposal, @fedirjh?

I trust your judgment, @fedirjh! Let's put this on hold for now and come back to it after the refactor.

@fedirjh I think the Form Refactor that is related to this issue is done. Please correct me if I misunderstood

@fedirjh Oh. I see this PR has already been merged for two weeks. So we need to wait until the issue is closed, right?

@fedirjh Can you help confirm this one https://github.com/Expensify/App/issues/30195#issuecomment-1823073334?

Just a happy bump for Melvin. I'm not 100% sure about your question, @DylanDylann, but I think you're correct that we need to wait until https://github.com/Expensify/App/issues/30312 is closed.

@fedirjh Update my proposal in section Alternative solutions

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

No required field validation on no value select on new dropdowns like state (validation works on older dropdown)

What is the root cause of that problem?

With the StatePicker component, blurring from it does not trigger the onBlur callback function that we have handled in Form component like others input
https://github.com/Expensify/App/blob/1c85ebeb55ffe81bf0cfa64665b4dc6625b94683/src/components/Form.js#L349-L356
Because the setTouchedInput(inputID) is not called, so it will not display error.

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

Now, the Form Refactor is done. So we just need to update StatePicker to

+ function StatePicker({value, errorText, onInputChange, forwardedRef, label, ...rest}) {
    const hidePickerModal = () => {
+      rest.onBlur && rest.onBlur();
        setIsPickerVisible(false);
    };
    return (
        <View>
            <MenuItemWithTopDescription
                ...
                onPress={() => {
+                  rest.onPress && rest.onPress();
                    showPickerModal();
                }}
            />
            ...
        </View>
    );
}

Result
Screencast from 22-11-2023 10:12:22.webm

Thanks for keeping this bumping, @fedirjh! @abdel-h66, can you take a look?

I'm OOO starting Monday, December 18 (today) through Tuesday, January 2.
Current status:
We're waiting on proposals
If this issue is open when I'm back from OOO, I'll take it back over. Thank you!

@fedirjh what do you think about my proposal?

@fedirjh I just test with the latest main, and we do not need to add the rest.onPress && rest.onPress();

Dupe of #27263

@situchan the #27263 is related to the CountrySelector component. The #30195 is related to StatePicker component

@fedirjh @DylanDylann paid

$50 contract extended to @dhanashree-sawant

@muttmuure it looks like I tagged you in when I went OOO for the holidays. I'm going to remove your assignment and take this off your plate. Thanks for pushing this forward!

I've issued payment to @dhanashree-sawant and will work on the regression test steps issue today.

authors: DylanDylann abdel-h66 johncschuster kbecciv muttmuure situchan


text:
[$500] Desktop - Draft message offline isn't saved after app reload if user remains in same chat

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: 1.4.21-3
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: Applause - Internal Team
Slack conversation:
Action Performed:

Open Desktop App
Go to any chat
Go offline
Enter any message and do not send it
Close Desktop App without changing chat
Go online
Start Desktop app again

Expected Result:
A draft message entered offline is saved in the chat composer field
Actual Result:
A draft message entered offline is not saved after restarting the app and going online if the user remains in the same chat
Workaround:
Unknown
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
[x] MacOS: Desktop

Screenshots/Videos
Add any screenshot/video evidence
https://github.com/Expensify/App/assets/93399543/16b4edfc-6e58-4b0e-8565-866a4bd6cbbd
View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~011a94f38191440005
Upwork Job ID: 1742849765768896512
Last Price Increase: 2024-01-04
Automatic offers:

DylanDylann | Contributor | 28121676

Proposal
Please re-state the problem that we are trying to solve in this issue.
Draft message offline isn't saved after app reload if user remains in same chat
What is the root cause of that problem?
We are getting the draft comment manually here from onyx when the page is rendered here
https://github.com/Expensiy/App/blob/dc3e19bdcc95023d74cd7fdd433f6208707eaaf8/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js#L119-L125
So when the onyx data is not ready when rendering (when navigating directly to the chat on opening the app) it will be empty
What changes do you think we should make in order to solve the problem?

We should subscribe to the data instead

        comment: {
            key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`,
            initialValue: '',
        },

What alternative solutions did you explore? (Optional)

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

Desktop - Draft message offline isn't saved after app reload if user remains in same chat

What is the root cause of that problem?

This error only appears when we open the app by the deep link https://staging.new.expensify.com/
When we open the app by the above deeplink, the screen stacks is something like [{name: "home"}, {name: CentralPaneNavigator, state: {routes: {name: Report, params: {reportID: " "}}}]
As we can see, the first time, the app will render the report screen with reportID params is " ", and because we have the bellow logic to set the initial value for the value state
https://github.com/Expensify/App/blob/5fedab802a07d672efc544839f90ba65513f5d44/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js#L119-L125
so the initial value of the value state will be " " (because the reportId is " ", so getDraftComment(" ") will return " " as well)
That leads to the current bug.

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

We can make sure that the component ComposerWithSuggestions is only rendered if the reportID is not empty.
For example, do it by updating:
https://github.com/Expensify/App/blob/5fedab802a07d672efc544839f90ba65513f5d44/src/pages/home/report/ReportActionCompose/ReportActionCompose.js#L413
to:

                                    {isReportReadyForDisplay &&  <ComposerWithSuggestions ... />}

What alternative solutions did you explore? (Optional)

NA

I can reproduce this, I think it might be part of the Vip-vsb project so adding it there. I'm not sure if this needs to be internal but I think it can be external.
@rushatgabhane feel free to continue with the proposal review!

I'm going to be ooo next week so adding a buddy to monitor this while I'm away.
Status- we're reviewing proposals

Sounds good!

@FitseTLT i think the root cause is not correct because getDraftComment() gets data from Onyx

@DylanDylann i think you have the correct root cause.
 {isReportReadyForDisplay &&  <ComposerWithSuggestions ... />}
Will waiting for reportId lead to a delayed composer? We want composer to display even if the report isn't ready yet.
So could we display empty composer initially and then fill it with draft comment once the report is ready? What do you think?

@rushatgabhane I my proposal, I also suggested that we can use   {reportID !== '0' &&  <ComposerWithSuggestions ... />} so the composer is not delayed

Ohh okay that would work, thank you 👍
🎀 👀 🎀 https://github.com/Expensify/App/issues/33938#issuecomment-1877354537

Actually, @DylanDylann even if reportId is 0 can we show it immediately and put the draft text once it's available

@rushatgabhane

can we show it immediately and put the draft text once it's available

How can we do that? Because once the ComposerWithSuggestions is rendered, the below will be called
https://github.com/Expensify/App/blob/5fedab802a07d672efc544839f90ba65513f5d44/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions/ComposerWithSuggestions.js#L119-L125
so it will set the value to ``''```

Is this dupe of https://github.com/Expensify/App/issues/31705?

@situchan No. It is not a dupe. The reproduction steps, the RCA and selected solution are different

@joekaufmanexpensify i crossed it because we need a more refined proposal
https://github.com/Expensify/App/issues/33938#issuecomment-1884210465

How can we do that? Because once the ComposerWithSuggestions is rendered, the below will be called

@DylanDylann can we put it under useEffect?

Can we re render once the draft comment is available?
useEffect(
...
[getDraftComment(reportID)])

@rushatgabhane

I dont think so. Because the RCA is that the report s composer always has the initial value state is ''. So if we re-render, the bug is not fixed

Can we pass the correct reportId to the composer once after reportId is ready?

Yes. That is the same as my solution

@rushatgabhane Are we close to being able to select a proposal here?

Pending further input from @rushatgabhane

Can we pass the correct reportId to the composer once after reportId is ready?
Yes. This is the same as my solution

@DylanDylann no because the solution won't render the composer when reportId is 0.

@puneetlath let's go with @DylanDylann's proposal
C+ reviewed 🎀 👀🎀

We should be able to proceed with the proposal here soon.

Reached out to Puneet to confirm if this proposal is good

Still pending confirmation about proposal

@puneetlath mind signing off on this proposal when you have a sec?

Sorry for the delay. Assigned.

@rushatgabhane PR https://github.com/Expensify/App/pull/35213 is ready to review

Bumped in Slack

Hey, looks like there was a dupe issue - https://github.com/Expensify/App/issues/34667
And it was fixed before us.

Please note that the solution in this issue and the solution in #34667 are the same and the solution https://github.com/Expensify/App/issues/33938#issuecomment-1877354537 came first. And the PR was delayed reviewing by a few weeks

Interesting. So the PR we opened in this issue isn't needed at all anymore. Is that right?

@joekaufmanexpensify Yes

Got it. Thanks! Looking into this more.

We discussed this internally, and here's where we landed:

@DylanDylann you'll get the full $500 bounty here since you were hired and raised the PR. This will be paid via Upwork.
@rushatgabhane, we're going with $0 for the C+ bounty here. Specifically because review on the PR was delayed by 10+ days here. If it hadn't been, this issue's PR would've likely been merged before we hired anyone in the other issue, meaning it would've been closed without compensation for the C+ there. Since we're paying the C+ bounty there, we're going to omit it here.

LMK if you have any questions!

@DylanDylann $500 sent and contract ended.

authors: Christinadobrzyn DylanDylann FitseTLT joekaufmanexpensify kbecciv puneetlath rushatgabhane situchan


text:
[HOLD for payment 2024-08-29] [$250] mWeb/Safari - Keypad overlap composer & blank space in header when open admins room from WS share pg

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.18-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: Exploratory around https://github.com/Expensify/App/pull/46855
Email or phone of affected tester (no customers): applausetester+1088ck@applause.expensifail.com
Issue reported by: Applause Internal Team
Action Performed:

Navigate to staging.new.expensify.com
Go to Settings >> Workspaces >> Select any workspace (or create a new one)
Open Profile >> Share option
Tap #admins text link
Tap on composer (Note: Keypad overlaps composer)
Start typing any message

Expected Result:

Keypad should not overlap on composer & user should be able to view the message entered in composer
No blank space displayed in header

Actual Result:

Keypad overlaps composer & user not able to view the message entered in composer
Blank space displayed in header

Workaround:
Unknown
Platforms:

[ ] Android: Native
[ ] Android: mWeb Chrome
[ ] iOS: Native
[x] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos
https://github.com/user-attachments/assets/cc9434b2-4622-49cb-86a4-f071a3f10d2e

View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~011db3b268b189dcbe
Upwork Job ID: 1822989061968230556
Last Price Increase: 2024-08-12
Automatic offers:

jjcoffee | Reviewer | 103560629

Issue OwnerCurrent Issue Owner: @mallenexpensify

We think that this bug might be related to #vip-vsb

@zanyrenney  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

polish but worth fixing due to the appearance/functionality.

adding external.

Proposal
Please re-state the problem that we are trying to solve in this issue.
Blank space and keyboard overlap the composer when focus on composer on iOS mWeb safari.
What is the root cause of that problem?
When we focus on the composer, the page is scrolled to the top so the composer is positioned above the keyboard and a margin-top is applied so the report header is shown at the top.
https://github.com/Expensify/App/blob/b7f7f1b4f36451b4e3a763ac717e1c8fd645ee5a/src/pages/home/ReportScreen.tsx#L280-L283
For webkkit based browsers, we have a hook to correct the visual viewport scroll if enable is true. It's basically to "prevent" the user from scrolling the visual viewport.
https://github.com/Expensify/App/blob/b7f7f1b4f36451b4e3a763ac717e1c8fd645ee5a/src/hooks/useTackInputFocus/index.ts#L31-L53
enable is true if shouldEnableMaxHeight and shouldAvoidScrollOnVirtualViewport are true and it will register a visual viewport scroll listener.
https://github.com/Expensify/App/blob/b7f7f1b4f36451b4e3a763ac717e1c8fd645ee5a/src/components/ScreenWrapper.tsx#L222
When the report screen viewport is scrolled so that the composer is shown at the top of the keyboard, the scroll listener is triggered which corrects the scroll position back to 0.
But the problem here is, ReportScreen's ScreenWrapper doesn't enable shouldEnableMaxHeight, so the hook should be disabled, but somehow the listener is still registered and triggered.
https://github.com/Expensify/App/blob/b7f7f1b4f36451b4e3a763ac717e1c8fd645ee5a/src/pages/home/ReportScreen.tsx#L733-L738
That's because when we open the workspace profile page > share > press #admins, the profile page is wrapped with WorkspacePageWithSections which enables shouldEnableMaxHeight.
https://github.com/Expensify/App/blob/b7f7f1b4f36451b4e3a763ac717e1c8fd645ee5a/src/pages/workspace/WorkspacePageWithSections.tsx#L171-L174
So, in short, the profile page screen wrapper settings affect the report screen.
What changes do you think we should make in order to solve the problem?

Disable the hook if the screen isn't focused.
const isFocused = useIsFocused();
const isAvoidingViewportScroll = useTackInputFocus(isFocused && shouldEnableMaxHeight && shouldAvoidScrollOnVirtualViewport && Browser.isMobileWebKit());

@jjcoffee please can you review the proposal above. It seems like a UI issue as opposed to anything critical on the functionality. Should be a simple enough!

@bernhardoj's proposal LGTM! (Really well written too, which is always a bonus!)
:ribbon::eyes::ribbon: C+ reviewed

PR is ready
cc: @jjcoffee

reapplying bug label for payout as I am going to be OOO until next week so will miss this payment date. making daily as payout in 3 days! thanks so much @mallenexpensify

The PR that introduced the bug has been identified. Link to the PR: https://github.com/Expensify/App/pull/23185
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: https://github.com/Expensify/App/commit/a32fa95627b8a5a9a093c8e05e79dd2facf9f8a9#r145836648
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: N/A - broader testing required
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.

Regression Test Proposal
iOS Safari only

Go to Settings >> Workspaces >> Select any workspace (or create a new one)
Open Profile >> Share option
Press the #admins text link
Focus on the composer
Verify the header still sits at the top and the composer is above the keyboard

Do we agree 👍 or 👎

payment didn't happen in my absence so handling it now as the initial assignee :)

payment summary
paid @jjcoffee  $250 via Upwork
@bernhardoj is due $250 and is paid via ND payments.

$250 approved for @bernhardoj

authors: IuliiaHerets JmillsExpensify bernhardoj jjcoffee zanyrenney


text:
[$250] Add posted date to Expensify Card transactions

Problem: Credit card transactions are unique because it's possible to buy something one day, but then the transaction isn't processed by the issuing bank until a couple of days later. Both dates are important, because the receipt will always have the transaction date, yet the accountant (and accounting system) need the posted date.
Solution: Let's add the posted date to all Expensify Card transactions. What this means in practice is that:

The transaction date will continue to show.
We'll add a "dot separator" pattern for Date label (just like Amount; see example below).
This will take the form Date * Posted %postedDate%.

I've annotated the mockup below so that this is hopefully all clear.

Other notes for emphasis:

The transaction date and the posted date only apply to Expensify Card transactions, so please make sure that's addressed in your proposal.
If there is no posted date yet, then * Posted %postedDate% will not show.

Issue OwnerCurrent Issue Owner: @ikevin127

Proposal
Please re-state the problem that we are trying to solve in this issue.
Add posted date to Expensify Card transactions
What is the root cause of that problem?
New feature
What changes do you think we should make in order to solve the problem?

Create a variable for date description as we did for amountDescription here

When the transaction is the card transaction, get the posted date from transaction data, if it exists add posted data to the dateDescription

https://github.com/Expensify/App/blob/e3632a9877542727230814ffc9fdd78bd087ecbd/src/components/ReportActionItem/MoneyRequestView.tsx#L235

Update the description prop here to dateDescription variable

description={translate('common.date')}

https://github.com/Expensify/App/blob/e3632a9877542727230814ffc9fdd78bd087ecbd/src/components/ReportActionItem/MoneyRequestView.tsx#L593
What alternative solutions did you explore? (Optional)

Updated proposal.

Add code change

♻ Reviewing proposal.

@mkzie2's proposal makes sense to me. The solution follows the Amount field's model for adding additional data which is what we're looking for based on the issue's emphasis notes. Other details can be worked on during the PR phase. The solution result would look like this:

Spoiler

🎀👀🎀 C+ reviewed

@mountiny Is the BE part for this NewFeature completed ? I wasn't able to find any variable like / related to postedDate in the FE transaction types - therefore we might have to wait for BE to return the variable before moving on with the FE implementation for this issue.

Thanks! no the be is not completed. I fell for the trap from @JmillsExpensify called External issue.
I will have to whip up a PR for this

Another thing I noticed when reviewing is that we have this check:
https://github.com/Expensify/App/blob/7256ad6eef046be3345e0b260252f6ec47fc4203/src/libs/TransactionUtils/index.ts#L637-L642
based on which we will show the Posted YYYY-MM-DD label if postedDate exists (based on proposed solution):
https://github.com/Expensify/App/blob/7256ad6eef046be3345e0b260252f6ec47fc4203/src/components/ReportActionItem/MoneyRequestView.tsx#L235-L242
but because of the !!transaction?.managedCard check which determines isCardTransaction, if managedCard (boolean) is false or doesn't exist in the transaction object -> isCardTransaction will be false -> label won't show.
@mountiny  Is managedCard always populated and true on card transactions ? I wanted to bring up with you to ensure that we're tight here from the BE side, otherwise we could have a potential regression post-merge.

@mountiny Please let me know once the backend PR is done and let me know what data in a transaction that I can use to get the postedDate

@mkzie2 will let you know

@ikevin127 I assume managedCard is for all card transactions give the logic you showed but I will check once I will work on the PR

No update

In the queue

@ikevin127 @mkzie2 the BE is up and it should be deployed on Monday.
The property in transaction will be posted and its a string with format YYYYMMDD so we will have to format it to what we need. This is how it is saved in the DB so I prefer not to change it to avoid confusion and just process it in the App. Types need to be updated too obviously

@mkzie2 @ikevin127 what is your eta on the pr? You can start working on it assuming the data is in posted field of a transaction. You cannot test card transactions nonetheless so you can go ahead

Then @mkzie2 can move forward with opening the PR and use posted as the variable and I'll start reviewing as soon as it's ready for review.

Will raise the PR today.

@mountiny Can you give me an example of posted with YYYYMMDD format?

@ikevin127  Create a draft here. cc @mountiny.

posted: "20241125"

posted: "20241125"

@mkzie2 Given the format detailed above, I think if this part of the PR works with the given date format, then we're good to go and you can open the PR making sure to add testing steps and when it comes to the screenshots/videos section see if you can simulate a card transaction locally using Onyx. You can follow this Slack thread for details on how to simulate a card transaction locally for testing purposes.

Thanks will complete the PR today.

[x] [@ikevin127] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.

Regression Test Proposal

Create a card transaction and ensure the transaction was paid and posted (processed by the issuing bank).

Go to the transaction thread report or the combined expense report.

Verify that the Posted {date} label is displayed on the right side of the Date label.

Do we agree 👍 or 👎.

cc @JmillsExpensify payment overdue, for visibility

Payment summary:

Contributor: $250 @mkzie2 via Upwork
Reviewer: $250 @ikevin127 via Upwork

Upworks offers sent to both and regression test also created.

@JmillsExpensify Offer accepted, thank you!

All contributors paid out and regression test request is place. Closing this issue.


text:
We created multiple workspaces for user in onboarding

This is being discussed here so I'm just creating a tracking issue for us to get to the bottom of it with @marcaaron @trjExpensify
https://expensify.slack.com/archives/C07HPDRELLD/p1731537294677189

I think @carlosmiceli is working on moving the policy creation to the CreateAccount step. I am wondering if this is at all related to those changes. At least, it seems like it will be fixed by them since we will no longer be creating the policy via an API call triggered in App.

Interesting. Carlos, how does that work if you create an account on new.expensify.com directly where there's no signUpQualifiers to choose? 🤔

What @marcaaron says is correct, that's what I'm working on. @trjExpensify so far, we would just not create a workspace for any signup that hasn't selected a qualifier. In fact, creating an account will not fail if for some reason the workspace fails to get created. But let's check with @anmurali to see if we'd like to add/change some logic about it.

@trjExpensify so far, we would just not create a workspace for any signup that hasn't selected a qualifier

Okay, which means they would then see the onboarding modal and selecting the manage my team path would create a workspace?

Sorry, maybe I'm not explaining myself well.

You said you're working on creating a workspace after you choose this on the expensify.com home page:

Today, the timing of creating the workspace is after you click this:

Q: In the expensify.com sign-up flow, how are you avoiding creating two workspaces on sign-up? Are you skipping that "How many employees do you have?" screen, and as a result, not triggering the CreateWorkspace command a second time?
Q: If you sign-up via new.expensify.com where there are no signUpQualifiers to select on the home page, will you still see the "How many employees do you have?" screen if you choose "Manage my team's expenses" in the first step, and thus still trigger CreateWorkspace at that point like today?

In the expensify.com sign-up flow, how are you avoiding creating two workspaces on sign-up? Are you skipping that "How many employees do you have?" screen, and as a result, not triggering the CreateWorkspace command a second time?

I haven't got to this point yet, but I think as Marc said, we're going to remove the current CreateWorkspace logic of creating the policy in App during onboarding (at least temporarily so we don't create duplicates).

If you sign-up via new.expensify.com where there are no signUpQualifiers to select on the home page, will you still see the "How many employees do you have?" screen if you choose "Manage my team's expenses" in the first step, and thus still trigger CreateWorkspace at that point like today?

To clarify, I'm not working in anything that would change any logic that happens after/during signup through new.expensify.com, other than replacing the logic as I mentioned above. You will see whatever you see while signing up now. This means that no workspace will get created when signing up through new.expensify.com until we actually want to implement some logic around that.
I hope I'm understanding the question 😅

Ah, gotcha! Yup, that sounds good, we can do that. By the way, while it's an edge case, we should consider this again then. I think with this new situation, it's justified to just hide the back button instead of having to implement more complicated logic to skip the workspace creation if they happen to go back.

Hm, but isn't that logic going to exist anyway? As in, we won't show the employee count selector screen if they've chosen a signUpQualifier on sign-up? So regardless of if they go back and forward again, the page that triggers the CreateWorkspace command won't show to them.

so I interpreted that clicking Continue in that screen is what triggered CreateWorkspace.

Correct, that's my understanding as well. But if the page is skipped, meaning it doesn't show at all for this use-case, then going back/forward doesn't matter does it? They'll never see that "employee count" page and therefore no Continue button to trigger creating a workspace. So it's not clear to me why we need to restrict going back..

Right, what I'm saying is that, currently, after a user signups and skips those screens and sees the accounting software screen in the onboarding modal, they can go back clicking on the top left of the modal and go back to the employee count. It's not skipped in terms of being hidden, but just being taken straight to the software sections; all previous sections can still be seen if navigating back.

Couldn't we hide it instead though? 🤔

Probably, but not sure how to do that since I haven't worked on that logic, that's probably a good question for someone with more knowledge there. Maybe @tgolen has some advice here? I still think hiding the back button is way easier, though 😅

Actually, thinking deeper about it, it's more future proof to hide the screens as you suggest, in case we keep expanding the onboarding steps.

Not overdue, this is getting worked on daily.

Still working through the updates in slack.

Will create issue today to align the ND fix with the BE work for https://github.com/Expensify/Expensify/issues/437845

Created here: https://github.com/Expensify/App/issues/53326

This is being worked on!

Still working on this on many fronts 😅

Still being worked on, I'm going to move it to weekly since it's just a tracking issue for more urgent issues.

The Front PR is in review so we're still making progress!

@carlosmiceli where are we at with this one? Still waiting on this to be done?
https://github.com/Expensify/App/issues/53326

The contributor was instructed of the changes to be implemented yesterday, while I'm pushing the last Web-E review suggestion today and we can have the BE ready to merge.

Any updates on whats going on with this one? Not sure if anyone is still making progress or if it will come next week?

I asked a few days ago for pullerbear to bring more reviewers to both BE PRs because the reviews have been very slow since @marcaaron went on paternity leave, as he was the reviewer giving consistent reviews. I'm paying attention to it daily but I guess many people are OOO these days too...

Having said that, if I get a new response from the other reviewers, I'll make the push/pitch to just merge since it's been confirmed to work and we can hopefully have it done either this or the next.

authors: carlosmiceli danielrvidal marcaaron trjExpensify


text:
[$250] IOU - Email is shown as a subtitle on LHN instead of added IOU amount

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: 9.0.64-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: https://expensify.testrail.io/index.php?/tests/view/5239196
Email or phone of affected tester (no customers): nathanmulugetatesting+2115@gmail.com
Issue reported by: Applause Internal Team
Action Performed:

Navigate to staging.new.expensify.com
Send an IOU to account A from account B
Observe LHN as account A
Send another IOU to account A from account B again
Observe LHN subtitle as account A

Expected Result:
LHN subtitle should show the added amount of the IOU's.
Actual Result:
LHN subtitle shows email instead of the added amounts of the IOU's
Workaround:
Unknown
Platforms:

[ ] Android: Standalone
[ ] Android: HybridApp
[x] Android: mWeb Chrome
[ ] iOS: Standalone
[ ] iOS: HybridApp
[ ] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos
https://github.com/user-attachments/assets/78fa1055-323d-467b-bab7-8ec05654f562

View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021863465747822418752
Upwork Job ID: 1863465747822418752
Last Price Increase: 2024-12-09

Issue OwnerCurrent Issue Owner: @aldo-expensify

Proposal
Please re-state the problem that we are trying to solve in this issue.
Email is shown as a subtitle on LHN instead of an added IOU amount.
What is the root cause of that problem?
The REPORT_PREVIEW action is not available in lastVisibleReportActions so the lastReportAction becomes null.
https://github.com/Expensify/App/blob/f12b73c3aff5fea8cb8d41f111d52d5bfd187eb1/src/libs/OptionsListUtils.ts#L491-L494
Since the report has not been opened yet, the lastOriginalReportAction is the default first CREATED report action.

Hence, the getLastMessageTextForReport function should not skip the check seen below but it does because lastReportAction is null.
https://github.com/Expensify/App/blob/f12b73c3aff5fea8cb8d41f111d52d5bfd187eb1/src/libs/OptionsListUtils.ts#L522
What changes do you think we should make in order to solve the problem?

Since report?.iouReportID and report?.lastActionType are present, we can create this condition, report?.iouReportID && report?.lastActionType === CONST.REPORT.ACTIONS.TYPE.REPORT_PREVIEW to get the lastMessageTextFromReport just after the ReportActionUtils.isReportPreviewAction check.
When the condition is true, we will do the same operations from the ReportActionUtils.isReportPreviewAction  check to set the lastMessageTextFromReport to the reportPreviewMessage .
https://github.com/Expensify/App/blob/f12b73c3aff5fea8cb8d41f111d52d5bfd187eb1/src/libs/OptionsListUtils.ts#L522-L523
   ...

} else if (report?.iouReportID && report?.lastActionType === CONST.REPORT.ACTIONS.TYPE.REPORT_PREVIEW) {
    const iouReport = ReportUtils.getReportOrDraftReport(report?.iouReportID);
    const lastIOUMoneyReportAction = allSortedReportActions[iouReport?.reportID ?? '-1']?.find(
        (reportAction, key): reportAction is ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU> =>
        ....
    );
    const reportPreviewMessage = ReportUtils.getReportPreviewMessage(
       !isEmptyObject(iouReport) ? iouReport : null,
       ....
    )
    lastMessageTextFromReport = ReportUtils.formatReportLastMessageText(reportPreviewMessage);
}

I can still reproduce this on chrome

@Tony-MK's proposal works here.
🎀👀🎀 C+ reviewed.

@aldo-expensify bump on the proposal approval!

Sorry, I was OOO on Friday, and I'm catching up today. I'll get to this asap!

I couldn't get to this today, I'll check tomorrow.

Started testing this today. The email that appears after submitting a second expense is not just the email, it is the email plus the first amount:

The amount was not visible for the tester because the email is too long.
I have been hesitant of accepting the current proposal because this doesn't look like a front end only problem. The onyx report data is incorrect:

Which signals that the backend is not pushing the right updates here.
A solely front end fix feels like a workaround that is not addressing the root cause here.

I'll take this Internal because the backend should be pushing the right lastMessageHtml / lastActionType when the first and second expenses are submitted.

Added Infra label by mistake because the UI buttons jumped around 😬

I believe this bug was caused by this PR here that is clearing the REPORTPREVIEW reportAction from the front end:
https://github.com/Expensify/Auth/pull/13112/files#diff-f11ec21da73b6b3e6067daf5047300859958522167fcfacca7ee4756502f0094R2914-R2920

PR up: https://github.com/Expensify/Auth/pull/13548

authors: IuliiaHerets Tony-MK adelekennedy akinwale aldo-expensify


text:
The error message doesn't disappear after checking the required checkboxes in Terms and fees page - reported by @adeel0202

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

Action Performed:

Navigate to Terms and fees page (Enter /settings/payments/enable-payments in the URL directly for testing purposes)
Click on Enable Payments button without checking both required checkboxes
When the error is displayed, check both the required checkboxes
Observe that the error message is still there

Expected Result:
The error message should disappear when the user checks both checkboxes
Actual Result:
The error message doesn't disappear after checking the checkboxes
Workaround:
unknown
Platform:

Where is this issue occurring?

Web

Version Number: 1.1.40-2
Reproducible in staging?: Y
Reproducible in production?: need repro
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
https://user-images.githubusercontent.com/43995119/159203333-bf68183a-8407-41d1-b6ed-23f8fc0e346d.mov
Expensify/Expensify Issue URL:
Issue reported by: @adeel0202
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1646672720972879
View all open jobs on GitHub

Proposal (If considered external)
We can add a new method to set the error state to false if both checkboxes are checked. (maybe we can write a better name for the method)
checkError() {
        if (this.state.hasAcceptedDisclosure
            && this.state.hasAcceptedPrivacyPolicyAndWalletAgreement) {
            this.setState({error: false});
        }
    }

We can then call this method after the state has been changed in toggleDisclosure and togglePrivacyPolicy methods, like this
toggleDisclosure() {
        this.setState(prevState => ({hasAcceptedDisclosure: !prevState.hasAcceptedDisclosure}), () => this.checkError());
    }

and
togglePrivacyPolicy() {
        this.setState(prevState => ({
            hasAcceptedPrivacyPolicyAndWalletAgreement: !prevState.hasAcceptedPrivacyPolicyAndWalletAgreement,
        }), () => this.checkError());
    }

It is external, and your proposal looks good to me @adeel0202! Once @MitchExpensify has created the upwork job go ahead and apply to it :)

Upwork Job

📣 @Santhosh-Sellavel You have been assigned to this job by @melvin-bot[bot]!
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 📖

Thanks @MitchExpensify, I have applied on Upwork.

Melvin-bot has assigned this job to @Santhosh-Sellavel 👀

@adeel0202 I'm assigned here for proposal review!

Looks good for me too! @stitesExpensify
🎀 👀 🎀 C+ reviewed
Let's assign him this one.
cc: @MitchExpensify
Feel free to submit  PR when you are ready @adeel0202
Also @adeel0202 ,
Suggestion checkError could be renamed as clearError

Sure, will rename it to clearError.

Thanks, @MitchExpensify.

Do I wait for this job to be assigned to me or can I just raise the PR?

You are good to go with the PR @adeel0202

PR raised.
@Santhosh-Sellavel, I got Prefer an early return to a conditionally-wrapped function body lint error on the first commit, so I refactored the clearError method.

@stitesExpensify Let's assign @adeel0202  this one to remove the Help Wanted label.

@stitesExpensify Let's assign @adeel0202 this one to remove the Help Wanted label.

Thanks, I forgot!

This was deployed to production yesterday

Setting a reminder to pay in 6 days if no regressions

We can remove the maxParticipantsReached from line 225 https://github.com/Expensify/App/blob/43910fe5c20584b05fe56013005e8e59ba93e8a6/src/pages/NewChatPage.js#L221-L226
And add the message text inside the the FixedFooter component

Paid @Santhosh-Sellavel & @adeel0202 !

Thanks @MitchExpensify, but I'm also eligible for the reporting bonus :)

Bonus paid @adeel0202 ! Sorry for missing it earlier

Thank you ☺

authors: MelvinBot MitchExpensify Santhosh-Sellavel adeel0202 mvtglobally stitesExpensify


text:
Fix for delete comment refactor

cc @PauloGasparSv i noticed that report actions were not optimistically updated in your PR here so i updated it in the code here.
Details

Fixed Issues

$ https://github.com/Expensify/App/issues/10753
Tests

Its hard to test the same issue in dev. One way i tested was by adding return; at that start here so we can test if optimistic data is set correctly.
Then i proceeded to delete a comment/attachment and confirmed it was gone from the UI properly.

[ ] Verify that no errors appear in the JS console

PR Review Checklist

Contributor (PR Author) Checklist

[ ] I linked the correct issue in the ### Fixed Issues section above
[ ] I wrote clear testing steps that cover the changes made in this PR

[ ] I added steps for local testing in the Tests section
[ ] I added steps for Staging and/or Production testing in the QA steps section
[ ] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[ ] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[ ] I included screenshots or videos for tests on all platforms
[ ] I ran the tests on all platforms & verified they passed on:

[ ] iOS / native
[ ] Android / native
[ ] iOS / Safari
[ ] Android / Chrome
[ ] MacOS / Chrome
[ ] MacOS / Desktop

[ ] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[ ] I followed proper code patterns (see Reviewing the code)

[ ] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[ ] I verified that comments were added to code that is not self explanatory
[ ] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[ ] I verified any copy / text shown in the product was added in all src/languages/* files
[ ] I verified any copy / text that was added to the app is correct English and approved by marketing by tagging the marketing team on the original GH to get the correct copy.
[ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[ ] I verified the JSDocs style guidelines (in STYLE.md) were followed

[ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[ ] I followed the guidelines as stated in the Review Guidelines
[ ] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[ ] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[ ] If a new component is created I verified that:

[ ] A similar component doesn't exist in the codebase
[ ] All props are defined accurately and each prop has a /** comment above it */
[ ] Any functional components have the displayName property
[ ] The file is named correctly
[ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[ ] The only data being stored in the state is data necessary for rendering and nothing else
[ ] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[ ] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[ ] All JSX used for rendering exists in the render method
[ ] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[ ] If a new CSS style is added I verified that:

[ ] A similar style doesn't already exist
[ ] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)

[ ] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[ ] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[ ] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

PR Reviewer Checklist
The Contributor+ will copy/paste it into a new comment and complete it after the author checklist is completed

[ ] I have verified the author checklist is complete (all boxes are checked off).
[ ] I verified the correct issue is linked in the ### Fixed Issues section above
[ ] I verified testing steps are clear and they cover the changes made in this PR

[ ] I verified the steps for local testing are in the Tests section
[ ] I verified the steps for Staging and/or Production testing are in the QA steps section
[ ] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[ ] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[ ] I checked that screenshots or videos are included for tests on all platforms
[ ] I verified tests pass on all platforms & I tested again on:

[ ] iOS / native
[ ] Android / native
[ ] iOS / Safari
[ ] Android / Chrome
[ ] MacOS / Chrome
[ ] MacOS / Desktop

[ ] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[ ] I verified proper code patterns were followed (see Reviewing the code)

[ ] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
[ ] I verified that comments were added to code that is not self explanatory
[ ] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[ ] I verified any copy / text shown in the product was added in all src/languages/* files
[ ] I verified any copy / text that was added to the app is correct English and approved by marketing by tagging the marketing team on the original GH to get the correct copy.
[ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[ ] I verified the JSDocs style guidelines (in STYLE.md) were followed

[ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[ ] I verified that this PR follows the guidelines as stated in the Review Guidelines
[ ] I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
[ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[ ] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[ ] If a new component is created I verified that:

[ ] A similar component doesn't exist in the codebase
[ ] All props are defined accurately and each prop has a /** comment above it */
[ ] Any functional components have the displayName property
[ ] The file is named correctly
[ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[ ] The only data being stored in the state is data necessary for rendering and nothing else
[ ] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[ ] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[ ] All JSX used for rendering exists in the render method
[ ] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[ ] If a new CSS style is added I verified that:

[ ] A similar style doesn't already exist
[ ] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)

[ ] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[ ] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[ ] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

QA Steps

[ ] Verify that no errors appear in the JS console

Screenshots

Web

Mobile Web

Desktop

iOS

Android

Added Cp-staging label since this tackles a deploy blocker.

@chiragsalian @luacmartins I think this was intentional from this comment and also I think this breaks one of the features!
When we send a message and we are offline, the message should appear strike-thoughed! Then, once it is online again it should vanish.
Setting the content of the message as empty like that makes it not appear when you are offline! The pendingAction was already making it disappear : (

🚀 Cherry-picked to staging by @luacmartins in version: 1.1.96-1 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

@PauloGasparSv good point. Would you mind investigating that please?

@luacmartins created https://github.com/Expensify/App/pull/10780 to revert the changes + adding a comment

🚀 Cherry-picked to staging by https://github.com/AndrewGable in version: 1.3.28-2 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

source: gharchive/pull-request

authors: OSBotify PauloGasparSv chiragsalian luacmartins