smartsheet / smartsheet-python-sdk

Library that uses Python to connect to Smartsheet services (using API 2.0).
Apache License 2.0
45 stars 22 forks source link

Add check for dict in serialize function #48

Closed OhmniD closed 3 months ago

OhmniD commented 3 months ago

Description:

This pull request introduces a change to the serialize function in smartsheet/util.py to ensure that the additionalDetails property is correctly serialized when present in the response for the Event Reporting API.

Changes:

  1. Code Changes:

    • Added an elif isinstance(obj, dict) statement in the serialize function to handle dictionary objects. This ensures that properties like additionalDetails, which are dictionaries with an unknown number/type of key:value pairs, are correctly serialized and included in the output.
  2. Test Changes:

    • Updated the integration test test_list_events in tests/integration/test_events.py to include a check for the additionalDetails field. This test now verifies that when additionalDetails is present, it is correctly populated and is of type dict.

Reason for Change:

The additionalDetails property was missing when querying Event Reporting. By adding the elif isinstance(obj, dict) statement, we ensure that dictionary properties are handled properly during the serialization process, preserving the integrity of the API response.

Testing:

Sample output:

Without handling dicts during serialization:

{
    "action": "AUTHORIZE",
    "additionalDetails": {},
    "eventId": "REDACTED",
    "eventTimestamp": "2024-03-20T16:03:17+00:00Z",
    "objectId": "REDACTED",
    "objectType": "ACCESS_TOKEN",
    "requestUserId": "REDACTED",
    "source": "UNKNOWN",
    "userId": "REDACTED"
}

With change to serialize dicts:

{
    "action": "AUTHORIZE",
    "additionalDetails": {
        "tokenExpirationTimestamp": "2024-03-27T16:03:17Z",
        "emailAddress": "REDACTED",
        "firstLoginTimestamp": "2018-09-18T20:07:03Z",
        "appName": "REDACTED",
        "accessScopes": "ADMIN_USERS,CREATE_SHEETS,ADMIN_WEBHOOKS,READ_USERS,WRITE_SHEETS,ADMIN_SHEETS,READ_SHEETS",
        "tokenDisplayValue": "REDACTED",
        "appClientId": "REDACTED"
    },
    "eventId": "REDACTED",
    "eventTimestamp": "2024-03-20T16:03:17+00:00Z",
    "objectId": "REDACTED",
    "objectType": "ACCESS_TOKEN",
    "requestUserId": "REDACTED",
    "source": "UNKNOWN",
    "userId": "REDACTED"
}

Note: I believe this supersedes Salman's PR here - additionalDetails is still empty when querying when I tested with his change.

an-shaik commented 3 months ago

@dmurray-smar : Pipeline seem to be failing. Can you please have a look?