-
Notifications
You must be signed in to change notification settings - Fork 11.8k
feat: Security Logs Page #35218
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
feat: Security Logs Page #35218
Conversation
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: f3cac5f The changes in this PR will be included in the next version bump. This PR includes changesets to release 37 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #35218 +/- ##
============================================
+ Coverage 59.62% 75.50% +15.88%
============================================
Files 2832 533 -2299
Lines 68353 24091 -44262
Branches 15139 5750 -9389
============================================
- Hits 40755 18190 -22565
+ Misses 24992 5401 -19591
+ Partials 2606 500 -2106
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
7ffafa3
to
2319c79
Compare
apps/meteor/client/views/audit/components/SecurityLogsTable.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/audit/components/SecurityLogsTable.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/audit/components/SecurityLogsTable.tsx
Outdated
Show resolved
Hide resolved
|
apps/meteor/client/views/audit/components/SecurityLogDisplayModal.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/audit/components/SecurityLogsTable.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/audit/components/SecurityLogsTable.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/audit/components/SettingSelect.spec.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/audit/hooks/useSettingSelectOptions.spec.ts
Outdated
Show resolved
Hide resolved
d4e7de6
to
390ed28
Compare
6c3219e
to
0689d7d
Compare
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.
Missing translations
Setting_changed
Changed_from
Changed_to
Actor
Timestamp
Setting
Dates are being prepopulated here to the actual day, but the initial results are not filtered. This filter should probably be empty when accessing this Page. (Or at least filled with a value that represents the actual filter).
There's several style issues that I've encountered:
(Adittional comment in code) In the table, there's different heights depending on the type of record. From the design, all records have a height of 44px
Actor description for types that are not user
should have their first letter capitalized.
In the modal, all the margins are different from the figma design. The color of the information (presented below the label) is the wrong collor (comment on code about InfoPanel component).
Also in the modal, if the setting is a big text (such as a custom script, or a license in the case of this example), the modal expands to the whole screen, and looks very weird. We should probably truncate this text a little bit, or limit the modal height and we can scroll the content.
Still in the modal, actors other than user are displaying the avatar.
There's also one thing that I didn't test, which is settings changed by an APP, but looking at the code, there's no information about what app made the changes. I'm also thinking that this information should somehow be in the table, but I'm not sure as it's not contemplated in the designs. We probably need to probe the design team on this. But I'm pretty sure we need to include at least the app ID somewhere, ideally the name.
apps/meteor/client/views/audit/components/SecurityLogDisplayModal.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/audit/hooks/useSettingSelectOptions.ts
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/audit/hooks/useSettingSelectOptions.ts
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/audit/components/SecurityLogDisplayModal.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/audit/components/SecurityLogsTable.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/audit/components/SecurityLogDisplayModal.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/audit/components/SecurityLogsTable.tsx
Outdated
Show resolved
Hide resolved
12bdfa2
to
1bb309f
Compare
f434366
to
05ef45c
Compare
Requires #35258
Requires #35361
Proposed changes (including videos or screenshots)
Issue(s)
LOG-11
Steps to test or reproduce
Further comments