feat(floware): change to OAuth flow from DWD for cron job#291
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughGmail auth switched from service-account delegation to OAuth (client_id/secret + refresh_token). ScheduledJobService now produces per-recipient XLSX reports with conditional column styling and per-recipient RLS; client hooks/types/UI, DI, and pyproject deps updated. ChangesGmail OAuth Migration
Scheduled Job XLSX + Recipient RLS
Sequence DiagramsequenceDiagram
participant DI as ApplicationContainer
participant Scheduler as ScheduledJobService
participant UserSvc as UserService
participant RoleRepo as RoleRepository
participant Plugin as DatasourcePlugin
participant Storage as CloudStorage
participant EmailSvc as EmailService
DI->>Scheduler: provide user_service, role_repository, user_role_repository, email_service
Scheduler->>UserSvc: load recipient metadata / associations
Scheduler->>RoleRepo: check admin roles for recipient
Scheduler->>Plugin: execute dynamic query with per-recipient RLS filter
Plugin->>Scheduler: return rows
Scheduler->>Scheduler: generate XLSX bytes and apply column styles (openpyxl)
Scheduler->>Storage: upload bytes when larger than attachment limit
Storage->>Scheduler: return presigned URL
Scheduler->>EmailSvc: send individualized email with attachment or download link
EmailSvc->>Scheduler: return delivery result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wavefront/client/src/pages/apps/[appId]/datasources/ScheduleEmailAlertDialog.tsx (1)
286-301:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve existing payload fields on update.
This update path reconstructs
payloadfrom a subset of form fields, so editing an existing job drops persisted keys likefilter,offset, andlimit.ScheduledJobService._execute_email_dynamic_query_job()still consumes those fields, so a no-op edit can widen the exported dataset or change the row count.Suggested fix
if (editingJobId) { + const existingPayload = ((jobs.find((job) => job.id === editingJobId)?.payload ?? + {}) as Record<string, unknown>); await floConsoleService.scheduledJobService.updateScheduledJob(editingJobId, { cron_expr: cronExpr.trim(), timezone: timezone.trim(), max_retries: retries, payload: { + ...existingPayload, datasource_id: datasourceId, query_id: queryId, recipient_user_ids: selectedRecipientUserIds, subject: subject.trim() || undefined, email_content: emailContent.trim() || undefined,🤖 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 `@wavefront/client/src/pages/apps/`[appId]/datasources/ScheduleEmailAlertDialog.tsx around lines 286 - 301, The update path for floConsoleService.scheduledJobService.updateScheduledJob(editingJobId, {...payload...}) rebuilds payload from form fields and drops persisted keys like filter/offset/limit; fetch the existing scheduled job (or its payload) first, shallow-merge its payload with the new payload object so any fields not present in the form are preserved, and then call updateScheduledJob with the merged payload (i.e., keep existingPayload = await floConsoleService.scheduledJobService.getScheduledJob(editingJobId) or similar, mergedPayload = {...existingPayload.payload, ...newPayloadFields}, and pass mergedPayload to updateScheduledJob) to ensure _execute_email_dynamic_query_job() still receives filter/offset/limit and other persisted keys.
🤖 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
`@wavefront/client/src/pages/apps/`[appId]/datasources/ScheduleEmailAlertDialog.tsx:
- Around line 265-279: The code currently accepts any JSON array as
parsedColumnStyles (in ScheduleEmailAlertDialog) and casts it to
ColumnStyleConfig[]; instead validate each entry before accepting: iterate
parsedColumnStyles (after JSON.parse) and ensure rule.operator is one of
'eq','neq','lt','lte','gt','gte','between' and for non-'between' rules
rule.value is a finite number and for 'between' rules rule.min and rule.max are
finite numbers (and min<=max); if any entry fails, call setError('Column styles
contain invalid rules'), setActiveTab('email') and return so malformed rules are
rejected instead of silently ignored. Ensure validation happens where
parsedColumnStyles is assigned/used and before any save/submit logic that sends
ColumnStyleConfig to the backend.
In `@wavefront/server/apps/floware/floware/services/scheduled_job_service.py`:
- Around line 737-817: The current try/except only wraps RLS and row fetch so
downstream failures (XLSX generation, cloud upload, presigned URL, send_email)
escape and cause full-job retry/duplicate sends; wrap the entire per-recipient
processing (start at calling _rls_filter_for_user and including
_fetch_dynamic_query_rows, _rows_to_xlsx_bytes,
cloud_storage_manager.save_small_file,
cloud_storage_manager.generate_presigned_url, building body/attachments, and
email_service.send_email) in a single try/except that catches Exception as exc,
logs the user-specific failure (including user_id and query_id), appends to
failed_recipient_user_ids, and continues the loop without re-raising so the job
only fails/raises downstream when every recipient is in
failed_recipient_user_ids (preserve existing delivered_count increment on
success).
- Around line 534-545: _in _resolve_fill_styles, the branch that handles
'#'‑prefixed colors currently strips the '#' into bg_hex without validating;
update that branch to validate the stripped value (bg_hex) is exactly 6 hex
characters (0-9A-F/a-f) before uppercasing and assigning, and if it fails
validation return (None, None) like the other invalid branch; ensure this same
validation logic is used consistently with the bare‑6-char branch so PatternFill
creation won't receive invalid bg_hex/font_hex values (references: function
_resolve_fill_styles, variables bg_hex and font_hex, and constants
COLUMN_FILL_COLORS / COLUMN_FILL_FONT_COLORS).
---
Outside diff comments:
In
`@wavefront/client/src/pages/apps/`[appId]/datasources/ScheduleEmailAlertDialog.tsx:
- Around line 286-301: The update path for
floConsoleService.scheduledJobService.updateScheduledJob(editingJobId,
{...payload...}) rebuilds payload from form fields and drops persisted keys like
filter/offset/limit; fetch the existing scheduled job (or its payload) first,
shallow-merge its payload with the new payload object so any fields not present
in the form are preserved, and then call updateScheduledJob with the merged
payload (i.e., keep existingPayload = await
floConsoleService.scheduledJobService.getScheduledJob(editingJobId) or similar,
mergedPayload = {...existingPayload.payload, ...newPayloadFields}, and pass
mergedPayload to updateScheduledJob) to ensure
_execute_email_dynamic_query_job() still receives filter/offset/limit and other
persisted keys.
🪄 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: d585fd44-f1cb-447c-a5d7-5cad441f9248
⛔ Files ignored due to path filters (1)
wavefront/server/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
wavefront/client/src/pages/apps/[appId]/datasources/ScheduleEmailAlertDialog.tsxwavefront/client/src/types/scheduled-job.tswavefront/server/apps/floware/floware/di/application_container.pywavefront/server/apps/floware/floware/server.pywavefront/server/apps/floware/floware/services/scheduled_job_service.pywavefront/server/apps/floware/pyproject.toml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
wavefront/client/src/api/app-user-service.ts (1)
25-27: ⚡ Quick win
listAppUsers()’s:appIdplaceholder is already substituted by the Axios request interceptor
wavefront/client/src/api/app-user-service.tssends/v1/:appId/floware/v1/users, andwavefront/client/src/lib/axios.tsreplaces:appIdinconfig.urlusingwindow.location.pathname.split('/')[2]before the request is issued.- Interceptor behavior depends on the current path matching
/apps/:appId/...(otherwise it may replace with'').🤖 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 `@wavefront/client/src/api/app-user-service.ts` around lines 25 - 27, The request URL in listAppUsers() relies on an Axios interceptor in lib/axios.ts to replace the :appId token, which is fragile if the current path doesn't match /apps/:appId/; make listAppUsers() build the URL explicitly by extracting appId = window.location.pathname.split('/')[2] (with a safe fallback) and using `/v1/${appId}/floware/v1/users` so the service no longer depends on the interceptor's runtime replacement; update listAppUsers() accordingly and keep the interceptor as-is for other callers.
🤖 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.
Nitpick comments:
In `@wavefront/client/src/api/app-user-service.ts`:
- Around line 25-27: The request URL in listAppUsers() relies on an Axios
interceptor in lib/axios.ts to replace the :appId token, which is fragile if the
current path doesn't match /apps/:appId/; make listAppUsers() build the URL
explicitly by extracting appId = window.location.pathname.split('/')[2] (with a
safe fallback) and using `/v1/${appId}/floware/v1/users` so the service no
longer depends on the interceptor's runtime replacement; update listAppUsers()
accordingly and keep the interceptor as-is for other callers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cc7abf0b-d69a-4f7d-b22d-54d7df93f71c
📒 Files selected for processing (9)
wavefront/client/src/api/app-user-service.tswavefront/client/src/hooks/data/fetch-hooks.tswavefront/client/src/hooks/data/mutation-hooks.tswavefront/client/src/hooks/data/query-functions.tswavefront/client/src/hooks/data/query-keys.tswavefront/client/src/pages/apps/[appId]/datasources/ScheduleEmailAlertDialog.tsxwavefront/client/src/pages/apps/[appId]/datasources/[datasourceId].tsxwavefront/client/src/pages/apps/users/index.tsxwavefront/server/apps/floware/floware/services/scheduled_job_service.py
🚧 Files skipped from review as they are similar to previous changes (2)
- wavefront/server/apps/floware/floware/services/scheduled_job_service.py
- wavefront/client/src/pages/apps/[appId]/datasources/ScheduleEmailAlertDialog.tsx
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
wavefront/client/src/pages/apps/[appId]/scheduled-jobs/ScheduledJobFormDialog.tsx (2)
546-559: 💤 Low valueSearch won't match users in the "Selected" group.
The
valuefor selected items isselected-${user.id}-${user.email}, so typing a user's name intoCommandInputfilters them out of the Selected group (cmdk matches againstvalue). Including the name in the value keeps selected users searchable.🤖 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 `@wavefront/client/src/pages/apps/`[appId]/scheduled-jobs/ScheduledJobFormDialog.tsx around lines 546 - 559, The Selected group items use a value of `selected-${user.id}-${user.email}` so cmdk search doesn't match a user's display name; update the CommandItem value for items in ScheduledJobFormDialog's selectedRecipientUsers group to include the user's display name (use the same name used by formatUserLabel) so typing the name in CommandInput will match—modify the `value` prop on the CommandItem(s) that call toggleRecipientUser to include the name along with id/email.
95-98: ⚡ Quick winSelected query IDs absent from the loaded yamls become invisible yet are still re-saved.
selectedQueryIdsis populated from the job payload on edit, but the selectable chips render onlyavailableQueryIds(derived from current yamls). If a previously-scheduled query's yaml was renamed/deleted, that ID stays in state and is re-submitted on save, but the user cannot see or deselect it. Consider surfacing such "stale" selections (e.g., render selected IDs not present inavailableQueryIdsas removable chips).Also applies to: 360-388
🤖 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 `@wavefront/client/src/pages/apps/`[appId]/scheduled-jobs/ScheduledJobFormDialog.tsx around lines 95 - 98, availableQueryIds is derived from yamls but selectedQueryIds (from the job payload) can contain IDs absent from yamls, causing them to be invisible yet still saved; update the logic that computes and renders query chips so it shows the union of availableQueryIds and selectedQueryIds: change the useMemo (availableQueryIds) or the chip-rendering code to build a combined list (available ∪ selected), mark items not present in availableQueryIds as "stale" or visually distinct, and ensure those stale chips are removable (handle removal by updating selectedQueryIds state). Also ensure the save path reads the current selectedQueryIds state so removing a stale chip prevents it from being re-submitted (affecting the same render area that currently handles chips around selectedQueryIds).wavefront/client/src/assets/icons/scheduled-jobs-icon.tsx (1)
1-2: ⚡ Quick winImport
SVGPropsfromreactto avoid relying on an ambientReacttype namespaceWith the modern JSX transform you don’t need
Reactfor JSX runtime, butReact.SVGProps<SVGSVGElement>is still a TypeScript type lookup that depends on the project havingReacttypes/namespaces available. ImportingSVGPropsmakes the icon independent of that config-sensitive ambient behavior.Suggested change
+import type { SVGProps } from 'react'; + -const ScheduledJobsIcon = ({ ...props }: React.SVGProps<SVGSVGElement>) => ( +const ScheduledJobsIcon = ({ ...props }: SVGProps<SVGSVGElement>) => (🤖 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 `@wavefront/client/src/assets/icons/scheduled-jobs-icon.tsx` around lines 1 - 2, The component ScheduledJobsIcon currently types its props using the ambient React namespace; import the SVGProps type from 'react' and use it explicitly (e.g., change the props type to SVGProps<SVGSVGElement>) so the component no longer relies on an ambient React type namespace—update the component signature (ScheduledJobsIcon) and add the import statement for SVGProps from 'react'.
🤖 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 `@wavefront/client/src/hooks/data/query-functions.ts`:
- Around line 416-418: getScheduledJobsQueryFn currently truncates results by
calling floConsoleService.scheduledJobService.listScheduledJobs({ limit: 200 })
and returning only the first page; change it to fetch all pages (either via
cursor/nextToken or offset-based pagination supported by listScheduledJobs) by
iteratively calling floConsoleService.scheduledJobService.listScheduledJobs
until no more pages, concatenating response.data.data.jobs into a single
ScheduledJob[] and returning that full array; ensure you respect the service's
pagination fields (e.g., nextCursor/hasMore, or offset/limit) and keep an
overall guard/upper bound to avoid endless loops.
In `@wavefront/client/src/pages/apps/`[appId]/scheduled-jobs/index.tsx:
- Around line 191-196: The status filter is missing a "completed" option even
though statusBadgeClass includes a completed style; add a SelectItem with
value="completed" and label "Completed" inside the SelectContent list (
alongside SelectItem value="all", "active", "paused", "running", "failed" ) so
the filter value matches the statusBadgeClass key and completed jobs can be
filtered directly.
In
`@wavefront/client/src/pages/apps/`[appId]/scheduled-jobs/ScheduledJobFormDialog.tsx:
- Around line 199-230: The validation in handleSave incorrectly treats empty or
whitespace maxRetries as 0 because retries is derived with Number(maxRetries);
update handleSave to first check that maxRetries.trim() is a non-empty numeric
string (e.g., /^\d+$/) before converting, and only then parse it to an integer;
if the string is empty, whitespace, or non-integer, call setError('Max retries
must be an integer between 0 and 10') and setActiveTab('schedule') to reject the
save. Ensure you reference the existing maxRetries variable and retries logic in
handleSave so the empty-string case no longer silently coerces to 0.
In `@wavefront/server/apps/floware/floware/services/scheduled_job_service.py`:
- Around line 1124-1135: The code currently materializes HTML for every prepared
report via query_tables and _rows_to_html_table even for reports delivered as
download links; change this so you only render full HTML for reports that will
be embedded in the email body and produce a lightweight summary/link for reports
listed in download_reports. Concretely: when building query_tables, iterate
prepared_reports but skip or replace rows for any r['query_id'] present in
download_reports (or if the template does not reference query_tables) and
instead set a compact placeholder (e.g., a summary string or link metadata);
only call _rows_to_html_table for reports you intend to embed. Keep passing
query_tables into _build_report_email_body so it can render embedded tables vs.
links appropriately.
- Around line 172-185: The new filter only checks
scheduled_job.payload->'queries' array and breaks old single-query payloads;
update the query_id WHERE clause in scheduled_job_service (and the other similar
places) to accept both shapes by adding a dual-read: keep the existing
EXISTS(jsonb_array_elements(...) WHERE elem->>'query_id' = :query_id) OR check
the legacy scalar path scheduled_job.payload->>'query_id' = :query_id, bind the
same :query_id param, and ensure the same logic is applied in the other
occurrences (the blocks around the other similar query_id filters);
alternatively, add/run a migration/backfill to convert legacy payloads to the
new queries array before rollout.
- Around line 403-418: In _normalize_query_specs, add validation to reject
duplicate query_id values in payload.queries: while iterating queries in
_normalize_query_specs(payload: dict) maintain a seen set of query_ids and if a
query_id repeats raise a ValueError (include the duplicate id and its index in
the message); this prevents collisions later in pipeline components that key
state by query_id (e.g., yaml_by_query_id, query_tables, filenames/report keys)
and ensures each entry in specs is unique.
---
Nitpick comments:
In `@wavefront/client/src/assets/icons/scheduled-jobs-icon.tsx`:
- Around line 1-2: The component ScheduledJobsIcon currently types its props
using the ambient React namespace; import the SVGProps type from 'react' and use
it explicitly (e.g., change the props type to SVGProps<SVGSVGElement>) so the
component no longer relies on an ambient React type namespace—update the
component signature (ScheduledJobsIcon) and add the import statement for
SVGProps from 'react'.
In
`@wavefront/client/src/pages/apps/`[appId]/scheduled-jobs/ScheduledJobFormDialog.tsx:
- Around line 546-559: The Selected group items use a value of
`selected-${user.id}-${user.email}` so cmdk search doesn't match a user's
display name; update the CommandItem value for items in ScheduledJobFormDialog's
selectedRecipientUsers group to include the user's display name (use the same
name used by formatUserLabel) so typing the name in CommandInput will
match—modify the `value` prop on the CommandItem(s) that call
toggleRecipientUser to include the name along with id/email.
- Around line 95-98: availableQueryIds is derived from yamls but
selectedQueryIds (from the job payload) can contain IDs absent from yamls,
causing them to be invisible yet still saved; update the logic that computes and
renders query chips so it shows the union of availableQueryIds and
selectedQueryIds: change the useMemo (availableQueryIds) or the chip-rendering
code to build a combined list (available ∪ selected), mark items not present in
availableQueryIds as "stale" or visually distinct, and ensure those stale chips
are removable (handle removal by updating selectedQueryIds state). Also ensure
the save path reads the current selectedQueryIds state so removing a stale chip
prevents it from being re-submitted (affecting the same render area that
currently handles chips around selectedQueryIds).
🪄 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: c634f1af-9d39-4af2-b86b-1c1bb8cea53a
📒 Files selected for processing (16)
wavefront/client/src/api/scheduled-job-service.tswavefront/client/src/assets/icons/index.tswavefront/client/src/assets/icons/scheduled-jobs-icon.tsxwavefront/client/src/hooks/data/fetch-hooks.tswavefront/client/src/hooks/data/query-functions.tswavefront/client/src/hooks/data/query-keys.tswavefront/client/src/pages/apps/[appId]/datasources/ScheduleEmailAlertDialog.tsxwavefront/client/src/pages/apps/[appId]/datasources/Yamls.tsxwavefront/client/src/pages/apps/[appId]/datasources/[datasourceId].tsxwavefront/client/src/pages/apps/[appId]/scheduled-jobs/ScheduledJobFormDialog.tsxwavefront/client/src/pages/apps/[appId]/scheduled-jobs/index.tsxwavefront/client/src/pages/apps/[appId]/scheduled-jobs/scheduled-job-utils.tswavefront/client/src/pages/apps/layout.tsxwavefront/client/src/router/routes.tsxwavefront/client/src/types/scheduled-job.tswavefront/server/apps/floware/floware/services/scheduled_job_service.py
💤 Files with no reviewable changes (1)
- wavefront/client/src/pages/apps/[appId]/datasources/ScheduleEmailAlertDialog.tsx
✅ Files skipped from review due to trivial changes (1)
- wavefront/client/src/assets/icons/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- wavefront/client/src/hooks/data/query-keys.ts
- wavefront/client/src/hooks/data/fetch-hooks.ts
Comment on lines
+199
to
+230
| const handleSave = async () => { | ||
| const retries = Number(maxRetries); | ||
| if (!datasourceId.trim()) { | ||
| setError('Datasource is required'); | ||
| setActiveTab('schedule'); | ||
| return; | ||
| } | ||
| if (selectedQueryIds.length === 0) { | ||
| setError('Select at least one dynamic query'); | ||
| setActiveTab('schedule'); | ||
| return; | ||
| } | ||
| if (!cronExpr.trim()) { | ||
| setError('Cron expression is required'); | ||
| setActiveTab('schedule'); | ||
| return; | ||
| } | ||
| if (!timezone.trim()) { | ||
| setError('Timezone is required'); | ||
| setActiveTab('schedule'); | ||
| return; | ||
| } | ||
| if (selectedRecipientUserIds.length === 0) { | ||
| setError('At least one recipient user is required'); | ||
| setActiveTab('email'); | ||
| return; | ||
| } | ||
| if (!Number.isInteger(retries) || retries < 0 || retries > 10) { | ||
| setError('Max retries must be an integer between 0 and 10'); | ||
| setActiveTab('schedule'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Empty/whitespace maxRetries silently coerces to 0.
Number('') and Number(' ') evaluate to 0, which passes the Number.isInteger(retries) && retries >= 0 check. Clearing the field saves a job with zero retries rather than surfacing a validation error.
Proposed guard
- const handleSave = async () => {
- const retries = Number(maxRetries);
+ const handleSave = async () => {
+ const trimmedRetries = maxRetries.trim();
+ const retries = Number(trimmedRetries);
...
- if (!Number.isInteger(retries) || retries < 0 || retries > 10) {
+ if (!trimmedRetries || !Number.isInteger(retries) || retries < 0 || retries > 10) {
setError('Max retries must be an integer between 0 and 10');
setActiveTab('schedule');
return;
}📝 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
| const handleSave = async () => { | |
| const retries = Number(maxRetries); | |
| if (!datasourceId.trim()) { | |
| setError('Datasource is required'); | |
| setActiveTab('schedule'); | |
| return; | |
| } | |
| if (selectedQueryIds.length === 0) { | |
| setError('Select at least one dynamic query'); | |
| setActiveTab('schedule'); | |
| return; | |
| } | |
| if (!cronExpr.trim()) { | |
| setError('Cron expression is required'); | |
| setActiveTab('schedule'); | |
| return; | |
| } | |
| if (!timezone.trim()) { | |
| setError('Timezone is required'); | |
| setActiveTab('schedule'); | |
| return; | |
| } | |
| if (selectedRecipientUserIds.length === 0) { | |
| setError('At least one recipient user is required'); | |
| setActiveTab('email'); | |
| return; | |
| } | |
| if (!Number.isInteger(retries) || retries < 0 || retries > 10) { | |
| setError('Max retries must be an integer between 0 and 10'); | |
| setActiveTab('schedule'); | |
| return; | |
| } | |
| const handleSave = async () => { | |
| const trimmedRetries = maxRetries.trim(); | |
| const retries = Number(trimmedRetries); | |
| if (!datasourceId.trim()) { | |
| setError('Datasource is required'); | |
| setActiveTab('schedule'); | |
| return; | |
| } | |
| if (selectedQueryIds.length === 0) { | |
| setError('Select at least one dynamic query'); | |
| setActiveTab('schedule'); | |
| return; | |
| } | |
| if (!cronExpr.trim()) { | |
| setError('Cron expression is required'); | |
| setActiveTab('schedule'); | |
| return; | |
| } | |
| if (!timezone.trim()) { | |
| setError('Timezone is required'); | |
| setActiveTab('schedule'); | |
| return; | |
| } | |
| if (selectedRecipientUserIds.length === 0) { | |
| setError('At least one recipient user is required'); | |
| setActiveTab('email'); | |
| return; | |
| } | |
| if (!trimmedRetries || !Number.isInteger(retries) || retries < 0 || retries > 10) { | |
| setError('Max retries must be an integer between 0 and 10'); | |
| setActiveTab('schedule'); | |
| return; | |
| } |
🤖 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
`@wavefront/client/src/pages/apps/`[appId]/scheduled-jobs/ScheduledJobFormDialog.tsx
around lines 199 - 230, The validation in handleSave incorrectly treats empty or
whitespace maxRetries as 0 because retries is derived with Number(maxRetries);
update handleSave to first check that maxRetries.trim() is a non-empty numeric
string (e.g., /^\d+$/) before converting, and only then parse it to an integer;
if the string is empty, whitespace, or non-integer, call setError('Max retries
must be an integer between 0 and 10') and setActiveTab('schedule') to reject the
save. Ensure you reference the existing maxRetries variable and retries logic in
handleSave so the empty-string case no longer silently coerces to 0.
thomastomy5
previously approved these changes
Jun 5, 2026
Comment on lines
+804
to
+805
| '<table border="1" cellpadding="6" cellspacing="0" ' | ||
| 'style="border-collapse:collapse;margin:12px 0;max-width:100%;">', |
thomastomy5
approved these changes
Jun 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Configuration Changes
New Features
Behavior & UI
API / Hooks