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.

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

Fix: Updating the same QBD Mapping in case of name case change #74

Merged
merged 2 commits into from
Mar 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 5 additions & 4 deletions apps/mappings/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,18 @@ def get_qbd_mapping_stat(source_type: str, workspace_id: int):
"""
get qbd mapping stat will return the count of total mappings available and unmapped mappings
"""

total_attributes_count = QBDMapping.objects.filter(
workspace_id=workspace_id,
attribute_type = source_type).count()
attribute_type = source_type
Copy link

Choose a reason for hiding this comment

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

The filtering conditions have been correctly updated to accurately count total and unmapped attributes based on attribute_type and destination_value. However, for consistency and adherence to PEP 8 style guidelines, consider adjusting the spacing around the equals sign in the filter conditions.

- attribute_type = source_type
+ attribute_type=source_type
- destination_value__isnull=True
+ destination_value__isnull=True

Also applies to: 16-16


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
attribute_type = source_type
attribute_type=source_type

).count()

unmapped_attributes_count = QBDMapping.objects.filter(
workspace_id=workspace_id,
attribute_type = source_type,
destination_value__isnull=True).count()
destination_value__isnull=True
).count()

return {
'all_attributes_count': total_attributes_count,
'unmapped_attributes_count': unmapped_attributes_count
}
}
3 changes: 3 additions & 0 deletions apps/mappings/connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ def sync_corporate_card(self):
query = {
'order': 'updated_at.desc',
}

generator = self.platform.v1beta.admin.corporate_cards.list_all(query)

for items in generator:
card_attributes = []
unique_card_numbers = []
Expand All @@ -47,5 +49,6 @@ def sync_corporate_card(self):
'value': value,
'source_id': card['id'],
})

if len(card_attributes) > 0:
QBDMapping.update_or_create_mapping_objects(card_attributes, self.workspace_id)
18 changes: 18 additions & 0 deletions apps/mappings/migrations/0002_auto_20240318_0616.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 3.1.14 on 2024-03-18 06:16

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('mappings', '0001_initial'),
]

operations = [
migrations.AlterField(
model_name='qbdmapping',
name='source_id',
field=models.CharField(help_text='Fyle ID', max_length=255, unique=True),
),
]
10 changes: 5 additions & 5 deletions apps/mappings/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class QBDMapping(models.Model):
id = models.AutoField(primary_key=True)
attribute_type = models.CharField(max_length=255, help_text='Type of expense attribute')
source_value = models.CharField(max_length=1000, help_text='Value of expense attribute')
source_id = models.CharField(max_length=255, help_text='Fyle ID')
source_id = models.CharField(max_length=255, help_text='Fyle ID', unique=True)
destination_value = models.CharField(max_length=1000,
null=True, blank=True, help_text='Value of destination attribute')
workspace = models.ForeignKey(Workspace, on_delete=models.PROTECT, help_text='Reference to Workspace model')
Expand All @@ -24,10 +24,10 @@ class Meta:
def update_or_create_mapping_objects(qbd_mapping_objects: List[Dict],workspace_id: int):
for qbd_mapping_object in qbd_mapping_objects:
QBDMapping.objects.update_or_create(
workspace_id= workspace_id,
source_value= qbd_mapping_object['value'],
attribute_type= qbd_mapping_object['attribute_type'],
workspace_id=workspace_id,
source_id=qbd_mapping_object['source_id'],
attribute_type=qbd_mapping_object['attribute_type'],
defaults={
'source_id': qbd_mapping_object['source_id'],
'source_value': qbd_mapping_object['value'],
}
)
6 changes: 6 additions & 0 deletions apps/mappings/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,15 @@ class Meta:
model = QBDMapping
fields = '__all__'
read_only_fields = ('workspace', 'created_at', 'updated_at')
extra_kwargs = {
'source_id': {
'validators': []
}
}

def create(self, validated_data):
workspace_id = self.context['request'].parser_context.get('kwargs').get('workspace_id')

Comment on lines +15 to +19
Copy link

Choose a reason for hiding this comment

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

The changes to dynamically obtain workspace_id from the request context are good. However, consider adding error handling to ensure workspace_id is always present in the context to avoid potential KeyError or NoneType errors.

qbd_mapping, _ = QBDMapping.objects.update_or_create(
source_id=validated_data['source_id'],
workspace_id=workspace_id,
Expand Down
2 changes: 1 addition & 1 deletion apps/mappings/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class QBDMappingView(LookupFieldMixin, generics.ListCreateAPIView):
ordering_fields = ('source_value',)


#mapping stats view
# Mapping stats view
class QBDMappingStatsView(generics.RetrieveAPIView):
"""
Stats for total mapped and unmapped count for a given attribute type
Expand Down
2 changes: 1 addition & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from quickbooks_desktop_api.tests import settings

from .test_fyle.fixtures import fixtures as fyle_fixtures
from .test_mapping.fixture import fixture as mapping_fixtures
from .test_mapping.fixtures import fixture as mapping_fixtures


@pytest.fixture
Expand Down
6 changes: 3 additions & 3 deletions tests/test_fyle/test_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

from apps.mappings.models import QBDMapping

from tests.test_mapping.fixture import fixture
from tests.test_mapping.fixtures import fixture

@pytest.mark.django_db(databases=['default'])
def test_sync_fyle_dimension_view(api_client, test_connection, mocker):
Expand Down Expand Up @@ -31,5 +31,5 @@ def test_sync_fyle_dimension_view(api_client, test_connection, mocker):
qbd_mapping = QBDMapping.objects.filter(workspace_id=workspace_id)

assert response.status_code == 200
assert len(qbd_mapping) == fixture['get_qbd_CCC_mapping']['count']
assert qbd_mapping[0].source_value == fixture['get_qbd_CCC_mapping']['results'][0]['source_value']
assert len(qbd_mapping) == fixture['get_qbd_ccc_mapping']['count']
assert qbd_mapping[0].source_value == fixture['get_qbd_ccc_mapping']['results'][0]['source_value']
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
]
}
],
'post_qbd_CCC_mapping': {
'post_qbd_ccc_mapping': {
"id": 1,
"attribute_type": "CORPORATE_CARD",
"source_value": "AMERICAN EXPRESS - 4818",
Expand All @@ -30,7 +30,7 @@
"updated_at": "2023-08-28T10:07:30.524503Z",
"workspace": 1
},
'get_qbd_CCC_mapping_state': {
'get_qbd_ccc_mapping_state': {
"all_attributes_count":2,
"unmapped_attributes_count":2
},
Expand All @@ -46,7 +46,7 @@
'source_id': 'baccEg1AzugNxZ',
}
],
'get_qbd_CCC_mapping': {
'get_qbd_ccc_mapping': {
"count": 2,
"next": "http://localhost:8008/api/workspaces/1/qbd_mappings/?attribute_type=CORPORATE_CARD",
"previous": '',
Expand Down
8 changes: 4 additions & 4 deletions tests/test_mapping/test_connector.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import pytest
from apps.mappings.connector import PlatformConnector
from apps.mappings.models import QBDMapping
from .fixture import fixture
from .fixtures import fixture


@pytest.mark.django_db(databases=['default'], transaction=True)
Expand All @@ -17,7 +17,7 @@ def test_sync_corporate_card(create_temp_workspace,
qbd_connection = PlatformConnector(workspace_id=workspace_id)
qbd_connection.sync_corporate_card()
qbd_mappings = QBDMapping.objects.filter(workspace_id=workspace_id, attribute_type = 'CORPORATE_CARD')
assert len(qbd_mappings) == len(fixture['get_qbd_CCC_mapping']['results'])
for i, item in enumerate(qbd_mappings):
assert qbd_mappings[i].source_value == fixture['get_qbd_CCC_mapping']['results'][i]['source_value']
assert len(qbd_mappings) == len(fixture['get_qbd_ccc_mapping']['results'])
for i, _ in enumerate(qbd_mappings):
assert qbd_mappings[i].source_value == fixture['get_qbd_ccc_mapping']['results'][i]['source_value']

25 changes: 14 additions & 11 deletions tests/test_mapping/test_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

from apps.mappings.models import QBDMapping

from .fixture import fixture
from .fixtures import fixture

@pytest.mark.django_db(databases=['default'])
def test_qbd_mapping_view(api_client, test_connection):
Expand All @@ -28,8 +28,8 @@ def test_qbd_mapping_view(api_client, test_connection):
response = api_client.get(url, {'attribute_type': 'CORPORATE_CARD', 'limit':10, 'offset':0})

assert response.status_code == 200
assert len(response.data['results']) == len(fixture['get_qbd_CCC_mapping']['results'])
assert response.data['count'] == fixture['get_qbd_CCC_mapping']['count']
assert len(response.data['results']) == len(fixture['get_qbd_ccc_mapping']['results'])
assert response.data['count'] == fixture['get_qbd_ccc_mapping']['count']

# post qbd mapping
url = reverse(
Expand All @@ -50,6 +50,8 @@ def test_qbd_mapping_view(api_client, test_connection):
post_value = QBDMapping.objects.filter(workspace_id=workspace_id,
attribute_type = payload['attribute_type'],
source_id = payload['source_id'])

print(response.data)
assert response.status_code == 201
assert post_value[0].destination_value == payload['destination_value']

Expand All @@ -59,19 +61,20 @@ def test_qbd_mapping_view(api_client, test_connection):
response = api_client.get(url, param)

assert response.status_code == 200
assert len(response.data['results']) == len(fixture['get_qbd_CCC_mapping']['results'])-1
assert response.data['count'] == fixture['get_qbd_CCC_mapping']['count']-1
assert response.data['results'][0]['source_value'] == fixture['get_qbd_CCC_mapping']['results'][0]['source_value']
assert len(response.data['results']) == len(fixture['get_qbd_ccc_mapping']['results'])-1
assert response.data['count'] == fixture['get_qbd_ccc_mapping']['count']-1
assert response.data['results'][0]['source_value'] == fixture['get_qbd_ccc_mapping']['results'][0]['source_value']

# get all unmapped rows (destination_value__isnull is true)

param = {'attribute_type': 'CORPORATE_CARD', 'limit':10, 'offset':0, 'destination_value__isnull': 'true'}
response = api_client.get(url, param)

assert response.status_code == 200
assert len(response.data['results']) == len(fixture['get_qbd_CCC_mapping']['results'])-1
assert response.data['count'] == fixture['get_qbd_CCC_mapping']['count']-1
assert response.data['results'][0]['source_value'] == fixture['get_qbd_CCC_mapping']['results'][1]['source_value']
assert len(response.data['results']) == len(fixture['get_qbd_ccc_mapping']['results'])-1
assert response.data['count'] == fixture['get_qbd_ccc_mapping']['count']-1
assert response.data['results'][0]['source_value'] == fixture['get_qbd_ccc_mapping']['results'][1]['source_value']


@pytest.mark.django_db(databases=['default'])
def test_qbd_mapping_stats_view(api_client, test_connection):
Expand All @@ -95,5 +98,5 @@ def test_qbd_mapping_stats_view(api_client, test_connection):
response = api_client.get(url, {'source_type': 'CORPORATE_CARD'})

assert response.status_code==200
assert response.data['all_attributes_count']==fixture['get_qbd_CCC_mapping_state']['all_attributes_count']
assert response.data['unmapped_attributes_count']==fixture['get_qbd_CCC_mapping_state']['unmapped_attributes_count']
assert response.data['all_attributes_count']==fixture['get_qbd_ccc_mapping_state']['all_attributes_count']
assert response.data['unmapped_attributes_count']==fixture['get_qbd_ccc_mapping_state']['unmapped_attributes_count']
11 changes: 5 additions & 6 deletions tests/test_qbd/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ def test_create_credit_card_purchases_iif_file_expense_vendor(
create_temp_workspace, add_accounting_export_bills,
add_accounting_export_expenses, add_fyle_credentials,
add_export_settings, add_field_mappings, add_advanced_settings,
add_ccc_mapping, mocker
mocker
):
"""
Test create credit card purchases iif file
Expand Down Expand Up @@ -331,7 +331,7 @@ def test_create_credit_card_purchases_iif_file_expense_employee(
create_temp_workspace, add_accounting_export_bills,
add_accounting_export_expenses, add_fyle_credentials,
add_export_settings, add_field_mappings, add_advanced_settings,
add_ccc_mapping, mocker
mocker
):
"""
Test create credit card purchases iif file
Expand Down Expand Up @@ -399,8 +399,7 @@ def test_create_credit_card_purchases_iif_file_expense_employee(
def test_create_credit_card_purchases_iif_file_expense_fail(
create_temp_workspace, add_accounting_export_bills,
add_accounting_export_expenses, add_fyle_credentials,
add_export_settings, add_advanced_settings,
add_ccc_mapping, mocker
add_export_settings, add_advanced_settings, mocker
):
"""
Test create credit card purchases iif file
Expand Down Expand Up @@ -455,7 +454,7 @@ def test_create_credit_card_purchases_iif_file_expense_fatal(
create_temp_workspace, add_accounting_export_bills,
add_accounting_export_expenses, add_fyle_credentials,
add_export_settings, add_field_mappings, add_advanced_settings,
add_ccc_mapping, mocker
mocker
):
"""
Test create credit card purchases iif file
Expand Down Expand Up @@ -822,7 +821,7 @@ def test_email_failure(
create_temp_workspace, add_accounting_export_bills,
add_accounting_export_expenses, add_fyle_credentials,
add_export_settings, add_field_mappings, add_advanced_settings,
add_ccc_mapping, mocker
mocker
):
"""
Test create journals iif file Failed for email failure
Expand Down
Loading