-
Notifications
You must be signed in to change notification settings - Fork 242
Task/FOUR-28659: Implement new Logs base UI section for Agents #8671
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
Conversation
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 the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on January 21
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
resources/js/admin/logs/components/Logs/BaseTable/BaseTable.vue
Outdated
Show resolved
Hide resolved
| }, | ||
| beforeDestroy() { | ||
| // Clean up event listener | ||
| this.$root.$off('bv::collapse::state'); |
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.
Event cleanup removes all listeners not just component's
Low Severity
The beforeDestroy hook calls this.$root.$off('bv::collapse::state') without passing the handler callback reference. In Vue, calling $off with only the event name removes ALL listeners for that event, not just the one this component registered. Other components in the codebase correctly pass the handler reference to $off (e.g., in ErrorHandlingMixin.vue). This could cause issues if the component is destroyed while other listeners depend on this event.
| }, | ||
| getExportUrl() { | ||
| return `/admin/logs/export/csv?type=${this.logType}&search=${this.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.
Search parameter not URL-encoded in export URL
Medium Severity
The getExportUrl computed property directly interpolates this.search into the URL query string without using encodeURIComponent(). If a user's search term contains special characters like &, =, ?, or #, the URL will be malformed. For example, searching for test&foo=bar would produce a URL where foo is incorrectly parsed as a separate query parameter rather than part of the search value, causing the export to return wrong results.
| }, | ||
| immediate: 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.
Duplicate API requests when changing log categories
Low Severity
Both the category and logType watchers call resetAndFetch() when their values change. When navigating between categories (e.g., from email to agents), both props change simultaneously, causing both watchers to fire and triggering two identical API requests. The watchers could be consolidated or use a debounce mechanism to avoid the duplicate network call.
| onHandleSearch() { | ||
| if (this.$refs.routerView) { | ||
| this.$refs.routerView.refresh({ search: this.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.
Search functionality broken due to incorrect RouterView ref
High Severity
The onHandleSearch method calls this.$refs.routerView.refresh(), but in Vue 2 the ref on <RouterView> points to the RouterView component wrapper, not the rendered child component (LogTable). Since RouterView doesn't have a refresh method, the search functionality will silently fail and users won't be able to filter log data. The refresh method exists on LogTable but is inaccessible through this ref pattern.
| next(); | ||
| } | ||
| }, | ||
| }, |
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 redirect route for /email causes blank page
Medium Severity
The routes configuration includes a redirect from /agents to /agents/design (lines 69-72), but there is no equivalent redirect for /email to /email/errors. The route /email/:logType requires the logType parameter, so navigating to /admin/logs/email without a log type results in no route match and a blank page. This creates inconsistent behavior where /agents works correctly but /email does not.
|




Description
Add a new section in admin > settings > logs for Agents logs
Related tickets and PRs
https://processmaker.atlassian.net/browse/FOUR-28659
Note
Adds a new Admin Logs section rendered via a Vue SPA and gated by permissions/package availability.
LogsController@index, Blade viewadmin.logs.index, web routes under/admin/logs(wildcard) with optional email CSV export; sidebar menu entry added inGenerateMenusresources/js/admin/logs) with router-based views: Email (errors|matched|total) and Agents (design|execution), conditional routing based on installed packagesSidebar,HeaderBar(category tabs + search),LogTable(column presets per category/type),BaseTable, andPagination; data fetched from/api/1.1/email-start-event/logs/:typeand/api/1.0/flow-genie/logs/(design|execution)/admin/logs/export/csv?type=...&search=...Written by Cursor Bugbot for commit 5f65a78. This will update automatically on new commits. Configure here.