Skip to content

Commit

Permalink
merge: #4036
Browse files Browse the repository at this point in the history
4036: Disambiguate Schema Name, Schema Variant Name, and Schema Variant Display Name across the UI. On import, if there's no display name, hydrate with Schema name. Same thing on regenerate. This is a step towards schema names being immutable. r=britmyerss a=britmyerss

This PR does the following: 
- On import, if the schema variant we're importing doesn't already have a display name, we copy the schema name to it
- On regenerate, if the variant doesn't have a display name, copy the schema name to it
- Return schema name separately in list routes
- Allow mutating the schema name separately from the variant name 
- Throughout the UI, default to Variant Display Name, and if it doesn't exist, show the Schema Name.   

This PR sets us up to be able to make schema names immutable by exposing what is necessary for us to update builtins with what we want the immutable schema name to be.  For now though, we must keep the names of builtins the same until we match by unique ID vs name on import.

<div><img src="https://media1.giphy.com/media/9PqOegBqgBnMHvERV3/giphy.gif?cid=5a38a5a2rbyw6q3hziwhfewsun0y3jrclt210uhrhjqfnsvb&amp;ep=v1_gifs_search&amp;rid=giphy.gif&amp;ct=g" style="border:0;height:231px;width:300px"/><br/>via <a href="https://giphy.com/channel/hauntsss/">Haunts</a> on <a href="https://giphy.com/gifs/hello-my-name-is-9PqOegBqgBnMHvERV3">GIPHY</a></div>

Co-authored-by: Brit Myers <[email protected]>
  • Loading branch information
si-bors-ng[bot] and britmyerss authored Jun 25, 2024
2 parents 4e6f01c + d0c5633 commit b0ba249
Show file tree
Hide file tree
Showing 27 changed files with 186 additions and 94 deletions.
3 changes: 2 additions & 1 deletion app/web/src/api/sdf/dal/diagram.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ export interface DiagramSchemaVariant {

export interface DiagramSchema {
id: string;
name: string;
name: string; // schema name
displayName: string; // variant display name
builtin: boolean;

variants: DiagramSchemaVariant[];
Expand Down
2 changes: 1 addition & 1 deletion app/web/src/components/AssetCard.vue
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
{{ asset.displayName }}
</template>
<template v-else>
{{ asset.name }}
{{ asset.schemaName }}
</template>
</div>
</Stack>
Expand Down
16 changes: 12 additions & 4 deletions app/web/src/components/AssetDetailsPanel.vue
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,22 @@
<div>
<ErrorMessage :requestStatus="execAssetReqStatus" />
</div>

<VormInput
id="schemaName"
v-model="editingAsset.schemaName"
type="text"
label="Asset Name"
compact
placeholder="(mandatory) Provide the asset a name"
@blur="updateAsset"
/>
<VormInput
id="name"
v-model="editingAsset.name"
type="text"
label="Name"
label="Asset Version Name"
compact
placeholder="(mandatory) Provide the asset a name"
placeholder="(mandatory) Provide the asset version a name"
@blur="updateAsset"
/>

Expand All @@ -78,7 +86,7 @@
type="text"
label="Display name"
compact
placeholder="(optional) Provide the asset a shorter display name"
placeholder="(optional) Provide the asset version a display name"
@blur="updateAsset"
/>
<VormInput
Expand Down
6 changes: 3 additions & 3 deletions app/web/src/components/AssetEditorTabs.vue
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
import isEqual from "lodash-es/isEqual";
import { watch, ref, computed } from "vue";
import { TabGroup, TabGroupItem } from "@si/vue-lib/design-system";
import { useAssetStore } from "@/store/asset.store";
import { useAssetStore, assetDisplayName } from "@/store/asset.store";
import { useFuncStore } from "@/store/func/funcs.store";
import AssetEditor from "./AssetEditor.vue";
import FuncEditor from "./FuncEditor/FuncEditor.vue";
Expand Down Expand Up @@ -84,10 +84,10 @@ watch(
if (!requestStatus.isSuccess) {
return;
}
const asset = assetStore.assetFromListById[assetId];
const assetTab = {
type: "asset",
label: assetStore.assetFromListById[assetId]?.name ?? "error",
label: asset ? assetDisplayName(asset) ?? "error" : "error",
id: assetId,
};
Expand Down
2 changes: 1 addition & 1 deletion app/web/src/components/AssetListPanel.vue
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ const categorizedAssets = computed(() =>
if (include && searchString.value.length) {
include = !!(
asset.name.toLocaleLowerCase().includes(searchString.value) ||
asset.schemaName.toLocaleLowerCase().includes(searchString.value) ||
asset.displayName?.toLocaleLowerCase().includes(searchString.value)
);
}
Expand Down
6 changes: 4 additions & 2 deletions app/web/src/components/AssetPalette.vue
Original file line number Diff line number Diff line change
Expand Up @@ -284,9 +284,11 @@ const schemaDisplayName = (
schemas: DiagramSchemaWithDisplayMetadata[],
) => {
const duplicates = schemas.filter((s) => s.name === schema.name);
let displayName = schema.variants[0]?.displayName;
if (!displayName) displayName = schema.name;
if (duplicates.length > 1) {
return `${schema.name} (${duplicates.indexOf(schema)})`;
} else return schema.name;
return `${displayName} (${duplicates.indexOf(schema)})`;
} else return displayName;
};
const updateMouseNode = (e: MouseEvent) => {
Expand Down
9 changes: 8 additions & 1 deletion app/web/src/components/FuncEditor/AttributeBindings.vue
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
<h1 class="pt-xs text-neutral-700 type-bold-sm dark:text-neutral-50">
Asset:
</h1>
<h2 class="pb-xs text-sm">{{ proto.schema }}</h2>
<h1 class="pt-xs text-neutral-700 type-bold-sm dark:text-neutral-50">
Asset version:
</h1>
<h2 class="pb-xs text-sm">{{ proto.schemaVariant }}</h2>

<h1 class="pt-xs text-neutral-700 type-bold-sm dark:text-neutral-50">
Expand Down Expand Up @@ -227,9 +231,11 @@ const prototypeViews = computed(() => {
];
const schemaVariant =
useComponentsStore().schemaVariantsById[schemaVariantId ?? ""]?.name ??
"none";
const schema =
useComponentsStore().schemaVariantsById[schemaVariantId ?? ""]
?.schemaName ?? "none";
const component =
funcStore.componentOptions.find((c) => c.value === proto.componentId)
?.label ?? "all";
Expand All @@ -256,6 +262,7 @@ const prototypeViews = computed(() => {
return {
id: proto.id,
schema,
schemaVariant,
component,
outputLocation,
Expand Down
9 changes: 5 additions & 4 deletions app/web/src/store/asset.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export interface DetachedValidationPrototype {
export interface ListedVariant {
id: AssetId;
defaultSchemaVariantId: string;
schemaName: string;
name: string;
displayName?: string;
category: string;
Expand Down Expand Up @@ -106,7 +107,7 @@ export type AssetCreateRequest = Omit<
export type AssetCloneRequest = Visibility & { id: AssetId; name: string };

export const assetDisplayName = (asset: Asset | AssetListEntry) =>
(asset.displayName ?? "").length === 0 ? asset.name : asset.displayName;
(asset.displayName ?? "").length === 0 ? asset.schemaName : asset.displayName;

export const useAssetStore = () => {
const changeSetsStore = useChangeSetsStore();
Expand Down Expand Up @@ -303,7 +304,8 @@ export const useAssetStore = () => {
return {
id: nilId(),
defaultSchemaVariantId: "",
name,
schemaName: name,
name: "v0",
displayName: name,
code: "",
color: this.generateMockColor(),
Expand Down Expand Up @@ -399,8 +401,6 @@ export const useAssetStore = () => {
return () => {
if (current) {
this.assetsById[asset.id] = current;
} else {
delete this.assetsById[asset.id];
}
};
},
Expand Down Expand Up @@ -494,6 +494,7 @@ export const useAssetStore = () => {
const variant = {
id: data.schemaId,
defaultSchemaVariantId: data.schemaVariantId,
schemaName: data.schemaName,
name: data.name,
displayName: "",
category: data.category,
Expand Down
1 change: 1 addition & 0 deletions app/web/src/store/components.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,7 @@ export const useComponentsStore = (forceChangeSetId?: ChangeSetId) => {
schemasWithAtLeastOneVariant.push({
id: schema.id,
name: schema.name,
displayName: schema.displayName,
builtin: schema.builtin,
variants: schema.variants,
category: schema.variants[0].category,
Expand Down
1 change: 1 addition & 0 deletions app/web/src/store/realtime/realtime_events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ export type WsEventPayloadMap = {
schemaId: string;
schemaVariantId: string;
name: string;
schemaName: string;
category: string;
color: string;
changeSetId: ChangeSetId;
Expand Down
14 changes: 13 additions & 1 deletion lib/dal/src/history_event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use telemetry::prelude::*;
use thiserror::Error;

use crate::actor_view::ActorView;
use crate::{pk, DalContext, Timestamp, UserPk};
use crate::{pk, DalContext, Timestamp, User, UserPk};
use crate::{Tenancy, TransactionsError};

#[remain::sorted]
Expand All @@ -23,6 +23,8 @@ pub enum HistoryEventError {
StandardModel(String),
#[error("transactions error: {0}")]
Transactions(#[from] TransactionsError),
#[error("user error: {0}")]
User(String),
}

pub type HistoryEventResult<T> = Result<T, HistoryEventError>;
Expand All @@ -41,6 +43,16 @@ impl HistoryActor {
HistoryActor::SystemInit => "unknown-backend".to_string(),
}
}
pub async fn email(&self, ctx: &DalContext) -> HistoryEventResult<String> {
Ok(match self {
HistoryActor::SystemInit => "[email protected]".to_string(),
HistoryActor::User(user_pk) => User::get_by_pk_or_error(ctx, *user_pk)
.await
.map_err(|e| HistoryEventError::User(e.to_string()))?
.email()
.clone(),
})
}
}

impl From<UserPk> for HistoryActor {
Expand Down
4 changes: 3 additions & 1 deletion lib/dal/src/pkg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use crate::{
SchemaVariantId, TransactionsError, UserPk, WorkspaceError, WorkspacePk, WsEvent,
WsEventResult, WsPayload,
};
use crate::{AttributePrototypeId, FuncId, PropId, PropKind};
use crate::{AttributePrototypeId, FuncId, HistoryEventError, PropId, PropKind};

use crate::module::ModuleError;
use crate::socket::connection_annotation::ConnectionAnnotationError;
Expand Down Expand Up @@ -64,6 +64,8 @@ pub enum PkgError {
FuncArgumentNotFoundByName(FuncId, String),
#[error("func {0} could not be found by name")]
FuncNotFoundByName(String),
#[error("history event error: {0}")]
HistoryEvent(#[from] HistoryEventError),
#[error("input socket error: {0}")]
InputSocket(#[from] InputSocketError),
#[error("Missing Func {1} for AttributePrototype {0}")]
Expand Down
14 changes: 9 additions & 5 deletions lib/dal/src/pkg/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ impl PkgExporter {
}
}

fn new_standalone_variant_exporter() -> Self {
Self::new_module_exporter("", "", None::<String>, "", vec![])
fn new_standalone_variant_exporter(schema_name: &str) -> Self {
Self::new_module_exporter(schema_name, "", None::<String>, "", vec![])
}

pub async fn export_as_bytes(&mut self, ctx: &DalContext) -> PkgResult<Vec<u8>> {
Expand Down Expand Up @@ -144,9 +144,11 @@ impl PkgExporter {
pub async fn export_variant_standalone(
ctx: &DalContext,
variant: &SchemaVariant,
schema_name: &str,
) -> PkgResult<(SchemaVariantSpec, Vec<FuncSpec>)> {
let mut exporter = Self::new_standalone_variant_exporter();

let mut exporter = Self::new_standalone_variant_exporter(schema_name);
let email = ctx.history_actor().email(ctx).await?;
exporter.created_by = email;
exporter.export_funcs_for_variant(ctx, variant.id()).await?;
exporter.export_intrinsics(ctx).await?;
let variant_spec = exporter.export_variant(ctx, variant, false).await?;
Expand All @@ -171,7 +173,6 @@ impl PkgExporter {
variant_spec_builder.name(variant.name());
variant_spec_builder.is_builtin(variant_is_builtin);
variant_spec_builder.unique_id(variant.id().to_string());

let mut data_builder = SchemaVariantSpecData::builder();

data_builder.name(variant.name());
Expand All @@ -180,6 +181,9 @@ impl PkgExporter {
if let Some(link) = variant.link() {
data_builder.try_link(link.to_string().deref())?;
}
if let Some(display_name) = variant.display_name() {
data_builder.display_name(display_name);
}

data_builder.component_type(get_component_type(ctx, variant).await?);

Expand Down
8 changes: 7 additions & 1 deletion lib/dal/src/pkg/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -971,6 +971,7 @@ pub(crate) async fn import_schema_variant(
let (variant, created) = match existing_schema_variant {
None => {
let spec = schema_spec.to_spec().await?;
let schema_name = &spec.name.clone();
let metadata = SchemaVariantJson::metadata_from_spec(spec)?;

let mut asset_func_id: Option<FuncId> = None;
Expand All @@ -987,12 +988,17 @@ pub(crate) async fn import_schema_variant(
asset_func_id = Some(asset_func.id)
}
}
let display_name = match metadata.display_name {
Some(name) => name,
None => schema_name.into(),
};
info!(%display_name, "display_name");
(
SchemaVariant::new(
ctx,
schema.id(),
variant_spec.name(),
metadata.menu_name,
display_name,
metadata.category,
metadata.color,
metadata.component_type,
Expand Down
Loading

0 comments on commit b0ba249

Please sign in to comment.