App
App copied to clipboard
[$500] IOU/LHN - IOU not been pinned by default
If you havenβt already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 1.3.99-0 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause - Internal Team Slack conversation:
Action Performed:
Precondition: user should be signed in
- Open app
- Create IOU money request with any user
- Navigate back to LHN
- Observe the IOU report position in LHN and its icons
Expected Result:
Step 4: Transaction thread report is pinned by default
Actual Result:
Step 4: Transaction thread report isn't pinned by default
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
- [x] Android: Native
- [x] Android: mWeb Chrome
- [x] iOS: Native
- [x] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [x] MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/Expensify/App/assets/78819774/02ee1e05-17a3-4d1d-b1e0-22c8751cdb10
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~0159bd7d65d5681c02
- Upwork Job ID: 1724881721908154368
- Last Price Increase: 2023-11-29
- Automatic offers:
- dukenv0307 | Contributor | 28037472
Issue Owner
Current Issue Owner: @neil-marcellini
Job added to Upwork: https://www.upwork.com/jobs/~0159bd7d65d5681c02
Triggered auto assignment to @bfitzexpensify (Bug
), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
Bug0 Triage Checklist (Main S/O)
- [ ] This "bug" occurs on a supported platform (ensure
Platforms
in OP are β ) - [ ] This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
- [ ] This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
- [ ] This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
- [ ] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync
Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (External
)
Test Rail: https://expensify.testrail.io/index.php?/tests/view/3974494&group_by=cases:section_id&group_order=asc&group_id=229063
Proposal
Please re-state the problem that we are trying to solve in this issue.
When requesting money in a conversation, the latter should be pinned.
What is the root cause of that problem?
IOU.requestMoney does not pin the report.
What changes do you think we should make in order to solve the problem?
I added the following line to IOU.requestMoney :
Report.notifyNewAction(chatReport.reportID, payeeAccountID);
Proposal
Please re-state the problem that we are trying to solve in this issue.
The problem is that the IOU request is not automatically pinning the conversation after it is made
Also the context menu containing the pin option but this would be resolved by having the IOU request return isPinned true.
What is the root cause of that problem?
Essentially the togglePinnedState is not being called in the current IOU.js RequestMoney function.
Report.togglePinnedState(chatReport.reportID, false);
additionally the context menu issue would be resolved by this fix because it would now be returning isPinnedChat flag as true
What changes do you think we should make in order to solve the problem?
Incorporating a Report.TogglePin on the already existing IOU.RequsetMoney function should return this to normalcy
What alternative solutions did you explore? (Optional)
Alternatively the actual backend for the requestMoney could perform a similar action and just handle this but this may be this may not be the most simple, predictable and trackable solution in the current setup
π£ @riqts! π£ Hey, it seems we donβt have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:
- Make sure you've read and understood the contributing guidelines.
- Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
- Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
- Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>
Contributor details Your Expensify account email: [email protected] Upwork Profile Link: https://www.upwork.com/freelancers/~014853d868137150b0
β Contributor details stored successfully. Thank you for contributing to Expensify!
This looks like a backend issue. IOU details report should have isPinned: true
when creating, but it isn't now
Proposal
Please re-state the problem that we are trying to solve in this issue.
- Transaction thread report isn't pinned by default
What is the root cause of that problem?
- BE return transaction thread report with
isPinned
asfalse
. When we build optimisticData for transaction thread report, we don't setisPinned
astrue
by default
What changes do you think we should make in order to solve the problem?
- BE should return transaction thread report with
isPinned
astrue
. When we buildoptimisticData
for transaction thread report, we also need to setisPinned
astrue
What alternative solutions did you explore? (Optional)
Few proposals ready for you to review @Santhosh-Sellavel
Sorry @bfitzexpensify Not sure how I missed this. I've unassigned newly assigned issues due to low bandwidth somehow missed it. If any C+ can review this immediately please Reassign. Otherwise I can look into this tomorrow.
All good! Tomorrow is fine.
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
Sorry again for the delay here
@lanitochka17 I'm confused here about Expected Result.
Step 4: created IOU report should be pinned by default in LHN as mentioned in the TestRail case steps
Are you referring to each IOU Request thread that should be pinned or each report Can you share me test rail link/ID
Steps 7 and 11: the pinned option should not be displayed in the context menu on IOU report details page
Again I'm why should pinned option shouldn't be displayed? What do you mean by pinned option here, showing option both pin/unpin options?
As @dukenv0307 explains here, are these following issues?
- Transaction thread report isn't pinned by default
- Transaction thread report has pin option
@bfitzexpensify Can you please confirm the bug and the expected behavior or add Engineering
label to add a internal to confirm?
@bfitzexpensify, @Santhosh-Sellavel Whoops! This issue is 2 days overdue. Let's get this updated quick!
bump @lanitochka17 cc: @bfitzexpensify
@Santhosh-Sellavel Test Rail: https://expensify.testrail.io/index.php?/tests/view/3974494&group_by=cases:section_id&group_order=asc&group_id=229063
@bfitzexpensify @Santhosh-Sellavel this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
@bfitzexpensify, @Santhosh-Sellavel Whoops! This issue is 2 days overdue. Let's get this updated quick!
Thanks for the test rail. @lanitochka17 was this test passing earlier, when it got added.
@bfitzexpensify Can you check and provide us clear expected OP. From test rail I found this internal issue link https://github.com/Expensify/Expensify/issues/298942 but I don't have access to this was this a regression if so please provide more details from which issue this got added, thanks!
@Santhosh-Sellavel OK, these are the TestRail steps:
- Create a IOU (or use an existing IOU conversation)
- Verify that IOU conversations (reports) are always pinned by default in the LHN
- Right-click on the IOU conversation (the one with a green dot) in the LHN
- Verify that there is no pin option on the context Menu
So @dukenv0307's identified issues here are correct:
- Transaction thread report isn't pinned by default
- Transaction thread report has pin option
Does that help clarify things? I also updated the actual/expected behaviours in the OP.
@bfitzexpensify Can we unpin & pin iou report or not? Is it pinned always?
Pinned always, so it shouldn't be possible to unpin
Thanks then @dukenv0307's proposal looks good to me. Can you apply an internal label as we need a BE fix for this first?
C+ Reviewed π π π
Triggered auto assignment to @neil-marcellini, see https://stackoverflow.com/c/expensify/questions/7972 for more details.