-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[No QA] Asking for company name and URL during invoice creation #46009
[No QA] Asking for company name and URL during invoice creation #46009
Conversation
companyInfo: 'Información de la empresa', | ||
companyInfoDescription: 'Necesitamos algunos detalles más antes de que pueda enviar su primera factura.', | ||
yourCompanyName: 'Nombre de su empresa', | ||
yourCompanyWebsite: 'Sitio web de su empresa', | ||
yourCompanyWebsiteNote: 'Si no tiene un sitio web, puede proporcionar el perfil de LinkedIn o de las redes sociales de su empresa.', | ||
invalidDomainError: 'Ha introducido un dominio no válido. Para continuar, introduzca un dominio válido.', | ||
publicDomainError: 'Ha introducido un dominio público. Para continuar, introduzca un dominio privado.', |
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.
I've asked for translations approve in the slack: https://expensify.slack.com/archives/C01GTK53T8Q/p1721825016527359
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2024-08-06.at.2.35.09.in.the.afternoon.movAndroid: mWeb ChromeScreen.Recording.2024-07-30.at.11.10.14.at.night.moviOS: NativeScreen.Recording.2024-08-06.at.2.49.34.in.the.afternoon.moviOS: mWeb SafariScreen.Recording.2024-07-30.at.11.27.36.at.night.movMacOS: Chrome / SafariScreen.Recording.2024-07-30.at.10.21.56.at.night.movMacOS: DesktopScreen.Recording.2024-08-06.at.2.19.33.in.the.afternoon.mov |
@getusha I've applied the requested updates, please take another look |
@getusha kind bump 🙂 |
@getusha can you prioritize getting this done please? |
👍 will be on it in 2 hours. |
@VickyStash seems like we don't allow urls without protocols, and the error message doesn't looks like misleading. should we have a new copy for this? wdyt @VickyStash? |
I've reused the existing isValidWebsite check for the website validity and the existing error message. |
I think this message is fine since we already validate this type of field |
…45174-invoice-company-info
@getusha Could you please continue your review? |
Note: you may have to try again a couple of times Screen.Recording.2024-08-06.at.2.25.10.in.the.afternoon.mov |
@VickyStash It always navigates me to enter both fields as new, despite filling it for previous invoices. Screen.Recording.2024-08-06.at.2.35.09.in.the.afternoon.mov |
It's cause you followed the pre-step described in the PR:
@getusha So it always shows you the screen for testing purposes cause you updated it so. When the API updated the check will works as expected. |
@getusha It looks like this bug isn't related to this PR, see relevant issue #46821 |
We will be handling that as a follow up? is it awaiting backend changes? |
Yes, I'll prepare a follow-up when API is ready |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/madmax330 in version: 9.0.18-0 🚀
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.0.18-10 🚀
|
Details
Updates the invoice creation flow to support adding company name and URL if it wasn't added before.
Fixed Issues
$ #45174
$ #45173
PROPOSAL: N/A
Tests
!Pre-step: Update hasInvoicingDetails check to always return false so you can always see the expected screen. Please, remember that API doesn't support adding company name and the website for now.
Next
button instead of Send Invoice.Next
. You should appear on the Company info page. Make sure it looks as expected.Send invoice
-> the invoice should be successfully created.Offline tests
Same as in the Tests section
QA Steps
N/A
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
android1.mp4
Android: mWeb Chrome
android_web1.mp4
iOS: Native
ios1.mp4
iOS: mWeb Safari
ios_web1.mp4
MacOS: Chrome / Safari
web1.mp4
MacOS: Desktop
desktop1.mp4