Skip to content

fix: added mandatory check in the delete handler#900

Open
sauraww wants to merge 1 commit intomainfrom
fix/mandatory-dimension
Open

fix: added mandatory check in the delete handler#900
sauraww wants to merge 1 commit intomainfrom
fix/mandatory-dimension

Conversation

@sauraww
Copy link
Collaborator

@sauraww sauraww commented Mar 3, 2026

Problem

Describe the problem you are trying to solve here

Solution

Provide a brief summary of your solution so that reviewers can understand your code

Environment variable changes

What ENVs need to be added or changed

Pre-deployment activity

Things needed to be done before deploying this change (if any)

Post-deployment activity

Things needed to be done after deploying this change (if any)

API changes

Endpoint Method Request body Response Body
API GET/POST, etc request response

Possible Issues in the future

Describe any possible issues that could occur because of this change

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Mandatory dimensions are now protected from deletion. When users attempt to delete a dimension marked as mandatory in workspace settings, the system will block the operation and display an error message. This protection prevents accidental removal of essential configuration elements that are critical for proper workspace functionality.

@sauraww sauraww requested a review from a team as a code owner March 3, 2026 05:16
@semanticdiff-com
Copy link

semanticdiff-com bot commented Mar 3, 2026

Review changes with  SemanticDiff

Changed Files
File Status
  crates/context_aware_config/src/api/dimension/handlers.rs  0% smaller
  crates/superposition/src/workspace/handlers.rs  0% smaller

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 3, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

The delete_handler in the dimension API now includes a mandatory dimension check. If a dimension marked as mandatory is targeted for deletion, the operation is rejected with a bad argument error before proceeding with deletion logic.

Changes

Cohort / File(s) Summary
Mandatory Dimension Protection
crates/context_aware_config/src/api/dimension/handlers.rs
Added validation in delete_handler to prevent deletion of dimensions flagged as mandatory in workspace settings. Early return with error if deletion is attempted on mandatory dimension.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

A rabbit guards the sacred dimensions deep,
No mandatory deletes shall slip and creep!
With early checks and error guards so true,
The data stays intact—hooray, we're through! 🐰✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: added mandatory check in the delete handler' accurately describes the main change: adding a mandatory dimension check to prevent deletion of mandatory dimensions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/mandatory-dimension

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

.schema_name(&workspace_context.schema_name)
.get_result(&mut conn)?;

let is_mandatory = workspace_context
Copy link
Collaborator

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

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.

@sauraww sauraww force-pushed the fix/mandatory-dimension branch from c4e4d2a to a915f94 Compare March 3, 2026 10:15
@sauraww sauraww requested a review from ayushjain17 March 3, 2026 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants