Skip to content

Conversation

@ArthurFreeb
Copy link

This pull request improves property mapping logic and test coverage for user ID-based queries, and refactors test data initialization for consistency and maintainability. The main changes include enhanced handling of property aliases ending with "Id" in mapping, the addition of user IDs to test data, and new test cases for querying by user ID.

Property mapping improvements

  • Updated TryGetProperty in PropertyMappingTree.cs to recognize property aliases ending with "Id" (e.g., mapping UserId to the actual Id property), improving support for common query patterns involving IDs.

Test data refactoring and enhancements

  • Refactored TestData.cs to define a shared Manager01 user instance, ensuring consistent manager data across test users and reducing code duplication. [1] [2] [3]
  • Added unique Id values to Jane and Harry users in test data to support ID-based queries in tests. [1] [2]

Test coverage improvements

  • Added a new test case in FilterTest.cs for querying users by userid or id, verifying correct mapping and retrieval based on user IDs.

Minor formatting fix

  • Fixed formatting for the IsEmailVerified property in a nested manager object in TestData.cs.

@ArthurFreeb
Copy link
Author

I added a narrow Id alias that resolves to the actual Id property on the current mapping. A few clarifications I can add:
• Confirm whether nested aliases like managerid are in scope; currently this only applies to the immediate SourceType, which keeps ambiguity low.
• Add a negative test for invalid GUID on userid to document parse/type behavior.
• Add a test showing the alias still resolves when Id has a JsonPropertyName override, since resolution keys off ActualPropertyName == "Id".

@Jamess-Lucass
Copy link
Member

Going to close this PR as UserId is not an alias for the Id field on the user. It's a separate GUID property that should be treated as such. It's unrelated to the Id property.

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