Fix F1ServerApp Features/Models review findings - #591
Merged
Conversation
Resolves a batch of low-severity findings from the repository review affecting F1ServerApp components and viewdata models: - Replace any usage with concrete types for MAT_DIALOG_DATA payloads, chart.js click/hover handlers and MatSelect change events. - Introduce shared TS enums and decoders (data/enums.ts) synchronized with F1Server.Core.Enumerations, replacing the duplicated and drifting numeric switch statements in sessionviewdata.ts, trackviewdata.ts, driverviewdata.ts and livesessionviewdata.ts. - Align ISessionViewData with SessionViewData (championshipId, hideImageButtons) and type loadTracks/loadTracksInternal as Promise<void> instead of Promise<any>. - Remove the unused LastSessionViewApiData/LastDriverApiData models and add the missing totalPacketsReceived field to SessionPacketMetricsViewApiData. - Replace magic numbers for session type comparisons with the new SessionType enum. - Implement OnInit/OnDestroy where missing, remove an unused field, and add error handling to a fire-and-forget HTTP call. - Route diagnostic console.log/console.info calls through the existing LoggerService instead of logging directly in production. - SignalrService: use strict equality, plain methods instead of arrow-function fields, proper error logging, and expose live session state via observables in addition to the existing getters. Closes #291, #276, #254, #252, #251, #250, #249, #270, #269, #267, #237, #235, #234
|
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
2 participants
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.
📖 Description
Fixes a batch of low-severity findings from the repository review affecting
F1ServerAppFeatures and Models:anyusage with concrete types forMAT_DIALOG_DATApayloads, chart.js click/hover handlers andMatSelectchange events (cartelemetry,home,showsession,deletesession,createchampionshipcomponents).data/enums.ts) synchronized withF1Server.Core.Enumerations, replacing the duplicated and drifting numericswitchstatements insessionviewdata.ts,trackviewdata.ts,driverviewdata.tsandlivesessionviewdata.ts.ISessionViewDatawithSessionViewData(championshipId,hideImageButtons) and typeloadTracks/loadTracksInternalasPromise<void>instead ofPromise<any>.LastSessionViewApiData/LastDriverApiDatamodels and add the missingtotalPacketsReceivedfield toSessionPacketMetricsViewApiData(the backend already computes and serializes it).showsession.component.tswith the newSessionTypeenum.OnInit/OnDestroywhere missing (statistics.component.ts,app.component.ts), remove an unusedtitlefield, and add error handling to a fire-and-forget HTTP call inapp.component.ts.console.log/console.infocalls through the existingLoggerServiceinstead of logging directly in production (championships,createchampionship,sessionscomponents).SignalrService: use strict equality, plain methods instead of arrow-function fields, proper error logging, and expose live session state via observables in addition to the existing getters.🎫 Issues
Closes #291, closes #276, closes #254, closes #252, closes #251, closes #250, closes #249, closes #270, closes #269, closes #267, closes #237, closes #235, closes #234
👩💻 Reviewer Notes
All changes are contained to
F1ServerApp. No backend changes were needed. The chart.js event handler types were verified against the actualng2-charts@10type declarations to keepstrictTemplateshappy.📑 Test Plan
ng build --configuration developmentpasses with no TypeScript or template errors.✅ Checklist
General
Frontend-specific (Angular)
F1ServerApp.Data/ViewDatacontracts.⏭ Next Steps
None.