NAS-138184 / 26.04 / fix: correct size sending for VM raw files

[mattwyatt-ix]

Changes:

• on the slidein to add a raw file to a VM, the 'Raw Filesize' field asked the user to input a file size in GiB, but sent this value off to the API unchanged when the API was expecting bytes. this may have lead to inconstent behavior in some cases. resolved by adding a formatter and parser to the field.

• this results in the correct values actually being sent to the API, so in the json-rpc data this will now send something like 1073741824 instead of just 1.

• updated tooltip to remove the GiB restriction.

• updated tests for new behavior and added a new test to ensure whitespace is handled.

Screenshots:

the file size defaults to 1 MiB

other sizes are supported

Testing:

1. navigate to the virtual machines page and create a new dummy VM. it doesn't need to be bootable, since all we're doing is attaching a device to it.
2. navigate to the 'Devices' page of the VM and click the 'Add' button in the top right.
3. choose 'Raw File' in the 'Type' box and begin inputting sizes. observe that the size input behaves like the other disk size menus throughout the UI. (like in the VM disk creation step) in the browser console, you can also observe that the json-rpc request sent contains the correct size in bytes.

[bugclerk]

Jira URL: https://ixsystems.atlassian.net/browse/NAS-138184

[claude[bot]]

Claude finished @mattwyatt-ix's task —— View job

---

Code Review Complete! ✅

Great work on fixing this bug! 🎉 The implementation correctly addresses the size conversion issue and the changes are well-tested. Here's my feedback:

Positive Aspects

The fix properly converts human-readable sizes (like "3 GiB") to bytes before sending to the API using the existing IxFormatterService, which is consistent with how disk sizes are handled elsewhere in the UI. The tooltip update removes the GiB restriction and provides helpful examples, and tests are comprehensive with good coverage of edge cases including whitespace handling.

Issues to Address

1. Type Safety Concern (device-form.component.ts:183-186)

The comment acknowledges that the formatter causes type inconsistency, but this is a code smell. The form field type is defined as number | string | null, which means throughout the component lifecycle, the size field could be a string even though the API expects a number.

// Current approach - type is loosely defined
size: [null as number | string | null],

Suggestion: Consider using a more type-safe approach by keeping the form field as number | null and handling the string parsing explicitly in the formatter/parser. The parser should always return a number or null, not a string. This would eliminate the need for the string-to-null conversion logic in getUpdateAttributes().

2. Incomplete Whitespace Handling (device-form.component.ts:591-594)

The whitespace check only handles empty strings, but doesn't trim leading/trailing whitespace from valid inputs:

if (typeof size === 'string' && size.trim() === '') {
  values.size = null;
}

Issue: If a user enters " 10 GiB " (with extra spaces), this code won't trim it. While the parser might handle this, it's not guaranteed.

Suggestion: Either:

• Trim all string values before processing: values.size = size.trim() || null;
• Or better, fix the parser to never return strings in the first place

3. Missing Type Assertion After Conversion (device-form.component.ts:591-606)

After converting size from string to null, there's no validation that the value is actually a number when it's not null. If the parser fails to convert "invalid input" to a number, it might still be a string.

Suggestion: Add runtime validation:

if ('size' in values) {
  const size = values.size;
  if (typeof size === 'string') {
    const trimmed = size.trim();
    values.size = trimmed === '' ? null : Number(trimmed);
  }
  // Validate the result is actually a number or null
  if (values.size !== null && typeof values.size !== 'number') {
    console.error('Invalid size value:', values.size);
    values.size = null;
  }
}

4. Inconsistent Form Field Type Declaration

The rawFileForm declares size as number | string | null, but the API expects number | null (based on the VmRawFileDevice interface). This type mismatch should be avoided.

5. Test Coverage Gap

While the whitespace test is excellent, there's no test for invalid input handling (e.g., user types "abc" or "10 GB" with wrong unit).

Suggestion: Add a test case for invalid input to ensure the form validation catches it before submission.

Recommendations

1. Refactor the type handling: Make the parser always return number | null, not strings
2. Add input validation: Ensure invalid formats are caught and displayed to the user
3. Simplify the conversion logic: If the parser works correctly, you shouldn't need the string-to-null conversion in getUpdateAttributes()
4. Add test for invalid input: Verify error handling for malformed size inputs

Architecture Note

The fundamental issue is that IxFormatterService.memorySizeParsing might be returning strings when it should only return numbers or null. Consider checking if this is a bug in the formatter service itself, rather than working around it in this component.

Overall, the fix works but could benefit from better type safety and more robust error handling! 🚀

---

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 86.28%. Comparing base (5daa552) to head (62b9cbb). :warning: Report is 14 commits behind head on master. :white_check_mark: All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #12760      +/-   ##
==========================================
+ Coverage   86.27%   86.28%   +0.01%     
==========================================
  Files        1824     1826       +2     
  Lines       67599    67742     +143     
  Branches     8140     8172      +32     
==========================================
+ Hits        58322    58454     +132     
- Misses       9277     9288      +11     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[bugclerk]

This PR has been merged and conversations have been locked. If you would like to discuss more about this issue please use our forums or raise a Jira ticket.