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.

Sign up for GitHub

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

feat: Get active counts of attribute #92

Merged
merged 3 commits into from
Oct 1, 2024
Merged

feat: Get active counts of attribute #92

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c1f7f2b
feat: Get active counts of attribute
Ashutosh619-sudo Sep 27, 2024
d1ef442
remove loggers
Ashutosh619-sudo Sep 27, 2024
b26230b
version bump up
Ashutosh619-sudo Oct 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions sageintacctsdk/apis/api_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ def __post_request(self, dict_body: dict, api_url: str):

if parsed_response['response']['control']['status'] == 'failure':
exception_msg = self.__decode_support_id(parsed_response['response']['errormessage'])

raise WrongParamsError('Some of the parameters are wrong', exception_msg)

if api_response['authentication']['status'] == 'failure':
Expand Down Expand Up @@ -358,6 +359,9 @@ def count(self):
'select': {
'field': 'RECORDNO'
},
'filter': {
'equalto': {'field': 'STATUS', 'value': 'active'}
},
Comment on lines +362 to +364
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider the impact of filtering only active records

The addition of a filter to only count active records is a significant change in the behavior of the count method. While this may be the desired functionality, it has several implications:

  1. It changes the expected behavior of the method without updating the method signature or docstring.
  2. Existing code that relies on this method to get a total count of all records (including inactive ones) may now produce incorrect results.

Consider the following improvements:

  1. Update the method name to reflect its new behavior (e.g., count_active).
  2. Add a parameter to make the active status filter optional, allowing users to choose whether they want to count all records or only active ones.
  3. Update the method's docstring to clearly explain the behavior and any parameters.

Example implementation:

def count(self, active_only: bool = True):
    """
    Count the number of records.
    
    :param active_only: If True, only count active records. If False, count all records. Default is True.
    :return: The count of records.
    """
    get_count = {
        'query': {
            'object': self.__dimension,
            'select': {
                'field': 'RECORDNO'
            },
            'pagesize': '1'
        }
    }
    
    if active_only:
        get_count['query']['filter'] = {
            'equalto': {'field': 'STATUS', 'value': 'active'}
        }

    response = self.format_and_send_request(get_count)
    return int(response['data']['@totalcount'])

This approach maintains backwards compatibility while allowing for the new functionality.

'pagesize': '1'
}
}
Expand Down