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 type/objectType for lists of ObjectId/Decimal128 #3235

Merged
merged 3 commits into from
Sep 21, 2020

Conversation

steffenagger
Copy link
Contributor

Issue:
Lists of ObjectId or Decimal128 still returns 'object id' & 'decimal' instead of 'objectId' & 'decimal128', when checking type/objectType.

Solution:
A util method local_string_for_property_type has been introduced, wrapping OS string_for_property_type - this is now used instead of string_for_property_type.
Tests expanded to include checks for objectType.

@steffenagger steffenagger self-assigned this Sep 18, 2020
@@ -145,7 +145,7 @@ static inline void parse_property_type(StringData object_name, Property& prop, S
prop.object_type = "";
}
else if (prop.object_type == "objectId") {
prop.type |= PropertyType::Decimal | PropertyType::Array;
prop.type |= PropertyType::ObjectId | PropertyType::Array;
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch!

@kneth
Copy link
Contributor

kneth commented Sep 21, 2020

Please add an entry in CHANGELOG.md: fixed a bug where .type is incorrect for some property types.

@steffenagger steffenagger mentioned this pull request Sep 21, 2020
22 tasks
@steffenagger steffenagger merged commit 54018c7 into v10 Sep 21, 2020
@steffenagger steffenagger deleted the sa/v10/local-type-naming branch September 21, 2020 11:39
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants