Skip to content

Conversation

@glacorSoul
Copy link
Contributor

@glacorSoul glacorSoul commented Apr 17, 2025

Description

Gridify should allow intermediate null values as per #268

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or that my feature works
  • I have made corresponding changes to the documentation
  • I have commented my code, particularly in hard-to-understand areas
  • New and existing unit tests pass locally with my changes

@what-the-diff
Copy link

what-the-diff bot commented Apr 17, 2025

PR Summary

  • Introduced BaseQueryBuilder for improved null propagation handling
    A new static class, BaseQueryBuilder, has been added which improves how the system deals with null properties. It's a significant improvement in managing data that may not always have a value.

  • Refactored methods for better management of binary expressions
    The BuildQuery method has been divided to include a new method, BuildQueryFromBinaryExpression. This new method specifically manages binary expressions, making the system more efficient in processing these kinds of data structures.

  • Added functionality to handle null object references in expressions
    A newly introduced function, AddNullPropagator, enhances the query building process by effectively handling situations where a reference in an expression does not have a value.

  • Enhanced configurations to control handling of null references
    Changes have been made to GridifyGlobalConfiguration and GridifyMapperConfiguration classes. A new feature AvoidNullReference has been added. This allows us to customize how the system should react when it encounters data references that don't have a value.

  • Introduced MemberNullPropagatorVisitor class for better null propagation management
    A new class MemberNullPropagatorVisitor has been created. It extends the ExpressionVisitor and handles null propagation in member accesses and method calls. It's designed to effectively manage certain data interactions particularly when null properties are involved.

  • Added unit tests to ensure enhancements work as expected
    To validate the improvements, new unit tests have been added under Issue286Tests. Through these, we ensure that the system can accommodate null properties during the query filtering process.

@glacorSoul glacorSoul changed the title Implements #286 Implements #268 Apr 18, 2025
@glacorSoul glacorSoul changed the title Implements #268 #268 Gridify should allow intermediate vallues Apr 18, 2025
@glacorSoul glacorSoul changed the title #268 Gridify should allow intermediate vallues #268 Gridify should allow intermediate null vallues Apr 18, 2025
@alirezanet alirezanet self-requested a review April 20, 2025 13:18
Copy link
Owner

@alirezanet alirezanet left a comment

Choose a reason for hiding this comment

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

Overall great improvements 💐👌,
There are some discussion points about the configuration settings and a few minor changes required.

also please don't forget documentation if you add a new feature.

Copy link
Owner

@alirezanet alirezanet left a comment

Choose a reason for hiding this comment

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

Awesome job, we're almost there.

I think two more changes is needed:

  • One mistake in the default value (it should be false)
  • Just to make sure it doesn't break the Gridify.EntityFramework integration, it would be nice if you could add two more test cases with database integration: one with AvoidNullReference = false and one with AvoidNullReference = true. Both tests should behave as expected.
    (I suggest to add it to the EntityFrameworkPostgreSqlIntegrationTests project)

Copy link
Owner

@alirezanet alirezanet left a comment

Choose a reason for hiding this comment

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

Awesome thanks 💐

@alirezanet alirezanet added the manual test needs manual testing for confirmation label May 26, 2025
@alirezanet alirezanet merged commit 33f4aec into alirezanet:master Oct 12, 2025
3 checks passed
@alirezanet alirezanet removed the manual test needs manual testing for confirmation label Oct 12, 2025
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