better charts2

common/build.rs

@@ -9,6 +9,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         .type_attribute(".komp_ac.accounting", serde)
         .type_attribute(".komp_ac.analytics", serde)
         .field_attribute(".komp_ac.analytics.ReportDashboardFilter.primary", "#[serde(default)]")
+        .field_attribute(".komp_ac.analytics.ReportDashboardDefinition.queries", "#[serde(default)]")
         .type_attribute(".komp_ac.auth", serde)
         .type_attribute(".komp_ac.common", serde)
         .type_attribute(".komp_ac.document_data", serde)

common/proto/analytics.proto

@@ -99,6 +99,7 @@ service ReportingService {
   rpc ListAssets(ListReportAssetsRequest) returns (ListReportAssetsResponse);
   rpc GetAsset(GetReportAssetRequest) returns (ReportAsset);
   rpc SaveDraft(SaveReportDraftRequest) returns (ReportAsset);
+  rpc Validate(ValidateReportRequest) returns (ReportAsset);
   rpc Publish(PublishReportRequest) returns (ReportAsset);
   rpc ListVersions(ReportAssetRef) returns (ListReportVersionsResponse);
   rpc RestoreDraft(RestoreReportDraftRequest) returns (ReportAsset);
@@ -376,6 +377,13 @@ message ReportDashboardDefinition {
   uint32 refresh_seconds = 6;
   repeated ReportDatasetVersionRef dataset_versions = 7;
   ReportPeriod period = 8;
+  // Queries owned by this report; keys are used by panels and filter targets.
+  repeated ReportQuery queries = 9;
+}
+
+message ReportQuery {
+  string key = 1;
+  ReportDatasetDefinition definition = 2;
 }
 
 message ReportDatasetVersionRef {
@@ -445,6 +453,11 @@ message SaveReportDraftRequest {
   ReportDefinition definition = 3;
 }
 
+message ValidateReportRequest {
+  ReportAssetRef asset = 1;
+  uint64 expected_revision = 2;
+}
+
 message PublishReportRequest {
   ReportAssetRef asset = 1;
   uint64 expected_revision = 2;

common/src/proto/komp_ac.analytics.rs

@@ -426,6 +426,18 @@ pub struct ReportDashboardDefinition {
     pub dataset_versions: ::prost::alloc::vec::Vec<ReportDatasetVersionRef>,
     #[prost(message, optional, tag = "8")]
     pub period: ::core::option::Option<ReportPeriod>,
+    /// Queries owned by this report; keys are used by panels and filter targets.
+    #[prost(message, repeated, tag = "9")]
+    #[serde(default)]
+    pub queries: ::prost::alloc::vec::Vec<ReportQuery>,
+}
+#[derive(serde::Serialize, serde::Deserialize)]
+#[derive(Clone, PartialEq, ::prost::Message)]
+pub struct ReportQuery {
+    #[prost(string, tag = "1")]
+    pub key: ::prost::alloc::string::String,
+    #[prost(message, optional, tag = "2")]
+    pub definition: ::core::option::Option<ReportDatasetDefinition>,
 }
 #[derive(serde::Serialize, serde::Deserialize)]
 #[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
@@ -547,6 +559,14 @@ pub struct SaveReportDraftRequest {
 }
 #[derive(serde::Serialize, serde::Deserialize)]
 #[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
+pub struct ValidateReportRequest {
+    #[prost(message, optional, tag = "1")]
+    pub asset: ::core::option::Option<ReportAssetRef>,
+    #[prost(uint64, tag = "2")]
+    pub expected_revision: u64,
+}
+#[derive(serde::Serialize, serde::Deserialize)]
+#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
 pub struct PublishReportRequest {
     #[prost(message, optional, tag = "1")]
     pub asset: ::core::option::Option<ReportAssetRef>,
@@ -1725,6 +1745,29 @@ pub mod reporting_service_client {
                 );
             self.inner.unary(req, path, codec).await
         }
+        pub async fn validate(
+            &mut self,
+            request: impl tonic::IntoRequest<super::ValidateReportRequest>,
+        ) -> std::result::Result<tonic::Response<super::ReportAsset>, tonic::Status> {
+            self.inner
+                .ready()
+                .await
+                .map_err(|e| {
+                    tonic::Status::unknown(
+                        format!("Service was not ready: {}", e.into()),
+                    )
+                })?;
+            let codec = tonic_prost::ProstCodec::default();
+            let path = http::uri::PathAndQuery::from_static(
+                "/komp_ac.analytics.ReportingService/Validate",
+            );
+            let mut req = request.into_request();
+            req.extensions_mut()
+                .insert(
+                    GrpcMethod::new("komp_ac.analytics.ReportingService", "Validate"),
+                );
+            self.inner.unary(req, path, codec).await
+        }
         pub async fn publish(
             &mut self,
             request: impl tonic::IntoRequest<super::PublishReportRequest>,
@@ -1966,6 +2009,10 @@ pub mod reporting_service_server {
             &self,
             request: tonic::Request<super::SaveReportDraftRequest>,
         ) -> std::result::Result<tonic::Response<super::ReportAsset>, tonic::Status>;
+        async fn validate(
+            &self,
+            request: tonic::Request<super::ValidateReportRequest>,
+        ) -> std::result::Result<tonic::Response<super::ReportAsset>, tonic::Status>;
         async fn publish(
             &self,
             request: tonic::Request<super::PublishReportRequest>,
@@ -2231,6 +2278,51 @@ pub mod reporting_service_server {
                     };
                     Box::pin(fut)
                 }
+                "/komp_ac.analytics.ReportingService/Validate" => {
+                    #[allow(non_camel_case_types)]
+                    struct ValidateSvc<T: ReportingService>(pub Arc<T>);
+                    impl<
+                        T: ReportingService,
+                    > tonic::server::UnaryService<super::ValidateReportRequest>
+                    for ValidateSvc<T> {
+                        type Response = super::ReportAsset;
+                        type Future = BoxFuture<
+                            tonic::Response<Self::Response>,
+                            tonic::Status,
+                        >;
+                        fn call(
+                            &mut self,
+                            request: tonic::Request<super::ValidateReportRequest>,
+                        ) -> Self::Future {
+                            let inner = Arc::clone(&self.0);
+                            let fut = async move {
+                                <T as ReportingService>::validate(&inner, request).await
+                            };
+                            Box::pin(fut)
+                        }
+                    }
+                    let accept_compression_encodings = self.accept_compression_encodings;
+                    let send_compression_encodings = self.send_compression_encodings;
+                    let max_decoding_message_size = self.max_decoding_message_size;
+                    let max_encoding_message_size = self.max_encoding_message_size;
+                    let inner = self.inner.clone();
+                    let fut = async move {
+                        let method = ValidateSvc(inner);
+                        let codec = tonic_prost::ProstCodec::default();
+                        let mut grpc = tonic::server::Grpc::new(codec)
+                            .apply_compression_config(
+                                accept_compression_encodings,
+                                send_compression_encodings,
+                            )
+                            .apply_max_message_size_config(
+                                max_decoding_message_size,
+                                max_encoding_message_size,
+                            );
+                        let res = grpc.unary(method, req).await;
+                        Ok(res)
+                    };
+                    Box::pin(fut)
+                }
                 "/komp_ac.analytics.ReportingService/Publish" => {
                     #[allow(non_camel_case_types)]
                     struct PublishSvc<T: ReportingService>(pub Arc<T>);

common/src/reporting.rs

@@ -133,6 +133,62 @@ pub fn validate_parameter(parameter: &ReportParameter, allow_lookup: bool) -> Re
     Ok(())
 }
 
+/// Drafts may have unfinished SQL and bindings. Their envelope and domain values remain typed.
+pub fn validate_draft(definition: &ReportDefinition) -> Result<(), String> {
+    if definition.schema_version != REPORT_SCHEMA_VERSION {
+        return Err(format!("Unsupported report schema version {}", definition.schema_version));
+    }
+    if serde_json::to_vec(definition).map_err(|error| error.to_string())?.len() > MAX_REPORT_BYTES {
+        return Err("Report definition exceeds 1 MiB".into());
+    }
+    asset_kind(definition)?;
+    if title(definition).len() > 160 || description(definition).len() > 4000 {
+        return Err("Report title or description is too long".into());
+    }
+    let parameter = |p: &ReportParameter| -> Result<(), String> {
+        enum_value::<ReportDataType>(p.data_type, "parameter type")?;
+        enum_value::<ReportFilterControl>(p.control, "filter control")?;
+        enum_value::<ReportDefaultKind>(p.default_kind, "default kind")?;
+        for scalar in p.default_values.iter().chain(p.choices.iter().filter_map(|choice| choice.value.as_ref())) {
+            let value = scalar.value.as_ref().ok_or("A scalar value is required")?;
+            if matches!(value, report_scalar::Value::NullValue(code) if *code != 0) { return Err("Invalid null value".into()); }
+        }
+        for choice in &p.choices { choice.value.as_ref().ok_or("A choice value is required")?; }
+        Ok(())
+    };
+    let dataset = |d: &ReportDatasetDefinition| -> Result<(), String> {
+        for p in &d.parameters { parameter(p)?; }
+        for c in &d.columns {
+            enum_value::<ReportDataType>(c.data_type, "column type")?;
+            enum_value::<ReportNumberFormat>(c.number_format, "number format")?;
+        }
+        Ok(())
+    };
+    match definition.content.as_ref().ok_or("A report definition is required")? {
+        report_definition::Content::Dataset(d) => dataset(d)?,
+        report_definition::Content::Dashboard(d) => {
+            for q in &d.queries { dataset(q.definition.as_ref().ok_or("A query definition is required")?)?; }
+            for f in &d.filters { parameter(f.parameter.as_ref().ok_or("A filter parameter is required")?)?; }
+            for p in &d.panels {
+                enum_value::<ReportPanelKind>(p.kind, "chart kind")?;
+                enum_value::<ReportOrientation>(p.orientation, "orientation")?;
+                enum_value::<ReportSortOrder>(p.sort_order, "sort order")?;
+                enum_value::<ReportNullPolicy>(p.null_policy, "null policy")?;
+                for action in &p.actions { action.target.as_ref().ok_or("An action target is required")?; }
+            }
+            for g in &d.grants {
+                g.subject.as_ref().ok_or("A grant subject is required")?;
+                for c in &g.capabilities { enum_value::<ReportCapability>(*c, "capability")?; }
+            }
+            if let Some(period) = &d.period { period.selection.as_ref().ok_or("A period selection is required")?; }
+            if let Some(report_period::Selection::DateRange(range)) = d.period.as_ref().and_then(|p| p.selection.as_ref()) {
+                enum_value::<ReportPeriodBoundary>(range.end_boundary, "period end boundary")?;
+            }
+        }
+    }
+    Ok(())
+}
+
 pub fn validate_definition(definition: &ReportDefinition) -> Result<(), String> {
     if definition.schema_version != REPORT_SCHEMA_VERSION {
         return Err(format!(
@@ -225,10 +281,18 @@ fn validate_dataset(dataset: &ReportDatasetDefinition) -> Result<(), String> {
 }
 
 fn validate_dashboard(dashboard: &ReportDashboardDefinition) -> Result<(), String> {
+    let mut query_keys = HashSet::new();
+    if dashboard.queries.len() > 64 { return Err("A report allows at most 64 queries".into()); }
+    for query in &dashboard.queries {
+        nonempty(&query.key, "Query key", 64)?;
+        if !query_keys.insert(query.key.as_str()) { return Err(format!("Duplicate query key '{}'", query.key)); }
+        let dataset = query.definition.as_ref().ok_or_else(|| format!("SQL · {}: query definition is missing", query.key))?;
+        validate_dataset(dataset).map_err(|error| format!("SQL · {}: {error}", query.key))?;
+    }
     let referenced = referenced_datasets(dashboard);
     let mut dataset_versions = HashSet::new();
     for reference in &dashboard.dataset_versions {
-        if !referenced.contains(reference.dataset_id.as_str()) || !dataset_versions.insert(&reference.dataset_id) {
+        if query_keys.contains(reference.dataset_id.as_str()) || !referenced.contains(reference.dataset_id.as_str()) || !dataset_versions.insert(&reference.dataset_id) {
             return Err("Dataset version references must be unique and used by the dashboard".into());
         }
     }
@@ -303,16 +367,16 @@ fn validate_dashboard(dashboard: &ReportDashboardDefinition) -> Result<(), Strin
         let sort: ReportSortOrder = enum_value(panel.sort_order, "sort order")?;
         let _: ReportNullPolicy = enum_value(panel.null_policy, "null policy")?;
         if !(1..=12).contains(&panel.width) || !(160..=1200).contains(&panel.height) {
-            return Err("Panel width must be 1–12 and height 160–1200".into());
+            return Err(format!("Charts · {} [{}]: Panel width must be 1–12 and height 160–1200", panel.title, panel.id));
         }
         if sort != ReportSortOrder::Source && panel.sort_field.is_empty() {
-            return Err("A sorted panel needs a sort field".into());
+            return Err(format!("Charts · {} [{}]: A sorted panel needs a sort field", panel.title, panel.id));
         }
         if panel.axis_min.is_some_and(|number| !number.is_finite())
             || panel.axis_max.is_some_and(|number| !number.is_finite())
             || matches!((panel.axis_min, panel.axis_max), (Some(min), Some(max)) if min >= max)
         {
-            return Err("Invalid chart axis bounds".into());
+            return Err(format!("Charts · {} [{}]: Invalid chart axis bounds", panel.title, panel.id));
         }
         if panel.colors.len() > 32
             || panel.colors.iter().any(|color| {
@@ -321,10 +385,10 @@ fn validate_dashboard(dashboard: &ReportDashboardDefinition) -> Result<(), Strin
                     || !color[1..].bytes().all(|byte| byte.is_ascii_hexdigit())
             })
         {
-            return Err("Chart colors must be hexadecimal CSS colors".into());
+            return Err(format!("Charts · {} [{}]: Chart colors must be hexadecimal CSS colors", panel.title, panel.id));
         }
         if panel.actions.len() > 8 {
-            return Err("A panel can have at most eight actions".into());
+            return Err(format!("Charts · {} [{}]: A panel can have at most eight actions", panel.title, panel.id));
         }
         for action in &panel.actions {
             nonempty(&action.label, "Action label", 160)?;
@@ -483,7 +547,7 @@ pub fn validate_dashboard_datasets(
         if panel.y_fields.iter().collect::<HashSet<_>>().len() != panel.y_fields.len()
             || panel.table_fields.iter().collect::<HashSet<_>>().len() != panel.table_fields.len()
         {
-            return Err("Panel field selections must be unique".into());
+            return Err(format!("Charts · {} [{}]: Panel field selections must be unique", panel.title, panel.id));
         }
         let field = |key: &str| {
             columns
@@ -506,12 +570,12 @@ pub fn validate_dashboard_datasets(
         }
         if kind != ReportPanelKind::Table {
             if panel.y_fields.is_empty() || panel.y_fields.len() > 16 {
-                return Err("Charts need between 1 and 16 measures".into());
+                return Err(format!("Charts · {} [{}]: Charts need between 1 and 16 measures", panel.title, panel.id));
             }
             for key in &panel.y_fields {
                 let kind: ReportDataType = enum_value(field(key)?.data_type, "measure type")?;
                 if !matches!(kind, ReportDataType::Integer | ReportDataType::Decimal) {
-                    return Err("Chart measures must be numeric".into());
+                    return Err(format!("Charts · {} [{}]: Chart measures must be numeric", panel.title, panel.id));
                 }
             }
             if !matches!(kind, ReportPanelKind::Kpi | ReportPanelKind::Gauge) {
@@ -529,7 +593,7 @@ pub fn validate_dashboard_datasets(
                     | ReportPanelKind::Kpi
             ) && panel.y_fields.len() != 1
             {
-                return Err("This panel kind requires exactly one measure".into());
+                return Err(format!("Charts · {} [{}]: This panel kind requires exactly one measure", panel.title, panel.id));
             }
             if kind == ReportPanelKind::Heatmap {
                 field(&panel.series_field)?;
@@ -540,7 +604,7 @@ pub fn validate_dashboard_datasets(
             ].into_iter().flatten() {
                 let data_type: ReportDataType = enum_value(field(key)?.data_type, "numeric field type")?;
                 if !matches!(data_type, ReportDataType::Integer | ReportDataType::Decimal) {
-                    return Err("Scatter coordinates and size fields must be numeric".into());
+                    return Err(format!("Charts · {} [{}]: Scatter coordinates and size fields must be numeric", panel.title, panel.id));
                 }
             }
         }
@@ -575,3 +639,48 @@ pub fn validate_dashboard_datasets(
     }
     Ok(())
 }
+
+#[cfg(test)]
+mod authoring_tests {
+    use super::*;
+
+    fn query() -> ReportDatasetDefinition {
+        ReportDatasetDefinition {
+            title: "Sales".into(), sql: "SELECT 1 AS total".into(), max_rows: 100,
+            columns: vec![ReportDatasetColumn { key: "total".into(), label: "Total".into(),
+                data_type: ReportDataType::Integer.into(), number_format: ReportNumberFormat::Number.into(), ..Default::default() }],
+            ..Default::default()
+        }
+    }
+
+    #[test]
+    fn unfinished_drafts_save_but_cannot_publish_and_unknown_types_are_rejected() {
+        let mut dataset = query();
+        dataset.sql.clear();
+        let definition = |dataset| ReportDefinition { schema_version: REPORT_SCHEMA_VERSION,
+            content: Some(report_definition::Content::Dataset(dataset)) };
+        assert!(validate_draft(&definition(dataset.clone())).is_ok());
+        assert!(validate_definition(&definition(dataset.clone())).is_err());
+        dataset.columns[0].data_type = 999;
+        assert!(validate_draft(&definition(dataset)).is_err());
+    }
+
+    #[test]
+    fn embedded_queries_require_unique_keys_and_cannot_have_external_version_pins() {
+        let mut dashboard = ReportDashboardDefinition {
+            title: "Sales".into(), queries: vec![ReportQuery { key: "sales".into(), definition: Some(query()) }],
+            panels: vec![ReportPanel { id: "total".into(), title: "Sales".into(), dataset_id: "sales".into(),
+                kind: ReportPanelKind::Table.into(), orientation: ReportOrientation::Vertical.into(),
+                sort_order: ReportSortOrder::Source.into(), null_policy: ReportNullPolicy::Gap.into(), width: 6, height: 360,
+                ..Default::default() }], ..Default::default()
+        };
+        let definition = |dashboard| ReportDefinition { schema_version: REPORT_SCHEMA_VERSION,
+            content: Some(report_definition::Content::Dashboard(dashboard)) };
+        assert!(validate_definition(&definition(dashboard.clone())).is_ok());
+        dashboard.queries.push(dashboard.queries[0].clone());
+        assert!(validate_definition(&definition(dashboard.clone())).is_err());
+        dashboard.queries.pop();
+        dashboard.dataset_versions.push(ReportDatasetVersionRef { dataset_id: "sales".into(), version: 1 });
+        assert!(validate_definition(&definition(dashboard)).is_err());
+    }
+}