Skip to content

feat: audit migration to mui - #1027

Open
santipalenque wants to merge 1 commit into
masterfrom
feature/migrate-audit-to-mui
Open

feat: audit migration to mui#1027
santipalenque wants to merge 1 commit into
masterfrom
feature/migrate-audit-to-mui

Conversation

@santipalenque

@santipalenque santipalenque commented Jul 28, 2026

Copy link
Copy Markdown

https://app.clickup.com/t/9014802374/86bb2ex8n

Summary by CodeRabbit

  • New Features

    • Added shared filtering for audit logs, including member and date-time criteria.
    • Added search, sorting, pagination, and configurable rows-per-page controls through the updated table interface.
  • Bug Fixes

    • Improved audit log filter handling and ensured filters remain applied during searches, sorting, and pagination.
    • Reset audit log filters and parameters when leaving the page.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Failed to post review comments.

We encountered an issue with GitHub. Use @coderabbitai full review to retry the review.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 09f09dcb-ed56-479a-88e2-5b7ccc069a32

📥 Commits

Reviewing files that changed from the base of the PR and between 52e285c and a5abcc6.

📒 Files selected for processing (3)
  • src/actions/audit-log-actions.js
  • src/components/audit-logs/index.js
  • src/pages/audit-log/audit-log-page.js
⏰ Context from checks skipped due to timeout. (4)
  • GitHub Check: build
  • GitHub Check: build
  • GitHub Check: build
  • GitHub Check: build
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-05-06T22:24:01.344Z
Learnt from: priscila-moneo
Repo: fntechgit/summit-admin PR: 896
File: src/pages/summits/summit-directory-page.js:220-221
Timestamp: 2026-05-06T22:24:01.344Z
Learning: When using `MuiTable` from `openstack-uicore-foundation/lib/components/mui/table` in this codebase, verify the callback payload types: `onDelete` is called with the primitive row identifier (e.g., `id`/`rowId`), not the full row object, while `onEdit` and `onSelect` receive the full row object. Use `onDelete={(id) => deleteSummit(id)}` (or equivalent) so the delete handler receives only the row ID. Note that the `getName` prop is used by `MuiTable` for displaying the delete dialog, not to transform the `onDelete` argument.

Applied to files:

  • src/pages/audit-log/audit-log-page.js
  • src/actions/audit-log-actions.js
  • src/components/audit-logs/index.js
📚 Learning: 2026-07-13T15:30:50.911Z
Learnt from: smarcet
Repo: fntechgit/summit-admin PR: 1005
File: src/actions/media-upload-actions.js:147-194
Timestamp: 2026-07-13T15:30:50.911Z
Learning: In pages that use the app-wide popup-dialog pattern (e.g., a dialog component like `MediaUploadDialog` that keeps the dialog open on save failure), ensure the parent page save handler intentionally does NOT swallow errors. Specifically: have the parent `handleSave` return the exact promise from the save action (e.g., `saveMediaUpload(...).then(...)`) without adding its own terminal `.catch()`; allow rejections to propagate up to the dialog component. The dialog’s `handleSubmit` should catch on the exact promise returned by the parent (pattern like `Promise.resolve(onSave(...)).then(() => onClose()).catch(() => {})`) so the dialog can remain open and preserve user input. Under this established popup-dialog error-propagation pattern, do not flag “missing `.catch()`” in the parent save-action promise chain.

Applied to files:

  • src/pages/audit-log/audit-log-page.js
📚 Learning: 2026-07-06T21:57:59.168Z
Learnt from: smarcet
Repo: fntechgit/summit-admin PR: 963
File: src/actions/sponsor-users-actions.js:535-546
Timestamp: 2026-07-06T21:57:59.168Z
Learning: In this repo, `src/reducers/base-reducer.js` treats `loading` as a flat 0/1 flag (not a counter). Therefore, in an async action chain, dispatching `stopLoading()` multiple times (e.g., both in a `.then()` and a `.finally()` for the same request) is redundant even if functionally harmless. During review, look for and clean up duplicate `stopLoading()` calls so it’s dispatched only once per async operation (typically in a single terminal path like `.finally()`).

Applied to files:

  • src/actions/audit-log-actions.js
🪛 ast-grep (0.45.0)
src/components/audit-logs/index.js

[warning] 131-131: Avoid using the initial state variable in setState
Context: setSearchTerm(newTerm)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(setstate-same-var)

📝 Walkthrough

Walkthrough

Audit log filtering now uses shared MUI grid criteria and array-based filter expressions. The audit log table, search, pagination, sorting, cleanup, and page composition were updated accordingly, with breadcrumb handling removed.

Changes

Audit log grid migration

Layer / File(s) Summary
Filter expression contract
src/actions/audit-log-actions.js
Audit log requests now clone array-based filters, append term searches, and default filters to an empty array.
Grid filter data flow
src/components/audit-logs/index.js
Shared grid criteria support member and datetime filters, while parsed filters flow through fetching, sorting, searching, pagination, table rendering, and cleanup.
Page composition cleanup
src/pages/audit-log/audit-log-page.js
The page removes breadcrumb and route-match handling while retaining the entry count and audit-log entity filter.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GridFilter
  participant AuditLogs
  participant getAuditLog
  GridFilter->>AuditLogs: Provide parsedFilter
  AuditLogs->>getAuditLog: Request filtered audit logs
  getAuditLog->>AuditLogs: Return audit log data
  AuditLogs->>GridFilter: Reset filters on unmount
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: migrating audit functionality to MUI.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/migrate-audit-to-mui

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/components/audit-logs/index.js (1)

95-142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Collapse the five near-identical getAuditLog calls into one helper.

The four handlers and the effect differ only in page/perPage/order/term. A single fetchLogs({ ... }) wrapper keeps the positional-argument contract in one place, which matters since the action takes seven positional params.

♻️ Suggested shape
+  const fetchLogs = ({
+    newTerm = searchTerm,
+    page = DEFAULT_CURRENT_PAGE,
+    newPerPage = perPage,
+    sortKey = order,
+    sortDir = orderDir
+  } = {}) =>
+    getAuditLog(
+      entityFilter,
+      newTerm,
+      page,
+      newPerPage,
+      sortKey,
+      sortDir,
+      parsedFilter
+    );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/audit-logs/index.js` around lines 95 - 142, Introduce a single
fetchLogs helper near handleSort that accepts page, perPage, order, orderDir,
and term overrides, applies the current entityFilter and parsedFilter, and calls
getAuditLog with the existing seven-argument order. Update handleSort,
handlePageChange, handlePerPageChange, handleSearch, and the related effect to
use this helper while preserving each handler’s current defaults and state
updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/components/audit-logs/index.js`:
- Line 37: Update the customParser callback to validate that f.value is a
non-empty array before mapping it; return an empty filter result for missing,
non-array, or empty values, while preserving the existing user_id expression for
valid selections.
- Around line 68-89: Update the action column in auditLogColumns to use the
normalized action field exposed by logEntries, replacing action_description with
action; alternatively, rename the reducer output to action_description and keep
all consumers consistent.

---

Nitpick comments:
In `@src/components/audit-logs/index.js`:
- Around line 95-142: Introduce a single fetchLogs helper near handleSort that
accepts page, perPage, order, orderDir, and term overrides, applies the current
entityFilter and parsedFilter, and calls getAuditLog with the existing
seven-argument order. Update handleSort, handlePageChange, handlePerPageChange,
handleSearch, and the related effect to use this helper while preserving each
handler’s current defaults and state updates.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 09f09dcb-ed56-479a-88e2-5b7ccc069a32

📥 Commits

Reviewing files that changed from the base of the PR and between 52e285c and a5abcc6.

📒 Files selected for processing (3)
  • src/actions/audit-log-actions.js
  • src/components/audit-logs/index.js
  • src/pages/audit-log/audit-log-page.js

multiple: true
}
},
customParser: (f) => [`user_id==${f.value.map((s) => s.value).join("||")}`]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Guard customParser against empty/non-array values.

If f.value is an empty array the parser emits a dangling user_id== expression that the API will reject; if the widget ever yields a single object (non-multiple), .map throws.

🛡️ Proposed guard
-    customParser: (f) => [`user_id==${f.value.map((s) => s.value).join("||")}`]
+    customParser: (f) => {
+      const values = (Array.isArray(f.value) ? f.value : [f.value])
+        .map((s) => s?.value)
+        .filter((v) => v != null);
+      return values.length ? [`user_id==${values.join("||")}`] : [];
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
customParser: (f) => [`user_id==${f.value.map((s) => s.value).join("||")}`]
customParser: (f) => {
const values = (Array.isArray(f.value) ? f.value : [f.value])
.map((s) => s?.value)
.filter((v) => v != null);
return values.length ? [`user_id==${values.join("||")}`] : [];
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/audit-logs/index.js` at line 37, Update the customParser
callback to validate that f.value is a non-empty array before mapping it; return
an empty filter result for missing, non-array, or empty values, while preserving
the existing user_id expression for valid selections.

Comment on lines 68 to 89
const auditLogColumns = [
{
columnKey: "created",
value: T.translate("audit_log.date"),
header: T.translate("audit_log.date"),
sortable: true
},
{
columnKey: "action_description",
value: T.translate("audit_log.action"),
header: T.translate("audit_log.action"),
sortable: false
},
{
columnKey: "event_id",
value: T.translate("audit_log.event"),
header: T.translate("audit_log.event"),
sortable: true
},
{ columnKey: "user", value: T.translate("audit_log.user"), sortable: false }
{
columnKey: "user",
header: T.translate("audit_log.user"),
sortable: false
}
];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the relevant file and nearby symbols
ast-grep outline src/components/audit-logs/index.js --view expanded

# Read the relevant section with line numbers
sed -n '1,220p' src/components/audit-logs/index.js

# Find table usages and any render/value accessor patterns in the repo
rg -n --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' 'columnKey|render:|value:' src components .

Repository: fntechgit/summit-admin

Length of output: 50379


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- src/components/audit-logs/index.js ---'
sed -n '1,220p' src/components/audit-logs/index.js

echo
echo '--- imports/usages around audit-log table ---'
rg -n --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' 'MuiTable|Table' src/components/audit-logs src/pages | head -n 80

echo
echo '--- package version hints ---'
rg -n --glob 'package.json' '"openstack-uicore-foundation"|uicore-foundation' .

Repository: fntechgit/summit-admin

Length of output: 13843


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- git diff for src/components/audit-logs/index.js ---'
git diff --unified=80 -- src/components/audit-logs/index.js

echo
echo '--- nearby audit-log action payload shape ---'
sed -n '1,260p' src/actions/audit-log-actions.js

echo
echo '--- any audit-log formatters/helpers ---'
rg -n --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' 'format.*audit|audit.*format|created.*format|user.*format|first_name|last_name|expand: "user"' src

Repository: fntechgit/summit-admin

Length of output: 50378


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- src/reducers/audit_log/audit-log-reducer.js ---'
sed -n '1,220p' src/reducers/audit_log/audit-log-reducer.js

echo
echo '--- current audit-log component columns plus data usage ---'
sed -n '1,240p' src/components/audit-logs/index.js

Repository: fntechgit/summit-admin

Length of output: 8149


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' 'action_description|audit_log\.action|columnKey: "action"|columnKey: "action_description"' src

Repository: fntechgit/summit-admin

Length of output: 322


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' 'formatAuditLog|parseSpeakerAuditLog|action_description' src/utils src/reducers src/actions src/components

Repository: fntechgit/summit-admin

Length of output: 696


Use the normalized action field here

logEntries exposes the formatted text as action, not action_description, so this column will render empty. Point columnKey at the reducer output field or rename the reducer key to match.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/audit-logs/index.js` around lines 68 - 89, Update the action
column in auditLogColumns to use the normalized action field exposed by
logEntries, replacing action_description with action; alternatively, rename the
reducer output to action_description and keep all consumers consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant