-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fullstory integration Update. #42046
Conversation
(cherry picked from commit e58ec49)
[CP Staging] Revert "Implement suggestion for edit composer" (cherry picked from commit 95aa19a)
(cherry picked from commit 390bf24)
…ertyBeingUsedByFullstory [CP Staging] Changing properties being used by Fullstory (cherry picked from commit 51372ab)
# Conflicts: # android/app/build.gradle # ios/NewExpensify/Info.plist # ios/NewExpensifyTests/Info.plist # ios/NotificationServiceExtension/Info.plist # package-lock.json # package.json
# Conflicts: # android/app/build.gradle # android/build.gradle # babel.config.js # ios/NewExpensify.xcodeproj/project.pbxproj # ios/NewExpensify/Info.plist # ios/NewExpensifyTests/Info.plist # ios/NotificationServiceExtension/Info.plist # ios/Podfile.lock # package-lock.json # package.json
@alitoshmatov Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
@alitoshmatov , removing you from this one since @ishpaul777 was already assigned to the original issue! |
@LCOleksii can you please apply this PR on top of yours? https://github.com/Expensify/App/pull/40807/files We've implemented the user metadata keys, so now you can use those to map everything instead of the session key. That was reverted with the other changes. |
Hey @LCOleksii Can you please resolve lint issues |
Looks like we're still missing this:
|
This is on my list for testing today, will pick this last because it requires a full rebuild of both native apps 🙇♂️ |
@ishpaul777 theres one more commit incoming, so please wait until that's done! |
…ertyBeingUsedByFullstory [CP Staging] Changing properties being used by Fullstory (cherry picked from commit 51372ab)
hey @LCOleksii App seems to crash when opening a video (IOS) Simulator.Screen.Recording.-.iPhone.15.Plus.-.2024-05-21.at.00.34.26.mp4Also, Can you please upload remaining test videos, preferably on github so its easy review them. |
Hi @ishpaul777
Please see the latest recording: Simulator Screen Recording - iPhone 15 Plus - 2024-05-20 at 22.37.25 If after this the issue still persist, please could you provide full crash log for investigation. |
On dev build, its still crashing (followed all of the above steps) |
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪 |
Okay it works well on Adhoc, let me ask someone else to build it on their DEV enviornment https://expensify.slack.com/archives/C02NK2DQWUX/p1716288064932099 Adhoc video trim.E5267B68-7FC5-4F18-BA73-A50631F18DE4.MOV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving since adhoc works fine 🤷
dee6710
to
dd22a4c
Compare
Attachment - Corrupted pdf can be uploaded without errorVersion Number: v1.4.74-4 Action Performed:
Expected Result:In Step 4, the preview will show "Failed to load PDF file" (production behavior). Actual Result:In Step 4, the preview shows filename.pdf. Workaround:n/a Platforms:Which of our officially supported platforms is this issue occurring on?
Screenshots/VideosAdd any screenshot/video evidence IMG_7006.MP4Bug6487529_1716302300396.Screen_Recording_20240521_223230_New_Expensify.mp4 |
Above is not a bug but expected behaviour from #38010 |
@ishpaul777 I've done a few other changes, can you please recheck/approve? this will only affect loading the fullstory lib in production, so you don't need to do all tests again |
Nice, once merge freeze is lifted, we'll merge this! |
Fullstory integration Update. (cherry picked from commit b7d6a68)
🚀 Cherry-picked to staging by https://github.com/puneetlath in version: 1.4.76-1 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
🚀 Deployed to production by https://github.com/Beamanator in version: 1.4.76-7 🚀
|
Details
Fixed Issues
$ #40910
$ #40784
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
https://drive.google.com/file/d/1EQRkfkWRo5Gm8uBQ_nSJg-h5XqDBJwqB/view?usp=drive_linkiOS: Native
https://drive.google.com/file/d/1R3ALatTtvHBW-E0x4PqhbcUrVv70s1UC/view?usp=drive_linkMacOS: Chrome / Safari
https://drive.google.com/file/d/1pK0UacdVlrCWuSCSLpsrGs7rHsjiGu-N/view?usp=drive_linkMacOS: Desktop
https://drive.google.com/file/d/1m2DdKWPDjrY7HAVVhHeRLZNhgmGlSVAq/view?usp=drive_link