-
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
Functionality to capture user activities #23
Functionality to capture user activities #23
Conversation
…ebhook-analytics into feature/17-create-user-activities-service
…ebhook-analytics into feature/17-create-user-activities-service
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.
@Sachinbisht27 a few changes are required. Have a look
api/models/user_activities.py
Outdated
.first() | ||
) | ||
|
||
def get_succeeded_activity_for_user(self, user_id, user_phone): |
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.
same here
api/services/flow_run_log_service.py
Outdated
if not today_flow_log: | ||
if flow_status == self.class_model.FlowRunStatus.COMPLETED: | ||
user_flow_log = self.update_log(latest_flow_log) | ||
else: | ||
user_flow_log = self.create_log(flow_uuid, flow_name, flow_type) |
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.
We should create a new log only if the start node is available - ie flow_status == "Started", thoughts?
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.
Yes, thanks for the suggestion. Let me update that.
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.
@Sachinbisht27 a few comments before we merge the changes. Have a look
.pre-commit-config.yaml
Outdated
rev: v1.10.0 | ||
hooks: | ||
- id: python-use-type-annotations | ||
- repo: https://github.com/sondrelg/pep585-upgrade |
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.
Why do we need this?
The GitHub repo is archived now, if this is helpful , we may need to find an alternative - https://github.com/sondrelg/pep585-upgrade
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.
I was exploring some pre-commit hooks so, we can maintain the TypeHinting in the codebase. I found 2 hooks related to it so added them here. I can explore more if this one is archived now.
#17
Please complete the following steps and check these boxes before filing your PR:
Types of changes
Short description of what this resolves:
Changes contain:
Testing payloads:
Checklist: