Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.55k stars 2.89k forks source link

[Regression test required] [$500] HIGH: Add `Avatar > About > Troubleshooting` section #34082

Closed quinthar closed 8 months ago

quinthar commented 10 months ago

Strategy: A billion users means a billion problems. To diagnose and solve them at scale, we capture client-side logs and upload centrally.

Problem: Some problems can't be diagnosed cleanly after the fact from the logs, for a few reasons:

Solution: Let's expand our ability to remotely diagnose production devices with better dev tools built into the app itself. Specifically:

This is blocking https://github.com/Expensify/App/issues/33040.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01c2a08b8b8ae90e21
  • Upwork Job ID: 1744202378047897600
  • Last Price Increase: 2024-01-15
  • Automatic offers:
    • fedirjh | Reviewer | 28115102
shawnborton commented 9 months ago

I think both of those icons are in the repo already, so perhaps @TMisiukiewicz can try each of those quickly and we can see which one we prefer? I could go either way... I do like that the lightbulb has a more positive connotation to it though!

TMisiukiewicz commented 9 months ago

@shawnborton @dannymcclain I couldn't find the lightbulb in the repo, here's the example view with Exlamation icon image

dannymcclain commented 9 months ago

I don't hate that, but I'm still curious to see the lightbulb icon.

Weirdly, I found the lightbulb here: assets/images/emojiCategoryIcons/light-bulb.svg Not sure why it's not in the main images directory with all the other icons as well though. Does that help at all?

TMisiukiewicz commented 9 months ago

Thanks for finding it, here's a version with a lightbulb: image

shawnborton commented 9 months ago

Big fan of that! Let's see what @dannymcclain and @dubielzyk-expensify think though!

dannymcclain commented 9 months ago

Yeah I like the lightbulb way better! Much less anxiety producing haha.

dubielzyk-expensify commented 9 months ago

Yeah. Lightbulb is cute. Dig it!

TMisiukiewicz commented 9 months ago

Do we need any message to display after pressing "Reset and refresh"? This might be confirmation modal (e.g. "Are you sure you want to wipe the data?"), or alert/toast saying the data was wiped. If so, what message should be there?

image

TMisiukiewicz commented 9 months ago

Opened up a Draft PR! While going through the checklist, noticed new copies have to be approved by marketing team. Can someone take a look on the copies suggested in the design above?

Once they are approved, I'll ask for Spanish translations

TMisiukiewicz commented 9 months ago

I'm also about to start working on a Console, it's not blocking me in any way for now but is there any update regarding the design for it?

shawnborton commented 9 months ago

Do we need any message to display after pressing "Reset and refresh"?

What exactly happens when this is pressed? Like do we even have a notion of "unsaved data" being thrown away? Otherwise I think we don't need a confirmation here.

or alert/toast saying the data was wiped

Again, not sure what data being wiped means in this context. But in terms of confirming to the user that tapping this row took an action, wouldn't the app itself would do a quick refresh, and thus that's the visual feedback the user would get? As in, on mobile I would see the splash screen again as the app loads up again? Or maybe we just use some kind of full page loading spinner while this happens?

shawnborton commented 9 months ago

I'm also about to start working on a Console, it's not blocking me in any way for now but is there any update regarding the design for it?

What does this mean exactly?

TMisiukiewicz commented 9 months ago

@shawnborton wiping the data basically means clearing all the Onyx data except those keys that are needed to run the app as a freshly logged-in user. There are some keys that could affect the user account, e.g. any draft messages would be gone.

You can see the currently implemented behavior on both web and mobile here:

https://github.com/Expensify/App/assets/13985840/f8996dc9-4d05-414a-9f55-e3b806a18de8

https://github.com/Expensify/App/assets/13985840/f02f6655-6a8b-44c1-8863-96cfc0e46748

As you can see, currently there is no full page spinner/splash screen after pressing it. Should we approach it differently?

I'm also about to start working on a Console, it's not blocking me in any way for now but is there any update regarding the design for it?

What does this mean exactly?

I'm talking about "View console" feature mentioned in the first post of this issue. I assume it would look and work pretty similar to the console known from web browsers e.g. Chrome. Would be great if we could figure out how to implement this view in the app

shawnborton commented 9 months ago

wiping the data basically means clearing all the Onyx data except those keys that are needed to run the app as a freshly logged-in user. There are some keys that could affect the user account, e.g. any draft messages would be gone.

Got it, thanks for clarifying. I could see where a small alert dialog would work in that case, which exists as an existing pattern (for instance, try to delete an outstanding money request and you will see the confirmation modal). Curious if @quinthar agrees though!

You can see the currently implemented behavior on both web and mobile here:

Got it. I think this is fine, especially if we have the confirmation modal.

I'm talking about "View console" feature mentioned in the https://github.com/Expensify/App/issues/34082#issue-2069570061. I assume it would look and work pretty similar to the console known from web browsers e.g. Chrome. Would be great if we could figure out how to implement this view in the app

I'm not entirely sure what the intention is behind this feature, so would love for @quinthar to chime in on this one too. Are you thinking we would display our own custom view of a console, or just launch the existing browser console, or something else?

TMisiukiewicz commented 9 months ago

I was able to create a basic implementation of in-app console that executes the arbitrary code, here is the demo:

https://github.com/Expensify/App/assets/13985840/2226aa3a-3798-43f3-90e4-51b8364eefe0

Next thing coming is connecting it with existing logs. Still waiting for any guidelines regarding the design.

@muttmuure could you help me get an approval for new copies listed in this comment? This would speed up opening the first PR with Troubleshoot page 🙏

muttmuure commented 9 months ago

For sure I can help

muttmuure commented 9 months ago

@LLPeckham is going to take a look at your copy

LLPeckham commented 9 months ago

Hiya! Added a few clarifying questions below for the first two bullets just so we can possibly make those more specific.

cc @muttmuure @TMisiukiewicz

muttmuure commented 9 months ago

My two cents:

Are more specific and help indicate what these new features do

LLPeckham commented 9 months ago

I'd be down for those, @muttmuure. I think just adding a little more context helps in this case. So we'd have:

Use the tools below to help troubleshoot the Expensify experience. If you encounter any issues, please submit a bug.

TMisiukiewicz commented 9 months ago

I'm thinking we should a tiny bit more detail so someone knows what exactly they're doing when they click this.

Agree on that one, however we might give a detailed description in a confirmation modal after pressing it

Possibly something like "Reset Onyx data and refresh" Or something like this?

Users probably don't know what Onyx is, so I think "Reset and refresh stashed data" proposed by @muttmuure should be fine and we can think about a little bit detailed description in a confirmation modal as I mentioned above

LLPeckham commented 9 months ago

Ok cool, so let's move forward with this for the copy if you agree, @TMisiukiewicz?

TMisiukiewicz commented 9 months ago

sounds great to me @LLPeckham 👍 Thank you!

TMisiukiewicz commented 9 months ago

Since copies are approved, I sent the draft PR #35306 for our internal review, I'll open it once I have an approval from CK side.

Today I was also moving forward with the console. I've already integrated existing logs with it and added functionality of saving it on a device. Tomorrow I'll be working on displaying real-time logs as well

https://github.com/Expensify/App/assets/13985840/540e555e-4a70-494c-a986-215ce34459d6

shawnborton commented 9 months ago

We can help provide better designs for this page: CleanShot 2024-01-31 at 10 18 35@2x

But to get you started, we should open up a new page in the RHP dedicated to this entire thing. So the page/view would be called Console.

shawnborton commented 9 months ago

@dubielzyk-expensify given that you worked on the original design for this, let us know if you want to take a stab at the console page design too.

Looks like the brief for that one is:

Add View console which opens a simple console:

  • This would first open up and initialize with all locally stored logs, so you can scroll back and see what happened
  • It would also append new logs in realtime
  • There would also be a Save button that just copies all available logs into a text file that is saved on the device
  • Add a Share button that brings up a Search selector and just posts the logs into the selected room (and then leaves the user navigated there, so they can talk about them). I'm imagining this would be useful for chatting with a real world user and asking them to do something and send you the logs.
  • At the bottom is a command line that works similar to the Chrome console, where you can execute arbitrary JavaScript, with the results output to the console
dubielzyk-expensify commented 9 months ago

Here's my take on the console screen. A few notes:

CleanShot 2024-02-01 at 13 11 08@2x

Keen on @Expensify/design thoughts

shawnborton commented 9 months ago

Looks great to me! Nice job on this.

dannymcclain commented 9 months ago

Look great! Hear me out, would the console be a good spot to bring in Expensify Mono?? Just ignore if you think it's unnecessary—you know me, always trying to find uses for ExMono 🙃

shawnborton commented 9 months ago

OMG YES DO IT!

TMisiukiewicz commented 9 months ago

The design looks great! 👍 I already started slowly implementing it today.

I have more technical problem now. Today I started working on adding real-time logs, that's why i thought switching my implementation to Onyx would be good. My idea was simply merging new logs when they appear in the logger, however that generates a problem of infinite loop, since we are getting merge() called... log endlessly in this scenario. So a few questions from my side:

shawnborton commented 9 months ago

Curious what @quinthar thinks about your questions, I'm not too sure how to answer them.

dubielzyk-expensify commented 9 months ago

100% with you on Mono font. I thought about it but was unsure if we even had access to it in the app. So glad you brought this up <3 Updated mocks with mono font: CleanShot 2024-02-02 at 08 18 28@2x

