Conversation
- add AnalyticsReport model and migrations - implement analytics services: - revenue forecast - team productivity - client revenue - expense trends - add InternalApi::V1::AnalyticsController endpoints - add caching layer (Redis/Rails.cache) - add analytics policy and authorization - add background job for cached reports refresh Frontend: - add analytics dashboard - add team, client, expense analytics pages - implement shared filters with URL persistence - integrate react-query and charts - add role-aware UI behavior Tests: - add service, policy, request and job specs
…cess - add manager/team scoped analytics access - add analytics audit logging via Ahoy - integrate analytics into Reports and Invoices - add threshold-based notifications with anti-spam protection - add near real-time team analytics refresh - add analytics insights/recommendations block - add client revenue heatmap - improve expense trend visualization with anomaly markers - add mobile-friendly analytics adaptations - document analytics module - expose Analytics navigation for employee users while preserving restrictions Project now fully satisfies analytics assignment requirements
…, exports, and demo data seeding
📝 WalkthroughWalkthroughIntroduces a comprehensive analytics module with revenue forecasting, team productivity metrics, client analysis, and expense trend reporting. Includes backend services for data aggregation, caching, and analysis; database models for saved reports; API endpoints for data retrieval and export; React frontend pages with filtering and visualization; authorization policies; background jobs for cache refresh and threshold alerts; and extensive test coverage across services, controllers, and UI. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
db/schema.rb (1)
1012-1015:⚠️ Potential issue | 🟠 MajorMissing foreign key on
expenses.project_id.
expenses.project_id(line 388) and its indexes were added, but there is noadd_foreign_key "expenses", "projects"in the foreign-keys section — other analogous columns on this table have FKs. Without it, nothing prevents orphaned rows, which will quietly break the newindex_expenses_on_company_date_project/ project trend queries inAnalytics::ExpenseTrendAnalyzer.🛡️ Proposed addition (schema-level; add via migration)
add_foreign_key "expenses", "expense_categories" + add_foreign_key "expenses", "projects" add_foreign_key "expenses", "users"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@db/schema.rb` around lines 1012 - 1015, The expenses table now has a project_id column and indexes (referenced by index_expenses_on_company_date_project) but is missing a foreign key to projects; add a schema-level FK by creating a migration that calls add_foreign_key "expenses", "projects" (or the equivalent in your migration DSL) for expenses.project_id, ensuring referential integrity and preventing orphaned rows that break Analytics::ExpenseTrendAnalyzer; run the migration and update schema.rb so the new add_foreign_key entry appears alongside the other expenses FKs.
🟡 Minor comments (24)
app/services/analytics/scope_resolver.rb-23-31 (1)
23-31:⚠️ Potential issue | 🟡 MinorAdd explicit client role handling and fail safely on unmapped roles.
Lines 23-31 default any unrecognized role to
"employee"and lack an explicit check for theclientrole defined inApplicationPolicy::ROLES. While the currentAnalyticsPolicyprevents client users from reaching this resolver via authorization gates, the resolver itself is fragile: if the policy check is removed, modified, or bypassed in the future, unmapped/client roles would silently grant employee-level scope. Explicitly handle the client role and returnnil(or raise) for unrecognized roles rather than silently defaulting to employee.Proposed fix
def effective_role return "owner" if user.has_cached_role?(:owner, company) return "admin" if user.has_cached_role?(:admin, company) return "book_keeper" if user.has_cached_role?(:book_keeper, company) return "manager" if user.has_cached_role?(:manager, company) return "employee" if user.has_cached_role?(:employee, company) + return "client" if user.has_cached_role?(:client, company) - "employee" + nil end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/services/analytics/scope_resolver.rb` around lines 23 - 31, The effective_role method in ScopeResolver currently maps several cached roles but silently defaults to "employee" for any unmapped value and omits the ApplicationPolicy::ROLES client role; update effective_role to explicitly check for the client role (e.g., user.has_cached_role?(:client, company) returning "client") and remove the silent fallback — instead return nil (or raise a descriptive error) when no known role matches so unmapped roles fail safely; update any callers to handle a nil/exception result if needed and reference ApplicationPolicy::ROLES, AnalyticsPolicy, and the effective_role method when making the change.lib/tasks/analytics_demo.rake-1-8 (1)
1-8:⚠️ Potential issue | 🟡 MinorConsider guarding demo seeding from production.
Analytics::DemoSeeder.processcreates demo users, clients, projects, invoices, etc. If this task is accidentally invoked against a production environment it will pollute real tenant data. A cheap safeguard would be refusing to run unlessRails.env.development?/test?(or requiring an explicitFORCE=1).Proposed change
namespace :analytics do desc "Seed reproducible demo data for analytics" task seed_demo: :environment do + if Rails.env.production? && ENV["FORCE"] != "1" + abort "Refusing to seed demo analytics data in production. Re-run with FORCE=1 if intentional." + end Analytics::DemoSeeder.process end end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/tasks/analytics_demo.rake` around lines 1 - 8, Guard the seed_demo rake task from running in production by adding an environment check in the task (namespace :analytics, task seed_demo) before calling Analytics::DemoSeeder.process: only allow execution when Rails.env.development? or Rails.env.test? OR when an explicit override like ENV["FORCE"] == "1" is provided; if the check fails, print/log a clear message and abort without calling Analytics::DemoSeeder.process. Ensure this logic lives inside the task block so accidental invocation in production will be refused.app/views/analytics_mailer/threshold_alert.text.erb-7-11 (1)
7-11:⚠️ Potential issue | 🟡 MinorTrim ERB tags to avoid blank lines in the plain-text output.
In a
.text.erbtemplate,<% %>tags leave empty lines behind, producing stray blank lines in the email body around the conditional and each loop iteration. Use the trim variant<%- -%>for control-flow tags.Proposed fix
-<% if `@alert`[:metadata].present? %> -<% `@alert`[:metadata].each do |key, value| %> +<%- if `@alert`[:metadata].present? -%> +<%- `@alert`[:metadata].each do |key, value| -%> <%= key.to_s.humanize %>: <%= value %> -<% end %> -<% end %> +<%- end -%> +<%- end -%>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/analytics_mailer/threshold_alert.text.erb` around lines 7 - 11, The template leaves blank lines because non-trim ERB control tags produce empty lines; update the control-flow tags around the metadata block to use trimmed variants so no stray newlines are emitted: change the if/opening tag to <%- if `@alert`[:metadata].present? -%>, the each iterator to <%- `@alert`[:metadata].each do |key, value| -%>, and the corresponding end tags to <%- end -%> (leave the <%= %> output expressions for key and value as-is so they still render).app/models/expense.rb-32-33 (1)
32-33:⚠️ Potential issue | 🟡 MinorConsider cross-company validation for
projectandexpense_category.Both associations are optional and belong to records that themselves belong to a
company. Without a validation thatproject.company_id == company_id(and same forexpense_category), nothing prevents an expense from being linked to another tenant's project/category, which will corrupt analytics aggregations inExpenseTrendsthat group/filter by project.Proposed validation
+ validate :project_belongs_to_same_company, if: -> { project.present? } + validate :expense_category_belongs_to_same_company, if: -> { expense_category.present? } + + private + + def project_belongs_to_same_company + errors.add(:project, "must belong to the same company") if project.company_id != company_id + end + + def expense_category_belongs_to_same_company + errors.add(:expense_category, "must belong to the same company") if expense_category.respond_to?(:company_id) && expense_category.company_id != company_id + end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/models/expense.rb` around lines 32 - 33, The Expense model currently allows linking to a Project or ExpenseCategory from another company; add a custom validation on Expense (e.g., validate :project_and_expense_category_belong_to_company) that checks, when project_id is present, that project.company_id == self.company_id and, when expense_category_id is present, that expense_category.company_id == self.company_id; add descriptive errors on :project_id and :expense_category_id when mismatched so ExpenseTrends and other analytics cannot be corrupted.spec/factories/analytics_reports.rb-7-7 (1)
7-7:⚠️ Potential issue | 🟡 MinorUse a Faker-backed factory value.
This factory name does not appear to need a fixed literal, so prefer generated data here.
Suggested change
- name { "Revenue forecast snapshot" } + name { Faker::Lorem.sentence }As per coding guidelines,
spec/factories/**/*.rb: “Prefer Faker-backed values in factories unless a spec needs a fixed literal.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/factories/analytics_reports.rb` at line 7, The factory currently uses a hard-coded name ("Revenue forecast snapshot"); change the name attribute in the analytics report factory (the name { ... } block inside the factory :analytics_report) to use a Faker-backed value (for example a generated product/company/short sentence) so tests get realistic dynamic names; ensure you require or reference Faker consistently with other factories and add uniqueness if other tests expect distinct names.app/javascript/src/components/Navbar/ModernNavbar.tsx-106-111 (1)
106-111:⚠️ Potential issue | 🟡 MinorLocalize the new Analytics nav label.
The rest of this nav uses
t("nav.*"); this hardcoded label will remain English in localized sessions.Suggested change
{ - label: "Analytics", + label: t("nav.analytics"), path: Paths.ANALYTICS, icon: BarChart3, allowedRoles: ["admin", "owner", "manager", "book_keeper", "employee"],Add the corresponding locale keys with the rest of the nav translations. Based on learnings, “Follow existing Rails and React patterns in this repo.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/src/components/Navbar/ModernNavbar.tsx` around lines 106 - 111, Replace the hardcoded label "Analytics" in ModernNavbar (the nav item with path Paths.ANALYTICS and icon BarChart3) with a localized string using the existing translator call (e.g., t("nav.analytics")) and ensure you add the corresponding locale keys ("nav.analytics") to the i18n resource files used by the app (both Rails/JSON and React locale files following the repo's patterns) so translations are available for all supported languages.app/javascript/src/components/Reports/ViewInAnalyticsButton.tsx-11-18 (1)
11-18:⚠️ Potential issue | 🟡 MinorLocalize this report action label.
This button is rendered inside report pages, including localized views, so the hardcoded English label will leak into non-English sessions.
Suggested change
import React from "react"; import { ChartBar } from "phosphor-react"; import { Link } from "react-router-dom"; import { Button } from "../ui/button"; +import { t } from "../../i18n"; @@ <Link to={to}> <ChartBar className="mr-2 h-4 w-4" /> - View in Analytics + {t("reports.viewInAnalytics")} </Link>Add the matching locale keys alongside the other report translations. Based on learnings, “Follow existing Rails and React patterns in this repo.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/src/components/Reports/ViewInAnalyticsButton.tsx` around lines 11 - 18, The "View in Analytics" label is hardcoded in ViewInAnalyticsButton; replace it with the repo's existing i18n lookup used by other report components (e.g. call the same translation helper used across React components in this repo inside ViewInAnalyticsButton instead of the literal string) and add the corresponding translation key (e.g. reports.view_in_analytics or the matching key pattern used for other report labels) to the report locale files (Rails/YAML and any frontend JSON locales) so localized views render the translated label; keep the component signature (ViewInAnalyticsButton, prop to) and the existing Button/Link/ChartBar usage unchanged.app/javascript/src/components/Navbar/Sidebar.tsx-112-125 (1)
112-125:⚠️ Potential issue | 🟡 MinorNew Analytics label/description bypass i18n.
Every other entry in
mainNavItemsusesi18n.t("navbar.*")for bothlabelanddescription, but this new item hard-codes English strings. That breaks localization for all non-English users and is inconsistent with the surrounding pattern.- label: "Analytics", + label: i18n.t("navbar.analytics"), path: Paths.ANALYTICS, ... - description: "Forecasts and performance insights", + description: i18n.t("navbar.analyticsDescription"),Please add
navbar.analytics/navbar.analyticsDescriptionkeys to the locale files. The same comment applies to the Analytics entry inModernNavbar.tsxif it hard-codes strings there too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/src/components/Navbar/Sidebar.tsx` around lines 112 - 125, The Analytics nav entry in mainNavItems is hard-coded in English; replace the literal label and description with i18n lookups (e.g., i18n.t("navbar.analytics") and i18n.t("navbar.analyticsDescription")) for the item where Paths.ANALYTICS is used so it matches the surrounding entries, add those keys to all locale files (translations for navbar.analytics and navbar.analyticsDescription), and apply the same change to the matching Analytics entry in ModernNavbar.tsx if it currently hard-codes strings.db/migrate/20260418134500_add_analytics_aggregation_indexes.rb-23-31 (1)
23-31:⚠️ Potential issue | 🟡 MinorAdd
algorithm: :concurrentlyto allremove_indexcalls indownmethod.The
upmethod correctly usesdisable_ddl_transaction!+algorithm: :concurrentlyto avoid write locks during concurrent index creation. Thedownmethod must similarly usealgorithm: :concurrentlyon eachremove_indexcall to preventACCESS EXCLUSIVEtable locks during rollback in production, since concurrent index drops are only possible when not within a transaction.Example fix for first removal:
- remove_index :invoices, name: "index_invoices_on_company_issue_date_status" if index_exists?(:invoices, name: "index_invoices_on_company_issue_date_status") + remove_index :invoices, name: "index_invoices_on_company_issue_date_status", algorithm: :concurrently if index_exists?(:invoices, name: "index_invoices_on_company_issue_date_status")Apply this pattern to all seven
remove_indexcalls (lines 24–30).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@db/migrate/20260418134500_add_analytics_aggregation_indexes.rb` around lines 23 - 31, The down method's remove_index calls (e.g., remove_index :invoices, name: "index_invoices_on_company_issue_date_status") must include algorithm: :concurrently to avoid ACCESS EXCLUSIVE locks; update all seven remove_index invocations in the down method to pass algorithm: :concurrently and ensure the migration uses disable_ddl_transaction! so these concurrent drops run outside a transaction.app/javascript/src/constants/routes.ts-243-247 (1)
243-247:⚠️ Potential issue | 🟡 MinorUse
Roles.MANAGERinstead of the string literal"manager".Line 170 destructures
{ ADMIN, OWNER, BOOK_KEEPER, EMPLOYEE, CLIENT }fromRoles, butMANAGERis not included. Line 246 then uses the string literal"manager"instead of the constant. This creates an inconsistency: every other role inROUTESand throughout the codebase (Sidebar.tsx line 119, Analytics components, etc.) uses theRolesenum. If the enum's canonical value ever changes in casing or spelling, the string literal would silently become stale, potentially allowing managers to see the sidebar entry but be denied at the route level, or vice versa.Add
MANAGERto the destructuring at line 170 and replace"manager"with the constant:Diff
-const { ADMIN, OWNER, BOOK_KEEPER, EMPLOYEE, CLIENT } = Roles; +const { ADMIN, OWNER, BOOK_KEEPER, EMPLOYEE, CLIENT, MANAGER } = Roles; ... - authorisedRoles: [ADMIN, OWNER, BOOK_KEEPER, EMPLOYEE, "manager"], + authorisedRoles: [ADMIN, OWNER, BOOK_KEEPER, EMPLOYEE, MANAGER],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/src/constants/routes.ts` around lines 243 - 247, The routes entry uses the string literal "manager" instead of the Roles constant and MANAGER was not destructured from Roles; update the destructuring that currently pulls { ADMIN, OWNER, BOOK_KEEPER, EMPLOYEE, CLIENT } from Roles to also include MANAGER, and change the authorisedRoles array for the Analytics route to use the MANAGER constant (i.e., the destructured MANAGER) instead of the string "manager" so the route uses the canonical Roles value.spec/services/analytics/demo_seeder_spec.rb-9-36 (1)
9-36:⚠️ Potential issue | 🟡 MinorIdempotency assertion filters by a different column than the creation assertion.
In the first example, client creation is verified via
name LIKE 'Analytics Demo%'; in the idempotency example, the clients count usesemail LIKE 'analytics.demo.%'. If the seeder ever changes one convention but not the other, the idempotency guard silently passes against zero rows. Recommend aligning both to the same filter (and adding a non-zero sanity check) so a regression in naming can't make the idempotency test vacuous:Proposed tightening
- first_counts = { - clients: company.clients.where("email LIKE ?", "analytics.demo.%").count, + first_counts = { + clients: company.clients.where("name LIKE ?", "Analytics Demo%").count, projects: company.projects.count, invoices: company.invoices.where("invoice_number LIKE ?", "AN-DEMO-%").count, expenses: company.expenses.where("description LIKE ?", "Analytics demo expense %").count } + expect(first_counts.values).to all(be > 0) described_class.new(company: company).process - expect(company.clients.where("email LIKE ?", "analytics.demo.%").count).to eq(first_counts[:clients]) + expect(company.clients.where("name LIKE ?", "Analytics Demo%").count).to eq(first_counts[:clients])Also worth noting:
projectsandpaymentshave no duplicate-safety assertion in the second example — if demo seeding is intended to be fully idempotent, consider covering them too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/services/analytics/demo_seeder_spec.rb` around lines 9 - 36, The idempotency test uses a different client filter than the creation assertion which can make the guard vacuous; update the idempotency block that calls described_class.new(company: company).process so the client count uses the same predicate as the creation test (use "name LIKE 'Analytics Demo%'" instead of "email LIKE 'analytics.demo.%'") and add a non-zero sanity expectation immediately after the first process call to ensure at least one demo record exists; also extend the idempotency comparisons to include projects and payments (compare project and payment counts before and after the second described_class.new(...).process) so duplicate-safety covers those entities as well.app/javascript/src/components/Analytics/components/AnalyticsInsights.tsx-41-47 (1)
41-47:⚠️ Potential issue | 🟡 MinorPlural form produces "anomalyies".
The template literal concatenates
anomaly+ies, yielding the misspelling "anomalyies" whenanomalyCount !== 1. Use full words instead of a suffix hack.🔧 Proposed fix
- description: `${anomalyCount} expense anomaly${anomalyCount === 1 ? " was" : "ies were"} detected in the selected period.`, + description: `${anomalyCount} expense ${anomalyCount === 1 ? "anomaly was" : "anomalies were"} detected in the selected period.`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/src/components/Analytics/components/AnalyticsInsights.tsx` around lines 41 - 47, The description string in the insights.push block incorrectly forms the plural as "anomalyies"; update the conditional to use full words instead of appending a suffix—use `${anomalyCount} ${anomalyCount === 1 ? "anomaly was" : "anomalies were"} detected in the selected period.` (locate the insights.push where anomalyCount is used in AnalyticsInsights.tsx and replace the current template literal with this full-word conditional).app/javascript/src/components/Invoices/InvoicePreview/ClientAnalyticsSummary.tsx-66-144 (1)
66-144:⚠️ Potential issue | 🟡 MinorInconsistent i18n: most strings are hardcoded.
The empty-state uses
i18n.t("reports.noResults"), but the card title and every metric label ("Client analytics", "Total revenue", "Average invoice", "Payment cycle", "Payment frequency", "Recent trend", "days") are inlined English. Either translate them all viai18n.t(...)or keep them hardcoded — don’t mix. This also matters because the user’s locale is respected elsewhere in the app.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/src/components/Invoices/InvoicePreview/ClientAnalyticsSummary.tsx` around lines 66 - 144, Replace the hardcoded English labels in the ClientAnalyticsSummary component with i18n lookups to match the empty-state usage (i18n.t("reports.noResults")); specifically change the CardTitle "Client analytics" and the metric labels "Total revenue", "Average invoice", "Payment cycle", "Payment frequency", "Recent trend" and the unit "days" to i18n.t(...) calls using appropriate translation keys (e.g., invoices.clientAnalytics.* or reports.*) so all strings use the same localization approach; update the span showing client.trend_direction to use i18n.t for the displayed direction key as well to ensure consistent locale output.app/javascript/src/components/Invoices/InvoicePreview/ClientAnalyticsSummary.tsx-128-138 (1)
128-138:⚠️ Potential issue | 🟡 MinorGuard
.toFixedagainst missing numeric fields.If the backend ever returns
null/undefinedforpayment_cycle_daysorpayment_frequency_days(e.g., client with no payments in the window),.toFixed(2)throws and breaks the invoice preview. A small coercion keeps the card resilient.🛡️ Proposed fix
- {client.payment_cycle_days.toFixed(2)} days + {Number(client.payment_cycle_days ?? 0).toFixed(2)} days ... - {client.payment_frequency_days.toFixed(2)} days + {Number(client.payment_frequency_days ?? 0).toFixed(2)} days🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/src/components/Invoices/InvoicePreview/ClientAnalyticsSummary.tsx` around lines 128 - 138, The component ClientAnalyticsSummary is calling client.payment_cycle_days.toFixed(2) and client.payment_frequency_days.toFixed(2) without guarding for null/undefined; update those usages to coerce or fallback to a safe numeric default before calling toFixed (e.g., use Number(client.payment_cycle_days ?? 0) or a conditional that renders "—"/"N/A" when the value is missing), ensuring the display still shows a sensible value without throwing when payment_cycle_days or payment_frequency_days is null/undefined.app/javascript/src/components/Invoices/InvoicePreview/ClientAnalyticsSummary.tsx-44-45 (1)
44-45:⚠️ Potential issue | 🟡 MinorUTC dates in analytics window causing potential off-by-one for users west of UTC.
new Date().toISOString().slice(0, 10)yields the UTC calendar day. For users west of UTC (e.g., PST, CST), local midnight converts to the previous UTC day—shifting the "last 365 days" window by one day and producing off-by-one analytics.Use
format(date, "yyyy-MM-dd")from date-fns instead, which uses the local timezone. Alternatively, useformatInTimeZonefromdate-fns-tzif explicit timezone handling is needed.Note: This pattern exists in other analytics pages (RevenueForecastPage, TeamAnalyticsPage, ClientInsightsPage, ExpenseTrendsPage) and should be fixed consistently across the analytics module.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/src/components/Invoices/InvoicePreview/ClientAnalyticsSummary.tsx` around lines 44 - 45, The date range uses UTC via new Date().toISOString().slice(0, 10) which causes off-by-one days for users west of UTC; update the from and to assignments in ClientAnalyticsSummary.tsx to use date-fns' local formatter (call format(new Date(), "yyyy-MM-dd") and format(subDays(new Date(), 365), "yyyy-MM-dd")) and add the corresponding import for format from 'date-fns'; apply the same replacement pattern for the other analytics modules (RevenueForecastPage, TeamAnalyticsPage, ClientInsightsPage, ExpenseTrendsPage) where variables like from/to are set so the window uses local dates rather than UTC.app/javascript/src/components/Analytics/components/AnalyticsSavedReports.tsx-124-132 (1)
124-132:⚠️ Potential issue | 🟡 MinorConfirm before deleting a saved report.
This is a destructive, one-click action with no undo. Add a confirmation dialog or reuse an existing destructive-action pattern before calling
deleteMutation.mutate(report.id).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/src/components/Analytics/components/AnalyticsSavedReports.tsx` around lines 124 - 132, The Delete button currently calls deleteMutation.mutate(report.id) directly; change this to require explicit confirmation before mutating by replacing the direct call in the Button's onClick with code that opens a confirmation dialog (or reuses your app's destructive-action modal) and only calls deleteMutation.mutate(report.id) if the user confirms; locate the Button that renders when report.can_delete || Number(report.created_by_id) === Number(user.id), add state to show/hide the confirmation (e.g., showDeleteConfirm), render a confirmation modal component or a window.confirm prompt, and move the deleteMutation.mutate(report.id) call into the confirmation handler so deletion happens only after the user confirms.app/services/analytics/exports/download_service.rb-49-71 (1)
49-71:⚠️ Potential issue | 🟡 MinorDon’t prepend summary-only headers to table exports.
When
export_payload[:tables]is present, the CSV still returns["Metric", "Value"]as the global header while also embedding each table’s own headers inrows. That produces mixed-column CSVs with a misleading first row. Prefer section headers inrows, or generate one consistent table per export format.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/services/analytics/exports/download_service.rb` around lines 49 - 71, In csv_content, stop emitting the global headers ["Metric", "Value"] when export_payload[:tables] exists: detect presence of export_payload[:tables] early in csv_content and only set headers = ["Metric", "Value"] when there are no tables; when tables are present set headers to nil (or an empty array) and rely on the section/table headers already being inserted into rows via export_payload[:tables] so the CSV doesn't mix a misleading global header with per-table columns—update the logic around the headers variable and the first_table conditional in csv_content to reflect this.app/services/analytics/cache_store.rb-11-17 (1)
11-17:⚠️ Potential issue | 🟡 MinorTreat only
nilas a cache miss.
present?makes valid cached values like{},[], orfalserecompute on every fetch. Use a nil check so the cache faithfully returns stored values.Suggested fix
def fetch(key, expires_in: DEFAULT_EXPIRY) cached_value = read(key) - return cached_value if cached_value.present? + return cached_value unless cached_value.nil? value = yield write(key, value, expires_in:) value end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/services/analytics/cache_store.rb` around lines 11 - 17, The fetch method in Analytics::CacheStore uses present? which treats {}, [], and false as cache misses; change the cache miss check in fetch(key, expires_in: DEFAULT_EXPIRY) from cached_value.present? to an explicit nil check (e.g., return cached_value unless cached_value.nil?) so only nil triggers recomputation; keep the existing write(key, value, expires_in:) and value return behavior intact.app/controllers/internal_api/v1/analytics_controller.rb-74-80 (1)
74-80:⚠️ Potential issue | 🟡 MinorUse
Integer()for strict horizon parsing to reject malformed input.
String#to_isilently accepts malformed values like"3months"as3, allowing invalid input to bypass validation. UseInteger(params[:horizon], 10)and rescueArgumentErrorto enforce strict parsing.Note: The same vulnerable pattern exists in
app/controllers/internal_api/v1/analytics_exports_controller.rb(line 96) and should be fixed consistently.Suggested fix
def validated_horizon! - horizon = params[:horizon].presence&.to_i || 3 + horizon = params[:horizon].present? ? Integer(params[:horizon], 10) : 3 return horizon if SUPPORTED_HORIZONS.include?(horizon) render_validation_error("horizon must be one of #{SUPPORTED_HORIZONS.join(', ')}") nil +rescue ArgumentError + render_validation_error("horizon must be one of #{SUPPORTED_HORIZONS.join(', ')}") + nil end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/internal_api/v1/analytics_controller.rb` around lines 74 - 80, The validated_horizon! method currently uses params[:horizon].to_i which silently accepts malformed values (e.g. "3months"); replace that parsing with strict Integer parsing: call Integer(params[:horizon], 10) inside validated_horizon! and rescue ArgumentError to render_validation_error("horizon must be one of ...") and return nil on parse failure, otherwise proceed to check SUPPORTED_HORIZONS; apply the same change to the corresponding method in analytics_exports_controller (the validated_horizon! implementation around line 96) so both controllers enforce strict integer parsing and reject malformed input.app/controllers/internal_api/v1/analytics_exports_controller.rb-95-101 (1)
95-101:⚠️ Potential issue | 🟡 MinorReject malformed
horizonvalues instead of coercing them.
to_iwill turn"3months"into3, so a request likeparams[:horizon]="3months"passes validation as a supported horizon. UseInteger(...)for strict parsing and returnnilafter rendering to avoid continuing execution after the response is sent.Suggested fix
def validated_horizon! - horizon = params[:horizon].presence&.to_i || 3 + horizon = params[:horizon].present? ? Integer(params[:horizon], 10) : 3 return horizon if SUPPORTED_HORIZONS.include?(horizon) render_validation_error("horizon must be one of #{SUPPORTED_HORIZONS.join(', ')}") - 3 + nil +rescue ArgumentError + render_validation_error("horizon must be one of #{SUPPORTED_HORIZONS.join(', ')}") + nil endCurrent specs test unsupported integers (e.g.,
horizon: 9) but not malformed input like"3months"or"3abc".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/internal_api/v1/analytics_exports_controller.rb` around lines 95 - 101, The method validated_horizon! currently coerces malformed strings via to_i (so "3months" becomes 3); change it to strictly parse with Integer(params[:horizon]) (rescuing ArgumentError/TypeError) and only accept the parsed value if SUPPORTED_HORIZONS.include?(horizon). If parsing fails or the value is unsupported, call render_validation_error with the same message and return nil immediately to stop further execution; update validated_horizon! accordingly.app/services/analytics/demo_seeder.rb-229-241 (1)
229-241:⚠️ Potential issue | 🟡 MinorPartial-payment branch does not update
outstanding_amount, producing inconsistent invoice state.The paid branch uses
invoice.settle!(payment)which presumably normalizesamount_paid/amount_due/outstanding_amount. The partial branch updates onlyamount_paidandamount_dueand leavesoutstanding_amountat whatever it was initialized to (0), so invoice analytics that rely onoutstanding_amountwill understate AR for these demo invoices.♻️ Suggested fix
- invoice.update!(amount_paid: partial_amount, amount_due: invoice.amount.to_f - partial_amount) + outstanding = invoice.amount.to_f - partial_amount + invoice.update!( + amount_paid: partial_amount, + amount_due: outstanding, + outstanding_amount: outstanding + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/services/analytics/demo_seeder.rb` around lines 229 - 241, The partial-payment branch updates only amount_paid and amount_due on the Invoice (via invoice.update!) but leaves outstanding_amount stale, causing inconsistent state; after creating the Payment in the partial branch, either call the same normalization used in the paid branch (invoice.settle!(payment)) to recalculate amount_paid/amount_due/outstanding_amount, or explicitly update outstanding_amount (e.g., set outstanding_amount = invoice.amount.to_f - invoice.amount_paid) in the same transaction so invoices created in demo_seeder.rb have the same normalized fields as those handled by settle!; modify the partial branch to perform one of these fixes immediately after Payment.create! and before incrementing counts[:payments].app/javascript/src/components/Analytics/index.tsx-70-75 (1)
70-75:⚠️ Potential issue | 🟡 MinorMake manager access consistent with the dashboard copy.
Line 73 includes managers in the financial role set, but Lines 167-168 say financial analytics are only for owners, admins, and book keepers. Update the copy or the role list so managers get a consistent experience.
Also applies to: 162-169
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/src/components/Analytics/index.tsx` around lines 70 - 75, The roles array used to compute isFinancialRole includes Roles.MANAGER but the UI copy states financial analytics are only for Owners, Admins, and Book Keepers; make them consistent by removing Roles.MANAGER from the isFinancialRole definition (the array in the isFinancialRole constant) so it only lists Roles.ADMIN, Roles.OWNER, Roles.BOOK_KEEPER, and verify any related conditional text/rendering (the financial analytics copy and its conditional checks around the same component) matches this same role set; alternatively, if you intend Managers to have access, update the copy text to include "Managers" everywhere the financial-analytics access message appears.app/javascript/src/components/Analytics/ClientInsightsPage.tsx-66-71 (1)
66-71:⚠️ Potential issue | 🟡 MinorAlign manager access with the restricted-state copy.
Line 69 treats
Roles.MANAGERas financial, so managers can load/export client revenue analytics, but Lines 164-165 say access is limited to owners, admins, and book keepers. Either remove managers from the role gate or include them in the copy.Also applies to: 160-166
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/src/components/Analytics/ClientInsightsPage.tsx` around lines 66 - 71, The role-check treats Roles.MANAGER as a financial role but the UI copy restricts access to only owners, admins, and book keepers; update them to match: either remove Roles.MANAGER from the isFinancialRole array (in the declaration named isFinancialRole that uses Roles.ADMIN, Roles.OWNER, Roles.MANAGER, Roles.BOOK_KEEPER) so managers cannot load/export analytics, or keep Roles.MANAGER in that array and update the user-facing copy strings that enumerate allowed roles (the text block around the client revenue access message currently listing owners, admins, and book keepers) to include "managers" as well—make the change consistently for the initial role check and the other copy/use of the same permission message referenced around the client revenue section.app/javascript/src/components/Analytics/RevenueForecastPage.tsx-87-92 (1)
87-92:⚠️ Potential issue | 🟡 MinorAlign manager access with the restricted message.
Line 90 allows managers to fetch and export revenue forecasts, but Lines 203-204 say the data is only available to owners, admins, and book keepers. Please make the role gate and copy agree.
Also applies to: 199-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/src/components/Analytics/RevenueForecastPage.tsx` around lines 87 - 92, The role gate currently includes Roles.MANAGER but the UI copy restricts access to only owners, admins, and book keepers; remove Roles.MANAGER from the isFinancialRole array so the check in RevenueForecastPage.tsx matches the text, and ensure any other role checks that refer to this same permission (e.g., export/fetch handlers that use isFinancialRole or duplicate checks around the export button) are using the updated isFinancialRole; also verify the user-facing string that says "only available to owners, admins, and book keepers" remains unchanged and aligns with the updated role array.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c81c305c-f5b5-49b1-ae31-42237485adf6
📒 Files selected for processing (97)
AGENTS.mdapp/controllers/api/v1/analytics/revenue_forecasts_controller.rbapp/controllers/internal_api/v1/analytics_controller.rbapp/controllers/internal_api/v1/analytics_exports_controller.rbapp/controllers/internal_api/v1/analytics_reports_controller.rbapp/javascript/src/apis/api.tsapp/javascript/src/components/Analytics/ClientInsightsPage.tsxapp/javascript/src/components/Analytics/ExpenseTrendsPage.tsxapp/javascript/src/components/Analytics/RevenueForecastPage.tsxapp/javascript/src/components/Analytics/TeamAnalyticsPage.tsxapp/javascript/src/components/Analytics/components/AnalyticsExportActions.tsxapp/javascript/src/components/Analytics/components/AnalyticsFilters.tsxapp/javascript/src/components/Analytics/components/AnalyticsInsights.tsxapp/javascript/src/components/Analytics/components/AnalyticsPageLayout.tsxapp/javascript/src/components/Analytics/components/AnalyticsSavedReports.tsxapp/javascript/src/components/Analytics/components/AnalyticsStates.tsxapp/javascript/src/components/Analytics/components/ClientRevenueHeatmap.tsxapp/javascript/src/components/Analytics/index.tsxapp/javascript/src/components/Analytics/savedReports.tsapp/javascript/src/components/Analytics/types.tsapp/javascript/src/components/Analytics/useAnalyticsFilters.tsapp/javascript/src/components/Analytics/utils.tsapp/javascript/src/components/Dashboard/DashboardLayout.tsxapp/javascript/src/components/Invoices/InvoicePreview/ClientAnalyticsSummary.tsxapp/javascript/src/components/Invoices/InvoicePreview/index.tsxapp/javascript/src/components/Navbar/ModernNavbar.tsxapp/javascript/src/components/Navbar/Sidebar.tsxapp/javascript/src/components/Navbar/utils.tsxapp/javascript/src/components/Reports/PaymentReport/index.tsxapp/javascript/src/components/Reports/RevenueByClientReport/index.tsxapp/javascript/src/components/Reports/TimeEntryReport/index.tsxapp/javascript/src/components/Reports/ViewInAnalyticsButton.tsxapp/javascript/src/constants/index.tsxapp/javascript/src/constants/routes.tsapp/jobs/analytics/refresh_cached_reports_job.rbapp/jobs/analytics/threshold_notifications_job.rbapp/mailers/analytics_mailer.rbapp/models/analytics_report.rbapp/models/company.rbapp/models/expense.rbapp/models/project.rbapp/models/user.rbapp/policies/analytics_policy.rbapp/policies/analytics_report_policy.rbapp/policies/application_policy.rbapp/services/analytics/cache_store.rbapp/services/analytics/client_revenue_analytics.rbapp/services/analytics/comparative_analysis_service.rbapp/services/analytics/demo_seeder.rbapp/services/analytics/expense_trend_analyzer.rbapp/services/analytics/exports/download_service.rbapp/services/analytics/exports/formatter.rbapp/services/analytics/exports/pdf_service.rbapp/services/analytics/query_service.rbapp/services/analytics/revenue_forecast_service.rbapp/services/analytics/scope_resolver.rbapp/services/analytics/team_productivity_analytics.rbapp/services/analytics/threshold_evaluator.rbapp/services/analytics/tracking_service.rbapp/services/pdf_generation/base_service.rbapp/views/analytics_mailer/threshold_alert.html.erbapp/views/analytics_mailer/threshold_alert.text.erbapp/views/pdfs/analytics_export.html.erbconfig/routes/api.rbconfig/solid_queue.ymldb/migrate/20260418130000_create_analytics_reports.rbdb/migrate/20260418130500_add_project_to_expenses.rbdb/migrate/20260418134500_add_analytics_aggregation_indexes.rbdb/schema.rbdocs/analytics.mdh origin developlib/tasks/analytics_demo.rakespec/factories/analytics_reports.rbspec/jobs/analytics/refresh_cached_reports_job_spec.rbspec/jobs/analytics/threshold_notifications_job_spec.rbspec/mailers/analytics_mailer_spec.rbspec/models/analytics_report_spec.rbspec/policies/analytics_policy_spec.rbspec/policies/analytics_report_policy_spec.rbspec/requests/api/v1/analytics/revenue_forecasts/index_spec.rbspec/requests/internal_api/v1/analytics_controller_spec.rbspec/requests/internal_api/v1/analytics_exports_controller_spec.rbspec/requests/internal_api/v1/analytics_reports_controller_spec.rbspec/services/analytics/client_revenue_analytics_spec.rbspec/services/analytics/comparative_analysis_service_spec.rbspec/services/analytics/demo_seeder_spec.rbspec/services/analytics/expense_trend_analyzer_spec.rbspec/services/analytics/query_service_spec.rbspec/services/analytics/revenue_forecast_service_spec.rbspec/services/analytics/scope_resolver_spec.rbspec/services/analytics/team_productivity_analytics_spec.rbspec/services/analytics/threshold_evaluator_spec.rbspec/system/analytics/index_spec.rbspec/system/analytics/saved_reports_spec.rbspec/system/invoices/details_spec.rbspec/system/navigation_spec.rbspec/system/reports/index_spec.rb
| onRetry={() => { | ||
| comparisonQuery.refetch(); | ||
| teamQuery.refetch(); | ||
| clientQuery.refetch(); | ||
| expenseQuery.refetch(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In TanStack Query React, does calling refetch on a query with enabled: false execute the queryFn?
💡 Result:
Yes, calling refetch on a query with enabled: false executes the queryFn.
Citations:
- 1: https://tanstack.com/query/latest/docs/framework/react/guides/disabling-queries
- 2: https://tanstack.com/query/v5/docs/framework/react/guides/disabling-queries
- 3: https://tanstack.com/query/v4/docs/framework/react/guides/disabling-queries
- 4: https://react-query.tanstack.com/guides/disabling-queries
🏁 Script executed:
# First, let's check the file and the exact code at lines 179-183
cat -n app/javascript/src/components/Analytics/index.tsx | sed -n '175,190p'Repository: saeloun/miru-web
Length of output: 726
🏁 Script executed:
# Find all query definitions in this file to understand the enabled pattern
rg "Query|refetch|enabled" app/javascript/src/components/Analytics/index.tsx -B 2 -A 2Repository: saeloun/miru-web
Length of output: 4425
🏁 Script executed:
# Search for isFinancialRole definition in the file
rg "isFinancialRole" app/javascript/src/components/Analytics/index.tsx -B 2 -A 2Repository: saeloun/miru-web
Length of output: 839
🏁 Script executed:
# Get more context around isFinancialRole definition to see the complete list
rg "const isFinancialRole" app/javascript/src/components/Analytics/index.tsx -A 10Repository: saeloun/miru-web
Length of output: 382
🏁 Script executed:
# Verify teamQuery definition to confirm it has no enabled restriction
rg "const teamQuery" app/javascript/src/components/Analytics/index.tsx -A 8Repository: saeloun/miru-web
Length of output: 305
🏁 Script executed:
# Run the build check as specified in coding guidelines
rtk mise exec -- timeout 30 bin/vite buildRepository: saeloun/miru-web
Length of output: 103
Guard financial query refetches with role check.
The financial queries are disabled with enabled: isFinancialRole, but the retry handler calls refetch() unconditionally on all queries. According to TanStack React Query documentation, calling refetch() on a disabled query executes the queryFn regardless of the enabled flag. This allows non-financial users (who lack ADMIN, OWNER, MANAGER, or BOOK_KEEPER roles) to trigger restricted endpoint calls when retrying after an analytics error.
Wrap the financial query refetches in a role check:
Proposed retry guard
onRetry={() => {
- comparisonQuery.refetch();
teamQuery.refetch();
- clientQuery.refetch();
- expenseQuery.refetch();
+
+ if (isFinancialRole) {
+ comparisonQuery.refetch();
+ clientQuery.refetch();
+ expenseQuery.refetch();
+ }
}}📝 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.
| onRetry={() => { | |
| comparisonQuery.refetch(); | |
| teamQuery.refetch(); | |
| clientQuery.refetch(); | |
| expenseQuery.refetch(); | |
| onRetry={() => { | |
| teamQuery.refetch(); | |
| if (isFinancialRole) { | |
| comparisonQuery.refetch(); | |
| clientQuery.refetch(); | |
| expenseQuery.refetch(); | |
| } | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/javascript/src/components/Analytics/index.tsx` around lines 179 - 183,
The retry handler currently calls comparisonQuery.refetch(),
teamQuery.refetch(), clientQuery.refetch(), and expenseQuery.refetch()
unconditionally; because clientQuery and expenseQuery are created with enabled:
isFinancialRole you must guard their refetches so non-financial users cannot
trigger restricted endpoints. Modify the onRetry handler in the component to
always refetch comparisonQuery and teamQuery but only call clientQuery.refetch()
and expenseQuery.refetch() when isFinancialRole is true (check the
isFinancialRole boolean used when creating those queries).
| const isEmployee = companyRole === Roles.EMPLOYEE; | ||
| const isManager = companyRole === Roles.MANAGER; | ||
| const currency = company?.base_currency || "USD"; | ||
|
|
||
| const optionsQuery = useQuery({ | ||
| queryKey: ["analytics", "team-options"], | ||
| queryFn: async () => { | ||
| const response = await teamApi.get("page=1&items=1000"); | ||
|
|
||
| return unmapList(response).map(member => ({ | ||
| id: Number(member.id), | ||
| label: member.name, | ||
| })) as AnalyticsOption[]; | ||
| }, | ||
| enabled: !isEmployee && !isManager, | ||
| }); | ||
|
|
||
| const query = useQuery({ | ||
| queryKey: ["analytics", "team", from, to, selectedIds], | ||
| queryFn: async () => { | ||
| const response = await analyticsApi.getTeamProductivity({ | ||
| from, | ||
| to, | ||
| view_context: "team_productivity", | ||
| user_ids: selectedIds.length > 0 ? selectedIds.join(",") : undefined, |
There was a problem hiding this comment.
Gate member filters with the same role check everywhere.
The UI hides member selection for employees/managers, but URL or saved-report selectedIds still flow into the analytics query, export payload, and saved filters. Use a single canFilterMembers/effectiveSelectedIds value so hidden filters cannot affect restricted-role requests.
Proposed fix
const isEmployee = companyRole === Roles.EMPLOYEE;
const isManager = companyRole === Roles.MANAGER;
+ const canFilterMembers = !isEmployee && !isManager;
+ const effectiveSelectedIds = canFilterMembers ? selectedIds : [];
const currency = company?.base_currency || "USD";
const optionsQuery = useQuery({
@@
- enabled: !isEmployee && !isManager,
+ enabled: canFilterMembers,
});
const query = useQuery({
- queryKey: ["analytics", "team", from, to, selectedIds],
+ queryKey: ["analytics", "team", from, to, effectiveSelectedIds],
queryFn: async () => {
const response = await analyticsApi.getTeamProductivity({
from,
to,
view_context: "team_productivity",
- user_ids: selectedIds.length > 0 ? selectedIds.join(",") : undefined,
+ user_ids:
+ effectiveSelectedIds.length > 0
+ ? effectiveSelectedIds.join(",")
+ : undefined,
});
@@
user_ids:
- !isEmployee && selectedIds.length > 0
- ? selectedIds.join(",")
+ effectiveSelectedIds.length > 0
+ ? effectiveSelectedIds.join(",")
: undefined,
@@
- selectedIds={isEmployee ? [] : selectedIds}
- onSelectedIdsChange={isEmployee || isManager ? undefined : setSelectedIds}
- options={isEmployee || isManager ? [] : optionsQuery.data || []}
- multiSelectLabel={isEmployee || isManager ? undefined : "Team members"}
+ selectedIds={effectiveSelectedIds}
+ onSelectedIdsChange={canFilterMembers ? setSelectedIds : undefined}
+ options={canFilterMembers ? optionsQuery.data || [] : []}
+ multiSelectLabel={canFilterMembers ? "Team members" : undefined}
@@
- ...(isEmployee
- ? {}
- : { members: selectedIds.length > 0 ? selectedIds.join(",") : undefined }),
+ ...(canFilterMembers
+ ? {
+ members:
+ effectiveSelectedIds.length > 0
+ ? effectiveSelectedIds.join(",")
+ : undefined,
+ }
+ : {}),Also applies to: 109-112, 146-149, 321-323
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/javascript/src/components/Analytics/TeamAnalyticsPage.tsx` around lines
61 - 85, The member filters are currently hidden for employees/managers but
selectedIds still flow into requests (optionsQuery,
query/analyticsApi.getTeamProductivity, export and saved filters), so add a
single gate variable (e.g., canFilterMembers) computed from companyRole (reuse
isEmployee/isManager logic) and derive an effectiveSelectedIds when building
requests; update optionsQuery.enabled, the queryFn payload (user_ids), export
payload builder, and saved-filter serialization to use effectiveSelectedIds
instead of selectedIds so hidden filters cannot affect restricted-role requests.
| const labels = trends[0].monthly_totals.map(point => point.label); | ||
|
|
||
| return labels.map((label, index) => | ||
| trends.reduce( | ||
| (row, trend) => ({ | ||
| ...row, | ||
| label, | ||
| [trend.name]: trend.monthly_totals[index]?.amount || 0, | ||
| }), | ||
| { label } as Record<string, string | number> | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Align trend series by label instead of array index.
Using trends[0] for labels and monthly_totals[index] for every other trend shifts values when any series has missing/reordered months, so the chart can show one month’s spend under another month’s label.
Proposed label-based alignment
- const labels = trends[0].monthly_totals.map(point => point.label);
+ const labels = Array.from(
+ new Set(
+ trends.flatMap(trend => trend.monthly_totals.map(point => point.label))
+ )
+ );
return labels.map((label, index) =>
trends.reduce(
- (row, trend) => ({
- ...row,
- label,
- [trend.name]: trend.monthly_totals[index]?.amount || 0,
- }),
+ (row, trend) => {
+ const point = trend.monthly_totals.find(point => point.label === label);
+
+ return {
+ ...row,
+ label,
+ [trend.name]: point?.amount ?? 0,
+ };
+ },
{ label } as Record<string, string | number>
)
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/javascript/src/components/Analytics/utils.ts` around lines 64 - 75, The
current logic builds labels from trends[0] and picks amounts by index causing
misalignment when series have missing or reordered months; change the
implementation to compute the union of all monthly_totals labels across trends
(e.g., collect label keys from each trend.monthly_totals into a Set), then map
over that unified list and for each trend look up the amount by label (build a
Map or object for trend.monthly_totals keyed by point.label) instead of using
monthly_totals[index], producing rows with { label, [trend.name]: amount || 0 }
so every series is aligned by label (update the code that defines labels and the
reduce that builds rows).
| { | ||
| logo: <ReportsIcon className="mr-0 md:mr-4" size={26} />, | ||
| label: "Analytics", | ||
| path: Paths.ANALYTICS, | ||
| allowedRoles: ["admin", "owner", "manager", "book_keeper", "employee"], | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C3 'MobileMenuOptions\b' app/javascript/src
rg -nP -C2 'from=\{|to=\{' app/javascript/src/components/NavbarRepository: saeloun/miru-web
Length of output: 10910
🏁 Script executed:
#!/bin/bash
# Check the navOptions definition including the Analytics entry and surrounding entries
sed -n '40,80p' app/javascript/src/components/Navbar/utils.tsx
# Check for i18n keys used in other navbar entries
rg -n 'i18n\.t.*navbar\.' app/javascript/src/components/Navbar/utils.tsx | head -20Repository: saeloun/miru-web
Length of output: 2178
🏁 Script executed:
#!/bin/bash
# Get MobileMenuOptions function implementation
sed -n '211,250p' app/javascript/src/components/Navbar/utils.tsx
# Also check the Navigation.tsx calls with full context
sed -n '34,45p' app/javascript/src/components/Navbar/Mobile/Navigation.tsx
sed -n '24,35p' app/javascript/src/components/Navbar/Mobile/MoreOptions.tsxRepository: saeloun/miru-web
Length of output: 1663
Localize the "Analytics" label and verify mobile slicing in browser.
-
The label
"Analytics"is hardcoded while all other navbar entries usei18n.t("navbar.*"). Add anavbar.analyticskey to match the pattern. -
The Analytics entry is inserted between Reports and Payments, shifting indices of subsequent items (Payments, Leaves, Expenses). The code uses
navOptions.slice(from, to)with hardcodedfrom={0}(Navigation.tsx) andfrom={4}(MoreOptions.tsx). While the slicing+filter logic should handle index shifts correctly, test the mobile navigation layout in browser to confirm top nav and "more" menu tabs display the correct items without visual glitches.
Proposed label fix
- label: "Analytics",
+ label: i18n.t("navbar.analytics"),📝 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.
| { | |
| logo: <ReportsIcon className="mr-0 md:mr-4" size={26} />, | |
| label: "Analytics", | |
| path: Paths.ANALYTICS, | |
| allowedRoles: ["admin", "owner", "manager", "book_keeper", "employee"], | |
| }, | |
| { | |
| logo: <ReportsIcon className="mr-0 md:mr-4" size={26} />, | |
| label: i18n.t("navbar.analytics"), | |
| path: Paths.ANALYTICS, | |
| allowedRoles: ["admin", "owner", "manager", "book_keeper", "employee"], | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/javascript/src/components/Navbar/utils.tsx` around lines 59 - 64, Replace
the hardcoded "Analytics" label in the navOptions entry with the localized key
i18n.t("navbar.analytics") and add the corresponding navbar.analytics string to
the translation files so it matches the pattern used by other entries; then
verify the mobile rendering in the browser because inserting Analytics shifts
indices used by Navigation.tsx and MoreOptions.tsx (they use
navOptions.slice(from, to) with from props) to ensure the top nav and the "more"
menu still show the intended items with no visual glitches.
| alerts.each do |alert| | ||
| next if recently_notified?(company.id, alert[:type]) | ||
|
|
||
| AnalyticsMailer.with( | ||
| company_id: company.id, | ||
| recipients: recipients, | ||
| alert: alert | ||
| ).threshold_alert.deliver_later | ||
|
|
||
| mark_notified(company.id, alert[:type]) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect threshold notification dedupe implementation and cache store configuration references.
# Expectation: dedupe should use an atomic claim or a durable unique record, not read-then-write.
rg -n -C4 'recently_notified\?|mark_notified|claim_notification|unless_exist|threshold_notification|cache_store' app config specRepository: saeloun/miru-web
Length of output: 6203
🏁 Script executed:
# Find the FALLBACK_CACHE definition and any configuration
rg -n 'FALLBACK_CACHE\|NOTIFICATION_TTL' app/jobs/analytics/threshold_notifications_job.rbRepository: saeloun/miru-web
Length of output: 42
🏁 Script executed:
# Check if there are tests for threshold_notifications_job to understand expected behavior
fd -t f '*threshold*' spec --exec grep -l 'ThresholdNotificationsJob\|recently_notified' {} \;Repository: saeloun/miru-web
Length of output: 350
🏁 Script executed:
# Verify the full threshold_notifications_job.rb file to understand context better
cat -n app/jobs/analytics/threshold_notifications_job.rbRepository: saeloun/miru-web
Length of output: 2246
Make notification deduplication atomic.
recently_notified? + mark_notified is a check-then-write race condition. Two concurrent job executions can both observe a cache miss and enqueue duplicate emails for the same company/alert type.
The current implementation uses MemoryStore (in-process only) which does not provide atomic operations across multiple processes. In production with multiple job processes or containers, this will reliably produce duplicates.
Replace with an atomic operation or use a database-backed uniqueness guard:
Database-backed approach (recommended)
Create a table to track notification state:
# db/migrate/...create_threshold_notification_logs.rb
create_table :threshold_notification_logs do |t|
t.bigint :company_id, null: false
t.string :alert_type, null: false
t.timestamp :notified_at, null: false
t.timestamps
t.index [:company_id, :alert_type], unique: true
t.foreign_key :companies, column: :company_id
endThen use insert_or_skip to claim atomically:
def claim_notification!(company_id, alert_type)
ThresholdNotificationLog.insert(
{ company_id:, alert_type:, notified_at: Time.current },
unique_by: [:company_id, :alert_type]
)
rescue ActiveRecord::RecordNotUnique
false
endAlternatively, switch to a process-safe atomic cache store (Redis) if available, and use unless_exist: true when writing the notification marker. Avoid relying on MemoryStore for cross-process deduplication.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/jobs/analytics/threshold_notifications_job.rb` around lines 16 - 25, The
check-then-write using recently_notified? and mark_notified is a race condition
across processes; replace it with an atomic claim. Implement a DB-backed guard
(e.g., create ThresholdNotificationLog with unique index on company_id +
alert_type and add a claim_notification! method that performs an insert_or_skip
/ insert with unique_by to atomically claim the notification before enqueuing
the mail), call claim_notification! in place of recently_notified?/mark_notified
to decide whether to send, and handle duplicate-insert failures as "already
claimed"; alternatively, if Redis is available switch mark_notified to set a key
with unless_exist/SET NX semantics and TTL so the operation is process-safe.
| rows: payload.fetch(:members, []).map do |member| | ||
| [ | ||
| member[:user_name], | ||
| member[:total_hours], | ||
| member[:billable_hours], | ||
| member[:non_billable_hours], | ||
| percentage(member[:utilization_rate]), | ||
| format_amount(member[:average_hourly_rate]) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect analytics CSV export generation for formula-injection sanitization.
# Expectation: CSV cell generation should neutralize formula-prefixed strings before writing.
fd -i 'download_service\.rb$' app/services/analytics/exports --exec sed -n '1,260p' {}
rg -n -C3 'CSV|formula|sanitize|escape|export_payload|summary_rows|tables' app/services/analytics/exports app/views/pdfs 2>/dev/null || trueRepository: saeloun/miru-web
Length of output: 14631
🏁 Script executed:
fd -i 'generatecsv\.rb$' -o 'generate_csv\.rb$' --exec cat {} \;Repository: saeloun/miru-web
Length of output: 213
🏁 Script executed:
rg -n 'class GenerateCsv' --type rbRepository: saeloun/miru-web
Length of output: 86
🏁 Script executed:
rg -n "CSV\." app/services app/lib --type rb | head -20Repository: saeloun/miru-web
Length of output: 86
🏁 Script executed:
sed -n '105,160p' app/services/analytics/exports/formatter.rbRepository: saeloun/miru-web
Length of output: 2339
🏁 Script executed:
sed -n '175,190p' app/services/analytics/exports/formatter.rbRepository: saeloun/miru-web
Length of output: 581
🏁 Script executed:
find . -type f -name '*generate*csv*' 2>/dev/nullRepository: saeloun/miru-web
Length of output: 143
🏁 Script executed:
rg -n 'class GenerateCsv' --no-headingRepository: saeloun/miru-web
Length of output: 42
🏁 Script executed:
rg -n 'CSV\.' | grep -E '(generate|write|open)' | head -15Repository: saeloun/miru-web
Length of output: 315
🏁 Script executed:
cat -n app/services/reports/generate_csv.rbRepository: saeloun/miru-web
Length of output: 522
🏁 Script executed:
sed -n '1,50p' app/services/analytics/exports/formatter.rbRepository: saeloun/miru-web
Length of output: 1923
🏁 Script executed:
rg -n 'CSV\.generate\(' --context 5Repository: saeloun/miru-web
Length of output: 1076
🏁 Script executed:
cat Gemfile | grep -i csvRepository: saeloun/miru-web
Length of output: 42
🏁 Script executed:
rg -n 'sanitize|escape' app/services/reports/generate_csv.rbRepository: saeloun/miru-web
Length of output: 42
Sanitize user-controlled data before CSV export to prevent spreadsheet formula injection.
User names, client names, and other labels sourced from the database flow directly into CSV export cells without sanitization. If a cell value begins with =, +, -, @, tab, or carriage return, opening the exported CSV in a spreadsheet application can execute arbitrary formulas.
The Reports::GenerateCsv class writes rows directly via csv << row without escaping formula-injection prefixes. Apply sanitization in the formatter (e.g., prefix suspicious values with a single quote ') or configure CSV output to quote all fields.
Affected locations: lines 77–84, 110–120, 147–154, 181–182 in app/services/analytics/exports/formatter.rb.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/services/analytics/exports/formatter.rb` around lines 77 - 84, Rows in
the CSV are written from user-controlled fields (e.g., member[:user_name] and
other labels) without sanitization, allowing spreadsheet formula injection; add
a sanitizer (e.g., sanitize_csv_cell) that prefixes values beginning with = + -
@ tab or CR/LF with a safe character like a single quote or otherwise escapes
them, and call it for every user-controlled value before pushing into row arrays
used by the formatter methods (apply to the arrays built around
member[:user_name], client/name fields, and any label-producing code paths that
use percentage and format_amount outputs), or alternatively enable quoting-all
mode in the CSV writer (Reports::GenerateCsv) and ensure sanitizer is still
applied to raw inputs; update usages where rows are constructed so sanitized
values replace the raw values.
| def build_payload | ||
| case report_type | ||
| when "revenue_forecast" | ||
| Analytics::RevenueForecastService.process(company:, horizon: filters.fetch(:horizon, 3), client_ids: filters[:client_ids]) | ||
| when "comparison" | ||
| Analytics::ComparativeAnalysisService.process(company:, from: filters.fetch(:from), to: filters.fetch(:to), user_ids: filters[:user_ids], client_ids: filters[:client_ids], project_ids: filters[:project_ids]) | ||
| when "client_analysis" | ||
| Analytics::ClientRevenueAnalytics.process(company:, from: filters.fetch(:from), to: filters.fetch(:to), client_ids: filters[:client_ids]) | ||
| when "team_productivity" | ||
| Analytics::TeamProductivityAnalytics.process(company:, from: filters.fetch(:from), to: filters.fetch(:to), user_ids: filters[:user_ids]) | ||
| when "expense_trends" | ||
| Analytics::ExpenseTrendAnalyzer.process(company:, from: filters.fetch(:from), to: filters.fetch(:to), project_ids: filters[:project_ids]) | ||
| else | ||
| raise ArgumentError, "Unsupported analytics report type: #{report_type}" | ||
| end | ||
| end |
There was a problem hiding this comment.
fetch(:from)/fetch(:to) will raise KeyError when saved reports are refreshed with incomplete filters.
AnalyticsReport#filters only validates the value is a Hash — not that from/to are present. When Analytics::RefreshCachedReportsJob iterates stored reports and calls this service, any report created without both keys (e.g., older rows, future UI regression, or a revenue_forecast report that also gets the comparison branch) will raise and kill the whole job for the day.
Either normalize defaults here (e.g., fall back to 30.days.ago.to_date / Date.current) per report type, or enforce presence in AnalyticsReport validations.
♻️ Proposed default-based fix
- when "comparison"
- Analytics::ComparativeAnalysisService.process(company:, from: filters.fetch(:from), to: filters.fetch(:to), user_ids: filters[:user_ids], client_ids: filters[:client_ids], project_ids: filters[:project_ids])
- when "client_analysis"
- Analytics::ClientRevenueAnalytics.process(company:, from: filters.fetch(:from), to: filters.fetch(:to), client_ids: filters[:client_ids])
- when "team_productivity"
- Analytics::TeamProductivityAnalytics.process(company:, from: filters.fetch(:from), to: filters.fetch(:to), user_ids: filters[:user_ids])
- when "expense_trends"
- Analytics::ExpenseTrendAnalyzer.process(company:, from: filters.fetch(:from), to: filters.fetch(:to), project_ids: filters[:project_ids])
+ when "comparison"
+ Analytics::ComparativeAnalysisService.process(company:, from: filter_from, to: filter_to, user_ids: filters[:user_ids], client_ids: filters[:client_ids], project_ids: filters[:project_ids])
+ when "client_analysis"
+ Analytics::ClientRevenueAnalytics.process(company:, from: filter_from, to: filter_to, client_ids: filters[:client_ids])
+ when "team_productivity"
+ Analytics::TeamProductivityAnalytics.process(company:, from: filter_from, to: filter_to, user_ids: filters[:user_ids])
+ when "expense_trends"
+ Analytics::ExpenseTrendAnalyzer.process(company:, from: filter_from, to: filter_to, project_ids: filters[:project_ids])
else
raise ArgumentError, "Unsupported analytics report type: #{report_type}"
end
end
+
+ def filter_from = filters[:from].presence || 30.days.ago.to_date
+ def filter_to = filters[:to].presence || Date.current📝 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.
| def build_payload | |
| case report_type | |
| when "revenue_forecast" | |
| Analytics::RevenueForecastService.process(company:, horizon: filters.fetch(:horizon, 3), client_ids: filters[:client_ids]) | |
| when "comparison" | |
| Analytics::ComparativeAnalysisService.process(company:, from: filters.fetch(:from), to: filters.fetch(:to), user_ids: filters[:user_ids], client_ids: filters[:client_ids], project_ids: filters[:project_ids]) | |
| when "client_analysis" | |
| Analytics::ClientRevenueAnalytics.process(company:, from: filters.fetch(:from), to: filters.fetch(:to), client_ids: filters[:client_ids]) | |
| when "team_productivity" | |
| Analytics::TeamProductivityAnalytics.process(company:, from: filters.fetch(:from), to: filters.fetch(:to), user_ids: filters[:user_ids]) | |
| when "expense_trends" | |
| Analytics::ExpenseTrendAnalyzer.process(company:, from: filters.fetch(:from), to: filters.fetch(:to), project_ids: filters[:project_ids]) | |
| else | |
| raise ArgumentError, "Unsupported analytics report type: #{report_type}" | |
| end | |
| end | |
| def build_payload | |
| case report_type | |
| when "revenue_forecast" | |
| Analytics::RevenueForecastService.process(company:, horizon: filters.fetch(:horizon, 3), client_ids: filters[:client_ids]) | |
| when "comparison" | |
| Analytics::ComparativeAnalysisService.process(company:, from: filter_from, to: filter_to, user_ids: filters[:user_ids], client_ids: filters[:client_ids], project_ids: filters[:project_ids]) | |
| when "client_analysis" | |
| Analytics::ClientRevenueAnalytics.process(company:, from: filter_from, to: filter_to, client_ids: filters[:client_ids]) | |
| when "team_productivity" | |
| Analytics::TeamProductivityAnalytics.process(company:, from: filter_from, to: filter_to, user_ids: filters[:user_ids]) | |
| when "expense_trends" | |
| Analytics::ExpenseTrendAnalyzer.process(company:, from: filter_from, to: filter_to, project_ids: filters[:project_ids]) | |
| else | |
| raise ArgumentError, "Unsupported analytics report type: #{report_type}" | |
| end | |
| end | |
| def filter_from = filters[:from].presence || 30.days.ago.to_date | |
| def filter_to = filters[:to].presence || Date.current |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/services/analytics/query_service.rb` around lines 32 - 47, build_payload
currently calls filters.fetch(:from) and filters.fetch(:to) which raises
KeyError when saved reports lack those keys; update build_payload in
QueryService to provide sensible defaults per report_type (e.g., for time-range
reports use from: filters.fetch(:from, 30.days.ago.to_date) and to:
filters.fetch(:to, Date.current) or other report-specific defaults) and only
require :from/:to for branches that need them (ensure revenue_forecast keeps its
horizon default). Modify the case branches for "comparison", "client_analysis",
"team_productivity", and "expense_trends" to use filters.fetch with fallback
values (or normalize filters at the start of build_payload) so
RefreshCachedReportsJob no longer errors on missing keys.
| def low_utilization? | ||
| team_summary[:utilization_rate].to_f < LOW_UTILIZATION_THRESHOLD | ||
| end |
There was a problem hiding this comment.
False-positive low-utilization alert on empty/unstaffed workspaces.
utilization_rate is 0 (or falls back to 0 via .to_f) when the company has no active team members for the window. That unconditionally fires a “Low utilization detected” alert and the paired email from AnalyticsMailer#threshold_alert — noisy for new workspaces, workspaces on leave, etc. Guard on a minimum team_size before emitting the alert.
🛡️ Proposed guard
def low_utilization?
- team_summary[:utilization_rate].to_f < LOW_UTILIZATION_THRESHOLD
+ team_size = team_summary[:team_size].to_i
+ return false if team_size.zero?
+
+ team_summary[:utilization_rate].to_f < LOW_UTILIZATION_THRESHOLD
end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/services/analytics/threshold_evaluator.rb` around lines 28 - 30, The
low_utilization? predicate currently treats a missing/zero utilization_rate as
low and triggers alerts for empty or unstaffed workspaces; modify the method
(low_utilization?) to short-circuit and return false unless the team size meets
a minimum threshold (e.g., check team_summary[:team_size].to_i >= MIN_TEAM_SIZE
or a configurable constant) before comparing
team_summary[:utilization_rate].to_f against LOW_UTILIZATION_THRESHOLD so
empty/unstaffed teams do not generate threshold_alert emails.
| browser_path = ENV["BROWSER_PATH"] || ENV["FERRUM_BROWSER_PATH"] || ENV["GOOGLE_CHROME_SHIM"] || ENV["CHROME_PATH"] | ||
| if browser_path && !browser_path.empty? | ||
| opts[:browser_path] = browser_path |
There was a problem hiding this comment.
Don’t let a blank BROWSER_PATH block the existing fallbacks.
Line 46 selects "" before checking emptiness, so a blank BROWSER_PATH prevents fallback to FERRUM_BROWSER_PATH, GOOGLE_CHROME_SHIM, or CHROME_PATH.
🐛 Proposed fix
- browser_path = ENV["BROWSER_PATH"] || ENV["FERRUM_BROWSER_PATH"] || ENV["GOOGLE_CHROME_SHIM"] || ENV["CHROME_PATH"]
- if browser_path && !browser_path.empty?
+ browser_path = [
+ ENV["BROWSER_PATH"],
+ ENV["FERRUM_BROWSER_PATH"],
+ ENV["GOOGLE_CHROME_SHIM"],
+ ENV["CHROME_PATH"]
+ ].find(&:present?)
+
+ if browser_path
opts[:browser_path] = browser_path
end📝 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.
| browser_path = ENV["BROWSER_PATH"] || ENV["FERRUM_BROWSER_PATH"] || ENV["GOOGLE_CHROME_SHIM"] || ENV["CHROME_PATH"] | |
| if browser_path && !browser_path.empty? | |
| opts[:browser_path] = browser_path | |
| browser_path = [ | |
| ENV["BROWSER_PATH"], | |
| ENV["FERRUM_BROWSER_PATH"], | |
| ENV["GOOGLE_CHROME_SHIM"], | |
| ENV["CHROME_PATH"] | |
| ].find(&:present?) | |
| if browser_path | |
| opts[:browser_path] = browser_path | |
| end |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/services/pdf_generation/base_service.rb` around lines 46 - 48, Current
logic assigns browser_path using the first ENV key even if it is an empty
string, blocking fallbacks; change the assignment for browser_path to iterate
the ENV values (BROWSER_PATH, FERRUM_BROWSER_PATH, GOOGLE_CHROME_SHIM,
CHROME_PATH) and pick the first non-nil, non-empty value (e.g., use Array#find
with a presence check) before setting opts[:browser_path] so blank BROWSER_PATH
does not prevent using other fallbacks.
| add_column :expenses, :project_id, :bigint | ||
| add_index :expenses, :project_id, algorithm: :concurrently | ||
| add_index :expenses, [:company_id, :date, :project_id], | ||
| algorithm: :concurrently, | ||
| name: "index_expenses_on_company_date_project" |
There was a problem hiding this comment.
Add a foreign key for expenses.project_id.
The new analytics filters depend on project-linked expenses; without a database constraint, orphaned project_id values can silently corrupt report results.
Suggested change
def change
add_column :expenses, :project_id, :bigint
add_index :expenses, :project_id, algorithm: :concurrently
add_index :expenses, [:company_id, :date, :project_id],
algorithm: :concurrently,
name: "index_expenses_on_company_date_project"
+ add_foreign_key :expenses, :projects, validate: false
+ validate_foreign_key :expenses, :projects
end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@db/migrate/20260418130500_add_project_to_expenses.rb` around lines 7 - 11,
Add a foreign key constraint for expenses.project_id to prevent orphaned project
references: update the migration that calls add_column :expenses, :project_id
(and the subsequent add_index calls) to also add a foreign key constraint
referencing the projects table (e.g., via add_foreign_key :expenses, :projects,
column: :project_id, name: 'fk_expenses_project_id'); ensure the constraint name
is unique and compatible with existing indexes and consider any needed
nullability or on_delete behavior while applying the change.
1 participant
Summary by CodeRabbit
New Features