-
Notifications
You must be signed in to change notification settings - Fork 0
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
Added generator in sdk and modified the calls in api #165
Changes from all commits
3c6c4c9
f71fe98
2259d90
050e6d5
8206449
c247f61
b633f30
fc66daa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
from django.utils.module_loading import import_string | ||
from fyle_accounting_mappings.models import DestinationAttribute | ||
from apps.workspaces.models import Sage300Credential | ||
from sage_desktop_sdk.sage_desktop_sdk import SageDesktopSDK | ||
|
@@ -66,7 +67,39 @@ def _update_latest_version(self, attribute_type: str): | |
|
||
return [] | ||
|
||
def _sync_data(self, data, attribute_type, display_name, workspace_id, field_names): | ||
def _add_to_destination_attributes(self, item, attribute_type, display_name, field_names): | ||
|
||
detail = {field: getattr(item, field) for field in field_names} | ||
if item.name: | ||
return self._create_destination_attribute( | ||
attribute_type, | ||
display_name, | ||
" ".join(item.name.split()), | ||
item.id, | ||
item.is_active, | ||
detail | ||
) | ||
|
||
def _get_attribute_class(self, attribute_type: str): | ||
""" | ||
Get the attribute class for the attribute type | ||
:param attribute_type: Type of the attribute | ||
:return: Attribute class | ||
""" | ||
ATTRIBUTE_CLASS_MAP = { | ||
'ACCOUNT': 'Account', | ||
'VENDOR': 'Vendor', | ||
'JOB': 'Job', | ||
'STANDARD_COST_CODE': 'StandardCostCode', | ||
'STANDARD_CATEGORY': 'StandardCategory', | ||
'COMMITMENT': 'Commitment', | ||
'COMMITMENT_ITEM': 'CommitmentItem', | ||
'COST_CODE': 'CostCode' | ||
} | ||
|
||
return ATTRIBUTE_CLASS_MAP[attribute_type] | ||
|
||
def _sync_data(self, data_gen, attribute_type, display_name, workspace_id, field_names, is_generator: bool = True): | ||
""" | ||
Synchronize data from Sage Desktop SDK to your application | ||
:param data: Data to synchronize | ||
|
@@ -76,21 +109,28 @@ def _sync_data(self, data, attribute_type, display_name, workspace_id, field_nam | |
:param field_names: Names of fields to include in detail | ||
""" | ||
|
||
destination_attributes = [] | ||
for item in data: | ||
detail = {field: getattr(item, field) for field in field_names} | ||
if item.name: | ||
destination_attributes.append(self._create_destination_attribute( | ||
attribute_type, | ||
display_name, | ||
" ".join(item.name.split()), | ||
item.id, | ||
item.is_active, | ||
detail | ||
)) | ||
|
||
DestinationAttribute.bulk_create_or_update_destination_attributes( | ||
destination_attributes, attribute_type, workspace_id, True) | ||
if is_generator: | ||
for data in data_gen: | ||
for items in data: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comment as above |
||
destination_attributes = [] | ||
for _item in items: | ||
attribute_class = self._get_attribute_class(attribute_type) | ||
item = import_string(f'sage_desktop_sdk.core.schema.read_only.{attribute_class}').from_dict(_item) | ||
destination_attr = self._add_to_destination_attributes(item, attribute_type, display_name, field_names) | ||
if destination_attr: | ||
destination_attributes.append(destination_attr) | ||
|
||
DestinationAttribute.bulk_create_or_update_destination_attributes( | ||
destination_attributes, attribute_type, workspace_id, True) | ||
else: | ||
destination_attributes = [] | ||
for item in data_gen: | ||
destination_attr = self._add_to_destination_attributes(item, attribute_type, display_name, field_names) | ||
if destination_attr: | ||
destination_attributes.append(destination_attr) | ||
|
||
DestinationAttribute.bulk_create_or_update_destination_attributes( | ||
destination_attributes, attribute_type, workspace_id, True) | ||
|
||
self._update_latest_version(attribute_type) | ||
|
||
|
@@ -177,7 +217,7 @@ def sync_commitment_items(self): | |
'code', 'version', 'description', 'cost_code_id', | ||
'category_id', 'created_on_utc', 'job_id', 'commitment_id' | ||
] | ||
self._sync_data(commitment_items, 'COMMITMENT_ITEM', 'commitment_item', self.workspace_id, field_names) | ||
self._sync_data(commitment_items, 'COMMITMENT_ITEM', 'commitment_item', self.workspace_id, field_names, False) | ||
|
||
def sync_cost_codes(self): | ||
""" | ||
|
@@ -193,6 +233,8 @@ def sync_cost_categories(self): | |
""" | ||
Synchronize categories from Sage Desktop SDK to your application | ||
""" | ||
cost_categories = self.connection.categories.get_all_categories() | ||
cost_categories_generator = self.connection.categories.get_all_categories() | ||
|
||
CostCategory.bulk_create_or_update(cost_categories, self.workspace_id) | ||
for cost_categories in cost_categories_generator: | ||
for categories in cost_categories: | ||
CostCategory.bulk_create_or_update(categories, self.workspace_id) |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,7 +2,6 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Sage Desktop Categories | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from sage_desktop_sdk.core.client import Client | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from sage_desktop_sdk.core.schema.read_only import Account | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class Accounts(Client): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -18,14 +17,12 @@ def get_all(self, version: int = None): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
:return: A generator yielding accounts in the Attachable Schema | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
:rtype: generator of Account objects | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = Accounts.GET_ACCOUNTS + '?page={0}' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if version: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Append the version query parameter if provided | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
query_params = '?version={0}'.format(version) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = Accounts.GET_ACCOUNTS + query_params | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = Accounts.GET_ACCOUNTS | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
query_params = f'&version={version}' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint += query_params | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Query the API to get all accounts | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
accounts = self._query_get_all(endpoint) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for account in accounts: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
yield Account.from_dict(account) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
accounts = self._query_get_all_generator(endpoint, is_paginated=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
yield accounts | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+20
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure consistent use of string formatting for query parameters. - endpoint = Accounts.GET_ACCOUNTS + '?page={0}'
+ endpoint = Accounts.GET_ACCOUNTS + f'?page={0}' This change ensures that the string formatting is consistent with the rest of the code, which uses f-string formatting. Committable suggestion
Suggested change
Comment on lines
+27
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yielding a generator object directly may not be the intended behavior. - yield accounts
+ for account in accounts:
+ yield account Directly yielding the generator object from Committable suggestion
Suggested change
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,7 +2,6 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Sage Desktop Categories | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from sage_desktop_sdk.core.client import Client | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from sage_desktop_sdk.core.schema.read_only import Category | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class Categories(Client): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -20,16 +19,12 @@ def get_all_categories(self, version: int = None): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
:return: A generator yielding job categories in the Jobs Schema | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
:rtype: generator of Category objects | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = Categories.GET_CATEGORIES + '?page={0}' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if version: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Append the version query parameter if provided | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
query_params = '?version={0}'.format(version) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = Categories.GET_CATEGORIES + query_params | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = Categories.GET_CATEGORIES | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
query_params = f'&version={version}' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint += query_params | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Query the API to get all job categories | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
categories = self._query_get_all(endpoint) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for category in categories: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Convert each category dictionary to a Category object and yield it | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
yield Category.from_dict(category) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
categories = self._query_get_all_generator(endpoint, is_paginated=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
yield categories | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+22
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure consistent use of string formatting for query parameters. - endpoint = Categories.GET_CATEGORIES + '?page={0}'
+ endpoint = Categories.GET_CATEGORIES + f'?page={0}' This change ensures that the string formatting is consistent with the rest of the code, which uses f-string formatting. Committable suggestion
Suggested change
Comment on lines
+29
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yielding a generator object directly may not be the intended behavior. - yield categories
+ for category in categories:
+ yield category Directly yielding the generator object from Committable suggestion
Suggested change
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -3,7 +3,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Sage Desktop Commitments | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from sage_desktop_sdk.core.client import Client | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from sage_desktop_sdk.core.schema.read_only import Commitment, CommitmentItem | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from sage_desktop_sdk.core.schema.read_only import CommitmentItem | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class Commitments(Client): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -22,19 +22,15 @@ def get_all(self, version: int = None): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
:return: A generator yielding commitments in the Commitments Schema | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
:rtype: generator of Commitment objects | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = Commitments.GET_COMMITMENTS + '?page={0}' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if version: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Append the version query parameter if provided | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
query_params = '?version={0}'.format(version) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = Commitments.GET_COMMITMENTS + query_params | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = Commitments.GET_COMMITMENTS | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
query_params = f'&version={version}' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint += query_params | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Query the API to get all commitments | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
commitments = self._query_get_all(endpoint) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for commitment in commitments: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Convert each commitment dictionary to a Commitment object and yield it | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
yield Commitment.from_dict(commitment) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
commitments = self._query_get_all_generator(endpoint) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
yield commitments | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+25
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure consistent use of string formatting for query parameters. - endpoint = Commitments.GET_COMMITMENTS + '?page={0}'
+ endpoint = Commitments.GET_COMMITMENTS + f'?page={0}' This change ensures that the string formatting is consistent with the rest of the code, which uses f-string formatting. Committable suggestion
Suggested change
Comment on lines
+32
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yielding a generator object directly may not be the intended behavior. - yield commitments
+ for commitment in commitments:
+ yield commitment Directly yielding the generator object from Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def get_commitment_items(self, commitment_id: str, version: int = None): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -58,4 +54,4 @@ def get_commitment_items(self, commitment_id: str, version: int = None): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
commitment_items = self._query_get_by_id(endpoint) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for commitment_item in commitment_items: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
yield CommitmentItem.from_dict(commitment_item) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
yield CommitmentItem.from_dict(commitment_item) |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,7 +2,6 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Sage Desktop Cost Codes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from sage_desktop_sdk.core.client import Client | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from sage_desktop_sdk.core.schema.read_only import CostCode | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class CostCodes(Client): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -20,16 +19,12 @@ def get_all_costcodes(self, version: int = None): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
:return: A generator yielding cost codes in the Cost Code Schema | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
:rtype: generator of CostCode objects | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = CostCodes.GET_COST_CODE + '?page={0}' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if version: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Append the version query parameter if provided | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
query_params = '?version={0}'.format(version) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = CostCodes.GET_COST_CODE + query_params | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = CostCodes.GET_COST_CODE | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
query_params = f'&version={version}' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint += query_params | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Query the API to get all cost codes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
cost_codes = self._query_get_all(endpoint) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for cost_code in cost_codes: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Convert each cost code dictionary to a CostCode object and yield it | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
yield CostCode.from_dict(cost_code) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
cost_codes = self._query_get_all_generator(endpoint, is_paginated=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
yield cost_codes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+22
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure consistent use of string formatting for query parameters. - endpoint = CostCodes.GET_COST_CODE + '?page={0}'
+ endpoint = CostCodes.GET_COST_CODE + f'?page={0}' This change ensures that the string formatting is consistent with the rest of the code, which uses f-string formatting. Committable suggestion
Suggested change
Comment on lines
+29
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yielding a generator object directly may not be the intended behavior. - yield cost_codes
+ for cost_code in cost_codes:
+ yield cost_code Directly yielding the generator object from Committable suggestion
Suggested change
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,7 +2,6 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Sage Desktop Jobs | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from sage_desktop_sdk.core.client import Client | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from sage_desktop_sdk.core.schema.read_only import Job, StandardCategory, StandardCostCode | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class Jobs(Client): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -22,19 +21,15 @@ def get_all_jobs(self, version: int = None): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
:return: A generator yielding jobs in the Jobs Schema | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
:rtype: generator of Job objects | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = Jobs.GET_JOBS + '?page={0}' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if version: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Append the version query parameter if provided | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
query_params = '?version={0}'.format(version) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = Jobs.GET_JOBS + query_params | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = Jobs.GET_JOBS | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
query_params = f'&version={version}' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint += query_params | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Query the API to get all jobs | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
jobs = self._query_get_all(endpoint) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for job in jobs: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Convert each job dictionary to a Job object and yield it | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
yield Job.from_dict(job) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
jobs = self._query_get_all_generator(endpoint, is_paginated=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
yield jobs | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+24
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure consistent use of string formatting for query parameters. - endpoint = Jobs.GET_JOBS + '?page={0}'
+ endpoint = Jobs.GET_JOBS + f'?page={0}' This change ensures that the string formatting is consistent with the rest of the code, which uses f-string formatting. Committable suggestion
Suggested change
Comment on lines
+31
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yielding a generator object directly may not be the intended behavior. - yield jobs
+ for job in jobs:
+ yield job Directly yielding the generator object from Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def get_standard_costcodes(self, version: int = None): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -46,19 +41,15 @@ def get_standard_costcodes(self, version: int = None): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
:return: A generator yielding standard cost codes in the Cost Code Schema | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
:rtype: generator of StandardCostCode objects | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = Jobs.GET_COST_CODES + '?page={0}' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if version: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Append the version query parameter if provided | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
query_params = '?version={0}'.format(version) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = Jobs.GET_COST_CODES + query_params | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = Jobs.GET_COST_CODES | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Query the API to get all standard cost codes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
costcodes = self._query_get_all(endpoint) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
query_params = f'&version={version}' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint += query_params | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for costcode in costcodes: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Convert each cost code dictionary to a StandardCostCode object and yield it | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
yield StandardCostCode.from_dict(costcode) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Query the API to get all jobs | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
cost_codes = self._query_get_all_generator(endpoint, is_paginated=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
yield cost_codes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+44
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure consistent use of string formatting for query parameters. - endpoint = Jobs.GET_COST_CODES + '?page={0}'
+ endpoint = Jobs.GET_COST_CODES + f'?page={0}' This change ensures that the string formatting is consistent with the rest of the code, which uses f-string formatting. Committable suggestion
Suggested change
Comment on lines
+51
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yielding a generator object directly may not be the intended behavior. - yield cost_codes
+ for cost_code in cost_codes:
+ yield cost_code Directly yielding the generator object from Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def get_standard_categories(self, version: int = None): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -70,16 +61,12 @@ def get_standard_categories(self, version: int = None): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
:return: A generator yielding standard categories in the Categories Schema | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
:rtype: generator of StandardCategory objects | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = Jobs.GET_CATEGORIES + '?page={0}' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if version: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Append the version query parameter if provided | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
query_params = '?version={0}'.format(version) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = Jobs.GET_CATEGORIES + query_params | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint = Jobs.GET_CATEGORIES | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
query_params = f'&version={version}' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint += query_params | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Query the API to get all standard categories | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
categories = self._query_get_all(endpoint) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for category in categories: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Convert each category dictionary to a StandardCategory object and yield it | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
yield StandardCategory.from_dict(category) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Query the API to get all jobs | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
categories = self._query_get_all_generator(endpoint, is_paginated=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
yield categories | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+64
to
+72
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure consistent use of string formatting for query parameters. - endpoint = Jobs.GET_CATEGORIES + '?page={0}'
+ endpoint = Jobs.GET_CATEGORIES + f'?page={0}' This change ensures that the string formatting is consistent with the rest of the code, which uses f-string formatting. Committable suggestion
Suggested change
Comment on lines
+71
to
+72
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yielding a generator object directly may not be the intended behavior. - yield categories
+ for category in categories:
+ yield category Directly yielding the generator object from Committable suggestion
Suggested change
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method
_get_attribute_class
uses a hard-coded mapping which could be externalized to a configuration file or database for easier maintenance.Consider externalizing the
ATTRIBUTE_CLASS_MAP
to a configuration file or database to facilitate easier updates and maintenance without needing to modify the code.