Skip to content
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.

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

Adding IsNull, IsNotNull, IsEmpty, IsNotEmpty filters. #48

Merged
merged 5 commits into from
Jan 28, 2022

Conversation

mehmetuken
Copy link
Contributor

No description provided.

Copy link
Owner

@enisn enisn left a comment

Choose a reason for hiding this comment

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

Great job ✨🚀, thanks

I've just wanted some formatting changes to keep consistency in the project

Comment on lines 45 to 47
return Expression.Equal( Expression.Property(expressionBody, targetProperty.Name), Expression.Constant(null));
case OperatorType.IsNotNull:
return Expression.NotEqual( Expression.Property(expressionBody, targetProperty.Name), Expression.Constant(null));
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
return Expression.Equal( Expression.Property(expressionBody, targetProperty.Name), Expression.Constant(null));
case OperatorType.IsNotNull:
return Expression.NotEqual( Expression.Property(expressionBody, targetProperty.Name), Expression.Constant(null));
return Expression.Equal(Expression.Property(expressionBody, targetProperty.Name), Expression.Constant(null));
case OperatorType.IsNotNull:
return Expression.NotEqual(Expression.Property(expressionBody, targetProperty.Name), Expression.Constant(null));

Comment on lines 60 to 65
public static OperatorComparisonAttribute LessThanOrEqual { get; }

public static OperatorComparisonAttribute IsNull { get; }

public static OperatorComparisonAttribute IsNotNull { get; }

Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
public static OperatorComparisonAttribute LessThanOrEqual { get; }
public static OperatorComparisonAttribute IsNull { get; }
public static OperatorComparisonAttribute IsNotNull { get; }
public static OperatorComparisonAttribute LessThanOrEqual { get; }
public static OperatorComparisonAttribute IsNull { get; }
public static OperatorComparisonAttribute IsNotNull { get; }

@enisn enisn added this to the v2.11 milestone Jan 28, 2022
@enisn
Copy link
Owner

enisn commented Jan 28, 2022

Also, some of the tests are failing currently

Can you please update the following test according to the new logic?

IQueryable<Book> GetAndQuery(IQueryable<Book> query, BookFilter_OperatorFilter_TotalPage f)
{
if (f.TotalPage.Eq != null)
query = query.Where(x => x.TotalPage == f.TotalPage.Eq);
if (f.TotalPage.Not != null)
query = query.Where(x => x.TotalPage != f.TotalPage.Not);
if (f.TotalPage.Gt != null)
query = query.Where(x => x.TotalPage > f.TotalPage.Gt);
if (f.TotalPage.Gte != null)
query = query.Where(x => x.TotalPage >= f.TotalPage.Gte);
if (f.TotalPage.Lt!= null)
query = query.Where(x => x.TotalPage < f.TotalPage.Lt);
if (f.TotalPage.Lte!= null)
query = query.Where(x => x.TotalPage <= f.TotalPage.Lt);
return query;

@mehmetuken
Copy link
Contributor Author

Also, some of the tests are failing currently

Not-Nullable types getting error. Adding the type is nullable check binary expressions.

@enisn enisn added autofilterer deploy-required Use this label if deployment is required for sandbox projects feature labels Jan 28, 2022
@enisn
Copy link
Owner

enisn commented Jan 28, 2022

I've just referenced the issue which is found by CodeFactor
#49

It's ok to merge now. I'll refactor it later. Using if chains is not a good solution, but it's ok for now.

@enisn enisn merged commit ce8e5a5 into enisn:develop Jan 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autofilterer deploy-required Use this label if deployment is required for sandbox projects feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants