Skip to content

Commit

Permalink
merge: #3361
Browse files Browse the repository at this point in the history
3361: Move validations qualification to backend r=paulocsanz a=paulocsanz



Co-authored-by: Paulo Cabral <[email protected]>
  • Loading branch information
si-bors-ng[bot] and paulocsanz authored Mar 4, 2024
2 parents 4844a25 + 1c295e4 commit 5a4a4a2
Show file tree
Hide file tree
Showing 12 changed files with 120 additions and 151 deletions.
63 changes: 0 additions & 63 deletions app/web/src/store/component_attributes.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import {
PropertyEditorValues,
} from "@/api/sdf/dal/property_editor";
import { nilId } from "@/utils/nilId";
import { Qualification } from "@/api/sdf/dal/qualification";
import { useFeatureFlagsStore } from "@/store/feature_flags.store";
import { useChangeSetsStore } from "./change_sets.store";
import { useRealtimeStore } from "./realtime/realtime.store";
import { ComponentId, useComponentsStore } from "./components.store";
Expand Down Expand Up @@ -82,8 +80,6 @@ export type AttributeTreeItem = {
};

export const useComponentAttributesStore = (componentId: ComponentId) => {
const featureFlagsStore = useFeatureFlagsStore();

const changeSetsStore = useChangeSetsStore();
const changeSetId = changeSetsStore.selectedChangeSetId;

Expand Down Expand Up @@ -203,65 +199,6 @@ export const useComponentAttributesStore = (componentId: ComponentId) => {
const componentsStore = useComponentsStore();
return componentsStore.componentsById[componentId];
},

schemaValidation(): Qualification {
const emptyQualification = {
title: "Schema Validation",
output: [],
result: {
status: "unknown" as "success" | "unknown",
sub_checks: [],
},
};

if (!featureFlagsStore.JOI_VALIDATIONS) {
/* eslint-disable no-console */
console.warn(
"Trying to get schemaValidation with feature flag turned off",
);
return emptyQualification;
}

if (!this.validations) {
return emptyQualification;
}

let status: "success" | "failure" = "success";
let failCounter = 0;
const output = [];
for (const [propId, validations] of Object.entries(
this.validations,
)) {
const prop = this.schema?.props[propId];
if (!prop) continue;

for (const [, validation] of validations) {
if (validation.status !== "Success") {
status = "failure";
failCounter++;
}
output.push({
line: `${prop.name}: ${validation.message}`,
stream: "stdout",
level: "log",
});
}
}

return {
title: "Schema Validation",
output,
result: {
status,
sub_checks: [
{
status,
description: `Component has ${failCounter} invalid value(s). Click "View Details" for more info.`,
},
],
},
};
},
},
actions: {
async FETCH_PROPERTY_EDITOR_SCHEMA() {
Expand Down
1 change: 0 additions & 1 deletion app/web/src/store/feature_flags.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const FLAG_MAPPING = {
COPY_PASTE: "copy_paste",
DONT_BLOCK_ON_ACTIONS: "dont_block_on_actions",
INVITE_USER: "invite_user",
JOI_VALIDATIONS: "joi_validations",
MODULES_TAB: "modules_tab",
MULTI_VARIANT_EDITING: "multiVariantEditing",
RESIZABLE_PANEL_UPGRADE: "resizable-panel-upgrade",
Expand Down
71 changes: 4 additions & 67 deletions app/web/src/store/qualifications.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ import * as _ from "lodash-es";
import { addStoreHooks, ApiRequest } from "@si/vue-lib/pinia";
import { Qualification } from "@/api/sdf/dal/qualification";
import { useWorkspacesStore } from "@/store/workspaces.store";
import { useComponentAttributesStore } from "@/store/component_attributes.store";
import { useFeatureFlagsStore } from "@/store/feature_flags.store";
import { useChangeSetsStore } from "./change_sets.store";
import { useRealtimeStore } from "./realtime/realtime.store";
import { ComponentId, useComponentsStore } from "./components.store";
Expand All @@ -21,8 +19,6 @@ type QualificationStats = {
};

export const useQualificationsStore = () => {
const featureFlagsStore = useFeatureFlagsStore();

const changeSetsStore = useChangeSetsStore();
const changeSetId = changeSetsStore.selectedChangeSetId;

Expand All @@ -33,79 +29,20 @@ export const useQualificationsStore = () => {
`ws${workspaceId || "NONE"}/cs${changeSetId || "NONE"}/qualifications`,
{
state: () => ({
// stats per component - this is the raw data
// we may change this to store qualification ids and individual statuses to make realtime updates easier
// NOTE(victor) Use the qualificationStatsByComponentId getter, it has validation data
qualificationStatsByComponentIdRaw: {} as Record<
qualificationStatsByComponentId: {} as Record<
ComponentId,
QualificationStats
>,

// NOTE(victor) Use the qualificationsByComponentId getter, it has validation data
qualificationsByComponentIdRaw: {} as Record<
qualificationsByComponentId: {} as Record<
ComponentId,
Qualification[]
>,

checkedQualificationsAt: null as Date | null,
}),
getters: {
// NOTE(victor) the following two getters only exist because Joi validations
// run on the frontend. If all qualification data goes back
// to coming from the API we can delete both *Raw entries from state
qualificationStatsByComponentId: (state) =>
_.mapValues(
state.qualificationStatsByComponentIdRaw,
(cs, componentId) => {
let total = cs.total;
let succeeded = cs.succeeded;
let failed = cs.failed;

if (featureFlagsStore.JOI_VALIDATIONS) {
const { result: validationResult } =
useComponentAttributesStore(componentId).schemaValidation;

if (validationResult) {
total += 1;
if (validationResult.status === "success") succeeded += 1;
else failed += 1;
}
}

return {
...cs,
total,
succeeded,
failed,
};
},
),
qualificationsByComponentId: (state) =>
_.mapValues(
state.qualificationsByComponentIdRaw,
(qualifications, componentId) => {
const compiledQualifications = _.cloneDeep(qualifications);

if (featureFlagsStore.JOI_VALIDATIONS) {
compiledQualifications.push(
useComponentAttributesStore(componentId).schemaValidation,
);
}

// TODO: maybe we want to sort these in the backend?
return _.orderBy(
compiledQualifications,
(response) =>
({
failure: 1,
warning: 2,
unknown: 3,
success: 4,
}[response.result?.status || "unknown"]),
);
},
),

// single status per component
qualificationStatusByComponentId(): Record<
ComponentId,
Expand Down Expand Up @@ -193,7 +130,7 @@ export const useQualificationsStore = () => {
"componentId",
);

this.qualificationStatsByComponentIdRaw = _.mapValues(
this.qualificationStatsByComponentId = _.mapValues(
byComponentId,
({ total, succeeded, warned, failed }) => ({
// transform the data slightly to add "running" so we can avoid recalculating again elsewhere
Expand Down Expand Up @@ -222,7 +159,7 @@ export const useQualificationsStore = () => {
visibility_change_set_pk: changeSetId,
},
onSuccess: (response) => {
this.qualificationsByComponentIdRaw[componentId] = response;
this.qualificationsByComponentId[componentId] = response;
},
});
},
Expand Down
1 change: 1 addition & 0 deletions lib/dal/src/attribute/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1078,6 +1078,7 @@ impl AttributeValue {
Ok(result)
}

#[instrument(level = "info", skip_all)]
pub async fn ids_for_component(
ctx: &DalContext,
component_id: ComponentId,
Expand Down
4 changes: 4 additions & 0 deletions lib/dal/src/component/qualification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ impl Component {
}
}

if let Some(view) = QualificationView::new_for_validations(ctx, component_id).await? {
qualification_views.push(view);
}

qualification_views.sort();
// We want the "all fields valid" to always be first
results.extend(qualification_views);
Expand Down
1 change: 1 addition & 0 deletions lib/dal/src/diagram/summary_diagram.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ pub async fn update_socket_summary(
Ok(())
}

#[instrument(level = "info", skip_all)]
pub async fn create_component_entry(
ctx: &DalContext,
component: &Component,
Expand Down
4 changes: 3 additions & 1 deletion lib/dal/src/job/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::{
job::producer::BlockingJobError, job::producer::JobProducerError, status::StatusUpdaterError,
AccessBuilder, ActionPrototypeError, ActionPrototypeId, AttributeValueError, ComponentError,
ComponentId, DalContext, DalContextBuilder, FixBatchId, FixResolverError, PropError,
StandardModelError, TransactionsError, Visibility, WsEventError,
StandardModelError, TransactionsError, ValidationResolverError, Visibility, WsEventError,
};

#[remain::sorted]
Expand Down Expand Up @@ -89,6 +89,8 @@ pub enum JobConsumerError {
#[error(transparent)]
UlidDecode(#[from] ulid::DecodeError),
#[error(transparent)]
ValidationResolver(#[from] ValidationResolverError),
#[error(transparent)]
WsEvent(#[from] WsEventError),
}

Expand Down
2 changes: 1 addition & 1 deletion lib/dal/src/job/definition.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
mod dependent_values_update;
pub mod dependent_values_update;
mod fix;
mod refresh;

Expand Down
26 changes: 24 additions & 2 deletions lib/dal/src/job/definition/dependent_values_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ use crate::{
},
job::producer::{JobProducer, JobProducerResult},
AccessBuilder, AttributeValue, AttributeValueError, AttributeValueId, AttributeValueResult,
DalContext, Prop, StandardModel, StatusUpdater, Visibility, WsEvent,
DalContext, Prop, StandardModel, StatusUpdater, ValidationResolver, ValidationStatus,
Visibility, WsEvent,
};
use crate::{FuncBindingReturnValue, InternalProvider};

Expand Down Expand Up @@ -471,7 +472,7 @@ async fn update_value(
component.id = %component_id,
)
)]
async fn update_summary_tables(
pub async fn update_summary_tables(
ctx: &DalContext,
component_value_json: &serde_json::Value,
component_id: ComponentId,
Expand Down Expand Up @@ -540,6 +541,27 @@ async fn update_summary_tables(
}
}
}

let mut success = None;
for resolver in ValidationResolver::find_by_attr(ctx, "component_id", &component_id).await? {
if success.is_none() {
success = Some(true);
}

if resolver.value()?.status != ValidationStatus::Success {
success = Some(false);
}
}

if let Some(success) = success {
total += 1;
if success {
succeeded += 1;
} else {
failed += 1;
}
}

let _row = ctx
.txns()
.await?
Expand Down
27 changes: 23 additions & 4 deletions lib/dal/src/prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,22 @@ use crate::standard_model::{
};
use crate::{
attribute::{prototype::AttributePrototype, value::AttributeValue},
component::ComponentViewError,
func::{
binding::{FuncBinding, FuncBindingError},
binding_return_value::FuncBindingReturnValueError,
},
impl_standard_model,
job::consumer::JobConsumerError,
label_list::ToLabelList,
pk,
property_editor::schema::WidgetKind,
standard_model, standard_model_accessor, standard_model_belongs_to, standard_model_has_many,
AttributeContext, AttributeContextBuilder, AttributeContextBuilderError,
AttributePrototypeError, AttributeReadContext, ComponentId, DalContext, Func, FuncError,
FuncId, HistoryEventError, SchemaVariantId, StandardModel, StandardModelError, Tenancy,
Timestamp, ValidationOutput, ValidationResolver, ValidationResolverError, ValidationStatus,
Visibility,
AttributePrototypeError, AttributeReadContext, ComponentId, ComponentView, DalContext, Func,
FuncError, FuncId, HistoryEventError, SchemaVariantId, StandardModel, StandardModelError,
Tenancy, Timestamp, ValidationOutput, ValidationResolver, ValidationResolverError,
ValidationStatus, Visibility,
};
use crate::{AttributeValueError, AttributeValueId, FuncBackendResponseType, TransactionsError};

Expand Down Expand Up @@ -137,6 +139,8 @@ pub enum PropError {
AttributePrototype(#[from] AttributePrototypeError),
#[error("AttributeValue error: {0}")]
AttributeValue(#[from] AttributeValueError),
#[error(transparent)]
ComponentView(#[from] Box<ComponentViewError>),
#[error("default diff function not found")]
DefaultDiffFunctionNotFound,
#[error("expected child prop not found with name {0}")]
Expand All @@ -149,6 +153,8 @@ pub enum PropError {
FuncBindingReturnValue(#[from] FuncBindingReturnValueError),
#[error("history event error: {0}")]
HistoryEvent(#[from] HistoryEventError),
#[error(transparent)]
JobConsumer(#[from] Box<JobConsumerError>),
#[error("Map prop {0} is missing element child")]
MapMissingElementChild(PropId),
#[error("missing a func: {0}")]
Expand Down Expand Up @@ -693,6 +699,7 @@ impl Prop {
self.set_diff_func_id(ctx, Some(*func.id())).await
}

#[instrument(level = "info", skip_all)]
pub async fn run_validation(
ctx: &DalContext,
prop_id: PropId,
Expand Down Expand Up @@ -774,6 +781,18 @@ impl Prop {
.await
.map_err(Box::new)?;
}

let component_value_json = ComponentView::new(ctx, component_id)
.await
.map_err(Box::new)?
.properties;
crate::job::definition::dependent_values_update::update_summary_tables(
ctx,
&component_value_json,
component_id,
)
.await
.map_err(Box::new)?;
}
Ok(())
}
Expand Down
Loading

0 comments on commit 5a4a4a2

Please sign in to comment.