Skip to content
This repository has been archived by the owner on Oct 14, 2024. It is now read-only.

Fix Numeric strings being converted to Datetime objects #348

Merged
merged 11 commits into from
Aug 23, 2024

Conversation

shemogumbe
Copy link
Contributor

Fixes microsoftgraph/msgraph-sdk-python#653

Overview

The PR fixes a problem where pendulum assumes all 4-digit numeric strings are Datetimes, e. 1212, 1214, 2530, 9057

Fixes # microsoftgraph/msgraph-sdk-python#653

Demo

The Script below,

async def get_my_messages():
    try:

        result = await user_client.sites.by_site_id(
            "yygbp.sharepoint.com,ab839da2-a22d-4e5c-bbeb-5060fad36e50,5cfc6d41-e5f6-4341-93fb-98728649efde"
        ).lists.by_list_id('6c8b411a-e440-4b24-9ee3-bfd3f06a9b4e'
                           ).items.by_list_item_id('1').get()
        print(f"sharepont list item id {result.id}")
        print(f"sharepoint list item fields {result.fields}")
        print(
            f"sharepoint list item text field {result.fields.additional_data['Doc_id']}"
        )

Output before

sharepoint list item fields FieldValueSet(additional_data={'@odata.etag': '"0f11e7ca-3f0e-46ce-b7e7-f2dfffe373da,2"', 'Title': 'Composition', 'LinkTitle': 'Composition', 'Doc_id': DateTime(1212, 1, 1, 0, 0, 0, tzinfo=Timezone('UTC')), 'Doc_numner': 1212.0, 'ContentType': 'Item', 'Modified': DateTime(2024, 8, 23, 14, 54, 17, tzinfo=Timezone('UTC')), 'Created': DateTime(2024, 8, 23, 14, 54, 17, tzinfo=Timezone('UTC')), 'AuthorLookupId': '9', 'EditorLookupId': '9', '_UIVersionString': '2.0', 'Attachments': True, 'Edit': '', 'LinkTitleNoMenu': 'Composition', 'ItemChildCount': '0', 'FolderChildCount': '0', '_ComplianceFlags': '', '_ComplianceTag': '', '_ComplianceTagWrittenTime': '', '_ComplianceTagUserId': ''}, id='1', odata_type=None)
sharepoint list item text field 1212-01-01 00:00:00+00:00

Output After

sharepoint list item fields FieldValueSet(additional_data={'@odata.etag': '"0f11e7ca-3f0e-46ce-b7e7-f2dfffe373da,2"', 'Title': 'Composition', 'LinkTitle': 'Composition', 'Doc_id': '1212', 'Doc_numner': 1212.0, 'ContentType': 'Item', 'Modified': DateTime(2024, 8, 23, 14, 54, 17, tzinfo=Timezone('UTC')), 'Created': DateTime(2024, 8, 23, 14, 54, 17, tzinfo=Timezone('UTC')), 'AuthorLookupId': '9', 'EditorLookupId': '9', '_UIVersionString': '2.0', 'Attachments': True, 'Edit': '', 'LinkTitleNoMenu': 'Composition', 'ItemChildCount': '0', 'FolderChildCount': '0', '_ComplianceFlags': '', '_ComplianceTag': '', '_ComplianceTagWrittenTime': '', '_ComplianceTagUserId': ''}, id='1', odata_type=None)
sharepoint list item text field 1212

Notes

This change could be reverted if pendulum does an update that fixes the bug.

Testing Instructions

@shemogumbe shemogumbe marked this pull request as ready for review August 23, 2024 17:08
@shemogumbe shemogumbe requested a review from a team as a code owner August 23, 2024 17:08
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!
Also:

  • unit tests
  • changelog
  • version bump
  • replicate in other serialization libraries

@@ -291,6 +293,10 @@ def _assign_field_values(self, item: U) -> None:
deserialize but the model doesn't support additional data"
)

def is_four_digit_number(self, value: str) -> bool:
Copy link
Member

Choose a reason for hiding this comment

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

just making sure this method is not accessible from outside of the class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's an instance method, and we really just need it here in the entire project, so nope, not accessible outside the class

Copy link
Member

Choose a reason for hiding this comment

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

shouldn't it start with __ ?

kiota_serialization_json/json_parse_node.py Outdated Show resolved Hide resolved
kiota_serialization_json/json_parse_node.py Show resolved Hide resolved
baywet
baywet previously approved these changes Aug 23, 2024
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

Thank you for making the changes!

kiota_serialization_json/_version.py Outdated Show resolved Hide resolved
@shemogumbe
Copy link
Contributor Author

Thanks for the contribution! Also:

  • unit tests
  • changelog
  • version bump
  • replicate in other serialization libraries

Added Unit tests, changelog and version bump

The other serialization libraries do not use pendulum thus do not have the problem

Copy link

@shemogumbe shemogumbe requested a review from baywet August 23, 2024 17:48
@baywet baywet merged commit 98c8b17 into main Aug 23, 2024
11 checks passed
@baywet baywet deleted the shem/fix_pendulum_conversion_problem branch August 23, 2024 18:39
@baywet baywet mentioned this pull request Aug 23, 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.

SharePoint Custom Fields (Single line of text) - Gets cast to Python Pendulam DateTime
2 participants