frontend: make internet checks non-binary#3043
Merged
patrickelectric merged 4 commits intobluerobotics:masterfrom Feb 6, 2025
Merged
frontend: make internet checks non-binary#3043patrickelectric merged 4 commits intobluerobotics:masterfrom
patrickelectric merged 4 commits intobluerobotics:masterfrom
Conversation
Member
Author
469352e to
f6b4c7d
Compare
Member
Author
Reviewer's Guide by SourceryThis pull request modifies the internet connection check to have three states: online, offline and limited. It also updates the UI to reflect the new states. Sequence diagram for internet connection checksequenceDiagram
participant PS as PingStore
participant BE as Backend
participant Sites as External Sites
PS->>BE: Check internet connection
BE->>Sites: Check reachability
Sites-->>BE: Sites status
BE-->>PS: Response with site statuses
Note over PS: Determine connection state:
Note over PS: - All sites: ONLINE
Note over PS: - No sites: OFFLINE
Note over PS: - Some sites: LIMITED
Note over PS: - Error: UNKNOWN
Class diagram showing updated PingStore and InternetConnectionStateclassDiagram
class PingStore {
-API_URL: string
-has_internet: InternetConnectionState
-services: Service[]
+setHasInternet(has_internet: InternetConnectionState)
+fetchData()
}
class InternetConnectionState {
<<enumeration>>
OFFLINE
UNKNOWN
LIMITED
ONLINE
}
PingStore --> InternetConnectionState: uses
State diagram for internet connection statesstateDiagram-v2
[*] --> UNKNOWN
UNKNOWN --> OFFLINE: No sites reachable
UNKNOWN --> ONLINE: All sites reachable
UNKNOWN --> LIMITED: Some sites reachable
OFFLINE --> UNKNOWN: Connection check error
ONLINE --> UNKNOWN: Connection check error
LIMITED --> UNKNOWN: Connection check error
OFFLINE --> ONLINE: All sites become reachable
OFFLINE --> LIMITED: Some sites become reachable
LIMITED --> ONLINE: All sites become reachable
LIMITED --> OFFLINE: No sites reachable
ONLINE --> LIMITED: Some sites unreachable
ONLINE --> OFFLINE: No sites reachable
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey @Williangalvani - I've reviewed your changes - here's some feedback:
Overall Comments:
- The tooltip text in InternetTrayMenu.vue should be updated to reflect the new connection states (LIMITED and UNKNOWN) rather than just showing binary online/offline messages
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| }), | ||
| computed: { | ||
| tooltip() { | ||
| return helper.has_internet ? 'Vehicle has internet access.' : 'Internet connection is not available.' |
There was a problem hiding this comment.
issue: Update tooltip to reflect all possible connection states
The tooltip should be updated to handle all four InternetConnectionState values to avoid misleading users about the actual connection state.
f6b4c7d to
3ceaa0c
Compare
joaomariolago
approved these changes
Feb 4, 2025
3ceaa0c to
e6a488f
Compare
patrickelectric
approved these changes
Feb 6, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


needs testing for the limited case
helps #2921
Summary by Sourcery
Refine the internet connection status to include limited connectivity states.
New Features:
Tests: