Skip to content

Commit 51d8fcd

Browse files
chore: add null checks (#1355)
A previous PR modified the conditions config to make the value field nullable. But the frontend is sometimes sending an empty string in case of null. This PR performs both the checks and rejects the request in case the operator is is null or is not null and the value is something other than null or an empty string. Co-authored-by: Nikhil Sinha <[email protected]>
1 parent 61697ea commit 51d8fcd

File tree

2 files changed

+30
-8
lines changed

2 files changed

+30
-8
lines changed

src/alerts/alerts_utils.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -347,15 +347,21 @@ pub fn get_filter_string(where_clause: &Conditions) -> Result<String, String> {
347347
LogicalOperator::And => {
348348
let mut exprs = vec![];
349349
for condition in &where_clause.condition_config {
350-
if let Some(value) = &condition.value {
350+
if condition
351+
.value
352+
.as_ref()
353+
.is_some_and(|v| !v.trim().is_empty())
354+
{
351355
// ad-hoc error check in case value is some and operator is either `is null` or `is not null`
352356
if condition.operator.eq(&WhereConfigOperator::IsNull)
353357
|| condition.operator.eq(&WhereConfigOperator::IsNotNull)
354358
{
355359
return Err("value must be null when operator is either `is null` or `is not null`"
356360
.into());
357361
}
358-
let value = NumberOrString::from_string(value.to_owned());
362+
let value = NumberOrString::from_string(
363+
condition.value.as_ref().unwrap().to_owned(),
364+
);
359365
match value {
360366
NumberOrString::Number(val) => exprs.push(format!(
361367
"\"{}\" {} {}",
@@ -387,7 +393,8 @@ fn match_alert_operator(expr: &ConditionConfig) -> Expr {
387393
// the form accepts value as a string
388394
// if it can be parsed as a number, then parse it
389395
// else keep it as a string
390-
if let Some(value) = &expr.value {
396+
if expr.value.as_ref().is_some_and(|v| !v.trim().is_empty()) {
397+
let value = expr.value.as_ref().unwrap();
391398
let value = NumberOrString::from_string(value.clone());
392399

393400
// for maintaining column case

src/alerts/mod.rs

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -343,14 +343,24 @@ impl Conditions {
343343
LogicalOperator::And | LogicalOperator::Or => {
344344
let expr1 = &self.condition_config[0];
345345
let expr2 = &self.condition_config[1];
346-
let expr1_msg = if let Some(val) = &expr1.value {
347-
format!("{} {} {}", expr1.column, expr1.operator, val)
346+
let expr1_msg = if expr1.value.as_ref().is_some_and(|v| !v.trim().is_empty()) {
347+
format!(
348+
"{} {} {}",
349+
expr1.column,
350+
expr1.operator,
351+
expr1.value.as_ref().unwrap()
352+
)
348353
} else {
349354
format!("{} {}", expr1.column, expr1.operator)
350355
};
351356

352-
let expr2_msg = if let Some(val) = &expr2.value {
353-
format!("{} {} {}", expr2.column, expr2.operator, val)
357+
let expr2_msg = if expr2.value.as_ref().is_some_and(|v| !v.trim().is_empty()) {
358+
format!(
359+
"{} {} {}",
360+
expr2.column,
361+
expr2.operator,
362+
expr2.value.as_ref().unwrap()
363+
)
354364
} else {
355365
format!("{} {}", expr2.column, expr2.operator)
356366
};
@@ -661,7 +671,12 @@ impl AlertConfig {
661671
WhereConfigOperator::IsNull | WhereConfigOperator::IsNotNull
662672
);
663673

664-
if needs_no_value && condition.value.is_some() {
674+
if needs_no_value
675+
&& condition
676+
.value
677+
.as_ref()
678+
.is_some_and(|v| !v.trim().is_empty())
679+
{
665680
return Err(AlertError::CustomError(
666681
"value must be null when operator is either `is null` or `is not null`"
667682
.into(),

0 commit comments

Comments
 (0)