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

ExpenseCustomField Rework #536

Merged
merged 6 commits into from
Dec 19, 2023
Merged

ExpenseCustomField Rework #536

merged 6 commits into from
Dec 19, 2023

Conversation

labhvam5
Copy link
Contributor

No description provided.

@@ -99,10 +98,6 @@ def pre_save_mapping_settings(self):
"""
mapping_settings = self.__mapping_settings

schedule_fyle_attributes_creation(self.__workspace_id)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have removed the scheduling logic from imports/triggers to mappings/signals.py . Hope this doesn't cause any issue.

workspace_id=workspace_general_settings.workspace_id,
is_custom=True
).count()

Copy link
Contributor

Choose a reason for hiding this comment

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

we're calling schedule_or_delete_fyle_import_tasks() from 2 places

mapping setting post save signal and post_save_mapping_settings trigger in onboarding API

I think we can remove mapping setting post save signal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

able to get the issue have removed the mapping setting post save signal

@@ -34,8 +34,15 @@ def schedule_or_delete_fyle_import_tasks(workspace_general_settings: WorkspaceGe
source_field__in=['PROJECT', 'COST_CENTER']
).count()

custom_field_import_fields_count = MappingSetting.objects.filter(
Copy link
Contributor

Choose a reason for hiding this comment

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

cleanup the whole function. if something: schedule. else: delete

apps/mappings/signals.py Outdated Show resolved Hide resolved
Copy link

Tests Skipped Failures Errors Time
219 0 💤 4 ❌ 0 🔥 51.432s ⏱️

Copy link

Tests Skipped Failures Errors Time
219 0 💤 5 ❌ 0 🔥 53.216s ⏱️

@labhvam5 labhvam5 requested a review from ashwin1111 December 19, 2023 13:28
Copy link

Tests Skipped Failures Errors Time
219 0 💤 5 ❌ 0 🔥 50.542s ⏱️

* Removing old code

* adding test (#538)

* adding test

* lint fixes

* fix test

* sub module changes

* fixing test

* fixing test

* Migration script (#539)
Copy link

Tests Skipped Failures Errors Time
221 0 💤 0 ❌ 0 🔥 49.177s ⏱️

Copy link

codecov bot commented Dec 19, 2023

Codecov Report

Merging #536 (c3ec1e5) into master (8ca3ed5) will decrease coverage by 0.16%.
Report is 1 commits behind head on master.
The diff coverage is 93.65%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #536      +/-   ##
==========================================
- Coverage   96.26%   96.10%   -0.16%     
==========================================
  Files          60       60              
  Lines        4467     4395      -72     
==========================================
- Hits         4300     4224      -76     
- Misses        167      171       +4     
Files Coverage Δ
apps/mappings/constants.py 100.00% <100.00%> (ø)
apps/mappings/queues.py 96.61% <100.00%> (-0.73%) ⬇️
apps/mappings/schedules.py 100.00% <100.00%> (ø)
apps/mappings/tasks.py 96.46% <ø> (-0.96%) ⬇️
apps/quickbooks_online/actions.py 100.00% <100.00%> (ø)
apps/workspaces/apis/import_settings/triggers.py 100.00% <100.00%> (ø)
apps/mappings/signals.py 94.87% <90.24%> (-5.13%) ⬇️

@labhvam5 labhvam5 added the deploy Triggers deployment of active branch to Staging label Dec 19, 2023
@labhvam5 labhvam5 merged commit 4ee16ef into master Dec 19, 2023
3 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy Triggers deployment of active branch to Staging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants