Bugfix: ab#2104 save vehicle types#318
Conversation
Feature/ab#2040 refactor build jobs
added Dauerzaehlung
WalkthroughThis PR modernizes the dave-admin-portal project by upgrading to version 5.1.1, restructuring GitHub Actions workflows for streamlined CI/CD, introducing a complete Helm chart for Kubernetes deployment, and adding dynamic city district management with configuration editing. It also extends the Zaehlung domain to support permanent count tracking. ChangesBuild Infrastructure & Deployment Configuration
City District Management & Permanent Count Features
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/src/components/map/ZaehlstelleMap.vue (1)
372-377:⚠️ Potential issue | 🟠 Major | ⚡ Quick winIncorrect Leaflet API usage for OpenStreetMap tiles.
OpenStreetMap serves standard XYZ raster tiles, not WMS (Web Map Service). Use
L.tileLayer()for XYZ tile endpoints with{z}/{x}/{y}URL patterns. TheL.tileLayer.wms()method is for WMS endpoints that require parameters likeLAYERS,REQUEST,SERVICE, etc. This will cause the layer to fail or render incorrectly.Proposed fix
- const osm = L.tileLayer.wms( - "https://tile.openstreetmap.org/{z}/{x}/{y}.png", - { - attribution: mapAttributionOpenStreetMap, - } - ); + const osm = L.tileLayer( + "https://tile.openstreetmap.org/{z}/{x}/{y}.png", + { + attribution: mapAttributionOpenStreetMap, + } + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/map/ZaehlstelleMap.vue` around lines 372 - 377, Replace the incorrect WMS call with an XYZ tile layer: the code currently creates the OpenStreetMap layer using L.tileLayer.wms(...) (variable osm), which is for WMS endpoints; change it to use L.tileLayer(...) with the same "{z}/{x}/{y}" URL and preserve options like attribution (mapAttributionOpenStreetMap) so the osm layer is instantiated with L.tileLayer rather than L.tileLayer.wms.frontend/src/components/zaehlstelle/UpdateZaehlstelleForm.vue (2)
163-174:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDead branch:
length >= 6is unreachable afterlength === 6is already handled.The
else if (length >= 6)guard at line 169 can never be reached because the precedingelse if (length === 6)at line 167 already covers that case. The intent was almost certainly> 6(i.e., length of 7 or more), otherwisereturn 100is never executed.🐛 Proposed fix
- } else if (zaehlstelle.value.nummer.length >= 6) { + } else if (zaehlstelle.value.nummer.length > 6) { return 100; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/zaehlstelle/UpdateZaehlstelleForm.vue` around lines 163 - 174, The computed getStadtbezirksviertelNummer contains an unreachable branch because it checks else if (length === 6) then else if (length >= 6); update the second guard to else if (zaehlstelle.value.nummer.length > 6) so lengths greater than 6 fall through to return 100, and ensure all references use zaehlstelle.value.nummer consistently when checking length and slicing.
145-161:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing assignment in
elsebranch —stadtbezirksviertelMapis alwaysundefinedfor regular districts.Line 150 calls
StadtbezirksviertelToBeschreibung.get(...)but discards the return value.stadtbezirksviertelMapremainsundefined, sogetStadtbezirksviertelalways falls through to"Das Stadtbezirksviertel konnte nicht ermittelt werden."for anystadtbezirkNummer < 100. This is directly related to the "not displayed" bug this PR is meant to fix.🐛 Proposed fix
} else { - StadtbezirksviertelToBeschreibung.get(zaehlstelle.value.stadtbezirkNummer); + stadtbezirksviertelMap = StadtbezirksviertelToBeschreibung.get(zaehlstelle.value.stadtbezirkNummer); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/zaehlstelle/UpdateZaehlstelleForm.vue` around lines 145 - 161, The computed getter getStadtbezirksviertel never assigns the result of StadtbezirksviertelToBeschreibung.get(...) in the else branch, so stadtbezirksviertelMap stays undefined; fix by assigning the map to stadtbezirksviertelMap in the else branch (i.e., set stadtbezirksviertelMap = StadtbezirksviertelToBeschreibung.get(zaehlstelle.value.stadtbezirkNummer)), keeping the existing >=100 branch as-is and leaving the subsequent lookup of stadtbezirksviertelMap.get(getStadtbezirksviertelNummer.value) unchanged.
🧹 Nitpick comments (4)
frontend/src/components/zaehlung/ZaehlungCard.vue (1)
331-333: 💤 Low valueTrivial computed wrapping a single property — can be inlined
dauerzaehlungdoes nothing beyond re-exposingzaehlung.value.dauerzaehlung. SincedefineModelrefs are auto-unwrapped in Vue 3 templates, the template can reference it directly, removing the boilerplate.♻️ Proposed simplification
Remove the computed:
-const dauerzaehlung = computed(() => { - return zaehlung.value.dauerzaehlung; -}); -And update the template reference:
- <v-card-subtitle v-if="dauerzaehlung">Dauerzählung</v-card-subtitle> + <v-card-subtitle v-if="zaehlung.dauerzaehlung">Dauerzählung</v-card-subtitle>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/zaehlung/ZaehlungCard.vue` around lines 331 - 333, Remove the trivial computed "dauerzaehlung" that only returns zaehlung.value.dauerzaehlung; delete the const dauerzaehlung = computed(...) in ZaehlungCard.vue and update template bindings to use the unwrapped model ref directly (e.g. zaehlung.dauerzaehlung) instead; also remove any now-unused imports or refs that existed solely for the computed.frontend/src/components/zaehlung/form/AllgemeineInfoForm.vue (1)
32-42: 💤 Low valueConsider grouping
dauerzaehlungwithsonderzaehlungfor visual consistency
dauerzaehlungis placed in the first row next toprojektNummer/projektName, while the analogoussonderzaehlungcheckbox sits in its own dedicated row (lines 141–153). Grouping both checkboxes together would make the form layout easier to scan.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/zaehlung/form/AllgemeineInfoForm.vue` around lines 32 - 42, Move the dauerzaehlung checkbox so it’s visually grouped with the sonderzaehlung checkbox for consistency: locate the v-checkbox using v-model="zaehlung.dauerzaehlung" and the v-checkbox using v-model="zaehlung.sonderzaehlung" (in AllgemeineInfoForm.vue) and place them in the same v-row/v-col structure (or adjust columns) so both checkboxes appear together in one row rather than dauerzaehlung at the top and sonderzaehlung in a separate row; ensure labels, color, and hide-details settings remain the same.stack/helm/dave-admin-portal/templates/configmap.yaml (1)
7-10: 💤 Low valueMinor:
nindent 12is non-standard for adata:block.The conventional value for top-level keys under
data:isnindent 2. Withnindent 12the rendered YAML is technically valid (any indent > 0 is legal YAML), but it looks unusual and may confuse tooling or future editors.🔧 Suggested alignment with Helm convention
data: {{- with .Values.extraEnvVars }} - {{- toYaml . | nindent 12 }} + {{- toYaml . | nindent 2 }} {{- end}}(The YAMLlint error on line 6 is a false positive — plain YAML linters cannot parse Helm
{{- ... }}template syntax.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stack/helm/dave-admin-portal/templates/configmap.yaml` around lines 7 - 10, The data: block is using an unusually large indentation (nindent 12) for rendered top-level keys; change the template to use toYaml . | nindent 2 so entries from .Values.extraEnvVars are indented with the conventional 2 spaces under the data: block (update the line that currently calls toYaml . | nindent 12 to use nindent 2, leaving the surrounding with .Values.extraEnvVars and toYaml invocation intact).frontend/src/types/enum/Stadtbezirksviertel.ts (1)
560-563: Offer to track this acknowledged technical debt.The TODO comment acknowledges this entire file is a placeholder. The Wolfsburg
[100, "Stadt Wolfsburg"]workaround works with the store's special-casing forbezirksnummer >= 100, but it's clearly a stopgap.Would you like me to open a new issue to track the migration of city-district/sub-district data to the backend service (leveraging the newly-added
CityDistrictDTOandCityInformationService)?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/types/enum/Stadtbezirksviertel.ts` around lines 560 - 563, This file contains a hardcoded stopgap map (StadtbezirksviertelWolfsburgToBeschreibung) and a TODO — create a tracked issue to migrate these city/district/sub-district values into the backend using the new CityDistrictDTO and CityInformationService, update the store logic that special-cases bezirksnummer >= 100 to instead fetch from CityInformationService, remove the frontend hardcoded Map, add a backend migration/seed for existing entries (including the Wolfsburg 100 entry), and add unit/integration tests to cover the service endpoint and the store data-fetch path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build_push_latest.yaml:
- Around line 59-116: The publish jobs assume source/artifacts are present but
never checkout or fetch them: add actions/checkout@v4 as the first step in the
publish_docker job before the docker/build-push-action (so Dockerfile and
context are present) and modify the build job to upload the Maven outputs
(apigateway/target/helm) with actions/upload-artifact, then in publish_helm add
actions/download-artifact (matching the artifact name from build) before running
helm commands so the packaged chart and Chart.yaml are reliably available;
reference the publish_docker and publish_helm job steps and the build job for
where to add upload/download.
In @.github/workflows/pr_build.yaml:
- Around line 66-80: The check_helm job currently assumes apigateway/target/helm
from build-maven will be present but workspace isn’t preserved; update CI so
build-maven uploads the Helm chart/artifacts (from apigateway/target/helm) using
actions/upload-artifact and then modify the check_helm job to either (A)
download that artifact with actions/download-artifact before running the steps
(so helm dep update and helm template run against the downloaded
apigateway/target/helm), or (B) perform a checkout and rebuild/regenerate the
Helm chart in check_helm itself (use actions/checkout and the same build steps
used in build-maven) so the working-directory apigateway/target/helm exists when
helm dep update and helm template run.
In @.github/workflows/release.yaml:
- Around line 94-150: The publish jobs lack a repo checkout and don't consume
build artifacts or the tag output from the build job; add an actions/checkout@v4
step at the start of both publish_docker and publish_helm, and change the build
job to (a) set the git tag as a job output (e.g. echo "TAG=...">>GITHUB_ENV and
echo "::set-output name=tag::$TAG" or use outputs via steps) and (b) upload the
built helm/maven output and Docker context using actions/upload-artifact (use
names like docker-context and helm-artifacts). In publish_docker replace the
local git describe with consuming needs.build.outputs.tag (or read the
downloaded artifact's tag file) and download the docker-context artifact with
actions/download-artifact before Build and push; in publish_helm download the
helm-artifacts artifact before run helm package/push and use the downloaded
files; ensure credentials/login steps remain after checkout. This targets the
publish_docker and publish_helm jobs and the build job's tag/artifact steps
referenced in the workflow.
In `@apigateway/application-local.yml`:
- Around line 1-3: The YAML uses the incorrect property name
"spring.profile.active" so the "prod" profile is never activated; update the
property to the correct Spring Boot key "spring.profiles.active" (note the
plural "profiles") in the application-local.yml to ensure the prod profile is
applied when loading the file.
In `@apigateway/pom.xml`:
- Around line 267-352: There are two duplicate maven-resources-plugin
declarations causing overlapping executions; consolidate them into one <plugin>
for artifactId maven-resources-plugin and merge the executions so you have the
copy-resources execution (id: copy-resources) and a single
resources-helm-deployment execution (id: resources-helm-deployment) under that
plugin, removing the duplicated block and ensuring the helm execution's
configuration (outputDirectory, delimiters, useDefaultDelimiters,
resources/includes/filtering) appears only once.
In `@apigateway/src/main/resources/logback-spring.xml`:
- Around line 57-101: The debug loggers (e.g., logger names "de.muenchen.dave",
"org.springframework.web", "org.springframework.security") and the STDOUT
appender were unintentionally left outside a springProfile, causing debug-level
logging and a duplicate "STDOUT" appender to apply globally; wrap the block that
defines the STDOUT appender and those logger entries in an appropriate
<springProfile name="...">...</springProfile> (or a negated production profile
like <springProfile name="!local & !unittest & !debugging">) so they only apply
to the intended environments, ensure you do not declare a second appender with
the same name (rename or remove the duplicate STDOUT if needed), and restore or
re-scope the production JSON/Logstash appender under the production profile so
structured logging remains enabled in prod.
In `@frontend/src/api/service/CityInformationService.ts`:
- Around line 5-14: The exported class name is wrong: rename the class exported
from this file from ConfigurationService to CityInformationService (update the
class declaration and default export) so it matches the filename and avoids
conflict with the existing ConfigurationService; ensure the static ENDPOINT
constant and method getAllDistricts remain unchanged and update any
imports/usages that referenced ConfigurationService to use
CityInformationService instead.
In `@frontend/src/api/service/ConfigurationService.ts`:
- Around line 24-32: The saveConfiguration method's declared return type
Promise<ConfigurationDTO[]> is incorrect; update
ConfigurationService.saveConfiguration to return the correct type (likely
Promise<ConfigurationItemDTO[]> if the backend echoes the saved items, or
Promise<void> if the endpoint returns no body). Adjust the signature and the
call to FetchService.postData to use ConfigurationItemDTO[] (or void) as the
expected response generic, and replace ConfigurationDTO[] with
ConfigurationItemDTO[] (or void) in the method return type so the types align
with the payload and backend behavior.
In `@frontend/src/App.vue`:
- Around line 153-155: CityInformationService.getAllDistricts() call lacks error
handling and can swallow rejections; update the promise chain where
getAllDistricts() is called (the block that currently calls
cityInformationStore.setCityDistricts(districts)) to add a .catch(error => ...)
that handles network errors — for consistency use the same handler pattern as
other created() calls (e.g., call snackbarStore.showApiError(error) or set an
error ref) so failures are surfaced to the user and not silently ignored.
In `@frontend/src/components/config/ConfigApplication.vue`:
- Around line 70-84: The validateDatatype function currently returns true for
empty values (if (!value) return true) which bypasses INTEGER/DOUBLE/BOOLEAN
validation; update validateDatatype(ConfigurationItemDTO) so it does not
silently accept empty strings for non-STRING datatypes: either remove the
early-return and add an emptiness check that returns a required error for
non-'STRING' items (e.g., if (!value && item.datatype !== 'STRING') return
'Erforderlich' or similar), or document/implement using a separate
validateRequired rule and ensure callers set :rules="[validateRequired,
validateDatatype(item)]"; reference the validateDatatype function and
ConfigurationItemDTO to locate the change.
- Around line 36-43: The Save button currently bypasses field validation; wrap
the editable fields in a <v-form ref="form"> and add a reactive ref named form
in the component, then update the saveConfigValues method to await
form.value?.validate() and return early if validation fails so invalid values
are not POSTed; locate the template block around the v-card-actions and the
saveConfigValues function to add the v-form wrapper and the validation guard.
In `@frontend/src/store/CityInformationStore.ts`:
- Around line 20-24: BeschreibungToStadtbezirk2 currently returns a Map<number,
string> by reversing insertion order of StadtbezirkToBeschreibung2 instead of
inverting key/value; change BeschreibungToStadtbezirk2 to return Map<string,
number> and build it by iterating over StadtbezirkToBeschreibung2.entries() and
inserting (description -> districtNumber) pairs (swap key/value), preserving
deterministic handling of duplicates (decide whether later or earlier wins) and
update the function signature and any callers expecting the old type
accordingly.
- Around line 8-17: The store mutates plain let bindings (cityDistricts,
stadtbezirke2, StadtbezirkToBeschreibung2) and reassigns cityDistricts in
setCityDistricts, which breaks Pinia reactivity; change these to refs (e.g.,
const cityDistricts = ref<Array<CityDistrictDTO>>([]), const stadtbezirke2 =
ref<Array<KeyVal>>([]), const StadtbezirkToBeschreibung2 =
ref<Map<number,string>>(new Map())) and update setCityDistricts to mutate .value
(clear and push into stadtbezirke2.value, set StadtbezirkToBeschreibung2.value,
and assign cityDistricts.value = payload) so components reading
store.cityDistricts, store.stadtbezirke2 and store.StadtbezirkToBeschreibung2
stay reactive.
In `@pom.xml`:
- Around line 13-15: The <scm><tag> element is pinned to a stale release value;
update the <scm><tag> XML entry (the <scm><tag> element in the POM) to "HEAD" on
development branches so it doesn't conflict with the current project version
(5.1.1-SNAPSHOT) or maven-release-plugin's @{project.version} behavior and to
avoid emitting stale SCM metadata during release:prepare.
In `@stack/helm/dave-admin-portal/templates/deployment.yaml`:
- Around line 38-42: The envFrom block must mirror the ConfigMap name selection
used above: update the envFrom.configMapRef.name so it uses the same conditional
as the volume name (check .Values.existingConfigMap vs include
"daveAdminPortal.fullname" .); i.e. wrap the envFrom configMapRef.name in the
same {{- if .Values.existingConfigMap }} ... {{- else }} ... {{- end }} logic
(also adjust the other envFrom occurrence around the 101-103 area) so the Pod
references the provided existingConfigMap when present instead of the
chart-generated name.
In `@stack/helm/dave-admin-portal/templates/service.yaml`:
- Around line 9-20: Remove the duplicate static ports mapping so there is only
one "ports" key: delete the static block that uses ".Values.service.port" with
"targetPort: http" (the first ports: section) and keep the dynamic "{{- range
.Values.service.ports }}" block; if you intended a single fixed port instead of
a range, move that port definition into ".Values.service.ports" in values.yaml
(or add a default entry) so the template only declares "ports" once.
In `@stack/helm/dave-admin-portal/values.yaml`:
- Around line 55-67: Replace the inline client_secret in values.yaml with a
reference to a Kubernetes Secret and update the chart templates to load the
secret via valueFrom.secretKeyRef: remove or blank out
auth.client.registration.keycloak.client_secret and instead add a field (e.g.,
auth.client.registration.keycloak.clientSecretSecretName and
clientSecretKeyName) that operators will populate with the Secret name and key;
then modify the Deployment/StatefulSet container env generation (the template
that currently reads auth.client.registration.keycloak.client_secret) to create
an env var using valueFrom.secretKeyRef with .Name set to the new
clientSecretSecretName and .Key set to clientSecretKeyName; document that
operators must create a Kubernetes Secret containing the OAuth client secret
(key matches clientSecretKeyName) before installing/upgrading the chart.
---
Outside diff comments:
In `@frontend/src/components/map/ZaehlstelleMap.vue`:
- Around line 372-377: Replace the incorrect WMS call with an XYZ tile layer:
the code currently creates the OpenStreetMap layer using L.tileLayer.wms(...)
(variable osm), which is for WMS endpoints; change it to use L.tileLayer(...)
with the same "{z}/{x}/{y}" URL and preserve options like attribution
(mapAttributionOpenStreetMap) so the osm layer is instantiated with L.tileLayer
rather than L.tileLayer.wms.
In `@frontend/src/components/zaehlstelle/UpdateZaehlstelleForm.vue`:
- Around line 163-174: The computed getStadtbezirksviertelNummer contains an
unreachable branch because it checks else if (length === 6) then else if (length
>= 6); update the second guard to else if (zaehlstelle.value.nummer.length > 6)
so lengths greater than 6 fall through to return 100, and ensure all references
use zaehlstelle.value.nummer consistently when checking length and slicing.
- Around line 145-161: The computed getter getStadtbezirksviertel never assigns
the result of StadtbezirksviertelToBeschreibung.get(...) in the else branch, so
stadtbezirksviertelMap stays undefined; fix by assigning the map to
stadtbezirksviertelMap in the else branch (i.e., set stadtbezirksviertelMap =
StadtbezirksviertelToBeschreibung.get(zaehlstelle.value.stadtbezirkNummer)),
keeping the existing >=100 branch as-is and leaving the subsequent lookup of
stadtbezirksviertelMap.get(getStadtbezirksviertelNummer.value) unchanged.
---
Nitpick comments:
In `@frontend/src/components/zaehlung/form/AllgemeineInfoForm.vue`:
- Around line 32-42: Move the dauerzaehlung checkbox so it’s visually grouped
with the sonderzaehlung checkbox for consistency: locate the v-checkbox using
v-model="zaehlung.dauerzaehlung" and the v-checkbox using
v-model="zaehlung.sonderzaehlung" (in AllgemeineInfoForm.vue) and place them in
the same v-row/v-col structure (or adjust columns) so both checkboxes appear
together in one row rather than dauerzaehlung at the top and sonderzaehlung in a
separate row; ensure labels, color, and hide-details settings remain the same.
In `@frontend/src/components/zaehlung/ZaehlungCard.vue`:
- Around line 331-333: Remove the trivial computed "dauerzaehlung" that only
returns zaehlung.value.dauerzaehlung; delete the const dauerzaehlung =
computed(...) in ZaehlungCard.vue and update template bindings to use the
unwrapped model ref directly (e.g. zaehlung.dauerzaehlung) instead; also remove
any now-unused imports or refs that existed solely for the computed.
In `@frontend/src/types/enum/Stadtbezirksviertel.ts`:
- Around line 560-563: This file contains a hardcoded stopgap map
(StadtbezirksviertelWolfsburgToBeschreibung) and a TODO — create a tracked issue
to migrate these city/district/sub-district values into the backend using the
new CityDistrictDTO and CityInformationService, update the store logic that
special-cases bezirksnummer >= 100 to instead fetch from CityInformationService,
remove the frontend hardcoded Map, add a backend migration/seed for existing
entries (including the Wolfsburg 100 entry), and add unit/integration tests to
cover the service endpoint and the store data-fetch path.
In `@stack/helm/dave-admin-portal/templates/configmap.yaml`:
- Around line 7-10: The data: block is using an unusually large indentation
(nindent 12) for rendered top-level keys; change the template to use toYaml . |
nindent 2 so entries from .Values.extraEnvVars are indented with the
conventional 2 spaces under the data: block (update the line that currently
calls toYaml . | nindent 12 to use nindent 2, leaving the surrounding with
.Values.extraEnvVars and toYaml invocation intact).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4d4154c4-f15a-400b-8f2d-3432ef08e623
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (38)
.github/workflows/build.yaml.github/workflows/build_push.yaml.github/workflows/build_push_latest.yaml.github/workflows/pr_build.yaml.github/workflows/release.yamlCHANGELOG.mdapigateway/.gitignoreapigateway/application-local.ymlapigateway/pom.xmlapigateway/runLocal.shapigateway/runLocalNoSecurity.shapigateway/src/main/resources/logback-spring.xmlfrontend/pom.xmlfrontend/src/App.vuefrontend/src/api/service/CityInformationService.tsfrontend/src/api/service/ConfigurationService.tsfrontend/src/components/config/ConfigApplication.vuefrontend/src/components/map/ZaehlstelleMap.vuefrontend/src/components/zaehlstelle/CreateZaehlstelleForm.vuefrontend/src/components/zaehlstelle/UpdateZaehlstelleForm.vuefrontend/src/components/zaehlung/ZaehlungCard.vuefrontend/src/components/zaehlung/form/AllgemeineInfoForm.vuefrontend/src/components/zaehlung/form/VerkehrsartForm.vuefrontend/src/components/zaehlung/form/ZaehlungForm.vuefrontend/src/store/CityInformationStore.tsfrontend/src/types/configuration/CityDistrictDTO.tsfrontend/src/types/configuration/ConfigurationItemDTO.tsfrontend/src/types/enum/Stadtbezirksviertel.tsfrontend/src/types/zaehlung/ZaehlungDTO.tsfrontend/src/views/ConfigView.vuepom.xmlstack/helm/dave-admin-portal/Chart.yamlstack/helm/dave-admin-portal/templates/_helpers.tplstack/helm/dave-admin-portal/templates/configmap.yamlstack/helm/dave-admin-portal/templates/deployment.yamlstack/helm/dave-admin-portal/templates/ingress.yamlstack/helm/dave-admin-portal/templates/service.yamlstack/helm/dave-admin-portal/values.yaml
💤 Files with no reviewable changes (2)
- .github/workflows/build.yaml
- .github/workflows/build_push.yaml
📜 Review details
🧰 Additional context used
🪛 YAMLlint (1.38.0)
stack/helm/dave-admin-portal/templates/service.yaml
[error] 14-14: duplication of key "ports" in mapping
(key-duplicates)
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
stack/helm/dave-admin-portal/templates/configmap.yaml
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
stack/helm/dave-admin-portal/templates/deployment.yaml
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
stack/helm/dave-admin-portal/templates/ingress.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (12)
frontend/src/components/zaehlung/form/VerkehrsartForm.vue (1)
121-127: Good sync fix for category-driven UI state.Watching
zaehlung.value.kategorienand callingresetForm()keeps the checkbox state aligned with external model changes (including resets), which matches the PR goal.frontend/src/components/zaehlung/form/ZaehlungForm.vue (1)
135-138: Reset-on-zaehlartchange is consistent and safe.Clearing
kategorienonzaehlarttransition prevents stale/incompatible vehicle-type selections and works correctly with the new category watcher inVerkehrsartForm.vue.frontend/src/components/map/ZaehlstelleMap.vue (1)
349-349: Clarify how this change relates to the PR objective.The PR objective states that "traffic types were correctly saved but not displayed." However, this change switches the default base layer from Stadtkarte to OpenStreetMap, which appears unrelated to displaying vehicle/traffic types. While commit messages mention a "workaround to show non Munich subdistricts," it's unclear how changing the base map layer would affect the display of saved traffic type data.
Can you clarify how switching the default map layer addresses the vehicle type display issue?
frontend/src/types/configuration/ConfigurationItemDTO.ts (1)
1-10: LGTM!Clean DTO interface. Using
stringforvaluefieldregardless ofdatatypeis the right call for a text-editing model where conversion lives in the backend.frontend/src/views/ConfigView.vue (1)
41-44: LGTM!The new tab follows the exact same pattern as the existing four tabs — consistent
TAB_KONFIGURATIONconstant, matchingv-tab/v-tabs-window-itempair, andcontentHeightforwarded correctly.Also applies to: 66-68, 77-77, 92-92
frontend/pom.xml (1)
6-10: LGTM — clean parent POM reference.Inheriting the version from the new parent (
5.1.1-SNAPSHOT) and keeping the distinctde.muenchen.dave.frontendgroupId is correct Maven practice.stack/helm/dave-admin-portal/templates/_helpers.tpl (1)
1-51: LGTM — standard Helm helpers boilerplate.apigateway/.gitignore (1)
35-37: LGTM — correctly excludes generated frontend assets from version control.frontend/src/types/configuration/CityDistrictDTO.ts (1)
1-7: LGTM — clean DTO definition.frontend/src/components/zaehlstelle/CreateZaehlstelleForm.vue (1)
200-206: ⚡ Quick winNo issues found. The store implementation safely handles all edge cases:
stadtbezirke2naming is intentional versioning consistent with the codebase (Stadtbezirksviertel maps 1–25, 32, 100, etc. are mirrored in the store's numbered naming convention).
getStadtbezirksviertel(undefined)is handled correctly: undefined/null/falsy values are converted to"None", which doesn't match any switch case in thestadtbezirksviertel()function, safely returning an empty array[]for the dropdown.stack/helm/dave-admin-portal/Chart.yaml (1)
5-6: ⚡ Quick winNo issue — Maven resource filtering is explicitly configured and properly handles version substitution.
Maven resource filtering is explicitly configured in
apigateway/pom.xmlfor the Helm chart directory. Themaven-resources-pluginexecutionresources-helm-deploymentcopies files fromstack/helm/toapigateway/target/helm/with<filtering>true</filtering>enabled during thevalidatephase. The custom delimiter${*}is configured to match${project.version}and substitute it with the actual version frompom.xml. When Helm commands run in the workflow (inapigateway/target/helm/), they operate on the already-filtered files with real versions, not placeholders.stack/helm/dave-admin-portal/templates/ingress.yaml (1)
1-48: LGTM. The YAMLlint syntax error at line 1 is a false positive — Helm's{{- if ... -}}delimiter is not valid plain YAML but is correct for Helm templates. The$svcPortfallback logic and TLS/rules structure look correct.
| publish_docker: | ||
| name: "Build and publish docker" | ||
| needs: build | ||
| runs-on: [self-hosted, linux, X64] | ||
| env: | ||
| VERSION_BUILD: ${{ needs.build.outputs.VERSION_BUILD }} | ||
|
|
||
| steps: | ||
| - name: "echo version" | ||
| run: | | ||
| echo "Version is $VERSION_BUILD" | ||
| - name: Login to Docker Hub | ||
| uses: docker/login-action@v3 | ||
| with: | ||
| username: ${{ env.DOCKERHUB_USERNAME }} | ||
| password: ${{ secrets.DOCKERHUB_TOKEN }} | ||
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@v3 | ||
|
|
||
| - name: Build and push | ||
| uses: docker/build-push-action@v5 | ||
| with: | ||
| context: . | ||
| file: ./Dockerfile | ||
| push: true | ||
| sbom: true | ||
| provenance: mode=max | ||
| tags: ${{ env.DOCKERHUB_ORG }}/${{ env.IMAGE_NAME }}:${{ env.VERSION_BUILD }}-${{ github.run_number }} | ||
|
|
||
| publish_helm: | ||
| name: "Publish helm chart" | ||
| needs: build | ||
| runs-on: [self-hosted, linux, X64] | ||
|
|
||
| steps: | ||
| - name: get helm | ||
| uses: azure/setup-helm@v4 | ||
|
|
||
| - name: login to Helm registry | ||
| run: helm registry login registry-1.docker.io -u starwit -p ${{ secrets.DOCKERHUB_TOKEN }} | ||
|
|
||
| - name: run helm update | ||
| working-directory: apigateway/target/helm/dave-admin-portal | ||
| run: helm dep update | ||
|
|
||
| - name: run helm package | ||
| working-directory: apigateway/target/helm | ||
| run: helm package dave-admin-portal | ||
|
|
||
| - name: get version number | ||
| working-directory: apigateway/target/helm/dave-admin-portal | ||
| run: | | ||
| echo "HELM_VERSION=$(grep version: Chart.yaml | head -1 | awk '{print $2}')" >> $GITHUB_ENV | ||
|
|
||
| - name: run helm push | ||
| working-directory: apigateway/target/helm | ||
| run: helm push dave-admin-portal-chart-${{ env.HELM_VERSION }}.tgz oci://registry-1.docker.io/starwitorg |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/build_push_latest.yamlRepository: it-at-m/dave-admin-portal
Length of output: 4071
Both publish jobs lack checkout and artifact handling, creating a flaky workflow.
publish_docker accesses ./Dockerfile without checking out code, and publish_helm accesses apigateway/target/helm without downloading artifacts from the build job. The workflow relies on workspace persistence across jobs, which GitHub Actions does not guarantee—even on self-hosted runners. Add actions/checkout@v4 to publish_docker and implement artifact upload/download between build and publish_helm to reliably preserve the Maven build outputs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build_push_latest.yaml around lines 59 - 116, The publish
jobs assume source/artifacts are present but never checkout or fetch them: add
actions/checkout@v4 as the first step in the publish_docker job before the
docker/build-push-action (so Dockerfile and context are present) and modify the
build job to upload the Maven outputs (apigateway/target/helm) with
actions/upload-artifact, then in publish_helm add actions/download-artifact
(matching the artifact name from build) before running helm commands so the
packaged chart and Chart.yaml are reliably available; reference the
publish_docker and publish_helm job steps and the build job for where to add
upload/download.
| check_helm: | ||
| name: "Check if Helm chart is correct" | ||
| needs: build-maven | ||
| runs-on: [self-hosted, linux, X64] | ||
|
|
||
| steps: | ||
| - name: get helm | ||
| uses: azure/setup-helm@v4 | ||
|
|
||
| - name: run helm dependency update | ||
| working-directory: apigateway/target/helm/dave-admin-portal | ||
| run: helm dep update | ||
|
|
||
| - name: run helm template | ||
| working-directory: apigateway/target/helm |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat .github/workflows/pr_build.yamlRepository: it-at-m/dave-admin-portal
Length of output: 2076
🌐 Web query:
GitHub Actions self-hosted runner workspace persistence between jobs
💡 Result:
GitHub Actions self-hosted runner workspaces are not something you should rely on for persistence between separate jobs in a workflow run. Each job may reuse the runner machine’s filesystem, but GitHub’s runner lifecycle includes cleanup/reset steps for the working directory and it can be emptied/unrecreated between jobs. What you can rely on instead 1) Persist data between jobs using actions/cache or artifacts. - A common approach is to use actions/cache to reuse dependencies and avoid re-downloading/installing in later jobs. - For passing build outputs between jobs, use actions/upload-artifact and actions/download-artifact. 2) If you truly need runner-local state, make it explicit and manage it yourself on the self-hosted machine (and accept the security/consistency tradeoffs). - GitHub notes that self-hosted runners are not required to be clean for every job execution, but that means you must handle any persistence/cleanup logic yourself. GitHub recommends ephemeral runners for better isolation. - You can also configure scripts that run before/after each job on your self-hosted runner to prepare/tear down directories. How to interpret “workspace persistence” correctly - Within a single job’s steps, the same workspace directory is used, so files you create in one step are available to later steps in that job. - Between jobs, don’t assume files remain in the runner’s checkout directory (the runner will attempt to clean/reset its _work directory / recreate repository content when needed). Self-hosted cleanup can also fail in edge cases like UID/GID mismatches, resulting in corruption or errors—so assuming persistence is brittle. Practical recommendations - Use actions/cache for dependency directories. - Use upload-artifact/download-artifact for build outputs needed by later jobs. - If you need a “warm” build directory on a persistent self-hosted runner, implement it deliberately (for example, store in a directory outside the runner’s _work checkout area, and manage locking and cleanup), rather than writing into the default workspace and hoping GitHub won’t wipe it. If you share your workflow snippet (especially runs-on labels and where you write files), I can suggest the safest caching/artifact wiring for your exact case.
Citations:
- 1: https://docs.github.com/en/actions/reference/runners/self-hosted-runners
- 2: https://docs.github.com/actions/hosting-your-own-runners
- 3: https://docs.github.com/en/actions/hosting-your-own-runners/managing-self-hosted-runners/running-scripts-before-or-after-a-job
- 4: Cleaning up the working directory fails if the application created files/directories under different UID/GID. actions/runner#3541
- 5: https://stackoverflow.com/questions/54483260/is-it-possible-to-persist-a-workdir-between-actions-in-github-actions
check_helm won't have access to the Helm chart from build-maven.
This job neither checks out the repository nor downloads artifacts, but it tries to read from apigateway/target/helm. GitHub Actions does not guarantee workspace persistence between jobs, even on self-hosted runners—the runner cleans up between jobs. Upload artifacts from build-maven and download them in check_helm, or regenerate the chart in this job.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/pr_build.yaml around lines 66 - 80, The check_helm job
currently assumes apigateway/target/helm from build-maven will be present but
workspace isn’t preserved; update CI so build-maven uploads the Helm
chart/artifacts (from apigateway/target/helm) using actions/upload-artifact and
then modify the check_helm job to either (A) download that artifact with
actions/download-artifact before running the steps (so helm dep update and helm
template run against the downloaded apigateway/target/helm), or (B) perform a
checkout and rebuild/regenerate the Helm chart in check_helm itself (use
actions/checkout and the same build steps used in build-maven) so the
working-directory apigateway/target/helm exists when helm dep update and helm
template run.
| publish_docker: | ||
| name: "Build and publish docker" | ||
| needs: build | ||
| runs-on: [self-hosted, linux, X64] | ||
|
|
||
| steps: | ||
| - name: Create GitHub Release | ||
| uses: it-at-m/lhm_actions/action-templates/actions/action-create-github-release@v1.0.20 | ||
| - name: Get tag name | ||
| run: | | ||
| echo "TAG=$( git describe --abbrev=0 )" >> $GITHUB_ENV | ||
|
|
||
| - name: Login to Docker Hub | ||
| uses: docker/login-action@v3 | ||
| with: | ||
| artifact-name: ${{ needs.release-maven.outputs.ARTIFACT_NAME }} | ||
| tag-name: ${{ needs.release-maven.outputs.MVN_ARTIFACT_ID }}-${{ github.event.inputs.releaseVersion }} | ||
| artifact-path: "./target/*.jar" No newline at end of file | ||
| username: ${{ env.DOCKERHUB_USERNAME }} | ||
| password: ${{ secrets.DOCKERHUB_TOKEN }} | ||
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@v3 | ||
|
|
||
| - name: Build and push | ||
| uses: docker/build-push-action@v5 | ||
| with: | ||
| context: . | ||
| file: ./Dockerfile | ||
| push: true | ||
| sbom: true | ||
| provenance: mode=max | ||
| tags: ${{ env.DOCKERHUB_ORG }}/${{ env.IMAGE_NAME }}:${{ env.TAG }} | ||
|
|
||
| publish_helm: | ||
| name: "Publish helm chart" | ||
| needs: build | ||
| runs-on: [self-hosted, linux, X64] | ||
|
|
||
| steps: | ||
| - name: get helm | ||
| uses: azure/setup-helm@v4 | ||
|
|
||
| - name: login to Helm registry | ||
| run: helm registry login registry-1.docker.io -u starwit -p ${{ secrets.DOCKERHUB_TOKEN }} | ||
|
|
||
| - name: run helm update | ||
| working-directory: apigateway/target/helm/dave-admin-portal | ||
| run: helm dep update | ||
|
|
||
| - name: run helm package | ||
| working-directory: apigateway/target/helm | ||
| run: helm package dave-admin-portal | ||
|
|
||
| - name: get version number | ||
| working-directory: apigateway/target/helm/dave-admin-portal | ||
| run: | | ||
| echo "HELM_VERSION=$(grep version: Chart.yaml | head -1 | awk '{print $2}')" >> $GITHUB_ENV | ||
|
|
||
| - name: run helm push | ||
| working-directory: apigateway/target/helm | ||
| run: helm push dave-admin-portal-chart-${{ env.HELM_VERSION }}.tgz oci://registry-1.docker.io/starwitorg |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, get the full release.yaml file to understand the build job and complete context
cat .github/workflows/release.yamlRepository: it-at-m/dave-admin-portal
Length of output: 4436
The publish jobs require explicit checkout and artifact handoff from the build job.
publish_docker runs git describe (needs repo checkout) and references ./Dockerfile, while publish_helm references apigateway/target/helm/ (a Maven build artifact). Neither job checks out the repo nor downloads artifacts from build. Pass the tag and build artifacts as job outputs/uploads so publishing doesn't depend on runner state leaking between jobs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release.yaml around lines 94 - 150, The publish jobs lack
a repo checkout and don't consume build artifacts or the tag output from the
build job; add an actions/checkout@v4 step at the start of both publish_docker
and publish_helm, and change the build job to (a) set the git tag as a job
output (e.g. echo "TAG=...">>GITHUB_ENV and echo "::set-output name=tag::$TAG"
or use outputs via steps) and (b) upload the built helm/maven output and Docker
context using actions/upload-artifact (use names like docker-context and
helm-artifacts). In publish_docker replace the local git describe with consuming
needs.build.outputs.tag (or read the downloaded artifact's tag file) and
download the docker-context artifact with actions/download-artifact before Build
and push; in publish_helm download the helm-artifacts artifact before run helm
package/push and use the downloaded files; ensure credentials/login steps remain
after checkout. This targets the publish_docker and publish_helm jobs and the
build job's tag/artifact steps referenced in the workflow.
| spring: | ||
| profile: | ||
| active: prod |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Spring Boot spring.profiles.active vs spring.profile.active correct property
💡 Result:
The correct property name in Spring Boot is spring.profiles.active (plural "profiles"), not spring.profile.active (singular "profile").[1][2] This property specifies which profiles are active and can be set in application.properties, YAML, environment variables, command line (--spring.profiles.active=prod), or programmatically.[1][2][3] No official documentation or sources mention spring.profile.active as a valid property; all references use the plural form spring.profiles.active.[1][4][5][2][6][7][8][3] Note that spring.profiles.active cannot be used in profile-specific documents activated by spring.config.activate.on-profile.[1][5]
Citations:
- 1: https://docs.spring.io/spring-boot/3.3/reference/features/profiles.html
- 2: https://docs.spring.io/spring-boot/reference/features/profiles.html
- 3: https://www.baeldung.com/spring-profiles
- 4: https://docs.spring.io/spring-boot/docs/3.1.3/reference/html/features.html
- 5: https://stackoverflow.com/questions/73062721/what-is-diff-between-spring-profiles-active-vs-spring-config-activate-on-profile
- 6: https://medium.com/@AlexanderObregon/mechanics-of-spring-boot-profiles-and-configurations-be81fac082f5
- 7: https://docs.spring.io/spring-boot/docs/1.2.0.M1/reference/html/boot-features-profiles.html
- 8: https://stackoverflow.com/questions/37700352/setting-the-default-active-profile-in-spring-boot
🏁 Script executed:
# Search for profile-related properties in codebase
find . -type f \( -name "*.yml" -o -name "*.yaml" -o -name "*.properties" \) -exec grep -l "spring\.profile" {} \;Repository: it-at-m/dave-admin-portal
Length of output: 126
🏁 Script executed:
# Find and inspect the application-local.yml file
find . -name "application-local.yml" -o -name "*application*.yml" | head -20Repository: it-at-m/dave-admin-portal
Length of output: 209
🏁 Script executed:
# Search for all spring.profile or spring.profiles references in YAML/properties files
rg "spring\.(profile|profiles)" --type yaml --type properties -nRepository: it-at-m/dave-admin-portal
Length of output: 104
🏁 Script executed:
# Read the application-local.yml file
cat -n ./apigateway/application-local.ymlRepository: it-at-m/dave-admin-portal
Length of output: 1622
🏁 Script executed:
# Search for profile configurations in YAML files
rg "profile" ./apigateway --type yaml -nRepository: it-at-m/dave-admin-portal
Length of output: 117
spring.profile.active is not a valid Spring Boot property — the profile will silently not be activated.
The correct Spring Boot property to activate profiles is spring.profiles.active (plural). spring.profile.active (singular) is unrecognized and will be silently ignored, meaning the prod profile is never activated when this file is loaded.
🔧 Proposed fix
spring:
- profile:
- active: prod
+ profiles:
+ active: prod📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| spring: | |
| profile: | |
| active: prod | |
| spring: | |
| profiles: | |
| active: prod |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apigateway/application-local.yml` around lines 1 - 3, The YAML uses the
incorrect property name "spring.profile.active" so the "prod" profile is never
activated; update the property to the correct Spring Boot key
"spring.profiles.active" (note the plural "profiles") in the
application-local.yml to ensure the prod profile is applied when loading the
file.
| <plugin> | ||
| <artifactId>maven-resources-plugin</artifactId> | ||
| <executions> | ||
| <!-- Copy frontend dist to static folder in Spring Boot app --> | ||
| <execution> | ||
| <id>copy-resources</id> | ||
| <phase>validate</phase> | ||
| <goals> | ||
| <goal>copy-resources</goal> | ||
| </goals> | ||
| <configuration> | ||
| <outputDirectory>${project.basedir}/src/main/resources/static</outputDirectory> | ||
| <resources> | ||
| <resource> | ||
| <directory>${project.basedir}/../frontend/dist</directory> | ||
| </resource> | ||
| </resources> | ||
| </configuration> | ||
| </execution> | ||
|
|
||
| <!-- Generate Helm Chart --> | ||
| <execution> | ||
| <id>resources-helm-deployment</id> | ||
| <phase>validate</phase> | ||
| <goals> | ||
| <goal>copy-resources</goal> | ||
| </goals> | ||
| <configuration> | ||
| <outputDirectory>${project.basedir}/target/helm</outputDirectory> | ||
| <delimiters> | ||
| <delimiter>${*}</delimiter> | ||
| </delimiters> | ||
| <useDefaultDelimiters>false</useDefaultDelimiters> | ||
| <resources> | ||
| <resource> | ||
| <directory> | ||
| ${project.basedir}/../stack/helm | ||
| </directory> | ||
| <filtering>true</filtering> | ||
| <includes> | ||
| <include>**/*.txt</include> | ||
| <include>**/*.yaml</include> | ||
| <include>**/*.yml</include> | ||
| <include>**/*.tpl</include> | ||
| </includes> | ||
| </resource> | ||
| </resources> | ||
| </configuration> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
|
|
||
|
|
||
| <plugin> | ||
| <artifactId>maven-resources-plugin</artifactId> | ||
| <executions> | ||
| <execution> | ||
| <id>resources-helm-deployment</id> | ||
| <phase>validate</phase> | ||
| <goals> | ||
| <goal>copy-resources</goal> | ||
| </goals> | ||
| <configuration> | ||
| <outputDirectory>${project.basedir}/target/helm</outputDirectory> | ||
| <delimiters> | ||
| <delimiter>${*}</delimiter> | ||
| </delimiters> | ||
| <useDefaultDelimiters>false</useDefaultDelimiters> | ||
| <resources> | ||
| <resource> | ||
| <directory> | ||
| ${project.basedir}/../stack/helm | ||
| </directory> | ||
| <filtering>true</filtering> | ||
| <includes> | ||
| <include>**/*.txt</include> | ||
| <include>**/*.yaml</include> | ||
| <include>**/*.yml</include> | ||
| <include>**/*.tpl</include> | ||
| </includes> | ||
| </resource> | ||
| </resources> | ||
| </configuration> | ||
| </execution> | ||
| </executions> | ||
| </plugin> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the exact structure of the apigateway/pom.xml file
wc -l apigateway/pom.xmlRepository: it-at-m/dave-admin-portal
Length of output: 92
🏁 Script executed:
# Read the specific lines mentioned in the review (267-352)
sed -n '267,352p' apigateway/pom.xml | cat -nRepository: it-at-m/dave-admin-portal
Length of output: 4818
🏁 Script executed:
# Search for all maven-resources-plugin declarations in the file
rg "maven-resources-plugin" apigateway/pom.xml -nRepository: it-at-m/dave-admin-portal
Length of output: 205
🏁 Script executed:
# Let's see the context around these plugin declarations to understand the full picture
sed -n '260,360p' apigateway/pom.xml | cat -nRepository: it-at-m/dave-admin-portal
Length of output: 5247
🌐 Web query:
Maven duplicate plugin declarations same artifactId behavior merge executions
💡 Result:
When the same plugin is declared more than once in a single POM build section using the same (groupId:artifactId) key, Maven does not “merge XML-wise”; instead it treats this as a malformed POM model and emits a warning. Maven 3 logs a warning, and Maven 4 will fail the build for the same condition [1]. So for your examples, you should not rely on “first wins / last wins / XML merge” semantics; Maven requires that you don’t declare the same plugin multiple times in the same build. Correct approach: keep a single element per unique groupId:artifactId and combine differences using either: 1) in that one plugin (for shared configuration), and 2) distinct entries within the single plugin (for different behavior), where execution ids are unique within that plugin and parent-child merges follow the execution-id rules [2]. If you currently have multiple plugin declarations only because you split configuration blocks, move those blocks under the single plugin’s existing structure (e.g., into ), rather than keeping multiple elements [2].
Citations:
- 1: https://maven.apache.org/pom.html
- 2: https://maven.apache.org/guides/mini/guide-configuring-plugins.html
Remove the duplicate maven-resources-plugin block and merge executions.
The pom.xml declares maven-resources-plugin twice (lines 268 and 321) with overlapping execution IDs. Maven does not merge duplicate plugin declarations; instead, Maven 3 warns and Maven 4 fails the build. Consolidate both plugin blocks into a single declaration with all three executions: copy-resources, and resources-helm-deployment (configured only once, not duplicated).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apigateway/pom.xml` around lines 267 - 352, There are two duplicate
maven-resources-plugin declarations causing overlapping executions; consolidate
them into one <plugin> for artifactId maven-resources-plugin and merge the
executions so you have the copy-resources execution (id: copy-resources) and a
single resources-helm-deployment execution (id: resources-helm-deployment) under
that plugin, removing the duplicated block and ensuring the helm execution's
configuration (outputDirectory, delimiters, useDefaultDelimiters,
resources/includes/filtering) appears only once.
| function BeschreibungToStadtbezirk2(): Map<number, string> { | ||
| return new Map( | ||
| [...StadtbezirkToBeschreibung2].reverse() | ||
| ); | ||
| } |
There was a problem hiding this comment.
BeschreibungToStadtbezirk2() reverses insertion order, not key↔value pairs — the mapping is wrong.
[...map].reverse() produces an array of [number, string] pairs in reversed insertion order, then rebuilds a Map<number, string>. The result is still a number→string map — identical in content to StadtbezirkToBeschreibung2 with only reversed ordering. The function name implies a description (string) → district number (number) lookup, but neither the implementation nor the declared return type Map<number, string> reflects that.
🐛 Proposed fix — actually invert the key and value types
- function BeschreibungToStadtbezirk2(): Map<number, string> {
+ function BeschreibungToStadtbezirk2(): Map<string, number> {
return new Map(
- [...StadtbezirkToBeschreibung2].reverse()
+ [...StadtbezirkToBeschreibung2].map(([num, desc]) => [desc, num] as [string, number])
);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/store/CityInformationStore.ts` around lines 20 - 24,
BeschreibungToStadtbezirk2 currently returns a Map<number, string> by reversing
insertion order of StadtbezirkToBeschreibung2 instead of inverting key/value;
change BeschreibungToStadtbezirk2 to return Map<string, number> and build it by
iterating over StadtbezirkToBeschreibung2.entries() and inserting (description
-> districtNumber) pairs (swap key/value), preserving deterministic handling of
duplicates (decide whether later or earlier wins) and update the function
signature and any callers expecting the old type accordingly.
| <connection>scm:git:https://github.com/starwit/dave-admin-portal.git</connection> | ||
| <developerConnection>scm:git:https://github.com/starwit/dave-admin-portal.git</developerConnection> | ||
| <tag>5.0.0</tag> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the pom.xml file
fd -name "pom.xml" -type f | head -5Repository: it-at-m/dave-admin-portal
Length of output: 300
🏁 Script executed:
# Find pom.xml file
fd -name "pom.xml" -type fRepository: it-at-m/dave-admin-portal
Length of output: 300
🏁 Script executed:
# Use git ls-files to locate pom.xml
git ls-files | grep -i pom.xmlRepository: it-at-m/dave-admin-portal
Length of output: 113
🏁 Script executed:
# Read the root pom.xml to inspect lines 13-15 and 26-30
cat -n pom.xml | head -50Repository: it-at-m/dave-admin-portal
Length of output: 2261
🏁 Script executed:
# Search for any other instances of <tag> elements in pom.xml
grep -n "tag>" pom.xmlRepository: it-at-m/dave-admin-portal
Length of output: 157
🏁 Script executed:
# Search for any Maven or SCM documentation in the repo
git ls-files | grep -iE "(README|CONTRIBUTING|docs|wiki)" | head -20Repository: it-at-m/dave-admin-portal
Length of output: 159
<scm><tag> is pinned to the wrong ref.
<tag>5.0.0</tag> at line 15 conflicts with both the current project version (5.1.1-SNAPSHOT) and maven-release-plugin's @{project.version} tag format. On the development branch this should stay HEAD; otherwise release:prepare can emit stale SCM metadata into the release POMs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pom.xml` around lines 13 - 15, The <scm><tag> element is pinned to a stale
release value; update the <scm><tag> XML entry (the <scm><tag> element in the
POM) to "HEAD" on development branches so it doesn't conflict with the current
project version (5.1.1-SNAPSHOT) or maven-release-plugin's @{project.version}
behavior and to avoid emitting stale SCM metadata during release:prepare.
| {{- if .Values.existingConfigMap }} | ||
| name: {{ .Values.existingConfigMap }} | ||
| {{- else }} | ||
| name: {{ include "daveAdminPortal.fullname" . }} | ||
| {{- end }} |
There was a problem hiding this comment.
Keep envFrom aligned with existingConfigMap.
The mounted config switches to .Values.existingConfigMap, but envFrom still hard-codes the chart-generated ConfigMap. If that generated ConfigMap is omitted in the existing-config path, the Pod will reference a missing object and fail to start.
Suggested fix
envFrom:
- configMapRef:
- name: {{ include "daveAdminPortal.fullname" . }}
+ {{- if .Values.existingConfigMap }}
+ name: {{ .Values.existingConfigMap }}
+ {{- else }}
+ name: {{ include "daveAdminPortal.fullname" . }}
+ {{- end }}Also applies to: 101-103
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@stack/helm/dave-admin-portal/templates/deployment.yaml` around lines 38 - 42,
The envFrom block must mirror the ConfigMap name selection used above: update
the envFrom.configMapRef.name so it uses the same conditional as the volume name
(check .Values.existingConfigMap vs include "daveAdminPortal.fullname" .); i.e.
wrap the envFrom configMapRef.name in the same {{- if .Values.existingConfigMap
}} ... {{- else }} ... {{- end }} logic (also adjust the other envFrom
occurrence around the 101-103 area) so the Pod references the provided
existingConfigMap when present instead of the chart-generated name.
| ports: | ||
| - port: {{ .Values.service.port }} | ||
| targetPort: http | ||
| protocol: TCP | ||
| name: http | ||
| ports: | ||
| {{- range .Values.service.ports }} | ||
| - port: {{ .port }} | ||
| targetPort: {{ .targetPort }} | ||
| protocol: {{ .protocol }} | ||
| name: {{ .name }} | ||
| {{- end }} |
There was a problem hiding this comment.
Duplicate ports key makes the Service resource invalid.
Lines 9–13 define a static ports block and lines 14–20 define the dynamic range-based ports block under the same spec. YAML prohibits duplicate keys, and the Kubernetes API server will only see the last definition, discarding the first entirely. The static block (lines 9-13) appears to be an accidental leftover.
🔧 Proposed fix – remove the static `ports` block
spec:
type: {{ .Values.service.type }}
- ports:
- - port: {{ .Values.service.port }}
- targetPort: http
- protocol: TCP
- name: http
ports:
{{- range .Values.service.ports }}
- port: {{ .port }}
targetPort: {{ .targetPort }}
protocol: {{ .protocol }}
name: {{ .name }}
{{- end }}🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 14-14: duplication of key "ports" in mapping
(key-duplicates)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@stack/helm/dave-admin-portal/templates/service.yaml` around lines 9 - 20,
Remove the duplicate static ports mapping so there is only one "ports" key:
delete the static block that uses ".Values.service.port" with "targetPort: http"
(the first ports: section) and keep the dynamic "{{- range .Values.service.ports
}}" block; if you intended a single fixed port instead of a range, move that
port definition into ".Values.service.ports" in values.yaml (or add a default
entry) so the template only declares "ports" once.
| auth: | ||
| enabled: true | ||
| client: | ||
| registration: | ||
| keycloak: | ||
| scope: openid | ||
| authorization_grant_type: authorization_code | ||
| provider: mykeycloak | ||
| client_secret: test | ||
| client_id: dave | ||
| provider: | ||
| mykeycloak: | ||
| issuer_uri: http://auth.cluster.local/auth/realms/realmname |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find stack/helm/dave-admin-portal -type f \( -name "*.yaml" -o -name "*.yml" -o -name "*.tpl" \) | head -20Repository: it-at-m/dave-admin-portal
Length of output: 415
🏁 Script executed:
cd stack/helm/dave-admin-portal && find . -type f | head -30Repository: it-at-m/dave-admin-portal
Length of output: 226
🏁 Script executed:
cat -n stack/helm/dave-admin-portal/values.yaml | head -80Repository: it-at-m/dave-admin-portal
Length of output: 3479
🏁 Script executed:
find stack/helm/dave-admin-portal -name "*.yaml" -o -name "*.yml" -o -name "*.tpl" | xargs grep -l "client_secret\|auth\|keycloak" 2>/dev/nullRepository: it-at-m/dave-admin-portal
Length of output: 165
🏁 Script executed:
cat -n stack/helm/dave-admin-portal/templates/deployment.yamlRepository: it-at-m/dave-admin-portal
Length of output: 6142
🏁 Script executed:
cat -n stack/helm/dave-admin-portal/templates/configmap.yamlRepository: it-at-m/dave-admin-portal
Length of output: 386
Use Kubernetes Secret for OAuth client credentials instead of storing in values.yaml.
client_secret is currently injected directly as an environment variable from values.yaml (line 63), which exposes the credential in git history and Helm release metadata. Refactor the deployment to use valueFrom.secretKeyRef and require operators to provision a separate Kubernetes Secret resource containing the client credentials.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@stack/helm/dave-admin-portal/values.yaml` around lines 55 - 67, Replace the
inline client_secret in values.yaml with a reference to a Kubernetes Secret and
update the chart templates to load the secret via valueFrom.secretKeyRef: remove
or blank out auth.client.registration.keycloak.client_secret and instead add a
field (e.g., auth.client.registration.keycloak.clientSecretSecretName and
clientSecretKeyName) that operators will populate with the Secret name and key;
then modify the Deployment/StatefulSet container env generation (the template
that currently reads auth.client.registration.keycloak.client_secret) to create
an env var using valueFrom.secretKeyRef with .Name set to the new
clientSecretSecretName and .Key set to clientSecretKeyName; document that
operators must create a Kubernetes Secret containing the OAuth client secret
(key matches clientSecretKeyName) before installing/upgrading the chart.
2 participants
Description
So far despite correctly saved, traffic types were not displayed.
Issue References
see starwit#7
Definition of Done
Summary by CodeRabbit
New Features
Chores