-
Notifications
You must be signed in to change notification settings - Fork 17
task/WP-930: Audit Trails UI #1594
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
base: main
Are you sure you want to change the base?
Conversation
…ounts", not sure if supposed to follow same way other tab on there are created or make something completely new, have UI set up in html under "terms and conditions" in manage account section.
… Changed names on dropdown menu for type of search, implemented username auto-search dropdown menu capability for easier selection, added css file for AuditTrials.tsx
…ing point if mess up
|
Need recommendation if I should create new hook files to handle getting the query information from the functions in audit/views.py, maybe like a useUserAudit.ts for the username search and a useFileAudit.ts for the file search. Also if I should ignore most of the missing module docstring errors I'm getting on serverside linting because other apps.py and urls.py in the codebase don't have them at all. |
designsafe/apps/accounts/templates/designsafe/apps/accounts/base.html
Outdated
Show resolved
Hide resolved
Yes, new hooks would be good. Serverside linting is r.e. trailing whitespace and missing newlines: |
…tsxm removed unnecessary files
…se.html Co-authored-by: Sal Tijerina <[email protected]>
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.
This is looking super clean, and easy to read! Thanks so much!
I think what we need now is some unit testing and to address the linting errors that are popping up. You can usually check these locally by running the same script that github actions does (npx nx format:check and pylint)
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.
There's some work to be done on these tests, still. But a great start!
… include (essential-graphrag.pdf, TAPIS_ERROR_ex_file.png) --case insensitive, still need to implement file tracking from submitJob action, need to figure out file tracking on tapis side, and fix small bugs on both ends
…Job included, and try to rework UI into more of a tree like format
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.
Some really good work here connecting pieces together. I think we can clean it up a bit
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 you point me to what changed in this file?
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.
Any reason we don't want to use class based views here? You could use the AuthenticatedApiView, which would cut down on repeated @login_required decorators
|
|
||
| {auditData?.data && auditData.data.length > 0 && ( | ||
| <div className="styles.tableWrapper"> | ||
| <table |
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.
In the future, I would suggest using antd's Table component
| //using in filetable as well, maybe move to another file? | ||
| const formatTimestamp = (timestamp: string) => { | ||
| const date = new Date(timestamp); | ||
| return date.toLocaleString('en-US', { | ||
| hour: 'numeric', | ||
| minute: '2-digit', | ||
| hour12: true, | ||
| month: 'numeric', | ||
| day: 'numeric', | ||
| year: '2-digit', | ||
| }); | ||
| }; |
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.
If duplicated, best to pull out into a util file somewhere, and import from there
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 thinking this too but not sure if it would have been best to pull out to utils, thank you!
|
|
||
| return ( | ||
| <div> | ||
| <form onSubmit={onSearch} style={{ marginBottom: 16 }}> |
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.
Antd components would give us some nice UI styling for free
| FROM public.portal_audit | ||
| WHERE username = %s | ||
| ORDER BY timestamp DESC | ||
| LIMIT 1 |
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.
Do we only want 1 response?
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, here we only limit it to 1 because we only look for the single most recent session.
| def get_path_without_filename(full_path): | ||
| """Extract path without filename""" | ||
| if not full_path or "." not in full_path.split("/")[-1]: | ||
| return full_path | ||
| return "/".join(full_path.rstrip("/").split("/")[:-1]) | ||
|
|
||
| def get_filename_from_path(full_path): | ||
| return full_path.rstrip("/").split("/")[-1].lower() | ||
|
|
||
| def normalize_dir_path(p): |
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 think we'll want to refactor this to not have nested functions
|
|
||
|
|
||
| @login_required | ||
| def get_rename_portal_search(request, filename): |
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.
Is this the same as get_upload_portal_search?
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.
No, so this one handles things a little differently, but uses get_upload_portal_search() if the upload row is found to get its full trace.
| 'designsafe.apps.api.publications_v2', | ||
| 'designsafe.apps.api.filemeta', | ||
| 'designsafe.apps.accounts', | ||
| 'designsafe.apps.audit', |
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.
Since we're not running tests, you'll want to remove this line.
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 would second Sal's comments, and there's a small part of turning off your unit tests that still needs to be done.
Overview:
This PR introduces an Audit Data Trails admin page that allows users to look up recent session activity by entering a username. It retrieves the most recent session and displays all associated actions across multiple portals.
PR Status:
Related Jira tickets:
Summary of Changes:
Created a new admin view under the Designsafe portal for audit trail lookups
Implemented backend logic to:
Testing Steps:
Get updated
designsafe.envfrom stache with Audit Trails secretsFrom the top-right dropdown menu, select "Audit Trails Admin"
For testing purposes:
Confirm that audit actions from the most recent session are being fetched and displayed correctly
UI Photos:
Notes: