Skip to content

fix: added mandatory check in the delete handler #900

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sauraww wants to merge 1 commit into
base: main
Choose a base branch
from
+38 −3
Open

fix: added mandatory check in the delete handler #900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions crates/context_aware_config/src/api/dimension/handlers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,19 @@ async fn delete_handler(
.schema_name(&workspace_context.schema_name)
.get_result(&mut conn)?;

let is_mandatory = workspace_context
Copy link
Collaborator

@ayushjain17 ayushjain17 Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checks are also needed in workspace handlers to verify if the given dimensions exists or not

Also, I think we should not take list of mandatory dimensions when we are creating a workspace as that cannot be validated at that time

Copy link
Collaborator Author

@sauraww sauraww Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

#[derive(Debug, Deserialize, Serialize)]
pub struct CreateWorkspaceRequest {
    pub workspace_admin_email: String,
    pub workspace_name: String,
    pub workspace_status: Option<WorkspaceStatus>,
    pub metrics: Option<Metrics>,
    #[serde(default)]
    pub allow_experiment_self_approval: bool,
    #[serde(default = "default_true")]
    pub auto_populate_control: bool,
    #[serde(default)]
    pub enable_context_validation: bool,
    #[serde(default)]
    pub enable_change_reason_validation: bool,
}

This request already doesn't have mandatory dimension. It is already take care of.
We can add the validation in the updateWorkspace api though , that is missing.

.settings
.mandatory_dimensions
.as_ref()
.map_or(false, |dims| dims.contains(&dimension_data.dimension));

if is_mandatory {
return Err(bad_argument!(
"Dimension `{}` is mandatory and cannot be deleted",
name
));
}

let context_ids = get_dimension_usage_context_ids(
&name,
&mut conn,
Expand Down
28 changes: 25 additions & 3 deletions crates/superposition/src/workspace/handlers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use superposition_types::{
custom_query::PaginationParams,
database::{
models::{Organisation, Workspace, WorkspaceStatus},
schema::config_versions::dsl as config_versions,
schema::{config_versions::dsl as config_versions, dimensions},
superposition_schema::superposition::{organisations, workspaces},
},
result as superposition,
Expand Down Expand Up @@ -182,9 +182,31 @@ async fn update_handler(
let workspace_name = workspace_name.into_inner();
let timestamp = Utc::now();
let schema_name = SchemaName(format!("{}_{}", *org_id, workspace_name));
// TODO: mandatory dimensions updation needs to be validated
// for the existance of the dimensions in the workspace
let DbConnection(mut conn) = db_conn;

if let Some(ref mandatory_dims) = request.mandatory_dimensions {
let existing_dimensions: Vec<String> = dimensions::dsl::dimensions
.select(dimensions::dimension)
.schema_name(&schema_name)
.load::<String>(&mut conn)
.map_err(|err| {
log::error!("failed to fetch dimensions: {}", err);
db_error!(err)
})?;

let missing: Vec<&String> = mandatory_dims
.iter()
.filter(|d| !existing_dimensions.contains(d))
.collect();

if !missing.is_empty() {
return Err(bad_argument!(
"The following mandatory dimensions do not exist in the workspace: {:?}",
missing
));
}
}

if let Some(I64Update::Add(version)) = request.config_version {
let _ = config_versions::config_versions
.select(config_versions::id)
Expand Down
Loading