Skip to content

Commit

Permalink
fix(dal): export default values correctly
Browse files Browse the repository at this point in the history
A bit of code left over from the old system (where default values were
the attribute value for a prop at the schema variant context) was
grabbing the first attribute value and making it the default for props
if a component existed on export. This fixes it and fixes the export of
default values for strings.

Default values now can only be defined in the asset spec.

Co-Authored-By: Paul Stack <paul@systeminit.com>
  • Loading branch information
zacharyhamm and stack72 committed Jun 28, 2024
1 parent f8fecca commit b20aa9d
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 34 deletions.
37 changes: 4 additions & 33 deletions lib/dal/src/pkg/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,17 +554,16 @@ impl PkgExporter {
builder: PropSpecBuilder,
prop_id: PropId,
parent_prop_id: Option<PropId>,
inside_map_or_array: bool,
}

let mut stack: Vec<(PropId, Option<PropId>, bool)> = Vec::new();
let mut stack: Vec<(PropId, Option<PropId>)> = Vec::new();
for child_tree_node in Prop::direct_child_prop_ids(ctx, root_prop.id()).await? {
stack.push((child_tree_node, None, false));
stack.push((child_tree_node, None));
}

let mut traversal_stack: Vec<TraversalStackEntry> = Vec::new();

while let Some((prop_id, parent_prop_id, inside_map_or_array)) = stack.pop() {
while let Some((prop_id, parent_prop_id)) = stack.pop() {
let child_prop = Prop::get_by_id_or_error(ctx, prop_id).await?;
let mut builder = PropSpec::builder();

Expand Down Expand Up @@ -608,16 +607,10 @@ impl PkgExporter {
builder,
prop_id,
parent_prop_id,
inside_map_or_array,
});

for child_tree_node in Prop::direct_child_prop_ids(ctx, child_prop.id).await? {
stack.push((
child_tree_node,
Some(prop_id),
matches!(child_prop.kind, PropKind::Array | PropKind::Map)
|| inside_map_or_array,
));
stack.push((child_tree_node, Some(prop_id)));
}
}

Expand Down Expand Up @@ -705,28 +698,6 @@ impl PkgExporter {
}
}

// TODO: handle default values for complex types. We also cannot set default values for
// children of arrays and maps, at any depth (currently), since that requires tracking the
// key or index
if matches!(
entry.builder.get_kind(),
Some(PropSpecKind::String)
| Some(PropSpecKind::Number)
| Some(PropSpecKind::Boolean)
) && !entry.inside_map_or_array
{
if let Some(av_id) = Prop::attribute_values_for_prop_id(ctx, entry.prop_id)
.await?
.pop()
{
let av = AttributeValue::get_by_id(ctx, av_id).await?;
if let Some(default_value) = av.value(ctx).await? {
entry.builder.has_data(true);
entry.builder.default_value(default_value);
}
}
}

let prop_spec = entry.builder.build()?;

match entry.parent_prop_id {
Expand Down
2 changes: 1 addition & 1 deletion lib/si-pkg/src/pkg/variant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ async fn create_prop_stack(
builder.has_data(false);

let default_value = match &spec {
SiPkgProp::Array { data, .. }
SiPkgProp::String { data, .. }
| SiPkgProp::Boolean { data, .. }
| SiPkgProp::Number { data, .. } => {
data.as_ref().and_then(|data| data.default_value.to_owned())
Expand Down

0 comments on commit b20aa9d

Please sign in to comment.