fedirjh commented 9 months ago
  • Add Reset and refresh, which wipes Onyx clean except for the minimum needed to call OpenApp (so you don't need to reauthenticate)

cc @quinthar What is the expected behavior for this feature in offline mode? Wiping the data may result in the app becoming unusable in offline mode. Should we consider enabling this option exclusively for online users, or is logging out the user in offline mode a more suitable approach?

Also, I think we need to have an internal engineer assigned to this issue.

🎀 👀 🎀 C+ reviewed

melvin-bot[bot] commented 9 months ago

Triggered auto assignment to @techievivek, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

shawnborton commented 9 months ago

I thought about it but was unsure if we even had access to it in the app.

We totally have access to it already! Just try using ticks for inline code or three ticks for a code block!

TMisiukiewicz commented 9 months ago

Small update, I connected logs with Onyx and created a list of logs that should be ignored, so I can avoid infinite loop when merging new items. Now items are displayed in real-time when console is open, however now it does not display previously collected logs, so I have to figure out how to correctly collect them in the background.

I also started working on a design implementation:

image
shawnborton commented 9 months ago

Looking good so far. For the buttons, why isn't the button text center-aligned?

TMisiukiewicz commented 9 months ago

Today I connected logs with Onyx and made it possible to display historical logs together with real-time logs. I feel like storing all the Onyx logs might be inefficient at some point, after some tests my file to download already had 100mbs. I think it'd be helpful to decide what exact logs should be stored there, eventually we should think of building some kind of mechanism that would flush old logs from the console. cc @quinthar

Here's the demo showing historical logs, adding real-time logs and executing commands

https://github.com/Expensify/App/assets/13985840/9d82ab13-01e5-471e-9a46-bed7e3a0bf44

Looking good so far. For the buttons, why isn't the button text center-aligned?

@shawnborton I was looking at the code responsible for displaying icon and text in the button and turns out when the left or right icon is displayed, the text is automatically aligned to the left 🤔 should it work different way?

shawnborton commented 9 months ago

It should always be center-aligned as far as I know... cc @Expensify/design for additional thoughts though.

dannymcclain commented 9 months ago

It should always be center-aligned as far as I know... cc @Expensify/design for additional thoughts though.

Definitely. Those buttons should look like Jon's mock.

dubielzyk-expensify commented 9 months ago

Yep, what Shawn and Danny said. Also worth mentioning that the logs area is the only area that should scale: CleanShot 2024-02-06 at 09 40 29@2x

greg-schroeder commented 9 months ago

Okay so according to https://github.com/Expensify/App/issues/34082#issuecomment-1923783535 this needs to be Internal instead, so I've adjusted it accordingly.

@techievivek you were assigned by the C+ review - do you want to take this, or should we make it a Hot Pick and call it out in the update? cc @quinthar

I guess I see @TMisiukiewicz working on part of it, so is the internal engineer working on a separate PR or just doing the review stages alongside C+? Sorry, just a bit confusing here

melvin-bot[bot] commented 9 months ago

Current assignee @fedirjh is eligible for the Internal assigner, not assigning anyone new.

TMisiukiewicz commented 9 months ago

@shawnborton @dannymcclain @dubielzyk-expensify button styles updated ✅ once we have it merged this style will apply globally

image

quinthar commented 9 months ago

Wow, this is all amazing! Just to confirm, the logs aren't saved to Onyx until you open this troubleshooting dialog, right? We don't want to do anything that will affect the 99.99% of users who won't open this dialog. Where are the logs normally stored, prior to this change?

TMisiukiewicz commented 9 months ago

@quinthar right now it's all stored in the Onyx from the beginning since we want to see historical logs or actions that happened when the app is backgrounded. I agree that is not the optimal solution as it would affect all the users. We can easily change it to collect logs only if user opens up a troubleshooting page, however once it is done, there would be no way to disable it other than logging out/flushing Onyx data, which may lead to uncontrolled amounts of data stored in Onyx . As an alternative, I'd suggest making logs collector as an opt-in, so they would be stored in Onyx only if the user specifically enables it with toggle (pretty much the same way the "Developer options" on Android works). This way those who want to track their actions might enable or disable it whenever they need. Disabling would automatically clear all the logs stored in Onyx. If the toggle is off, "View debug console" should not be visible. WDYT?

shawnborton commented 9 months ago

button styles updated ✅ once we have it merged this style will apply globally

Looks great, thanks!

TMisiukiewicz commented 9 months ago

Today's update:

Tomorrow I have a plan to implement sending log for selected report

https://github.com/Expensify/App/assets/13985840/621f359b-7dd0-4c3d-9be2-18922e292430

TMisiukiewicz commented 9 months ago

hi @quinthar today I’ve been working on sharing the logs. I was investigating how logged data grows on a mobile and how could we effectively collect it. So right now I have ~2300 logs which is around 2,1mb. My first idea was to send it as formatted message, however composer accepts maximum 10000 characters. So sending this as a txt file seems more efficient, as it gives a possibility to send a file up to 24mbs. We could automatically remove logs older than e.g. 3 days to keep it as lean as possible and avoid building files that exceeds this limit. What do you think about this solution?

Additionally, do we want to display all logs that are going through logger, or do we want to collect only some of them? Second solution would definitely have a positive impact on a size of the file

I also posted an idea to make collecting logs an opt-in here. Wondering what are your thoughts about this, since as you mentioned, we don’t want to affect 99% of the users who won’t open the debug console.