-
Notifications
You must be signed in to change notification settings - Fork 3
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
exportable expense API and date range filter for expense group view API #454
Conversation
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #454 +/- ##
==========================================
- Coverage 95.12% 95.04% -0.09%
==========================================
Files 49 49
Lines 4636 4682 +46
==========================================
+ Hits 4410 4450 +40
- Misses 226 232 +6
|
|
|
apps/fyle/views.py
Outdated
workspace_id=self.kwargs['workspace_id'], | ||
exported_at__isnull=True, | ||
fund_source__in=fund_source | ||
).values_list('id', flat=True) |
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.
can move all of the above code to a helper function and get ids from there
can help make the view look a bit cleaner
tests/test_fyle/test_views.py
Outdated
assert response.status_code==200 | ||
|
||
response = json.loads(response.content) | ||
assert response['exportable_expense_group_ids'] == [1, 2] |
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.
new line
|
|
apps/fyle/actions.py
Outdated
from apps.workspaces.models import Configuration | ||
|
||
|
||
def get_expense_group_ids(workspace_id, ): |
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.
action file should do some sort of updates. This seem more like a helper func, move it to helpers.py
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.
remove extra comma in arg
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.
name it get_exportable_expense_group_ids()
|
* employee email added to expense group * added field export_url in expense group and util to generate URL (#457) * add employee name in expense and script to populate data * test fixture changes * added field export_url in expense group and util to generate URL * updated test and fixtures * changed scripts to batch update export url * bug fix * comment resolved * added more fields in expense serializer for redirection (#458) * added more fields in expense serializer * Sync import API (#459) * added expense group sync API * minor changes * added url for expense group sync view * remove redundant script
|
|
No description provided.