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

Fix for custom convertor returning null throws NRE. #220 #221

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

moxplod
Copy link
Contributor

@moxplod moxplod commented Sep 11, 2024

Description

If the custom convertor returned null. That threw an NRE.

Fixes # #220

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

Copy link

what-the-diff bot commented Sep 11, 2024

PR Summary

  • Enhancement in BaseQueryBuilder.cs
    Adjustments were made in the query builder to better handle instances when the AllowNullSearch configuration is set to null. This improvement ensures that the search function can accommodate such scenarios without causing any unexpected errors or behaviors.

  • Addition of a Test Scenario in GridifyExtensionsShould.cs
    A new test has been added to validate custom conversion circumstances. This test is designed to verify if it correctly returns a null value when the input specifically is "null". This addition is crucial to making sure our conversion process handles all edge-cases and works reliably under different conditions.

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.

LGTM 👌

@alirezanet alirezanet merged commit 8bee70a into alirezanet:master Sep 12, 2024
3 checks passed
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