Skip to content

Commit

Permalink
Refactor/3250/ngrx (#3271)
Browse files Browse the repository at this point in the history
fix: color buildings of delta mode  again and replace redux through ngrx

Also fix at least one broken memorization leading to a lot of unnecessary calculations

close #3250
fix #3268 (which was probably caused by racing conditions)
  • Loading branch information
Torsten Knauf authored Apr 28, 2023
1 parent 4101072 commit d46fa03
Show file tree
Hide file tree
Showing 494 changed files with 3,741 additions and 5,954 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/)
### Fixed 🐞

- Fix suspicious metrics and risk profile docs not loading [#3272](https://github.com/MaibornWolff/codecharta/pull/3272)
- Show again delta of a building which have nothing in common in red or green [#3271](https://github.com/MaibornWolff/codecharta/pull/3271)

### Chore 👨‍💻 👩‍💻

- Replace custom Redux adapter through real NgRx [#3271](https://github.com/MaibornWolff/codecharta/pull/3271)

## [1.115.1] - 2023-04-06

Expand Down
15 changes: 10 additions & 5 deletions visualization/app/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,35 @@ import { platformBrowserDynamic } from "@angular/platform-browser-dynamic"
import { HttpClientModule } from "@angular/common/http"
import { FormsModule, ReactiveFormsModule } from "@angular/forms"
import { MaterialModule } from "./material/material.module"
import { EffectsModule } from "./codeCharta/state/angular-redux/effects/effects.module"
import { EffectsModule } from "@ngrx/effects"
import { UnfocusNodesEffect } from "./codeCharta/state/effects/unfocusNodes/unfocusNodes.effect"
import { AddBlacklistItemsIfNotResultsInEmptyMapEffect } from "./codeCharta/state/effects/addBlacklistItemsIfNotResultsInEmptyMap/addBlacklistItemsIfNotResultsInEmptyMap.effect"
import { dialogs } from "./codeCharta/ui/dialogs/dialogs"
import { OpenNodeContextMenuEffect } from "./codeCharta/state/effects/nodeContextMenu/openNodeContextMenu.effect"
import { BlacklistSearchPatternEffect } from "./codeCharta/ui/searchPanel/searchBar/blacklistSearchPattern.effect"
import { ResetColorRangeEffect } from "./codeCharta/state/store/dynamicSettings/colorRange/resetColorRange.effect"
import { ResetChosenMetricsEffect } from "./codeCharta/state/effects/resetChosenMetrics/resetChosenMetrics.effect"
import { UpdateEdgePreviewsEffect } from "./codeCharta/state/effects/updateEdgePreviews/updateEdgePreviews.effect"
import { ChangelogDialogModule } from "./codeCharta/ui/dialogs/changelogDialog/changelogDialog.module"
import { VersionService } from "./codeCharta/services/version/version.service"
import { RenderCodeMapEffect } from "./codeCharta/state/effects/renderCodeMapEffect/renderCodeMap.effect"
import { AutoFitCodeMapEffect } from "./codeCharta/state/effects/autoFitCodeMapChange/autoFitCodeMap.effect"
import { CodeChartaModule } from "./codeCharta/codeCharta.module"
import { UpdateVisibleTopLabelsEffect } from "./codeCharta/state/effects/updateVisibleTopLabels/updateVisibleTopLabels.effect"
import { ResetSelectedEdgeMetricWhenItDoesntExistAnymoreEffect } from "./codeCharta/state/effects/resetSelectedEdgeMetricWhenItDoesntExistAnymore/resetSelectedEdgeMetricWhenItDoesntExistAnymore.effect"
import { LinkColorMetricToHeightMetricEffect } from "./codeCharta/state/effects/linkColorMetricToHeightMetric/linkColorMetricToHeightMetric.effect"
import { UpdateFileSettingsEffect } from "./codeCharta/state/effects/updateFileSettings/updateFileSettings.effect"
import { CodeChartaComponent } from "./codeCharta/codeCharta.component"
import { NodeContextMenuCardModule } from "./codeCharta/state/effects/nodeContextMenu/nodeContextMenuCard/nodeContextMenuCard.module"
import { StoreModule } from "@ngrx/store"
import { appReducers, setStateMiddleware } from "./codeCharta/state/store/state.manager"
import { ResetColorRangeEffect } from "./codeCharta/state/store/dynamicSettings/colorRange/resetColorRange.effect"
import { ResetChosenMetricsEffect } from "./codeCharta/state/effects/resetChosenMetrics/resetChosenMetrics.effect"
import { ResetSelectedEdgeMetricWhenItDoesntExistAnymoreEffect } from "./codeCharta/state/effects/resetSelectedEdgeMetricWhenItDoesntExistAnymore/resetSelectedEdgeMetricWhenItDoesntExistAnymore.effect"
import { SetLoadingIndicatorEffect } from "./codeCharta/state/effects/setLoadingIndicator/setLoadingIndicator.effect"

@NgModule({
imports: [
BrowserModule,
HttpClientModule,
StoreModule.forRoot(appReducers, { metaReducers: [setStateMiddleware] }),
EffectsModule.forRoot([
UnfocusNodesEffect,
AddBlacklistItemsIfNotResultsInEmptyMapEffect,
Expand All @@ -42,7 +46,8 @@ import { NodeContextMenuCardModule } from "./codeCharta/state/effects/nodeContex
UpdateVisibleTopLabelsEffect,
LinkColorMetricToHeightMetricEffect,
ResetSelectedEdgeMetricWhenItDoesntExistAnymoreEffect,
UpdateFileSettingsEffect
UpdateFileSettingsEffect,
SetLoadingIndicatorEffect
]),
MaterialModule,
FormsModule,
Expand Down
4 changes: 2 additions & 2 deletions visualization/app/codeCharta/codeCharta.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import { TestBed } from "@angular/core/testing"
import { LoadInitialFileService } from "./services/loadInitialFile/loadInitialFile.service"
import { CodeChartaModule } from "./codeCharta.module"
import { CodeChartaComponent } from "./codeCharta.component"
import { Store } from "./state/angular-redux/store"
import { setIsLoadingFile } from "./state/store/appSettings/isLoadingFile/isLoadingFile.actions"
import { Store } from "@ngrx/store"

describe("codeChartaComponent", () => {
beforeEach(() => {
Expand All @@ -20,7 +20,7 @@ describe("codeChartaComponent", () => {
const codeChartaComponent = new CodeChartaComponent(mockedStore, mockedLoadInitialFileService)
await codeChartaComponent.ngOnInit()

expect(mockedStore.dispatch).toHaveBeenCalledWith(setIsLoadingFile(true))
expect(mockedStore.dispatch).toHaveBeenCalledWith(setIsLoadingFile({ value: true }))
expect(mockedLoadInitialFileService.loadFileOrSample).toHaveBeenCalled()
})

Expand Down
4 changes: 2 additions & 2 deletions visualization/app/codeCharta/codeCharta.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { setIsLoadingFile } from "./state/store/appSettings/isLoadingFile/isLoad
import packageJson from "../../package.json"
import { LoadInitialFileService } from "./services/loadInitialFile/loadInitialFile.service"
import { Component, OnInit, ViewEncapsulation } from "@angular/core"
import { Store } from "./state/angular-redux/store"
import { Store } from "@ngrx/store"

@Component({
selector: "cc-code-charta",
Expand All @@ -21,7 +21,7 @@ export class CodeChartaComponent implements OnInit {
}

async ngOnInit(): Promise<void> {
this.store.dispatch(setIsLoadingFile(true))
this.store.dispatch(setIsLoadingFile({ value: true }))
await this.loadInitialFileService.loadFileOrSample()
}
}
36 changes: 14 additions & 22 deletions visualization/app/codeCharta/codeCharta.model.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import { Vector3 } from "three"
import { Action } from "redux"
import { ExportCCFile } from "./codeCharta.api.model"
import { FileState } from "./model/files/files"
import { CustomConfig } from "./model/customConfig/customConfig.api.model"
import Rectangle from "./util/algorithm/streetLayout/rectangle"
import { RightClickedNodeData } from "./state/store/appStatus/rightClickedNodeData/rightClickedNodeData.actions"
import { Scaling } from "./state/store/appSettings/scaling/scaling.actions"

export type Scaling = {
x: number
y: number
z: number
}

export interface NameDataPair {
fileName: string
Expand Down Expand Up @@ -85,7 +88,7 @@ export enum SortingOption {
NUMBER_OF_FILES = "Number of Files"
}

export interface colorLabelOptions {
export interface ColorLabelOptions {
positive: boolean
negative: boolean
neutral: boolean
Expand Down Expand Up @@ -155,7 +158,7 @@ export interface AppSettings {
sharpnessMode: SharpnessMode
experimentalFeaturesEnabled: boolean
screenshotToClipboardEnabled: boolean
colorLabels: colorLabelOptions
colorLabels: ColorLabelOptions
isColorMetricLinkedToHeightMetric: boolean
enableFloorLabels: boolean
}
Expand Down Expand Up @@ -347,7 +350,7 @@ export interface Node {
outgoingEdgePoint: Vector3
}

export interface State {
export interface CcState {
fileSettings: FileSettings
dynamicSettings: DynamicSettings
appSettings: AppSettings
Expand Down Expand Up @@ -382,23 +385,12 @@ export function stateObjectReviver(_, valueToRevive) {
return valueToRevive
}

export interface CCAction extends Action {
// TODO: Do not use any here! Make sure all our actions are properly declared.
//
// As a starting point:
//
// RecursivePartial<MetricData & DynamicSettings & FileSettings & AppSettings & FileState> & {
// metricData: MetricData
// dynamicSettings: DynamicSettings
// fileSettings: FileSettings
// appSettings: AppSettings
// files: FileState[]
// }
payload?: any
}

export interface AppStatus {
hoveredNodeId: number | null
selectedBuildingId: number | null
rightClickedNodeData: RightClickedNodeData
rightClickedNodeData: {
nodeId: number
xPositionOfRightClickEvent: number
yPositionOfRightClickEvent: number
} | null
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { TestBed } from "@angular/core/testing"
import { LoadFileService } from "./loadFile.service"
import { TEST_FILE_CONTENT } from "../../util/dataMocks"
import { CCFile, NodeMetricData, NodeType } from "../../codeCharta.model"
import { removeFile, setDeltaReference, setFiles, setStandard } from "../../state/store/files/files.actions"
import { CCFile, CcState, NodeMetricData, NodeType } from "../../codeCharta.model"
import { removeFile, setDeltaReference, setStandard } from "../../state/store/files/files.actions"
import { ExportBlacklistType, ExportCCFile } from "../../codeCharta.api.model"
import { getCCFiles, isPartialState } from "../../model/files/files.helper"
import { CCFileValidationResult, ERROR_MESSAGES } from "../../util/fileValidator"
Expand All @@ -11,11 +11,11 @@ import { clone } from "../../util/clone"
import { klona } from "klona"
import { ErrorDialogComponent } from "../../ui/dialogs/errorDialog/errorDialog.component"
import { loadFilesValidationToErrorDialog } from "../../util/loadFilesValidationToErrorDialog"
import { Store } from "../../state/angular-redux/store"
import { State } from "../../state/angular-redux/state"
import { fileRoot } from "./fileRoot"
import { MatDialog } from "@angular/material/dialog"
import { metricDataSelector } from "../../state/selectors/accumulatedData/metricData/metricData.selector"
import { State, Store, StoreModule } from "@ngrx/store"
import { appReducers, setStateMiddleware } from "../../state/store/state.manager"

const mockedMetricDataSelector = metricDataSelector as unknown as jest.Mock
jest.mock("../../state/selectors/accumulatedData/metricData/metricData.selector", () => ({
Expand All @@ -24,8 +24,8 @@ jest.mock("../../state/selectors/accumulatedData/metricData/metricData.selector"

describe("loadFileService", () => {
let codeChartaService: LoadFileService
let store: Store
let state: State
let store: Store<CcState>
let state: State<CcState>
let dialog: MatDialog
let validFileContent: ExportCCFile
let metricData: NodeMetricData[]
Expand All @@ -41,14 +41,16 @@ describe("loadFileService", () => {
{ name: "mcc", maxValue: 1, minValue: 1 },
{ name: "rloc", maxValue: 2, minValue: 1 }
]
store.dispatch(setFiles([]))
})

afterEach(() => {
codeChartaService.referenceFileSubscription.unsubscribe()
})

function restartSystem() {
TestBed.configureTestingModule({
imports: [StoreModule.forRoot(appReducers, { metaReducers: [setStateMiddleware] })]
})
store = TestBed.inject(Store)
state = TestBed.inject(State)
dialog = { open: jest.fn() } as unknown as MatDialog
Expand Down Expand Up @@ -365,7 +367,7 @@ describe("loadFileService", () => {
{ fileName: "SecondFile", content: validFileContent, fileSize: 42 }
])

store.dispatch(removeFile("FirstFile"))
store.dispatch(removeFile({ fileName: "FirstFile" }))
expect(state.getValue().files).toHaveLength(1)
expect(state.getValue().files[0].file.fileMeta.fileName).toEqual("SecondFile")
})
Expand All @@ -376,12 +378,12 @@ describe("loadFileService", () => {
const updateRootDataSpy = jest.spyOn(fileRoot, "updateRoot")

const newReferenceFile = state.getValue().files[0].file
store.dispatch(setDeltaReference(newReferenceFile))
store.dispatch(setDeltaReference({ file: newReferenceFile }))
expect(updateRootDataSpy).toHaveBeenCalledTimes(1)
expect(updateRootDataSpy).toHaveBeenCalledWith(state.getValue().files[0].file.map.name)

// set reference file to a partial selected file. Therefore reference file becomes undefined
store.dispatch(setStandard([state.getValue().files[0].file]))
store.dispatch(setStandard({ files: [state.getValue().files[0].file] }))
expect(updateRootDataSpy).toHaveBeenCalledTimes(1)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@ import { clone } from "../../util/clone"
import { CCFileValidationResult } from "../../util/fileValidator"
import { setFiles, setStandardByNames } from "../../state/store/files/files.actions"
import { FileState } from "../../model/files/files"
import { NameDataPair } from "../../codeCharta.model"
import { NameDataPair, CcState } from "../../codeCharta.model"
import { referenceFileSelector } from "../../state/selectors/referenceFile/referenceFile.selector"
import { ErrorDialogComponent } from "../../ui/dialogs/errorDialog/errorDialog.component"
import { loadFilesValidationToErrorDialog } from "../../util/loadFilesValidationToErrorDialog"
import { Store } from "../../state/angular-redux/store"
import { State } from "../../state/angular-redux/state"
import { enrichFileStatesAndRecentFilesWithValidationResults } from "./fileParser"
import { fileRoot } from "./fileRoot"
import { Store, State } from "@ngrx/store"

@Injectable({ providedIn: "root" })
export class LoadFileService implements OnDestroy {
Expand All @@ -29,7 +28,7 @@ export class LoadFileService implements OnDestroy {
)
.subscribe()

constructor(private store: Store, private state: State, private dialog: MatDialog) {}
constructor(private store: Store<CcState>, private state: State<CcState>, private dialog: MatDialog) {}

ngOnDestroy(): void {
this.referenceFileSubscription.unsubscribe()
Expand All @@ -52,11 +51,11 @@ export class LoadFileService implements OnDestroy {
throw new Error("No files could be uploaded")
}

this.store.dispatch(setFiles(fileStates))
this.store.dispatch(setFiles({ value: fileStates }))

const recentFile = recentFiles[0]
const rootName = this.state.getValue().files.find(f => f.file.fileMeta.fileName === recentFile).file.map.name
this.store.dispatch(setStandardByNames(recentFiles))
this.store.dispatch(setStandardByNames({ fileNames: recentFiles }))

fileRoot.updateRoot(rootName)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
import { TestBed } from "@angular/core/testing"
import { HttpClient } from "@angular/common/http"
import { CCFile, LayoutAlgorithm } from "../../codeCharta.model"
import { State } from "../../state/angular-redux/state"
import { GLOBAL_SETTINGS } from "../../util/dataMocks"
import { GlobalSettingsHelper } from "../../util/globalSettingsHelper"
import { LoadInitialFileService } from "./loadInitialFile.service"
import { Store } from "../../state/angular-redux/store"
import { LoadFileService } from "../loadFile/loadFile.service"
import { ErrorDialogComponent } from "../../ui/dialogs/errorDialog/errorDialog.component"
import sample1 from "../../assets/sample1.cc.json"
import sample2 from "../../assets/sample2.cc.json"
import { FileSelectionState, FileState } from "../../model/files/files"
import { setFiles } from "../../state/store/files/files.actions"
import { MatDialog } from "@angular/material/dialog"
import { State, Store, StoreModule } from "@ngrx/store"
import { appReducers, setStateMiddleware } from "../../state/store/state.manager"

describe("LoadInitialFileService", () => {
let loadInitialFileService: LoadInitialFileService
Expand All @@ -23,9 +23,8 @@ describe("LoadInitialFileService", () => {

mockedDialog = { open: jest.fn() } as unknown as MatDialog
TestBed.configureTestingModule({
imports: [[StoreModule.forRoot(appReducers, { metaReducers: [setStateMiddleware] })]],
providers: [
Store,
State,
{ provide: MatDialog, useValue: mockedDialog },
{ provide: HttpClient, useValue: {} },
{ provide: LoadFileService, useValue: { loadFiles: jest.fn() } }
Expand Down Expand Up @@ -74,36 +73,36 @@ describe("LoadInitialFileService", () => {
it("should set files to standard mode when no 'mode' parameter is given", () => {
const store = TestBed.inject(Store)
const fileState: FileState[] = [{ file: {} as CCFile, selectedAs: FileSelectionState.None }]
store.dispatch(setFiles(fileState))
store.dispatch(setFiles({ value: fileState }))
store.dispatch = jest.fn()
loadInitialFileService["urlUtils"].getParameterByName = () => ""

loadInitialFileService["setRenderStateFromUrl"]()

expect(store.dispatch).toHaveBeenCalledWith({ payload: [{}], type: "SET_STANDARD" })
expect(store.dispatch).toHaveBeenCalledWith({ files: [{}], type: "SET_STANDARD" })
})

it("should set files to multiple mode when any string (except 'Delta') is given", () => {
const store = TestBed.inject(Store)
const fileState: FileState[] = [{ file: {} as CCFile, selectedAs: FileSelectionState.None }]
store.dispatch(setFiles(fileState))
store.dispatch(setFiles({ value: fileState }))
store.dispatch = jest.fn()
loadInitialFileService["urlUtils"].getParameterByName = () => "invalidMode"

loadInitialFileService["setRenderStateFromUrl"]()

expect(store.dispatch).toHaveBeenCalledWith({ payload: [{}], type: "SET_STANDARD" })
expect(store.dispatch).toHaveBeenCalledWith({ files: [{}], type: "SET_STANDARD" })
})

it("should set files to delta mode when 'mode=delta' parameter is given", () => {
const store = TestBed.inject(Store)
const fileState: FileState[] = [{ file: {} as CCFile, selectedAs: FileSelectionState.None }]
store.dispatch(setFiles(fileState))
store.dispatch(setFiles({ value: fileState }))
store.dispatch = jest.fn()
loadInitialFileService["urlUtils"].getParameterByName = () => "Delta"

loadInitialFileService["setRenderStateFromUrl"]()

expect(store.dispatch).toHaveBeenCalledWith({ payload: [{}], type: "SET_STANDARD" })
expect(store.dispatch).toHaveBeenCalledWith({ files: [{}], type: "SET_STANDARD" })
})
})
Loading

0 comments on commit d46fa03

Please sign in to comment.