feat: allow to manage display_order value for building block definiti…#218
feat: allow to manage display_order value for building block definiti…#218j0g3sc wants to merge 3 commits into
Conversation
📊 Test Coverage
❌ Acceptance tests failed (advisory — the job runs against the last merged meshfed-release backend, so a change needing a companion backend PR fails here until that backend merges; the real gate is meshfed-release's Uncovered functions (combined run)Combined with |
…on I/O without affecting the calculated hash for change detection CU-86cabn76y
5321918 to
49e6c12
Compare
|
|
grubmeshi
left a comment
There was a problem hiding this comment.
Have a look at my proposal of implementing it differently. I think only addressing this inside the resource is not enough and it must be handled inside versionContentHash itself.
| return result | ||
| } | ||
|
|
||
| func displayOrdersDiffer(a, b client.MeshBuildingBlockDefinitionVersionSpec) bool { |
There was a problem hiding this comment.
f: hm, that's not quite the correct way of addressing this IMHO. the versionContentHash already has a part where it sets fields to constant/zero values.
My proposal would be instead of adding this function (remove it), address it entirely at this place. Loop over inputs/ouputs and set displayOrder to 0 no matter what the DTO sent, making the hash stable against any changes in there.
It might require making the DisplayOrder a *int64. I'm not sure of map entries only get ignored by our hashing algo if it's a nil pointer or also if it's a zero value (0 in case for int64)
| MarkdownDescription: "Numeric value controlling the display ordering of this output in meshPanel. " + | ||
| "Used only for arranging outputs; it does not affect the version's content hash. Defaults to `0` when omitted.", |
There was a problem hiding this comment.
d: sounds a bit AI sloppy (also fix likewise for input above), maybe:
| MarkdownDescription: "Numeric value controlling the display ordering of this output in meshPanel. " + | |
| "Used only for arranging outputs; it does not affect the version's content hash. Defaults to `0` when omitted.", | |
| MarkdownDescription: "Controls the display ordering of this output in meshStack UI. " + | |
| "It does not affect the version's content hash. Defaults to `0` when omitted. Values can be non-sequential to support groups, such as `01`, `02`, `03`, ..., `10`, `11`, `12`, ..., across all inputs. Duplicate values are resolved by alphanumeric sorting of keys as a fallback.", |
also, is meshPanel the official product name? I'd have called it "meshStack UI" 🤔
…nd checks. Include BB definition version input and output displayOrder field into hashsum calculation as they cannot be changed on released versions.
d7eec02 to
816cdbc
Compare
…on I/O without affecting the calculated hash for change detection
CU-86cabn76y
Summary
Added a manageable display_order field to building block definition version inputs and outputs.
Implementation:
Client
DisplayOrder int64 with json:"displayOrder,omitempty" on both MeshBuildingBlockDefinitionInput and MeshBuildingBlockDefinitionOutput.
Schema
display_order Int64Attribute on inputs and outputs, Optional + Computed + Default(0) — the Default is what keeps re-plans clean given the backend returns 0 when unset.
Content hash
stripped to 0 before hashing, so reordering never changes content_hash or forces a new version
Released immutability
displayOrdersDiffer guard in Update — needed because display_order is outside the hash, so this restores the "released versions are immutable" behavior with a clear error.
Mock
mirrors input→output display_order for manual BBDs.