Skip to content

Enhance RemoteStartTransactionTask to support charging profiles and transaction IDs#104

Merged
juherr merged 2 commits intomainfrom
features/steve-1816
Sep 15, 2025
Merged

Enhance RemoteStartTransactionTask to support charging profiles and transaction IDs#104
juherr merged 2 commits intomainfrom
features/steve-1816

Conversation

@juherr
Copy link
Owner

@juherr juherr commented Sep 15, 2025

Backport of steve-community#1809 and steve-community#1816

Summary by CodeRabbit

  • New Features

    • Remote Start Transaction: optionally select a Charging Profile when starting a transaction; form shows a Charging Profile dropdown and supported profiles include the profile in remote-start requests.
    • Set Charging Profile: added Transaction ID field for TxProfile flows; Connector ID input improved to numeric with clearer validation.
  • Refactor

    • Centralized charging-profile mapping, validation and transaction-id handling to ensure consistent request construction and enforce TxProfile constraints.

@coderabbitai
Copy link

coderabbitai bot commented Sep 15, 2025

Walkthrough

Adds optional ChargingProfile support to RemoteStartTransaction (DTO, task, service wiring, UI/controller changes) and centralizes OCPP ChargingProfile mapping and transactionId handling in SetChargingProfileTask, with validation and UI form updates.

Changes

Cohort / File(s) Summary
Remote Start + profile wiring
steve-core/src/main/java/de/rwth/idsg/steve/ocpp/task/RemoteStartTransactionTask.java, steve-core/src/main/java/de/rwth/idsg/steve/service/ChargePointServiceClient.java, steve-core/src/main/java/de/rwth/idsg/steve/web/dto/ocpp/RemoteStartTransactionParams.java, steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/Ocpp12Controller.java, steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/Ocpp16Controller.java, steve-ui-jsp/src/main/resources/webapp/WEB-INF/views/op-forms/RemoteStartTransactionForm.jsp
RemoteStartTransactionTask now exposes a public nested params type supporting an optional OCPP ChargingProfile, adds constructors that accept a ChargingProfileRepository and resolve/validate a profile, and includes the profile in OCPP requests when present. Service client call sites use the new constructors. DTO RemoteStartTransactionParams gains chargingProfilePk and class-level setters. UI/controller hooks and form select added to choose a profile.
SetChargingProfile mapping & transactionId
steve-core/src/main/java/de/rwth/idsg/steve/ocpp/task/SetChargingProfileTask.java, steve-core/src/main/java/de/rwth/idsg/steve/web/dto/ocpp/SetChargingProfileParams.java, steve-ui-jsp/src/main/resources/webapp/WEB-INF/views/op-forms/SetChargingProfileForm.jsp
Adds mapToOcpp(ChargingProfile.Details, Integer) to build a complete ocpp.cp._2015._10.ChargingProfile (including transactionId). Centralizes OCPP mapping and enforces that TX_PROFILE requires a transactionId while other purposes forbids it. Success path avoids persisting TX_PROFILE associations. DTO and JSP gain transactionId input/binding and connector input adjusted.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant UI as RemoteStart Form
  participant Ctrl16 as Ocpp16Controller
  participant Svc as ChargePointServiceClient
  participant Task as RemoteStartTransactionTask
  participant Repo as ChargingProfileRepository
  participant CP as Charge Point

  User->>UI: open RemoteStart form
  UI->>Ctrl16: request form
  Ctrl16->>Ctrl16: setCommonAttributesForRemoteStartTx(model)
  Ctrl16-->>UI: profileList + flags

  User->>UI: submit params (idTag, connectorId, chargingProfilePk?)
  UI->>Svc: remoteStartTransaction(params)
  Svc->>Task: new RemoteStartTransactionTask(params, ..., chargingProfileRepository)

  alt chargingProfilePk provided
    Task->>Repo: findByPk(pk)
    Repo-->>Task: ChargingProfile.Details
    Task->>Task: map details -> ocpp.ChargingProfile
  else no profile
    Note over Task: proceed without profile
  end

  Task->>CP: RemoteStartTransactionRequest [idTag, connectorId, chargingProfile?]
  CP-->>Task: Response
  Task-->>Svc: invoke callbacks / persist as applicable
Loading
sequenceDiagram
  autonumber
  participant UI as SetChargingProfileForm
  participant Ctrl as Controller
  participant Task as SetChargingProfileTask
  participant Repo as ChargingProfileRepository
  participant CP as Charge Point

  UI->>Ctrl: POST connectorId, chargingProfilePk, transactionId?
  Ctrl->>Task: execute(params)
  Task->>Task: checkAdditionalConstraints(purpose, transactionId)
  alt valid
    Task->>Repo: resolve ChargingProfile.Details (if needed)
    Task->>Task: mapToOcpp(details, transactionId)
    Task->>CP: SetChargingProfileRequest
    CP-->>Task: Status
    alt Accepted and purpose != TX_PROFILE
      Task->>Repo: persist profile association
    else Skip persistence
    end
  else invalid
    Task-->>Ctrl: BadRequest
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Possibly related PRs

Poem

A rabbit with a tiny map in paw,
Resolves profiles, checks each law.
TX IDs hop, schedules align,
Controllers set the form just fine.
"Start!" it twitches — chargers shine. 🐇⚡

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately reflects the primary change: adding charging-profile support and transaction ID handling to RemoteStartTransactionTask and related components, which aligns with the task, DTO, service, UI, and SetChargingProfile changes in the diff. It is concise, specific, and clearly communicates the main intent to reviewers scanning history.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch features/steve-1816

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4e17fea and 48aaefe.

📒 Files selected for processing (2)
  • steve-ui-jsp/src/main/resources/webapp/WEB-INF/views/op-forms/RemoteStartTransactionForm.jsp (1 hunks)
  • steve-ui-jsp/src/main/resources/webapp/WEB-INF/views/op-forms/SetChargingProfileForm.jsp (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • steve-ui-jsp/src/main/resources/webapp/WEB-INF/views/op-forms/RemoteStartTransactionForm.jsp
  • steve-ui-jsp/src/main/resources/webapp/WEB-INF/views/op-forms/SetChargingProfileForm.jsp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: build (ubuntu-22.04, 21, mariadb:10.6.14)
  • GitHub Check: build (ubuntu-24.04, 24, mariadb:10.5.21)
  • GitHub Check: build (ubuntu-22.04, 21, mariadb:10.5.21)
  • GitHub Check: build (ubuntu-24.04, 24, mariadb:10.3)
  • GitHub Check: build (ubuntu-24.04, 24, mariadb:10.6.14)
  • GitHub Check: build (ubuntu-24.04, 24, mariadb:10.4.30)
  • GitHub Check: build (ubuntu-24.04, 21, mariadb:10.6.14)
  • GitHub Check: build (ubuntu-24.04, 21, mariadb:10.3)
  • GitHub Check: build (ubuntu-24.04, 24, mysql:8.0)
  • GitHub Check: build (ubuntu-22.04, 24, mariadb:10.4.30)
  • GitHub Check: build (ubuntu-22.04, 21, mariadb:10.4.30)
  • GitHub Check: build (ubuntu-22.04, 21, mysql:8.0)
  • GitHub Check: build (ubuntu-22.04, 24, mysql:8.0)
  • GitHub Check: build (ubuntu-22.04, 24, mariadb:10.5.21)
  • GitHub Check: build (ubuntu-24.04, 21, mariadb:10.5.21)
  • GitHub Check: build (ubuntu-22.04, 24, mariadb:10.6.14)
  • GitHub Check: build (ubuntu-22.04, 21, mariadb:10.3)
  • GitHub Check: build (ubuntu-22.04, 24, mariadb:10.3)
  • GitHub Check: build (ubuntu-24.04, 21, mysql:8.0)
  • GitHub Check: build (ubuntu-24.04, 21, mariadb:10.4.30)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
steve-core/src/main/java/de/rwth/idsg/steve/ocpp/task/SetChargingProfileTask.java (1)

153-178: Guard against potential NPE from autounboxing connectorId in constraint checks.

If connectorId is null, comparisons like != 0 or < 1 will NPE. Make checks null-safe.

-        Optional.ofNullable(request.getCsChargingProfiles())
-                .map(ocpp.cp._2015._10.ChargingProfile::getChargingProfilePurpose)
-                .ifPresent(purpose -> {
-                    if (purpose == ChargingProfilePurposeType.CHARGE_POINT_MAX_PROFILE
-                            && request.getConnectorId() != 0) {
+        Optional.ofNullable(request.getCsChargingProfiles())
+                .map(ocpp.cp._2015._10.ChargingProfile::getChargingProfilePurpose)
+                .ifPresent(purpose -> {
+                    final Integer cid = request.getConnectorId();
+                    if (purpose == ChargingProfilePurposeType.CHARGE_POINT_MAX_PROFILE
+                            && !Integer.valueOf(0).equals(cid)) {
                         throw new SteveException.InternalError(
                                 "ChargePointMaxProfile can only be set at Charge Point ConnectorId 0");
                     }
 
-                    if (purpose == ChargingProfilePurposeType.TX_PROFILE && request.getConnectorId() < 1) {
+                    if (purpose == ChargingProfilePurposeType.TX_PROFILE && (cid == null || cid < 1)) {
                         throw new SteveException.InternalError(
                                 "TxProfile should only be set at Charge Point ConnectorId > 0");
                     }
 
-                    if (ChargingProfilePurposeType.TX_PROFILE == purpose
-                            && request.getCsChargingProfiles().getTransactionId() == null) {
+                    var ocppProfile = request.getCsChargingProfiles();
+                    if (ChargingProfilePurposeType.TX_PROFILE == purpose
+                            && ocppProfile.getTransactionId() == null) {
                         throw new SteveException.InternalError("transaction id is required for TxProfile");
                     }
 
-                    if (ChargingProfilePurposeType.TX_PROFILE != purpose
-                            && request.getCsChargingProfiles().getTransactionId() != null) {
+                    if (ChargingProfilePurposeType.TX_PROFILE != purpose
+                            && ocppProfile.getTransactionId() != null) {
                         throw new SteveException.InternalError("transaction id should only be set for TxProfile");
                     }
                 });
🧹 Nitpick comments (6)
steve-core/src/main/java/de/rwth/idsg/steve/ocpp/task/RemoteStartTransactionTask.java (4)

56-61: Avoid direct field access; expose a getter for chargingProfile

Accessing a private field of the nested params class ties the version handler to an implementation detail. Provide a getter and use it here.

Apply this diff:

-                                            .withConnectorId(task.getParams().getConnectorId())
-                                            .withChargingProfile(task.getParams().chargingProfile),
+                                            .withConnectorId(task.getParams().getConnectorId())
+                                            .withChargingProfile(task.getParams().getChargingProfile()),

Then add a getter in RemoteStartTransactionWithProfileParams (see separate comment).


81-99: Add accessor for chargingProfile to decouple callers from field visibility

Keeps params encapsulated and prevents future breakage if the storage changes.

Apply this diff:

     public static class RemoteStartTransactionWithProfileParams extends RemoteStartTransactionParams {
         private final ocpp.cp._2015._10.@Nullable ChargingProfile chargingProfile;
 
         public RemoteStartTransactionWithProfileParams(
                 RemoteStartTransactionParams params, ocpp.cp._2015._10.@Nullable ChargingProfile chargingProfile) {
             super();
             this.setIdTag(params.getIdTag());
             this.setConnectorId(params.getConnectorId());
             this.chargingProfile = chargingProfile;
         }
 
+        public @Nullable ChargingProfile getChargingProfile() {
+            return chargingProfile;
+        }
+
         @Override
         public @Nullable Integer getChargingProfilePk() {
             if (chargingProfile == null) {
                 return null;
             }
             return chargingProfile.getChargingProfileId();
         }
     }

64-80: Consider deferring DB lookup of ChargingProfile until needed

Constructors always resolve the profile even when the target CP is OCPP 1.2/1.5 (where it’s unused). This adds avoidable DB load and can fail a request that wouldn’t use the profile anyway.

Option: carry only chargingProfilePk in params and resolve inside the V_16 version handler or lazily cache on first access.


101-115: Improve error message with context

Include the profile id in the TX_PROFILE check failure to aid ops/debugging.

Apply this diff:

-        if (chargingProfile.getChargingProfilePurpose() != ChargingProfilePurposeType.TX_PROFILE) {
-            throw new SteveException.BadRequest("ChargingProfilePurposeType is not TX_PROFILE");
+        if (chargingProfile.getChargingProfilePurpose() != ChargingProfilePurposeType.TX_PROFILE) {
+            throw new SteveException.BadRequest(
+                    "ChargingProfilePurposeType is not TX_PROFILE for profileId " + chargingProfile.getChargingProfileId());
         }
steve-ui-jsp/src/main/resources/webapp/WEB-INF/views/op-forms/SetChargingProfileForm.jsp (1)

37-40: Make transactionId input numeric, positive, and show validation errors.

Improve UX and client-side validation; mirror @positive on the DTO.

-        <tr>
-            <td>Transaction ID (integer):</td>
-            <td><form:input path="transactionId" placeholder="only necessary for TxProfile"/></td>
-        </tr>
+        <tr>
+            <td>Transaction ID (integer):</td>
+            <td>
+                <form:input path="transactionId" type="number" inputmode="numeric" min="1"
+                            placeholder="Only required for TxProfile"/>
+                <form:errors path="transactionId" cssClass="error"/>
+            </td>
+        </tr>
steve-core/src/main/java/de/rwth/idsg/steve/ocpp/task/SetChargingProfileTask.java (1)

121-151: Mapping looks correct; consider null-conditional writes for optional fields.

Optional: only call withMinChargingRate when non-null to avoid emitting xsi:nil in some SOAP stacks.

-        var schedule = new ChargingSchedule()
+        var schedule = new ChargingSchedule()
                 .withDuration(details.getDurationInSeconds())
                 .withStartSchedule(toOffsetDateTime(details.getStartSchedule()))
                 .withChargingRateUnit(ChargingRateUnitType.fromValue(details.getChargingRateUnit()))
-                .withMinChargingRate(details.getMinChargingRate())
                 .withChargingSchedulePeriod(schedulePeriods);
+        if (details.getMinChargingRate() != null) {
+            schedule.setMinChargingRate(details.getMinChargingRate());
+        }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6e272c2 and fb7be73.

📒 Files selected for processing (9)
  • steve-core/src/main/java/de/rwth/idsg/steve/ocpp/task/RemoteStartTransactionTask.java (2 hunks)
  • steve-core/src/main/java/de/rwth/idsg/steve/ocpp/task/SetChargingProfileTask.java (4 hunks)
  • steve-core/src/main/java/de/rwth/idsg/steve/service/ChargePointServiceClient.java (1 hunks)
  • steve-core/src/main/java/de/rwth/idsg/steve/web/dto/ocpp/RemoteStartTransactionParams.java (1 hunks)
  • steve-core/src/main/java/de/rwth/idsg/steve/web/dto/ocpp/SetChargingProfileParams.java (1 hunks)
  • steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/Ocpp12Controller.java (3 hunks)
  • steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/Ocpp16Controller.java (1 hunks)
  • steve-ui-jsp/src/main/resources/webapp/WEB-INF/views/op-forms/RemoteStartTransactionForm.jsp (1 hunks)
  • steve-ui-jsp/src/main/resources/webapp/WEB-INF/views/op-forms/SetChargingProfileForm.jsp (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java

📄 CodeRabbit inference engine (AGENTS.md)

Java source code must be formatted using the Spotless Maven plugin (run spotless:apply/spotless:check before committing)

Files:

  • steve-core/src/main/java/de/rwth/idsg/steve/web/dto/ocpp/SetChargingProfileParams.java
  • steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/Ocpp16Controller.java
  • steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/Ocpp12Controller.java
  • steve-core/src/main/java/de/rwth/idsg/steve/service/ChargePointServiceClient.java
  • steve-core/src/main/java/de/rwth/idsg/steve/ocpp/task/RemoteStartTransactionTask.java
  • steve-core/src/main/java/de/rwth/idsg/steve/web/dto/ocpp/RemoteStartTransactionParams.java
  • steve-core/src/main/java/de/rwth/idsg/steve/ocpp/task/SetChargingProfileTask.java
🧬 Code graph analysis (2)
steve-core/src/main/java/de/rwth/idsg/steve/ocpp/task/RemoteStartTransactionTask.java (2)
steve-core/src/main/java/de/rwth/idsg/steve/SteveException.java (1)
  • SteveException (25-83)
steve-core/src/main/java/de/rwth/idsg/steve/repository/dto/ChargingProfile.java (1)
  • ChargingProfile (33-87)
steve-core/src/main/java/de/rwth/idsg/steve/web/dto/ocpp/RemoteStartTransactionParams.java (2)
steve-core/src/main/java/de/rwth/idsg/steve/ocpp/task/SetChargingProfileTask.java (1)
  • Getter (44-179)
steve-core/src/main/java/de/rwth/idsg/steve/web/dto/ocpp/SetChargingProfileParams.java (1)
  • Setter (32-41)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: build (ubuntu-22.04, 21, mariadb:10.4.30)
  • GitHub Check: build (ubuntu-24.04, 21, mariadb:10.4.30)
  • GitHub Check: build (ubuntu-24.04, 21, mariadb:10.3)
  • GitHub Check: build (ubuntu-24.04, 24, mariadb:10.3)
  • GitHub Check: build (ubuntu-24.04, 24, mysql:8.0)
  • GitHub Check: build (ubuntu-24.04, 24, mariadb:10.5.21)
  • GitHub Check: build (ubuntu-24.04, 21, mariadb:10.5.21)
  • GitHub Check: build (ubuntu-24.04, 24, mariadb:10.4.30)
  • GitHub Check: build (ubuntu-24.04, 21, mariadb:10.6.14)
  • GitHub Check: build (ubuntu-22.04, 24, mariadb:10.5.21)
  • GitHub Check: build (ubuntu-22.04, 21, mysql:8.0)
  • GitHub Check: build (ubuntu-24.04, 21, mysql:8.0)
  • GitHub Check: build (ubuntu-22.04, 21, mariadb:10.6.14)
  • GitHub Check: build (ubuntu-22.04, 24, mariadb:10.3)
  • GitHub Check: build (ubuntu-22.04, 24, mariadb:10.6.14)
  • GitHub Check: build (ubuntu-22.04, 21, mariadb:10.5.21)
  • GitHub Check: build (ubuntu-22.04, 24, mariadb:10.4.30)
  • GitHub Check: build (ubuntu-22.04, 24, mysql:8.0)
  • GitHub Check: build (ubuntu-22.04, 21, mariadb:10.4.30)
  • GitHub Check: build (ubuntu-22.04, 21, mariadb:10.3)
🔇 Additional comments (12)
steve-core/src/main/java/de/rwth/idsg/steve/web/dto/ocpp/SetChargingProfileParams.java (1)

40-41: TX-profile transactionId support looks correct

Optional, positive-only Integer aligns with OCPP expectations; task-level checks enforce purpose-specific presence. No further changes needed here.

Please confirm UI binding sends null (not 0) when the Transaction ID field is left empty.

steve-core/src/main/java/de/rwth/idsg/steve/web/dto/ocpp/RemoteStartTransactionParams.java (2)

35-35: Class-level @Setter: verify Lombok interplay with custom setter

Lombok won’t generate a setter when a custom one exists, so your setConnectorId(Integer) remains authoritative. Just confirming this is intentional.


43-44: Optional chargingProfilePk added — validation is appropriate

Nullable + @positive is fine. Downstream task enforces TX_PROFILE, so DTO-level constraints are sufficient.

Ensure the form/JSON binding sends null for blank input to satisfy @positive (avoid empty string coercion issues).

steve-core/src/main/java/de/rwth/idsg/steve/service/ChargePointServiceClient.java (2)

192-193: Wiring repository into RemoteStartTransactionTask: LGTM

Constructor migration looks correct; task creation remains single-target via forFirst(), consistent with RemoteStart semantics.


199-200: Caller-aware overload updated correctly

Matches new task constructor; no additional changes required.

steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/Ocpp16Controller.java (1)

83-86: Supplying profile data for RemoteStart form: LGTM; verify it’s invoked

Attributes are correct. Ensure the base controller calls setCommonAttributesForRemoteStartTx(model) for the RemoteStartTransaction GET/POST paths so the JSP sees them.

steve-ui-jsp/src/main/resources/webapp/WEB-INF/views/op-forms/SetChargingProfileForm.jsp (1)

43-43: EOF newline is fine.

steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/Ocpp12Controller.java (2)

161-161: Hook invocation in GET/POST paths looks correct.

Keeps form re-render consistent on validation errors.

Also applies to: 245-245


85-92: Confirmed — Ocpp16Controller overrides setCommonAttributesForRemoteStartTx and sets both attributes.
Ocpp16Controller (extends Ocpp15Controller) calls model.addAttribute("profileForRemoteStartTx", Boolean.TRUE) and model.addAttribute("profileList", chargingProfileRepository.getBasicInfo()). File: steve-ui-jsp/src/main/java/de/rwth/idsg/steve/web/controller/Ocpp16Controller.java:83–85.

steve-ui-jsp/src/main/resources/webapp/WEB-INF/views/op-forms/RemoteStartTransactionForm.jsp (1)

49-49: EOF newline is fine.

steve-core/src/main/java/de/rwth/idsg/steve/ocpp/task/SetChargingProfileTask.java (2)

94-96: Correct: do not persist TX_PROFILE on acceptance.

Matches OCPP intent; TX-scoped profiles shouldn’t be stored as station-scoped.


114-119: Good centralization via mapToOcpp.

Reduces duplication and guarantees consistent transactionId population.

Comment on lines +36 to +46
<c:if test="${profileForRemoteStartTx}">
<tr>
<td>Charging Profile ID:</td>
<td>
<form:select path="chargingProfilePk">
<option value="" selected>-- Empty --</option>
<form:options items="${profileList}" itemLabel="itemDescription" itemValue="chargingProfilePk"/>
</form:select>
</td>
</tr>
</c:if>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Avoid forcing the empty option selected; it breaks re-selection after validation errors.

Let Spring bind the selected option; add error display.

-        <c:if test="${profileForRemoteStartTx}">
+        <c:if test="${profileForRemoteStartTx}">
             <tr>
                 <td>Charging Profile ID:</td>
                 <td>
-                    <form:select path="chargingProfilePk">
-                        <option value="" selected>-- Empty --</option>
-                        <form:options items="${profileList}" itemLabel="itemDescription" itemValue="chargingProfilePk"/>
-                    </form:select>
+                    <form:select path="chargingProfilePk">
+                        <form:option value="">-- Empty --</form:option>
+                        <form:options items="${profileList}" itemLabel="itemDescription" itemValue="chargingProfilePk"/>
+                    </form:select>
+                    <form:errors path="chargingProfilePk" cssClass="error"/>
                 </td>
             </tr>
         </c:if>

Ensure chargingProfilePk is a nullable wrapper (Integer/Long) so empty option binds to null:


🏁 Script executed:

#!/bin/bash
rg -n 'class RemoteStartTransactionParams' -n steve-core/src/main/java
rg -nP '(\bInteger\b|\bLong\b)\s+chargingProfilePk' steve-core/src/main/java/de/rwth/idsg/steve/web/dto/ocpp/RemoteStartTransactionParams.java -C2

Length of output: 279


Avoid forcing the empty option selected; let Spring bind and show validation errors.

Use <form:option value=""> (no selected attribute) and add form:errors so the previously selected value is restored after validation. chargingProfilePk is @nullable Integer — verified in steve-core/src/main/java/de/rwth/idsg/steve/web/dto/ocpp/RemoteStartTransactionParams.java:43.

-        <c:if test="${profileForRemoteStartTx}">
+        <c:if test="${profileForRemoteStartTx}">
             <tr>
                 <td>Charging Profile ID:</td>
                 <td>
-                    <form:select path="chargingProfilePk">
-                        <option value="" selected>-- Empty --</option>
-                        <form:options items="${profileList}" itemLabel="itemDescription" itemValue="chargingProfilePk"/>
-                    </form:select>
+                    <form:select path="chargingProfilePk">
+                        <form:option value="">-- Empty --</form:option>
+                        <form:options items="${profileList}" itemLabel="itemDescription" itemValue="chargingProfilePk"/>
+                    </form:select>
+                    <form:errors path="chargingProfilePk" cssClass="error"/>
                 </td>
             </tr>
         </c:if>
📝 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.

Suggested change
<c:if test="${profileForRemoteStartTx}">
<tr>
<td>Charging Profile ID:</td>
<td>
<form:select path="chargingProfilePk">
<option value="" selected>-- Empty --</option>
<form:options items="${profileList}" itemLabel="itemDescription" itemValue="chargingProfilePk"/>
</form:select>
</td>
</tr>
</c:if>
<c:if test="${profileForRemoteStartTx}">
<tr>
<td>Charging Profile ID:</td>
<td>
<form:select path="chargingProfilePk">
<form:option value="">-- Empty --</form:option>
<form:options items="${profileList}" itemLabel="itemDescription" itemValue="chargingProfilePk"/>
</form:select>
<form:errors path="chargingProfilePk" cssClass="error"/>
</td>
</tr>
</c:if>
🤖 Prompt for AI Agents
In
steve-ui-jsp/src/main/resources/webapp/WEB-INF/views/op-forms/RemoteStartTransactionForm.jsp
around lines 36 to 46, the empty option is hard-coded with selected which
prevents Spring form binding from restoring a previously chosen value and
suppresses validation feedback; replace the hard-coded HTML option with a Spring
form:option value="" (no selected attribute) so the form taglib can bind the
value, ensure the select uses form:options as already present, and add a
<form:errors path="chargingProfilePk" /> next to the select to display
validation messages; chargingProfilePk is a nullable Integer so the empty value
must be allowed.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
steve-ui-jsp/src/main/resources/webapp/WEB-INF/views/op-forms/SetChargingProfileForm.jsp (3)

35-39: Connector ID: enforce integer-only input (step=1).

Prevents decimals in browsers that allow them for type=number; aligns with Integer backend type.

-                <form:input path="connectorId" type="number" inputmode="numeric" min="0" placeholder="0 = charge point as a whole"/>
+                <form:input path="connectorId" type="number" inputmode="numeric" min="0" step="1" placeholder="0 = charge point as a whole"/>

41-45: Transaction ID: add step=1 and provide an accessible hint.

  • step=1 to avoid decimals.
  • Placeholder-only guidance isn’t accessible; add a hint and link via aria-describedby.
-                <form:input path="transactionId" type="number" inputmode="numeric" min="1" placeholder="only necessary for TxProfile"/>
-                <form:errors path="transactionId" cssClass="error"/>
+                <form:input path="transactionId" type="number" inputmode="numeric" min="1" step="1" aria-describedby="transactionId-hint" placeholder="only necessary for TxProfile"/>
+                <span id="transactionId-hint" class="hint">Only necessary for TxProfile.</span>
+                <form:errors path="transactionId" cssClass="error"/>

Optional: externalize the hint/labels via message bundle instead of hardcoded English.


35-45: Make transactionId conditional/required for TxProfile in the UI

Backend already enforces transactionId for ChargingProfilePurposeType.TX_PROFILE and forbids it for non‑TX profiles; add a client-side toggle to hide/disable the Transaction ID input and mark it required only when Purpose == TX_PROFILE to reduce user errors. (See: steve-ui-jsp/src/main/resources/webapp/WEB-INF/views/op-forms/SetChargingProfileForm.jsp — transactionId input; steve-core/src/main/java/de/rwth/idsg/steve/web/dto/ocpp/SetChargingProfileParams.java — transactionId is optional; steve-core/src/main/java/de/rwth/idsg/steve/ocpp/task/SetChargingProfileTask.java — server-side validation.)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fb7be73 and 4e17fea.

📒 Files selected for processing (2)
  • steve-ui-jsp/src/main/resources/webapp/WEB-INF/views/op-forms/RemoteStartTransactionForm.jsp (1 hunks)
  • steve-ui-jsp/src/main/resources/webapp/WEB-INF/views/op-forms/SetChargingProfileForm.jsp (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • steve-ui-jsp/src/main/resources/webapp/WEB-INF/views/op-forms/RemoteStartTransactionForm.jsp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: build (ubuntu-22.04, 21, mysql:8.0)
  • GitHub Check: Analyze (java-kotlin)
  • GitHub Check: build (ubuntu-24.04, 21, mariadb:10.5.21)
  • GitHub Check: build (ubuntu-24.04, 21, mariadb:10.4.30)
  • GitHub Check: build (ubuntu-24.04, 24, mariadb:10.5.21)
  • GitHub Check: build (ubuntu-24.04, 24, mariadb:10.6.14)
  • GitHub Check: build (ubuntu-24.04, 21, mariadb:10.6.14)
  • GitHub Check: build (ubuntu-24.04, 24, mariadb:10.4.30)
  • GitHub Check: build (ubuntu-24.04, 24, mariadb:10.3)
  • GitHub Check: build (ubuntu-24.04, 24, mysql:8.0)
  • GitHub Check: build (ubuntu-24.04, 21, mariadb:10.3)
  • GitHub Check: build (ubuntu-22.04, 24, mariadb:10.5.21)
  • GitHub Check: build (ubuntu-24.04, 21, mysql:8.0)
  • GitHub Check: build (ubuntu-22.04, 24, mariadb:10.3)
  • GitHub Check: build (ubuntu-22.04, 21, mariadb:10.6.14)
  • GitHub Check: build (ubuntu-22.04, 24, mariadb:10.4.30)
  • GitHub Check: build (ubuntu-22.04, 21, mariadb:10.5.21)
  • GitHub Check: build (ubuntu-22.04, 21, mysql:8.0)
  • GitHub Check: build (ubuntu-22.04, 24, mysql:8.0)
  • GitHub Check: build (ubuntu-22.04, 21, mariadb:10.3)

@juherr juherr force-pushed the features/steve-1816 branch from 4e17fea to 48aaefe Compare September 15, 2025 20:38
@juherr juherr merged commit a1c7228 into main Sep 15, 2025
50 checks passed
@juherr juherr deleted the features/steve-1816 branch September 15, 2025 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant