-
Notifications
You must be signed in to change notification settings - Fork 0
9 scanning screen with startstop and results #18
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
9 scanning screen with startstop and results #18
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request introduces a comprehensive live feed feature for scan events, enabling users to monitor real-time scan progress and view warnings in the UI. The implementation includes backend API support, new exception handling, UI updates with live feed components, and enhanced user interaction patterns.
- Added live feed API endpoint and service methods with proper error handling
- Updated UI layout to accommodate live feed display with scan progress and diff counting
- Implemented polling mechanism for real-time updates and scan completion handling with user prompts
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| HomeScreen.java | Updated UI layout removing old scan button and adding live feed components |
| HomeScreen.form | Modified form layout to expand scan tab grid and add live feed UI elements |
| BaseScreen.java | Added generic option dialog method for user interaction prompts |
| LiveFeedUtils.java | New utility class for formatting live feed entries and counting warnings |
| ScanDetailsPresenter.java | Fixed potential null pointer exception with list initialization |
| HomePresenter.java | Major refactoring to implement live feed polling and scan completion handling |
| LiveFeedResponse.java | New record for encapsulating live feed API response data |
| Endpoint.java | Added LIVE_FEED endpoint constant |
| ScanService.java | Implemented getLiveFeed method with proper error handling |
| LiveFeedException.java | New exception class for live feed specific errors |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/main/java/org/pwss/view/screen/ScanDetailsScreen.java:1
- The variable name
rootpanelon line 19 should be renamed to follow camelCase convention like the other fields. It should berootPanelto match the existing field.
package org.pwss.view.screen;
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@pwgit-create Finished the last small details during my lunch break :) |
pwgit-create
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Det är en ren fröjd att se vilket bra resultat ni fått av Frontend appen genom era presentationer samt koden här. Vilket arbete ni har lagt ner kamrat , imponerade! 😃
Vill att koden ska bli så bra som möjligt (när ni lagt ner så mycket arbete på denna PR) så var därför jag bad om ändringar.
Det är i princip bara att följa alla Github Copilots instruktioner (har markerat visssa med Agree). De är bra , har verifierat ändringarna Copilot ville se - de kommer gå snabbt att implementera!
Utöver Copilots förslag har jag också lagt ett en egen ändring som jag ville se. Den går också relativt fort att implementera. Gör resolve på alla review comments när ni är färdiga med dem.
Med detta sagt , 100 på arbetet här kamrat! Var stolt över dig själv - nu börjar File-Integrity-Scanner faktiskt bli någonting 💫
| * or an empty Optional if the index is out of bounds. | ||
| */ | ||
| public Diff getDiffAt(int rowIndex) { | ||
| public Optional<Diff> getDiffAt(int rowIndex) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bra @lilstiffy !
| MonitoredDirectory dir = model.getDirectoryAt(modelRow); | ||
| startScanSuccess = scanService.startScanById(dir.id()); | ||
| Optional<MonitoredDirectory> dir = model.getDirectoryAt(modelRow); | ||
| if (dir.isPresent()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:) 👍
| /** | ||
| * A utility class that holds various string constants used throughout the application. | ||
| */ | ||
| public final class StringConstants { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bättre än innan även om jag föredrar att String konstanter som bara används i en klass också placeras i samma klass. Är en smaksak så kommer inte påverka PR. Vill ni inte ha konstanterna i minnet så kan ni använda en XML fil och Parsa String värdet när ni behöver det. Det är lite trixat att implementera sådana lösningar eftersom ni måste:
- Läsa värdet från en XML fil (Läsa bytes från en stream mao) och göra om det till en Sträng.
- För att vinna något på den strategin får ni inte spara Strängen i en persistent variabel. Detta innebär att ni får läsa strängen från fil på nytt varje gång den används. Det kommer ta tid.
- IT-säkerhet. Kommer behövas att tänka på input sanitation!
Det finns nog mer effektiva sätt att hantera strängar från filer, så ni kan gärna undersöka det om ni vill. Jag tycker dock att det sätt ni har gjort på är tillräckligt bra, och det bästa är nog att ni fokuserar på att få ut första versionen av releasen. Efter det kan ni titta på att förbättra lagringen av strängarna om ni vill. Men gör som ni vill, det är bara mitt tips 😊
Är ni intresserad kolla här: https://www.baeldung.com/java-parse-xml-string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Valid! Vi kanske kan skapa en puck på att ha strängarna i XML istället! :)
| SwingUtilities.invokeLater(() -> { | ||
| if (stopScanSuccess) { | ||
| screen.showSuccess("Scan stopped successfully!"); | ||
| screen.showSuccess(StringConstants.SCAN_STOPPED_SUCCESS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 😃
| return Optional.of(scans.get(rowIndex)); | ||
| } | ||
| return null; | ||
| return Optional.empty(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
| package org.pwss.utils; | ||
|
|
||
| /** | ||
| * A utility class that holds various string constants used throughout the application. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Java Docs 💯 +
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We like it!
Java docs is 🔥
pwgit-create
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 Ni har verkligen fixat allt på ett toppenbra sätt! 🙌 Nu kan ni merga in koden från en riktigt välgjord PR till er Develop Branch. 💻✨ Fantastiskt jobbat, Stefan! 😄
Tack så jättemycket kamrat! 🫡 🥇 |
This pull request introduces the live feed feature for scan events, allowing users to view real-time scan progress and warnings in the UI. The changes include backend support for fetching live feed data, new exception handling, UI updates to display the live feed, and utility methods for formatting and counting warnings. The polling logic is updated to fetch live feed updates periodically and handle scan completion scenarios.
Backend and API changes:
LiveFeedExceptionclass to handle errors specific to live feed retrieval.LIVE_FEEDendpoint in theEndpointenum and implemented thegetLiveFeedmethod inScanService, which fetches live feed updates and handles various error scenarios. [1] [2]LiveFeedResponserecord to encapsulate scan running status and live feed data.UI and presenter logic:
HomePresenterto poll live feed updates during scans, display live feed and diff count in the UI, and handle scan completion with user prompts for viewing details. [1] [2] [3] [4]HomeScreen.formto accommodate new live feed components.Utilities and user interaction:
LiveFeedUtilswith methods for formatting live feed entries and counting warnings, improving readability and diff tracking.showOptionDialogmethod inBaseScreenfor user prompts, used when a scan completes with differences.NOTE: MARKED AS DRAFT
Discussion is needed for the comment i put in HomePresenter l:216-219