rogers-obrien-rad / ProPyCore

Interact with Procore through Python for Data Connection Applications (no user authentication)
MIT License
3 stars 0 forks source link

[BUG] Some `id` values for budget columns are strings #66

Open HagenFritz opened 2 weeks ago

HagenFritz commented 2 weeks ago

Current Behavior

Looking at the response from budget.columns.get() function, there are columns with id values of both strings and integers.

[
    {
        "id": "budget_code",
        "name": "Budget Code",
        "groupable": false,
        "aggregatable": false,
        "type": "standard",
        "filterable": false,
        "position": -5
    },
    {
        "id": "3141780",
        "name": "Commtiment Subcontracts",
        "aggregatable": true,
        "groupable": false,
        "position": 4,
        "type": "source",
        "filterable": false
    }
]

This means that the budget.columns.find() function won't work as expected since we assumed all id would be integers:

def find(self, company_id, project_id, budget_view_id, identifier):
        """
        Finds specified budget view column by looping through the results from get_budget_columns()

        Parameters
        ----------
        company_id : int
            unique identifier for the company
        project_id : int
            unique identifier for the project
        budget_view_id : int
            unique identifier for the budget view
        identifier : int or str
            identifier for view which can be an id (int) or name (str)

        Returns
        -------
        column : dict
            column data

        Raises
        ------
        NotFoundItemError
        """
        if isinstance(identifier, int):
            key = "id"
        else:
            key = "name"

        for column in self.get_budget_columns(company_id=company_id, project_id=project_id, budget_view_id=budget_view_id):
            if column[key] == identifier:
                return column

        raise NotFoundItemError(f"Could not find column {identifier}")

Expected behavior

So we need to update the function so that we can use strings with id. Perhaps this is some flag instead or we try with all entries on the id first and then search by string.

Additional context

This discovery might have ramifications across multiple resources and not just columns which means updating everything. If we do end up including a flag input, then, for consistencies-sake, we would need to update all find() methods.

HagenFritz commented 2 weeks ago

I was in error; the id field is always a string value, even if it is an integer. It is really a string representation of an integer.