[Phase 2] Fetch report expenses inside modal #66
Conversation
|
|
||
| report_expenses_dialog = modal_messages.get_report_expenses_dialog(user=user, report=report, report_expenses=report_expenses) | ||
|
|
||
| slack_client.views_update(user=slack_user_id, view=report_expenses_dialog, view_id=modal_view_id) |
There was a problem hiding this comment.
[NOTE FOR SELF] - Add a tracker here as well
| @@ -13,7 +13,7 @@ Django==3.1.14 | |||
| django-picklefield==3.0.1 | |||
| django-q==1.3.4 | |||
| future==0.18.2 | |||
| fyle==0.19.0 | |||
| fyle==0.22.0 | |||
There was a problem hiding this comment.
GET /approver/expenses api changes published in this latest sdk version
fyle_slack_service/settings.py
Outdated
| @@ -152,7 +152,7 @@ | |||
| 'name': 'fyle_slack_service', | |||
| 'compress': True, | |||
| 'save_limit': 0, | |||
| 'workers': 4, | |||
| 'workers': 1, | |||
There was a problem hiding this comment.
[NOTE FOR SELF] - Revert this change before merging (did this for local development)
fyle_slack_service/settings.py
Outdated
| @@ -152,7 +152,7 @@ | |||
| 'name': 'fyle_slack_service', | |||
| 'compress': True, | |||
| 'save_limit': 0, | |||
| 'workers': 4, | |||
| 'workers': 1, | |||
| query_params = { | ||
| 'report_id': 'eq.{}'.format(report['id']), | ||
| 'order': 'created_at.desc', | ||
| 'limit': '30', |
There was a problem hiding this comment.
for report_id, we should find only one, no?
|
|
||
| report_expenses_dialog = modal_messages.get_report_expenses_dialog(user=user, report=report, report_expenses=report_expenses) | ||
|
|
||
| slack_client.views_update(user=slack_user_id, view=report_expenses_dialog, view_id=modal_view_id) |
| 'type': 'context', | ||
| 'elements': [ | ||
| { | ||
| 'type': 'image', |
There was a problem hiding this comment.
need some explanation of the placeholder image
| } | ||
| ] | ||
|
|
||
| # Compose and append the expense title message |
There was a problem hiding this comment.
move the expense message composing to a method.
There was a problem hiding this comment.
Rule of thumb: If a method/function is more than 15 lines, then it is time to split up the method/function.
| from fyle_slack_app.fyle import utils as fyle_utils | ||
|
|
||
|
|
||
| def get_report_expenses_dialog(user: User, report: dict, report_expenses: dict) -> Dict: |
There was a problem hiding this comment.
think about breaking this method into multiple methods
| 'order': 'created_at.desc', | ||
| 'limit': '15', | ||
| 'offset': '0' | ||
| } |
There was a problem hiding this comment.
FYI - Using limit as 15, since we will only be showing total 15 expenses at-max inside the modal
Therefore, fetching only latest 15 expenses via API
| } | ||
| } | ||
| report_notification_message.insert(3, report_deleted_section) | ||
| report_notification_message = common_messages.get_no_report_access_message(notification_message=notification_message) |
There was a problem hiding this comment.
FYI - Created a function to return a "No access to report anymore" message (in case if report gets deleted)
| modal_view_id = modal['view']['id'] | ||
|
|
||
| async_task( | ||
| 'fyle_slack_app.slack.interactives.block_action_handlers.fetch_report_and_expenses', |
There was a problem hiding this comment.
async tasks should be in tasks.py
| } | ||
|
|
||
| tracking.identify_user(user.email) | ||
| tracking.track_event(user.email, 'Report Approved from Slack Modal', event_data) |
There was a problem hiding this comment.
This will have 2 events triggered, one from the original report approve function and one from here, how will we differentiate from where the report was approved from?
There was a problem hiding this comment.
Working on fixing this
| 'emoji': True | ||
| }, | ||
| 'action_id': 'pre_auth_message_approve', | ||
| 'value': 'pre_auth_message_approve', | ||
| } | ||
| } | ||
|
|
||
|
|
||
| def get_no_report_access_message(notification_message: List[Dict]) -> List[Dict]: |
There was a problem hiding this comment.
It should be more of a generic function name which can take a message and show it irrespective of state of report.
|
|
||
|
|
||
|
|
||
| def fetch_report_and_expenses(user: User, team_id: str, private_metadata: Dict, modal_view_id: str) -> JsonResponse: |
There was a problem hiding this comment.
name is not in relation to what the function is doing, function is doing much more than fetching report and it's expenses.
Changes done -
Screen recording -
new_flow_1.mov