Skip to content
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

Sprint 3 improvements #30

Merged
merged 18 commits into from
Feb 21, 2024
Merged

Sprint 3 improvements #30

merged 18 commits into from
Feb 21, 2024

Conversation

yaslam-dev
Copy link
Collaborator

@yaslam-dev yaslam-dev commented Feb 12, 2024

Addressed Issues

  1. There is a 1-2 second delay between clicking 'more details' and the left panel updating
  2. Discarding a problem using the 'Discard' button should remove the problem from the left panel (It already removes the highlight) The inconsistency will lead to confusion
  3. While hovering over "More Details" and "Fix" we should hide an ugly URL instead user-friendly text will appear.
  4. The color button that appears below the recommendation is fixed.
  5. Other Files with problems shows the same file multiple times, e.g., views.py is fixed.
  6. If we open the other file with problems by clicking “open”, on the other file it says: “Please open a project to analyze”, even though we should rather update the ‘other files with problems’ list to be non-inclusive of the file that you are on, but include the file you came from if it still has unsolved problems is fixed.
  7. The list is gone if we leave the metabob extension panel and go back to it.

Changes

  1. The back button is added to the webview.
  2. Dynamic color theme based on user preferences.
  3. Other files with problems feature is added.
  4. Discarding the problem happens quickly now.
  5. A new API endpoint feedback/read is integrated.

@yaslam-dev yaslam-dev force-pushed the sprint-3-improvements branch from 5e6158b to 79e6f21 Compare February 15, 2024 20:20
Copy link
Contributor

@AviGopal AviGopal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like not all the previous issues are resolved, namely:

  • The discard button now no longer removes the highlighting, in addition to continuing to show a discarded problem's info. Switching between files will remove the highlighting, but the problem still remains
  • The apply button similarly doesn't change the UI, nor does it actually change the code in the highlighted region, however it does remove the problem highlight when switching back from another file.

In addition there are a few new functionality issues. Below are the two main ones:

View problems in other files does not update
If a user analyzes a file, let's call it file_1 and then goes to another file, file_2, after the results appear but before viewing any results the side panel in file_2 will correctly show "file_1" as an option with files in the state, if they choose to analyze file_2, wait for the results to populate but then change back to file_1 the side panel in file_1 will not show "file_2" as another file with problems.

File 1
image
File 2
image

Both of these images are taken after file_1 and file_2 have a valid analysis, with no seen problems.

To rectify, file_1's side panel should show "Problems in other files" for files analyzed after file_1.

Recommendations are global
Currently generating a recommendation, along with the recommendation history, are global to the application and not specifically tied to each problem. This leads to incorrect recommendations already shown when clicking on other detections in the list.

Example:
A recommendation was generated for the problem detection below.
image
However when switching to another problem in the file (or even in another file)
image
This will lead to some confusion as the state of the side panel is not properly represented.
To rectify:
The generated content should be stored for each problem, rather than globally.

Other issues:

  • The buttons to change between generated recommendation drafts should change color when you're at the beginning or end of the list

@yaslam-dev
Copy link
Collaborator Author

yaslam-dev commented Feb 16, 2024

@AviGopal, I have made changes to address the issues. Before testing, please do npm i to install the latest packages.

  • The discard button now no longer removes the highlighting, in addition to continuing to show a discarded problem's info. Switching between files will remove the highlighting, but the problem still remains

The issue was due to negative line number being sent by Backend primarily in endline field. The value that vscode support is in the range of [0, lineNumber]. Hopefully should be fixed

  • The apply button similarly doesn't change the UI, nor does it actually change the code in the highlighted region, however it does remove the problem highlight when switching back from another file.

Similar to discard problem mentioned above; Hopefully should not happen now.

  • The buttons to change between generated recommendation drafts should change color when you're at the beginning or end of the list

The issue is fixed.

  • Recommendations are global
    Currently generating a recommendation, along with the recommendation history, are global to the application and not specifically tied to each problem. This leads to incorrect recommendations already shown when clicking on other detections in the list.

The issue is fixed. I investigated the issue in detail and found out that I was not resetting the state upon application state changes and also localized recomendation to its problem. Should be fixed now.

  • View problems in other files does not update

I spent a lot of time on this issue to investigate in detail; the first issue was we were not sending data upon webview initialization - what I mean by this is when a user closes the extension then opens again; Vscode initializes the webview from scratch i.e local state is destroyed; I have added persistence to local storage of webview which is not destroyed by vscode; that leads to smooth user-expierence.

The second issue was getting errors in the message that information webview of the current file, that is fixed as well.

Let me know your thoughts after testing again, Hopefully, you will have smooth and stable expierence.

@yaslam-dev yaslam-dev requested a review from AviGopal February 16, 2024 22:46
Copy link
Contributor

@AviGopal AviGopal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the issues with discarding problems and generating/applying fixes have been partially resolved. As they now correctly remove the problem from the state for that file however, there is still some unintended behavior.

There is now leakage of state after interacting with the problem information panel.

Details:
If there is a completed analyses in two or more files. Reviewing a problem in one file and interacting with the discard, endorse or apply buttons will merge all problem detections into the current file.

To replicate:

  • Run an analysis in two or more files and wait for each to complete
  • Discard a problem in the first file analyzed
  • You should see that the number of highlighted regions have increased; with the detection areas from the other file being present
  • Clicking on any of these will show the detection information for a file other than the active file, The logs will report this this as well.

Note the log messages at the bottom
image

Furthermore, the detected problems in other files dialog will not show the other analyzed file.

image

Since this was not an issue in the previous build, there must have been an oversight when modifying the decoration handler

To rectify:
Double check that decorations are only added for the currently active file.

@yaslam-dev yaslam-dev requested a review from AviGopal February 20, 2024 18:05
@AviGopal
Copy link
Contributor

Everything seems to be working okay after my testing today. I'll merge this branch in a bit if there's nothing left for you to add

@AviGopal AviGopal merged commit 798c3bd into main Feb 21, 2024
1 check passed
@yaslam-dev yaslam-dev deleted the sprint-3-improvements branch February 29, 2024 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants