-
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
[HOLD for payment 2024-04-05] [$500] Not able to share logs to own self DM from "Debug console" #38147
Comments
Triggered auto assignment to @CortneyOfstad ( |
Was able to recreate — believe this falls under VIP-VSB 👍 |
Asked here |
Alright, David confirmed it is under VIP-VSB – going to have the priority be low, as the logs can be downloaded or sent into an admin channel as workarounds. |
Job added to Upwork: https://www.upwork.com/jobs/~013e74b76766270067 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @mkhutornyi ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.The user is unable to share logs to self dm What is the root cause of that problem?in the App/src/libs/OptionsListUtils.ts Lines 1751 to 1764 in 8f32e60
What changes do you think we should make in order to solve the problem?add includeSelfDM:true,
includeThreads: true, this way we are sure that no regressions will occur in the |
ProposalPlease re-state the problem that we are trying to solve in this issue.Not able to share logs to own self DM from "Debug console" What is the root cause of that problem?This is a two parts issue. First we need to set Second we need to remove It is safe to remove What changes do you think we should make in order to solve the problem?As explained in the root cause section. What alternative solutions did you explore? (Optional)n/a ![]() |
ProposalPlease re-state the problem that we are trying to solve in this issue.The problem is that the user is unable to share logs to their own self DM from the "Debug console". What is the root cause of that problem?The root cause is that in the getShareLogOptions function, we are not including the user's own DM as a valid option to share logs to. This is because:
So in the getShareLogOptions use case, we need to both pass the What changes do you think we should make in order to solve the problem?To solve this, we should make two key changes in OptionsListUtils.ts:
function getShareLogOptions(reports: OnyxCollection<Report>, personalDetails: OnyxEntry<PersonalDetailsList>, searchValue = '', betas: Beta[] = []): GetOptions {
return getOptions(reports, personalDetails, {
betas,
searchInputValue: searchValue.trim(),
includeRecentReports: true,
includeMultipleParticipantReports: true,
sortByReportTypeInSearch: true,
includePersonalDetails: true,
forcePolicyNamePreview: true,
includeOwnedWorkspaceChats: true,
includeSelfDM: true, // Add this line
});
}
const optionsToExclude: Option[] = [{login: CONST.EMAIL.NOTIFICATIONS}];
if (!(includeSelfDM && currentUserLogin)) {
optionsToExclude.push({login: currentUserLogin});
} What alternative solutions did you explore?I considered an alternative of always including the user's DM and not having the |
ProposalPlease re-state the problem that we are trying to solve in this issue.What is the root cause of that problem?Currently by default we don't include App/src/libs/OptionsListUtils.ts Line 1548 in 8f32e60
This in turn doesn't allow us to show current user details What changes do you think we should make in order to solve the problem?Add option const optionsToExclude: Option[] = includeSelfDM ? [{ login: CONST.EMAIL.NOTIFICATIONS }] : [{ login: currentUserLogin }, { login: CONST.EMAIL.NOTIFICATIONS }]; The above changes will check the value of
What alternative solutions did you explore? (Optional)N/A |
my logic looks a little odd, but just want to clarify that my proposal is not saying get rid of |
@mkhutornyi thoughts on the proposals above? If you can provide any feedback by EOD, that would be great! |
I believe enabling the option to send logs to what do you think @CortneyOfstad ? |
I LOVE this idea. I think it makes it much cleaner from a user perspective as well, helping to keep the chat organized without having giant blocks of texts. Great suggestion @abzokhattab! @mkhutornyi Would love to get your input on this as well! |
yes agree but before doing that, I'd like to ask author who implmeneted this feature if it was intentional. There might have been context/discussion for this. |
There might be deleted threads in the search results as well, and hence they might have disabled the threads option 🤔 |
started a discussion here: https://github.com/Expensify/App/pull/36337/files#r1526981210 |
Continuing to wait on a response from Vivek and Tomaz and the question here. If we don't hear anything by EOD, going to bump 👍 |
The PR is ready! |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.57-5 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 2024-04-05. 🎊 For reference, here are some details about the assignees on this 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:
|
@abzokhattab sent you an offer here Please let me know once you both accept. Also @mkhutornyi please have the checklist above completed by EOD today — thanks! |
Thank you! Just accepted the offer |
Payment Summary
BugZero Checklist (@CortneyOfstad)
|
@mkhutornyi looks like you haven't accepted the offer yet. Can you do that when you have a chance and let me know here? Thanks! |
Bump @mkhutornyi ^^^ |
It shows that @mkhutornyi is out sick, so going to keep this open until they are back 👍 |
According to Slack @mkhutornyi is still sick, so continuing to keep this open 👍 |
@mkhutornyi — just checking to see if you're back? Thanks! |
@mkhutornyi going to close this, feel free to reopen when you're back — hope all is well! |
@mkhutornyi is back! can you please accept the job and reply here once you have? @CortneyOfstad can you finish off payment once it's ready? |
Welcome back @mkhutornyi! |
Accepted offer. Thanks |
Payment Summary@mkhutornyi — paid $500 via Upwork |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 1.4.50-5
Reproducible in staging?: y
Reproducible in production?:
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: @quinthar
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1710208120099339
Action Performed:
Expected Result:
User should be able to share to own DM to access in desktop
Actual Result:
Does not give the option in the list
Workaround:
unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
RPReplay_Final1710258700.MP4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @mkhutornyiThe text was updated successfully, but these errors were encountered: