-
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
Fix redbrick path for workspace #12329
Conversation
The changes look fine. I couldn't create a @sketchydroide All yours
ScreenshotsWebMobile Web - ChromeMobile Web - SafariDesktopiOSAndroid |
@sketchydroide looks like this was merged without the checklist test passing. Please add a note explaining why this was done and remove the |
Not an emergency, the checklist tests were passing, just rechecked again... Melvin is weird about this |
🚀 Deployed to staging by @sketchydroide in version: 1.2.24-0 🚀
|
🚀 Deployed to production by @yuwenmemon in version: 1.2.24-4 🚀
|
1 similar comment
🚀 Deployed to production by @yuwenmemon in version: 1.2.24-4 🚀
|
🚀 Deployed to production by @yuwenmemon in version: 1.2.24-4 🚀
|
@sketchydroide @aldo-expensify This is deployed to production for 7 days and it is due for payment. Can you please help with this? |
@miljakljajic this is a bit of a weird case:
In the end, @mananjadhav reviewed two PRs for this GH issue. Does @mananjadhav get payed for each PR? (I'm not familiarized how payment works for reviewing) |
@miljakljajic did you get a chance to look at @aldo-expensify's comment? |
@mananjadhav after internal review, we've decided we'll go ahead and pay out for both PRs! Creating an upwork job shortly |
@mananjadhav please apply here! |
Applied @dylanexpensify |
@dylanexpensify I just realized that the amount for the job was $2K. While the first PR was a bit time consuming and took me effort, the second took relatively less time. Do you want to reevaluate and reduce the budget there? |
offer sent @mananjadhav ! |
Accepted @dylanexpensify |
payment sent, contract closed |
Details
The redbrick path is cut if the error is in the policy itself and not its members or custom units.
Fixed Issues
$ #12327
Tests
Using an account that is a domain
QA Steps
With an account that is an admin of the domain @expensifail.com
PR Review Checklist
PR Author Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
filesWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)PR Reviewer Checklist
The reviewer will copy/paste it into a new comment and complete it after the author checklist is completed
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
).src/languages/*
filesWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
have been tested & I retested again)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Screenshots
Web
Mobile Web - Chrome
Desktop
iOS
Android