-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[$1000] [HOLD for payment 2023-05-23] [Manual Requests] Fix participants on the IOUConfirmation page in the case when an admin requests money from the workspace #18708
Comments
Triggered auto assignment to @sakluger ( |
Bug0 Triage Checklist (Main S/O)
|
Just a clarification, the admin is not shown as participant, the other members are. |
Assinged @trjExpensify for payments and @s77rt to fix this |
Dope! @s77rt to confirm, are these the repro steps?
Also, the bug only occurs when the admin of the workspace requests money in their workspace chat. In the above example, if userB requested money in their own workspaceChat, would a list of all workspace members appear underneath the |
@tjferriss Yes, correct, but not need to finish the request process you can see that the participant list is incorrect at the confirmation step. As userB the participants list only contains the workspace (no other members) which is the expected behaviour. Checking now, will post more info asap |
The screenshot doesn't align with the issue. |
Poor TJ man, always getting pinged for @trjExpensify. 😅 😂
Ah okay, got it. I've updated the steps and added them to the OP for posterity. |
Yeah, the title of this issue is also wrong. Look at the video it's on the IOUConfirmation screen #18215 (comment) |
Any luck @s77rt? (If there's a slack thread somewhere for this I missed, feel free to point me to it). |
a sec.. writing 😁 |
Problem
SolutionI think the straightforward solution is to filter the report based on diff (may change based on point 2)diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js
index 83a0b53b40..6dceeb3308 100644
--- a/src/libs/OptionsListUtils.js
+++ b/src/libs/OptionsListUtils.js
@@ -100,26 +100,21 @@ Onyx.connect({
* @returns {Array}
*/
function getPolicyExpenseReportOptions(report) {
- if (!ReportUtils.isPolicyExpenseChat(report)) {
- return [];
- }
- const filteredPolicyExpenseReports = _.filter(policyExpenseReports, (policyExpenseReport) => policyExpenseReport.policyID === report.policyID);
- return _.map(filteredPolicyExpenseReports, (expenseReport) => {
- const policyExpenseChatAvatarSource = ReportUtils.getWorkspaceAvatar(expenseReport);
- return {
- ...expenseReport,
- keyForList: expenseReport.policyID,
- text: expenseReport.displayName,
+ return [
+ {
+ ...report,
+ keyForList: report.policyID,
+ text: report.displayName,
alternateText: Localize.translateLocal('workspace.common.workspace'),
icons: [
{
- source: policyExpenseChatAvatarSource,
- name: expenseReport.displayName,
+ source: ReportUtils.getWorkspaceAvatar(report),
+ name: report.displayName,
type: CONST.ICON_TYPE_WORKSPACE,
},
],
- };
- });
+ },
+ ];
}
/**
diff --git a/src/pages/iou/MoneyRequestModal.js b/src/pages/iou/MoneyRequestModal.js
index b1bfb9f1ff..abee99e4e9 100644
--- a/src/pages/iou/MoneyRequestModal.js
+++ b/src/pages/iou/MoneyRequestModal.js
@@ -111,7 +111,7 @@ const MoneyRequestModal = (props) => {
const [previousStepIndex, setPreviousStepIndex] = useState(-1);
const [currentStepIndex, setCurrentStepIndex] = useState(0);
const [selectedOptions, setSelectedOptions] = useState(
- ReportUtils.isPolicyExpenseChat(props.report)
+ ReportUtils.isPolicyExpenseChat(props.report) && props.report.isOwnPolicyExpenseChat
? OptionsListUtils.getPolicyExpenseReportOptions(props.report)
: OptionsListUtils.getParticipantsOptions(props.report, props.personalDetails),
); |
Are you saying that a policyExpenseChat ("Workspace chat") doesn't get created for anyone other than an admin on the workspace? 🤔 I would find that very strange if it was the case. Every member of a workspace should have a workspace chat. |
They are created, maybe the optimistic part does not work smoothly but they are being created for sure I dont think your solution is ideal in this case, the problem is that the getPolicyExpenseReportOptions returns all the options for the user and its intended for the use in the Participants selection view. But we also use the results in here for the confirmation page and we check if the user is not same. Its honestly a bit confusing as its set to the state I think there is an easier fix, making PR |
@trjExpensify @mountiny I'm not referring to the main workspace chat. This is created for all. I'm referring to the other member-2-member policy chats. The ones with avatar of workspace visible under member avatars. Those only exist for the admin. |
Ah, I see. That's still a @mountiny put a PR up with the fix for this, do you want to take a look through that? |
@trjExpensify Right, but for members there seems to be only one Hey I tagged you correctly twice in a row. That's an achievement 😅 🎉 |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.14-14 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2023-05-23. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
|
Job added to Upwork: https://www.upwork.com/jobs/~019f72dca339218ee5 |
Current assignee @trjExpensify is eligible for the External assigner, not assigning anyone new. |
Current assignee @s77rt is eligible for the External assigner, not assigning anyone new. |
Current assignee @mountiny is eligible for the External assigner, not assigning anyone new. |
Current assignee @s77rt is eligible for the Internal assigner, not assigning anyone new. |
Alrighty, @s77rt sent you an offer on this job for $1,000 for the C+ review. |
I'm not sure if I am eligible for payment here as I didn't get to finish the reviewer checklist. |
I think we can do a partial $250 because there was no checklist but you have helped. Does that work? |
Sounds good! |
@trjExpensify Can you please adjust the offer? |
Yep, done! |
@trjExpensify Accepted! Thanks! |
Settled up! |
Coming from #18215 (comment)
When admin requests money from their workspace as a employee, all the other members of the workspace are shown as participants on the confirmation page when the request is being made
Repro steps
+
>Request Money
> enter an amount > nextTo
are %workspaceName% & userB on the confirmation screen.Note: The bug only occurs when the admin of the workspace requests money in their own workspace chat. In the above example, if userB requested money in their own workspaceChat, then the
To
would only show the %workspaceName% as is expected:cc @luacmartins @Julesssss
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: