-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Updated logic to Not allow future date as incorporation date #5939
Conversation
i.e. Displayed a more specific error message if incorporation date is in the future. e.g. "Incorporation date cannot be in the future."
@puneetlath can you please review the pr again, I also updated screen record with latest implementation. It shows one file conflict, I think it can be resolved before merge. Please let me know if anything need from my side. Thanks. |
@PrashantMangukiya I'll give it another review! Can you please go ahead and resolve the merge conflict? |
@puneetlath thanks, this pull request shows me this message Only those with write access to this repository can merge pull requests. It looks like I have no write access to this repo. Can you please guide me if any way to resolve conflict from my side ? I will be happy to do it. |
What do you see when you click the "resolve conflicts" button? |
@puneetlath thanks for helping. It allowed me to edit code and resolve when clicked Resolve Conflicts button. I just resolved it. Let us wait to finish all checks. |
Happy to help! |
@Luke9389 Updated |
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.
Thanks for the changes 👍
I ask every contributor this one last question after reviews are over. Have you tested this on all platforms after the most recent change? This helps us reduce regressions and often ends up in the contributor being paid faster.
Also, we have a [HOLD] right now due to a company retreat, so we need to wait on merging this.
Yes, I did testing locally before every commit I pushed to pr, so you can freely merge it once hold released. |
f50eb39
Removed Company Offsite HOLD, feel free to merge @puneetlath |
✋ 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 @puneetlath in version: 1.1.15-18 🚀
|
🚀 Deployed to production by @roryabraham in version: 1.1.16-10 🚀
|
@puneetlath PR is ready for review.
Details
Updated logic to show error whenever future date entered as incorporation date.
So now it will not allow to proceed if future date entered as company incorporation date.
Proposal: #5722 (comment)
Confirmation: #5722 (comment)
Fixed Issues
$ #5722
Tests | QA Steps
For example 01.01.2022 or Any date greater than current date.
It will show error message "Please enter a valid date" under incorporation date if future date entered.
So It will not allow to proceed if future date entered as incorporation date.
Tested On
Screenshots
Web
Web.mp4
Mobile Web
MobileWeb.mov
Desktop
Desktop.mp4
iOS
iOS.mov
Android
Android.mp4