-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Audit logs #5646
Audit logs #5646
Conversation
6ed389c
to
a6bdde7
Compare
text: element.innerText, | ||
classes: element.className, |
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.
Need to think what kind of information we are going to record about pressed buttons. Maybe need to add descriptive class name to all the buttons. Now not everything has it and it is not always clear what button has been pressed.
} | ||
|
||
private isEventToBeRecorded(node: HTMLElement, filter: string[]): boolean { | ||
return filter.some((cssClass: string) => node.classList.contains(cssClass)); |
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.
For now the implementation is allright, but in the future we probably need to generalize it somehow
@@ -53,3 +53,5 @@ dnspython==2.2.0 | |||
setuptools==65.5.1 | |||
django-health-check==3.17.0 | |||
psutil==5.9.4 | |||
clickhouse-connect==0.5.10 | |||
django-crum==0.7.9 |
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.
The app isn't supported for 2 years already: https://github.com/ninemoreminutes/django-crum
task_id = serializers.IntegerField(required=False, allow_null=True) | ||
job_id = serializers.IntegerField(required=False, allow_null=True) | ||
user_id = serializers.IntegerField(required=False, allow_null=True) | ||
org_id = serializers.IntegerField(required=False, allow_null=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.
Let's add org_slug
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.
Please look for kibana
word and cleanup
f243abe
to
6ddfbf2
Compare
cvat/apps/events/export.py
Outdated
writer.writerows(result.result_rows) | ||
|
||
archive_ctime = os.path.getctime(output_filename) | ||
scheduler = django_rq.get_scheduler(settings.CVAT_QUEUES.IMPORT_DATA.value) |
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 IMPORT_DATA queue?
@@ -217,6 +218,7 @@ def add_ssh_keys(): | |||
# FIXME | |||
# 'corsheaders.middleware.CorsPostCsrfMiddleware', | |||
'django.contrib.auth.middleware.AuthenticationMiddleware', | |||
'crum.CurrentRequestUserMiddleware', |
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 will need to remove the extension. It isn't well supported. Let's think how to do that.
return get_current_user() | ||
|
||
def user_id(instance): | ||
current_user = _get_current_user(instance) |
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 to call _get_current_user? We handle a request. The request has an authentication user. Why should we get a user from job.segment.task.owner?
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.
Let's merge the PR. I will propose to discuss some tricky moments on Monday.
Motivation and context
How has this been tested?
Checklist
develop
branch(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.