fix: make version number id consistent.#1926
Merged
JoshuaSBrown merged 1 commit intodevelfrom Apr 1, 2026
Merged
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnsures that schema IDs created via the /api/sch/create endpoint always include an explicit :0 version suffix, matching Python CommandLib behavior. Sequence diagram for /api/sch/create with version suffix normalizationsequenceDiagram
participant Client
participant ExpressApp as ExpressApp
participant SchemaCreateHandler as SchemaCreateHandler
participant Backend as BackendService
Client->>ExpressApp: POST /api/sch/create { id, ... }
ExpressApp->>SchemaCreateHandler: handleRequest(a_req, a_resp)
SchemaCreateHandler->>SchemaCreateHandler: check a_req.body.id
alt id has no colon
SchemaCreateHandler->>SchemaCreateHandler: a_req.body.id = a_req.body.id + :0
else id already has version
SchemaCreateHandler->>SchemaCreateHandler: keep existing id
end
SchemaCreateHandler->>Backend: sendMessage SchemaCreateRequest(a_req.body)
Backend-->>SchemaCreateHandler: SchemaCreateReply
SchemaCreateHandler-->>Client: JSON response
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Mutating
a_req.body.idin-place could have unintended side effects for downstream middleware or logging; consider copying the value to a local variable or a dedicated payload object before callingsendMessage. - The version suffix logic is now duplicated with Python CommandLib; it may be worth centralizing this normalization in a shared utility to ensure future changes stay consistent across components.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Mutating `a_req.body.id` in-place could have unintended side effects for downstream middleware or logging; consider copying the value to a local variable or a dedicated payload object before calling `sendMessage`.
- The version suffix logic is now duplicated with Python CommandLib; it may be worth centralizing this normalization in a shared utility to ensure future changes stay consistent across components.
## Individual Comments
### Comment 1
<location path="web/datafed-ws.js" line_range="1887-1888" />
<code_context>
app.post("/api/sch/create", (a_req, a_resp) => {
+ // Mirror Python CommandLib: ensure :0 version suffix on create
+ if (a_req.body.id && a_req.body.id.indexOf(":") === -1) {
+ a_req.body.id = a_req.body.id + ":0";
+ }
sendMessage("SchemaCreateRequest", a_req.body, a_req, a_resp, function (reply) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against non-string `id` to avoid runtime errors when calling `indexOf`.
If `a_req.body.id` is ever non-string (e.g. number or object), calling `indexOf` will throw. To avoid runtime errors on malformed input, first ensure it’s a string, e.g. `if (typeof a_req.body.id === 'string' && a_req.body.id.indexOf(':') === -1) { ... }`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1887
to
+1888
| if (a_req.body.id && a_req.body.id.indexOf(":") === -1) { | ||
| a_req.body.id = a_req.body.id + ":0"; |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Guard against non-string id to avoid runtime errors when calling indexOf.
If a_req.body.id is ever non-string (e.g. number or object), calling indexOf will throw. To avoid runtime errors on malformed input, first ensure it’s a string, e.g. if (typeof a_req.body.id === 'string' && a_req.body.id.indexOf(':') === -1) { ... }.
JoshuaSBrown
added a commit
that referenced
this pull request
Apr 1, 2026
JoshuaSBrown
added a commit
that referenced
this pull request
Apr 1, 2026
* [DAPS-1925] - fix: make version number id consistent. (#1926) Co-authored-by: JoshuaSBrown <brownjs@ornl.gov>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket
Description
How Has This Been Tested?
Artifacts (if appropriate):
Tasks
Summary by Sourcery
Bug Fixes: