Skip to content

Conversation

@Bilguun0410
Copy link
Collaborator

@Bilguun0410 Bilguun0410 commented Sep 4, 2025

Summary by Sourcery

Introduce a full app token management feature in the settings UI and adjust various settings routes and sidebar items based on organization type.

New Features:

  • Add an Apps settings table with columns for name, expiration date, and inline actions to copy or remove tokens
  • Implement a Create Token form with Zod schema validation and GraphQL mutation to add new app tokens
  • Provide custom hooks for querying, adding, and removing app tokens using Apollo Client

Enhancements:

  • Hide the Notification settings route and sidebar entry
  • Show File Upload and Mail Config settings only for organizations of type 'os'
  • Add an Apps section to the settings sidebar and update breadcrumbs and header actions
  • Update SelectPositions badge variant to use 'secondary'

Summary by CodeRabbit

  • New Features

    • Apps settings: token management table with expiration column, create-token form with validation, copy-token and delete (with confirmation), breadcrumb + "Create App token" action, nested in-page routes for Apps and Create Token.
  • Changes

    • Apps routing supports wildcard/nested paths and is added to settings navigation.
    • File Upload and Mail Config now shown only for OS organizations.
    • Notification item removed from Account.
  • Style

    • Position badges now use secondary variant.

Important

Introduces app token management, modifies settings visibility by organization type, and updates UI components for consistency.

  • App Token Management:
    • Adds AppsSettings component for managing app tokens with columns for name and expiration date.
    • Implements CreateToken component with form validation and GraphQL mutation for adding tokens.
    • Provides useAddAppToken, useAppsTokens, and useRemoveToken hooks for token operations.
  • Settings Visibility:
    • Hides Notification settings route and sidebar entry.
    • Shows File Upload and Mail Config settings only for 'os' type organizations in SettingsRoutes.tsx and SettingsSidebar.tsx.
  • UI Enhancements:
    • Adds AppsHeader with breadcrumb navigation and "Create App token" button.
    • Updates SelectPositions badge variant to 'secondary' in SelectPositions.tsx.
  • GraphQL:
    • Adds ADD_TOKEN, REMOVE_TOKEN, and GET_APPS GraphQL operations for app token management.
  • Miscellaneous:
    • Updates SettingsPath and SettingsWorkspacePath enums to include new paths for app settings.

This description was created by Ellipsis for 338dc3d. You can customize this summary. It will automatically update as commits are pushed.

Bilguun Munkhzul added 3 commits September 4, 2025 16:36
@coderabbitai
Copy link

coderabbitai bot commented Sep 4, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds Apps token management (list, create, copy, remove) with GraphQL queries/mutations, hooks, schema and types; integrates in-page routing and UI for Apps; gates some Settings routes by organization type and adds an apps catch-all route.

Changes

Cohort / File(s) Summary
Settings routing & sidebar
frontend/core-ui/src/modules/app/components/SettingsRoutes.tsx, frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx, frontend/core-ui/src/modules/types/paths/SettingsPath.ts, frontend/core-ui/src/modules/settings/constants/data.ts
Read currentOrganizationState via useAtomValue and compute isOs; hide FileUpload/MailConfig when not OS; add Apps nav entry; add AppsCatchAll = 'apps/*' enum member; switch Apps route to catch-all; comment/remove Notification entry.
Apps page & UI components
frontend/core-ui/src/pages/settings/workspace/AppSettingsPage.tsx, frontend/core-ui/src/modules/settings/apps/components/AppsHeader.tsx, frontend/core-ui/src/modules/settings/apps/components/AppsSettings.tsx, frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx, frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx
Add AppsHeader with breadcrumb and Create action; introduce in-page Routes (index → list, create-new-app → create form); replace simple view with RecordTable-based apps list and skeleton; add CreateToken form component; add Copy and Remove action buttons with confirmation/toasts.
GraphQL documents & barrels
frontend/core-ui/src/modules/settings/apps/graphql/queries/getApps.ts, .../queries/index.ts, .../graphql/index.ts, .../mutations/addToken.ts, .../mutations/removeToken.ts, .../mutations/index.ts
Add GET_APPS query, ADD_TOKEN and REMOVE_TOKEN mutations; add mutation/query barrel files and re-export queries from graphql index.
Apps hooks
frontend/core-ui/src/modules/settings/apps/hooks/useAppsTokens.tsx, .../useAddAppToken.tsx, .../useRemoveToken.tsx, .../useCreateAppForm.tsx
New hooks: useAppsTokens (fetch apps), useAddAppToken (add token mutation), useRemoveToken (remove token with toast and refetch), and useCreateAppForm (react-hook-form + zod setup).
Schema & types
frontend/core-ui/src/modules/settings/apps/schema.ts, .../types.ts
Add CREATE_TOKEN_SCHEMA (zod) and TCreateAppForm; refactor IApp model to token-centric fields (accessToken, refreshToken, allowAllPermission, isEnabled, noExpire, createdAt: Date, expireDate: Date, userGroupId) and remove clientId/clientSecret.
UI minor style & state typing
frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx, frontend/libs/ui-modules/src/states/currentOrganizationState.ts
Change PositionBadge variant from 'default' to 'secondary'; extend CurrentOrganization type with optional type?: string.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Page as Apps Settings Page
  participant Table as Apps List (RecordTable)
  participant Form as CreateToken Form
  participant HookAdd as useAddAppToken
  participant HookRemove as useRemoveToken
  participant Apollo as Apollo Client
  participant Server

  User->>Page: open /settings/apps
  Page->>Apollo: GET_APPS (via useAppsTokens)
  Apollo-->>Table: apps data
  Table-->>User: render list

  User->>Page: navigate to create-new-app
  Page->>Form: render CreateToken
  User->>Form: submit
  Form->>HookAdd: appsAdd(ADD_TOKEN)
  HookAdd->>Apollo: mutate(ADD_TOKEN)
  Apollo->>Server: AppsAdd(...)
  Server-->>Apollo: result
  Apollo-->>HookAdd: onCompleted (toast + navigate)
  Apollo->>Table: refetch GET_APPS -> update list

  User->>Table: click remove
  Table->>HookRemove: appsRemove(REMOVE_TOKEN)
  HookRemove->>Apollo: mutate(REMOVE_TOKEN, refetchQueries:['Apps'])
  Apollo->>Server: AppsRemove(_id)
  Server-->>Apollo: success
  Apollo-->>HookRemove: onCompleted (toast)
  Apollo->>Table: refetch GET_APPS -> update list
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • Settings #221 — Overlaps changes to settings workspace routing and Apps path/enum updates.
  • update fix #52 — Touches the same SettingsRoutes file and routing/suspense rendering.
  • remove tabler #223 — Related to Tabler icon usage and IconApi import changes.

Poem

I nibble keys and hop through routes,
I copy tokens and tidy up spouts.
A little hop — a token born,
I shake my whiskers at break of morn.
Hooray! New apps in neat little rows. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately summarizes the PR’s primary changes—hiding Notification settings, gating File Upload and Mail Config by organization type, and adding app token functionality—so it is directly related to the changeset and informative for reviewers; it avoids file lists or emojis and conveys the main intent. The phrasing is slightly awkward (capitalization and punctuation) but not misleading. Overall, it gives a clear, concise signal of the PR’s purpose to a teammate scanning history.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch settings-structure

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7d327fd and 338dc3d.

📒 Files selected for processing (4)
  • frontend/core-ui/src/modules/app/components/SettingsRoutes.tsx (5 hunks)
  • frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx (2 hunks)
  • frontend/core-ui/src/modules/settings/constants/data.ts (3 hunks)
  • frontend/core-ui/src/modules/types/paths/SettingsPath.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • frontend/core-ui/src/modules/app/components/SettingsRoutes.tsx
  • frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx
  • frontend/core-ui/src/modules/types/paths/SettingsPath.ts
  • frontend/core-ui/src/modules/settings/constants/data.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: core-ui
  • GitHub Check: tourism_ui-ci
  • GitHub Check: frontline_ui-ci
  • GitHub Check: Sourcery review
  • GitHub Check: Analyze (javascript-typescript)

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

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

@sourcery-ai
Copy link

sourcery-ai bot commented Sep 4, 2025

Reviewer's Guide

This PR overhauls the Apps settings UI to support token CRUD and conditional feature visibility by organization type, refactors navigation/header components, and introduces new GraphQL hooks, types, and schema for app tokens.

File-Level Changes

Change Details Files
Implement token management in Apps settings
  • Add RecordTable view with columns for name, expiration, and actions
  • Create RemoveButton and CopyTokenButton with confirmation and clipboard logic
  • Introduce CreateToken form component with Zod validation and datepicker
  • Add GraphQL queries/mutations and React hooks (useAppsTokens, useAddAppToken, useRemoveToken, useCreateAppForm)
frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx
frontend/core-ui/src/modules/settings/apps/components/AppsSettings.tsx
frontend/core-ui/src/pages/settings/workspace/AppSettingsPage.tsx
frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx
frontend/core-ui/src/modules/settings/apps/hooks/useAppsTokens.tsx
frontend/core-ui/src/modules/settings/apps/hooks/useAddAppToken.tsx
frontend/core-ui/src/modules/settings/apps/hooks/useRemoveToken.tsx
frontend/core-ui/src/modules/settings/apps/hooks/useCreateAppForm.tsx
frontend/core-ui/src/modules/settings/apps/graphql/queries/getApps.ts
frontend/core-ui/src/modules/settings/apps/graphql/mutations/addToken.ts
frontend/core-ui/src/modules/settings/apps/graphql/mutations/removeToken.ts
frontend/core-ui/src/modules/settings/apps/schema.ts
frontend/core-ui/src/modules/settings/apps/types.ts
Enable conditional routing and hide workspace features by org type
  • Read currentOrganizationState to detect SaaS vs OS
  • Comment out Notification route and wrap FileUpload/MailConfig under OS-only guards
  • Filter sidebar nav items to exclude file-upload and mail-config for SaaS orgs
  • Add AppsCatchAll path to SettingsWorkspacePath enum
frontend/core-ui/src/modules/app/components/SettingsRoutes.tsx
frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx
frontend/core-ui/src/modules/types/paths/SettingsPath.ts
frontend/libs/ui-modules/src/states/currentOrganizationState.ts
Revise Apps header and navigation structure
  • Replace shield icon with API icon and add dynamic breadcrumb for New token
  • Add Create App token button linking to /create-new-app
  • Nest AppsSettings under Suspense with Routes for listing and create views
  • Register Apps path in SETTINGS_PATH_DATA nav array
frontend/core-ui/src/modules/settings/apps/components/AppsHeader.tsx
frontend/core-ui/src/pages/settings/workspace/AppSettingsPage.tsx
frontend/core-ui/src/modules/settings/constants/data.ts
Miscellaneous UI and utility adjustments
  • Switch date formatting import to formatDate and standardize cell rendering
  • Use toast notifications for success/error flows
  • Update SelectPositions badge variant to secondary
frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx
frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes - here's some feedback:

  • The organization-type filtering logic in SettingsRoutes and SettingsSidebar is duplicated; consider extracting it into a shared helper or custom hook to follow DRY principles.
  • This PR spans routing, UI components, GraphQL hooks, schema, and more—splitting it into smaller, focused PRs would make review and future maintenance much easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The organization-type filtering logic in SettingsRoutes and SettingsSidebar is duplicated; consider extracting it into a shared helper or custom hook to follow DRY principles.
- This PR spans routing, UI components, GraphQL hooks, schema, and more—splitting it into smaller, focused PRs would make review and future maintenance much easier.

## Individual Comments

### Comment 1
<location> `frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx:129` </location>
<code_context>
+                </Form.Item>
+              )}
+            />
+            {!noExpire && (
+              <Form.Field
+                control={control}
</code_context>

<issue_to_address>
Expire date field default value may be confusing.

Defaulting expireDate to the current date could cause confusion. Suggest prompting the user to choose an expiration date or setting a more meaningful default.

Suggested implementation:

```typescript
  const { control, handleSubmit, formState } = useForm({
    defaultValues: {
      name: "",
      expireDate: undefined, // No default, user must choose
      // ...other defaults
    },
  });

```

```typescript
            {!noExpire && (
              <Form.Field
                control={control}
                name="expireDate"
                render={({ field }) => (
                  <Form.Item className="col-span-2">
                    <Form.Label>Expiration Date</Form.Label>
                    <Form.Control>
                      <Input
                        {...field}
                        type="date"
                        placeholder="Select expiration date"
                        min={new Date().toISOString().split("T")[0]} // Prevent past dates
                      />
                    </Form.Control>
                    <Form.Message />
                  </Form.Item>
                )}
              />

```

If you have validation logic elsewhere that expects `expireDate` to always be set, you may need to update it to handle the case where the field is empty or undefined. Also, ensure that the UI clearly indicates that the user must select an expiration date.
</issue_to_address>

### Comment 2
<location> `frontend/core-ui/src/modules/settings/apps/schema.ts:12` </location>
<code_context>
+    noExpire: z.boolean().default(false),
+  })
+  .superRefine((data, ctx) => {
+    if (!data.allowAllPermission && !data.userGroupId) {
+      ctx.addIssue({
+        code: 'custom',
</code_context>

<issue_to_address>
Custom validation may not handle empty string userGroupId.

Please update the validation to also check for empty strings in userGroupId to prevent invalid input from passing.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

</Form.Item>
)}
/>
{!noExpire && (
Copy link

Choose a reason for hiding this comment

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

suggestion: Expire date field default value may be confusing.

Defaulting expireDate to the current date could cause confusion. Suggest prompting the user to choose an expiration date or setting a more meaningful default.

Suggested implementation:

  const { control, handleSubmit, formState } = useForm({
    defaultValues: {
      name: "",
      expireDate: undefined, // No default, user must choose
      // ...other defaults
    },
  });
            {!noExpire && (
              <Form.Field
                control={control}
                name="expireDate"
                render={({ field }) => (
                  <Form.Item className="col-span-2">
                    <Form.Label>Expiration Date</Form.Label>
                    <Form.Control>
                      <Input
                        {...field}
                        type="date"
                        placeholder="Select expiration date"
                        min={new Date().toISOString().split("T")[0]} // Prevent past dates
                      />
                    </Form.Control>
                    <Form.Message />
                  </Form.Item>
                )}
              />

If you have validation logic elsewhere that expects expireDate to always be set, you may need to update it to handle the case where the field is empty or undefined. Also, ensure that the UI clearly indicates that the user must select an expiration date.

Copy link
Collaborator

Choose a reason for hiding this comment

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

fix

Copy link

Choose a reason for hiding this comment

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

Thanks for the quick update!

Currently, the default value for expireDate in your form hook is still set to new Date(), which means the expiration date will default to today unless the user changes it. Your schema validation does require the user to set an expiration date when noExpire is false, and the UI uses a DatePicker that defaults to today if no value is set.

If you want to ensure users actively choose an expiration date (rather than accepting today's date by default), you could set the default to undefined and rely on your validation and UI to prompt the user. Otherwise, your current setup is functional and will prevent empty expiration dates when required.

Let me know if you'd like further suggestions or a more detailed review of the updated logic!

noExpire: z.boolean().default(false),
})
.superRefine((data, ctx) => {
if (!data.allowAllPermission && !data.userGroupId) {
Copy link

Choose a reason for hiding this comment

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

issue: Custom validation may not handle empty string userGroupId.

Please update the validation to also check for empty strings in userGroupId to prevent invalid input from passing.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Caution

Changes requested ❌

Reviewed everything up to b4eb468 in 1 minute and 54 seconds. Click for details.
  • Reviewed 905 lines of code in 23 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx:317
  • Draft comment:
    Confirm that changing the PositionBadge variant from 'default' to 'secondary' aligns with the intended UI design.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the PR author to confirm their intention regarding a UI change. It doesn't provide a specific suggestion or point out a potential issue, but rather asks for confirmation of intent, which violates the rules.
2. frontend/libs/ui-modules/src/states/currentOrganizationState.ts:3
  • Draft comment:
    Document the acceptable values for the 'type' property in CurrentOrganization (e.g., 'os', 'saas') to improve maintainability.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx:61
  • Draft comment:
    Ensure consistency between organization type checks: Sidebar filters out FileUpload/MailConfig for 'saas' while SettingsRoutes displays them for 'os'. Verify that this logic correctly reflects the business requirements.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
4. frontend/core-ui/src/modules/app/components/SettingsRoutes.tsx:178
  • Draft comment:
    There's a typographical error in the component name: 'PropertiesSettins' appears to be misspelled. It should likely be 'PropertiesSettings'.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_26wXolDqHFvtDQ0l

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

/>
<Route path={SettingsPath.Profile} element={<SettingsProfile />} />
<Route
{/* <Route
Copy link

Choose a reason for hiding this comment

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

Remove stale commented-out routes (e.g. Notification, Experience, General) if they are no longer needed to keep the code clean.

import { IconChevronLeft, IconPlus } from '@tabler/icons-react';
import {
Button,
Checkbox,
Copy link

Choose a reason for hiding this comment

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

Remove the unused import 'Checkbox' to avoid unnecessary code clutter.

Suggested change
Checkbox,

const CopyTokenButton = ({ cell }: { cell: Cell<IApp, unknown> }) => {
const { accessToken } = cell.row.original;
const copy = () => {
navigator.clipboard.writeText(accessToken);
Copy link

Choose a reason for hiding this comment

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

Wrap the clipboard writeText call in a try/catch block to handle possible errors from navigator.clipboard.writeText.

Copy link
Collaborator

Choose a reason for hiding this comment

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

zasah

import { IApp } from '@/settings/apps/types';
import { QueryHookOptions, useQuery } from '@apollo/client';

interface IApssResponse {
Copy link

Choose a reason for hiding this comment

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

Typo: The interface name 'IApssResponse' appears to contain a typographical error. It might be intended to be 'IAppsResponse'.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 12

♻️ Duplicate comments (1)
frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx (1)

320-321: Same variant change here

Same checks and nit as above apply.

🧹 Nitpick comments (28)
frontend/libs/ui-modules/src/states/currentOrganizationState.ts (1)

15-16: Narrow type to a literal union to prevent gating bugs.

Suggest restricting to the allowed values (e.g., 'os' | 'saas') to avoid silent UI mismatches from typos.

-  type?: string;
+  type?: 'os' | 'saas';

If additional org types exist, extend the union accordingly.

frontend/core-ui/src/modules/settings/apps/components/AppsHeader.tsx (3)

3-8: Route check: prefer useMatch over pathname.includes.

Using substring checks can produce false positives. Use useMatch('/settings/apps/create-new-app') for accuracy.

Example:

import { Link, useMatch } from 'react-router-dom';

export function AppsHeader() {
  const creating = Boolean(useMatch('/settings/apps/create-new-app'));
  

Also consider importing route constants instead of hardcoded strings to avoid drift with SettingsWorkspacePath.


21-30: Nit: JSX prop style and breadcrumb condition.

  • Use concise string props per guidelines.
-                  <Button variant={'ghost'} asChild>
+                  <Button variant="ghost" asChild>
  • If you switch to useMatch, replace the includes condition with the boolean creating.

35-40: Hide CTA on the create page to avoid redundant action.

When already on “create token”, the “Create App token” button is redundant.

-        <Button asChild>
-          <Link to="/settings/apps/create-new-app">
-            <IconPlus />
-            Create App token
-          </Link>
-        </Button>
+        {!Boolean(useMatch('/settings/apps/create-new-app')) && (
+          <Button asChild>
+            <Link to="/settings/apps/create-new-app">
+              <IconPlus />
+              Create app token
+            </Link>
+          </Button>
+        )}

Minor copy nit: prefer sentence case “Create app token”.

frontend/core-ui/src/modules/settings/apps/graphql/mutations/removeToken.ts (1)

3-9: Prefer ID! for _id and add error handling to useRemoveToken.

  • Change the mutation signature in removeToken.ts from $_id: String! to $_id: ID! if your GraphQL schema supports it.
  • In useRemoveToken.tsx, you’re already using refetchQueries: ['Apps'], but add an onError callback (or wrap the mutation call in a try/catch) to surface and handle failures gracefully.
frontend/core-ui/src/modules/settings/apps/graphql/mutations/index.ts (1)

1-2: Barrel looks good; consider surfacing mutations at the root GraphQL index for DX.

Re-exports are fine. To enable import { ADD_TOKEN } from '@/settings/apps/graphql', also re-export mutations from the root graphql/index.ts.

frontend/core-ui/src/modules/settings/apps/graphql/index.ts (1)

1-1: Export mutations here too for a single import surface.

Apply:

 export * from './queries';
+export * from './mutations';
frontend/core-ui/src/pages/settings/workspace/AppSettingsPage.tsx (1)

12-17: Ensure Spinner is visible in fallback.

If Spinner hides when show is falsy/undefined, pass it explicitly.

Apply:

-            <Spinner />
+            <Spinner show />
frontend/core-ui/src/modules/settings/apps/components/AppsSettings.tsx (3)

12-15: Drop the unsafe cast by fixing the hook’s return type.

Casting apps as IApp[] hides type issues. Make the hook return IApp[] by default so you can pass apps directly.

Apply in this file:

-        data={apps as IApp[]}
+        data={apps}

And update the hook (see comment in useAppsTokens.tsx) to default apps as IApp[].


7-7: Surface errors to users.

Expose GraphQL errors to avoid a blank table when requests fail.

-  const { apps, loading } = useAppsTokens();
+  const { apps, loading, error } = useAppsTokens();
             <RecordTable.RowList />
-            {loading && <RecordTable.RowSkeleton rows={30} />}
+            {loading && <RecordTable.RowSkeleton rows={30} />}
+            {error && (
+              <div className="text-sm text-destructive px-3 py-2">
+                Failed to load apps.
+              </div>
+            )}

Also applies to: 19-21


9-11: Use fieldset+legend for better semantics.

legend should be a child of fieldset.

-    <section className="max-w-xl w-full mx-auto">
-      <legend className="font-semibold text-lg pt-4 pb-6">Apps settings</legend>
+    <section className="max-w-xl w-full mx-auto">
+      <fieldset>
+        <legend className="font-semibold text-lg pt-4 pb-6">Apps settings</legend>
...
-      </RecordTable.Provider>
-    </section>
+      </RecordTable.Provider>
+      </fieldset>
+    </section>
frontend/core-ui/src/modules/settings/apps/hooks/useAppsTokens.tsx (1)

5-7: Fix typo and return a correctly typed default to remove downstream casts.

Rename IApssResponseIAppsResponse and default apps as IApp[].

-interface IApssResponse {
+interface IAppsResponse {
   apps: IApp[];
 }
 
-export const useAppsTokens = (options?: QueryHookOptions<IApssResponse>) => {
-  const { data, error, loading } = useQuery<IApssResponse>(GET_APPS, {
+export const useAppsTokens = (options?: QueryHookOptions<IAppsResponse>) => {
+  const { data, error, loading } = useQuery<IAppsResponse>(GET_APPS, {
     ...options,
   });
-  const apps = data?.apps || [];
+  const apps = data?.apps ?? ([] as IApp[]);
   return {
     apps,
     loading,
     error,
   };
 };

Also applies to: 9-19

frontend/core-ui/src/modules/app/components/SettingsRoutes.tsx (1)

92-98: Remove unused Notification route import to reduce bundle.

NotificationSettingsRoutes is lazily imported but the route is commented out. Drop the import while hidden.

-const NotificationSettingsRoutes = lazy(() =>
-  import('@/notification/settings/components/NotificationsRoutes').then(
-    (module) => ({
-      default: module.NotificationSettingsRoutes,
-    }),
-  ),
-);

No change needed in the commented route block.

Also applies to: 119-123

frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx (1)

71-78: Avoid double Sidebar.MenuItem wrappers.

NavigationMenuLinkItem already renders a Sidebar.MenuItem. The extra wrapper can break styling/ARIA.

-            .map((item) => (
-              <Sidebar.MenuItem key={item.name}>
-                <NavigationMenuLinkItem
-                  pathPrefix={AppPath.Settings}
-                  path={item.path}
-                  name={item.name}
-                />
-              </Sidebar.MenuItem>
-            ))}
+            .map((item) => (
+              <NavigationMenuLinkItem
+                key={item.name}
+                pathPrefix={AppPath.Settings}
+                path={item.path}
+                name={item.name}
+              />
+            ))}
frontend/core-ui/src/modules/settings/apps/hooks/useCreateAppForm.tsx (2)

9-15: Defaults currently violate schema constraints; avoid instant error state.

With allowAllPermission=false and noExpire=false by default, userGroupId and expireDate are required. userGroupId is set to an empty string, which fails validation on first blur. Prefer schema-aligned defaults to keep the initial form valid.

   defaultValues: {
     name: '',
-    noExpire: false,
-    allowAllPermission: false,
-    expireDate: new Date(),
-    userGroupId: '',
+    allowAllPermission: true,
+    noExpire: true,
+    expireDate: undefined,
+    userGroupId: undefined,
   },

Alternative: keep current defaults but switch mode to 'onSubmit' to avoid premature errors.


18-20: Return the form instance directly to reduce nesting.

useCreateAppForm() can just return the UseFormReturn instead of { methods } to simplify consumers.

-  return {
-    methods: form,
-  };
+  return form;

Update usage in CreateToken.tsx accordingly:

const methods = useCreateAppForm();
frontend/core-ui/src/modules/settings/constants/data.ts (1)

1-17: Remove unused icon import after hiding Notification.

IconBellRinging remains imported but the Notification item is commented out. Drop the import to reduce bundle size.

 import {
-  IconAdjustmentsAlt,
-  IconApi,
-  IconBellRinging,
+  IconAdjustmentsAlt,
+  IconApi,
   IconChartPie2,
   IconChessKnight,
   IconColorSwatch,
   IconFile,
   IconMail,

Also applies to: 128-132

frontend/core-ui/src/modules/settings/apps/hooks/useRemoveToken.tsx (2)

1-1: Drop unused React import.

Not used in this hook.

-import React from 'react';

3-3: Use absolute import path per guidelines.

Aligns with the rest of the module imports.

-import { REMOVE_TOKEN } from '../graphql/mutations';
+import { REMOVE_TOKEN } from '@/settings/apps/graphql/mutations';
frontend/core-ui/src/modules/settings/apps/hooks/useAddAppToken.tsx (2)

1-1: Import the listing query for safer refetch.

Use the query DocumentNode instead of the operation name string.

-import { ADD_TOKEN } from '@/settings/apps/graphql/mutations';
+import { ADD_TOKEN } from '@/settings/apps/graphql/mutations';
+import { GET_APPS } from '@/settings/apps/graphql/queries';

3-7: Type variables for the mutation.

Avoid any for variables; use your schema-generated types or a dedicated variables interface.

-import { MutationFunctionOptions, useMutation } from '@apollo/client';
+import { MutationFunctionOptions, useMutation } from '@apollo/client';
+// Example: replace `AddTokenVariables` with the generated type for ADD_TOKEN variables
+// type AddTokenVariables = { /* ... */ };
@@
-  const [mutate, { loading, error }] = useMutation<IResult>(ADD_TOKEN);
-  const handleAdd = (options?: MutationFunctionOptions<IResult, any>) => {
+  const [mutate, { loading, error }] = useMutation<IResult /*, AddTokenVariables*/>(ADD_TOKEN);
+  const handleAdd = (options?: MutationFunctionOptions<IResult, /*AddTokenVariables*/ any>) => {
frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx (4)

7-16: Remove unused imports.

Checkbox and cn are not used.

   Button,
-  Checkbox,
-  cn,
   DatePicker,
   Form,
   Input,
   Spinner,
   Switch,
   toast,

27-31: Clear hidden-field values when toggles disable them.

Avoid submitting stale userGroupId/expireDate when allowAllPermission/noExpire are turned on.

// Add below hooks
React.useEffect(() => {
  if (allowAllPermission) {
    methods.setValue('userGroupId', undefined, { shouldValidate: true });
  }
}, [allowAllPermission, methods]);

React.useEffect(() => {
  if (noExpire) {
    methods.setValue('expireDate', undefined, { shouldValidate: true });
  }
}, [noExpire, methods]);

32-43: Consider awaiting the mutation for better flow control.

If useAddAppToken returns the promise (see hook change), you can await to ensure sequential UI actions.

-  const onSubmit = (data: TCreateAppForm) => {
-    appsAdd({
+  const onSubmit = async (data: TCreateAppForm) => {
+    await appsAdd({
       variables: data,
       onCompleted: () => {
         toast({ title: 'Created a token' });
         navigate(SettingsWorkspacePath.Apps);
         reset();
       },
       onError: (error) =>
         toast({ title: error.message, variant: 'destructive' }),
     });
   };

151-154: Optionally disable submit when form is invalid.

Prevents accidental submits before satisfying schema rules.

-  <Button type="submit" className="isolate ml-auto" disabled={loading}>
+  <Button
+    type="submit"
+    className="isolate ml-auto"
+    disabled={loading || !methods.formState.isValid}
+  >
frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx (3)

14-37: Delete flow: add error handling and minor UX nits

Add onError to surface failures and small copy fix. Consider an accessible label on the icon-only button.

Apply:

   return (
     <Button
-      variant="outline"
-      size={'icon'}
+      variant="outline"
+      size="icon"
+      aria-label={`Remove app ${name}`}
       onClick={() =>
         confirm({
-          message: `Are you sure you want to remove the app '${name}'`,
+          message: `Are you sure you want to remove the app '${name}'?`,
           options: confirmOptions,
         }).then(() =>
           appsRemove({
             variables: { _id },
             onCompleted: () => {
               toast({
                 title: `App '${name}' removed`,
                 variant: 'default',
               });
             },
+            onError: (error) => {
+              toast({
+                title: `Failed to remove app '${name}'`,
+                description: error?.message,
+                variant: 'destructive',
+              });
+            },
           }),
         )
       }
     >

40-54: Clipboard: handle promise rejection and add a11y label

Use async/await with try/catch and label the icon button.

Apply:

 const CopyTokenButton = ({ cell }: { cell: Cell<IApp, unknown> }) => {
   const { accessToken } = cell.row.original;
-  const copy = () => {
-    navigator.clipboard.writeText(accessToken);
-    toast({
-      title: 'Token copied to clipboard',
-      variant: 'default',
-    });
-  };
+  const copy = async () => {
+    try {
+      await navigator.clipboard.writeText(accessToken ?? '');
+      toast({ title: 'Token copied to clipboard', variant: 'default' });
+    } catch {
+      toast({ title: 'Failed to copy token', variant: 'destructive' });
+    }
+  };
   return (
-    <Button variant="outline" size={'icon'} onClick={() => copy()}>
+    <Button variant="outline" size="icon" aria-label="Copy access token" onClick={copy}>
       <IconCopy />
     </Button>
   );
 };

81-91: Minor: add header label for actions

Improves table clarity and accessibility.

Apply:

   {
     id: 'actions',
+    header: 'Actions',
     cell: ({ cell }) => {
       return (
         <RecordTableInlineCell className="flex justify-center gap-2">
           <RemoveButton cell={cell} />
           <CopyTokenButton cell={cell} />
         </RecordTableInlineCell>
       );
     },
     size: 100,
   },
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ad5d038 and b4eb468.

📒 Files selected for processing (23)
  • frontend/core-ui/src/modules/app/components/SettingsRoutes.tsx (5 hunks)
  • frontend/core-ui/src/modules/settings/apps/components/AppsHeader.tsx (2 hunks)
  • frontend/core-ui/src/modules/settings/apps/components/AppsSettings.tsx (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/graphql/index.ts (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/graphql/mutations/addToken.ts (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/graphql/mutations/index.ts (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/graphql/mutations/removeToken.ts (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/graphql/queries/getApps.ts (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/graphql/queries/index.ts (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/hooks/useAddAppToken.tsx (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/hooks/useAppsTokens.tsx (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/hooks/useCreateAppForm.tsx (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/hooks/useRemoveToken.tsx (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/schema.ts (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/types.ts (1 hunks)
  • frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx (2 hunks)
  • frontend/core-ui/src/modules/settings/constants/data.ts (3 hunks)
  • frontend/core-ui/src/modules/types/paths/SettingsPath.ts (1 hunks)
  • frontend/core-ui/src/pages/settings/workspace/AppSettingsPage.tsx (1 hunks)
  • frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx (1 hunks)
  • frontend/libs/ui-modules/src/states/currentOrganizationState.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx}: Use functional and declarative programming patterns; avoid classes.
Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use maps instead.

Files:

  • frontend/core-ui/src/modules/settings/apps/graphql/queries/index.ts
  • frontend/libs/ui-modules/src/states/currentOrganizationState.ts
  • frontend/core-ui/src/modules/settings/apps/graphql/mutations/index.ts
  • frontend/core-ui/src/modules/settings/apps/graphql/queries/getApps.ts
  • frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx
  • frontend/core-ui/src/modules/settings/apps/graphql/mutations/addToken.ts
  • frontend/core-ui/src/modules/settings/apps/graphql/index.ts
  • frontend/core-ui/src/modules/settings/apps/hooks/useCreateAppForm.tsx
  • frontend/core-ui/src/modules/settings/apps/schema.ts
  • frontend/core-ui/src/modules/types/paths/SettingsPath.ts
  • frontend/core-ui/src/modules/settings/apps/hooks/useRemoveToken.tsx
  • frontend/core-ui/src/pages/settings/workspace/AppSettingsPage.tsx
  • frontend/core-ui/src/modules/settings/apps/hooks/useAppsTokens.tsx
  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx
  • frontend/core-ui/src/modules/settings/apps/components/AppsHeader.tsx
  • frontend/core-ui/src/modules/settings/apps/hooks/useAddAppToken.tsx
  • frontend/core-ui/src/modules/settings/apps/graphql/mutations/removeToken.ts
  • frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx
  • frontend/core-ui/src/modules/settings/apps/components/AppsSettings.tsx
  • frontend/core-ui/src/modules/app/components/SettingsRoutes.tsx
  • frontend/core-ui/src/modules/settings/apps/types.ts
  • frontend/core-ui/src/modules/settings/constants/data.ts
  • frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx,js,jsx}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Avoid console logs.
Always use absolute paths when importing.
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.

**/*.{ts,tsx,js,jsx}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Use absolute path when import.

Files:

  • frontend/core-ui/src/modules/settings/apps/graphql/queries/index.ts
  • frontend/libs/ui-modules/src/states/currentOrganizationState.ts
  • frontend/core-ui/src/modules/settings/apps/graphql/mutations/index.ts
  • frontend/core-ui/src/modules/settings/apps/graphql/queries/getApps.ts
  • frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx
  • frontend/core-ui/src/modules/settings/apps/graphql/mutations/addToken.ts
  • frontend/core-ui/src/modules/settings/apps/graphql/index.ts
  • frontend/core-ui/src/modules/settings/apps/hooks/useCreateAppForm.tsx
  • frontend/core-ui/src/modules/settings/apps/schema.ts
  • frontend/core-ui/src/modules/types/paths/SettingsPath.ts
  • frontend/core-ui/src/modules/settings/apps/hooks/useRemoveToken.tsx
  • frontend/core-ui/src/pages/settings/workspace/AppSettingsPage.tsx
  • frontend/core-ui/src/modules/settings/apps/hooks/useAppsTokens.tsx
  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx
  • frontend/core-ui/src/modules/settings/apps/components/AppsHeader.tsx
  • frontend/core-ui/src/modules/settings/apps/hooks/useAddAppToken.tsx
  • frontend/core-ui/src/modules/settings/apps/graphql/mutations/removeToken.ts
  • frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx
  • frontend/core-ui/src/modules/settings/apps/components/AppsSettings.tsx
  • frontend/core-ui/src/modules/app/components/SettingsRoutes.tsx
  • frontend/core-ui/src/modules/settings/apps/types.ts
  • frontend/core-ui/src/modules/settings/constants/data.ts
  • frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{tsx,jsx}: Favor named exports for components.
Use declarative JSX.
Use Shadcn UI, Radix, and Tailwind for components and styling.
Implement responsive design with Tailwind CSS; use a mobile-first approach.

**/*.{tsx,jsx}: Structure files: exported component, subcomponents, helpers, static content, types.
Favor named exports for components.
Use declarative JSX.
Use Shadcn UI, Radix, and Tailwind for components and styling.
Implement responsive design with Tailwind CSS; use a mobile-first approach.

Files:

  • frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx
  • frontend/core-ui/src/modules/settings/apps/hooks/useCreateAppForm.tsx
  • frontend/core-ui/src/modules/settings/apps/hooks/useRemoveToken.tsx
  • frontend/core-ui/src/pages/settings/workspace/AppSettingsPage.tsx
  • frontend/core-ui/src/modules/settings/apps/hooks/useAppsTokens.tsx
  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx
  • frontend/core-ui/src/modules/settings/apps/components/AppsHeader.tsx
  • frontend/core-ui/src/modules/settings/apps/hooks/useAddAppToken.tsx
  • frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx
  • frontend/core-ui/src/modules/settings/apps/components/AppsSettings.tsx
  • frontend/core-ui/src/modules/app/components/SettingsRoutes.tsx
  • frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx
**/*.tsx

📄 CodeRabbit inference engine (.cursorrules)

**/*.tsx: Use functional components with TypeScript interfaces.
Minimize 'useEffect', and 'setState'.
Wrap client components in Suspense with fallback.
Use dynamic loading for non-critical components.
Limit 'use client': Favor server components and Next.js SSR. Use only for Web API access in small components. Avoid for data fetching or state management.

Files:

  • frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx
  • frontend/core-ui/src/modules/settings/apps/hooks/useCreateAppForm.tsx
  • frontend/core-ui/src/modules/settings/apps/hooks/useRemoveToken.tsx
  • frontend/core-ui/src/pages/settings/workspace/AppSettingsPage.tsx
  • frontend/core-ui/src/modules/settings/apps/hooks/useAppsTokens.tsx
  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx
  • frontend/core-ui/src/modules/settings/apps/components/AppsHeader.tsx
  • frontend/core-ui/src/modules/settings/apps/hooks/useAddAppToken.tsx
  • frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx
  • frontend/core-ui/src/modules/settings/apps/components/AppsSettings.tsx
  • frontend/core-ui/src/modules/app/components/SettingsRoutes.tsx
  • frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx
**/*.{webp,tsx,jsx}

📄 CodeRabbit inference engine (.cursorrules)

Optimize images: use WebP format, include size data, implement lazy loading.

Files:

  • frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx
  • frontend/core-ui/src/modules/settings/apps/hooks/useCreateAppForm.tsx
  • frontend/core-ui/src/modules/settings/apps/hooks/useRemoveToken.tsx
  • frontend/core-ui/src/pages/settings/workspace/AppSettingsPage.tsx
  • frontend/core-ui/src/modules/settings/apps/hooks/useAppsTokens.tsx
  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx
  • frontend/core-ui/src/modules/settings/apps/components/AppsHeader.tsx
  • frontend/core-ui/src/modules/settings/apps/hooks/useAddAppToken.tsx
  • frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx
  • frontend/core-ui/src/modules/settings/apps/components/AppsSettings.tsx
  • frontend/core-ui/src/modules/app/components/SettingsRoutes.tsx
  • frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx
🧠 Learnings (1)
📚 Learning: 2025-07-21T06:09:41.203Z
Learnt from: CR
PR: erxes/erxes-next#0
File: .cursorrules:0-0
Timestamp: 2025-07-21T06:09:41.203Z
Learning: Structure files: exported component, utils, contexts, hooks, types.

Applied to files:

  • frontend/core-ui/src/modules/settings/apps/graphql/mutations/index.ts
🧬 Code graph analysis (12)
frontend/core-ui/src/modules/settings/apps/hooks/useCreateAppForm.tsx (2)
frontend/core-ui/src/modules/settings/apps/types.ts (1)
  • TCreateAppForm (17-17)
frontend/core-ui/src/modules/settings/apps/schema.ts (1)
  • CREATE_TOKEN_SCHEMA (3-27)
frontend/core-ui/src/modules/settings/apps/hooks/useRemoveToken.tsx (1)
frontend/core-ui/src/modules/settings/apps/graphql/mutations/removeToken.ts (1)
  • REMOVE_TOKEN (9-9)
frontend/core-ui/src/pages/settings/workspace/AppSettingsPage.tsx (4)
frontend/core-ui/src/modules/settings/apps/components/AppsHeader.tsx (1)
  • AppsHeader (6-44)
frontend/libs/erxes-ui/src/components/spinner.tsx (1)
  • Spinner (42-134)
frontend/core-ui/src/modules/settings/apps/components/AppsSettings.tsx (1)
  • AppsSettings (6-26)
frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx (1)
  • CreateToken (21-159)
frontend/core-ui/src/modules/settings/apps/hooks/useAppsTokens.tsx (1)
frontend/core-ui/src/modules/settings/apps/types.ts (1)
  • IApp (4-15)
frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx (9)
frontend/core-ui/src/modules/settings/apps/hooks/useCreateAppForm.tsx (1)
  • useCreateAppForm (6-21)
frontend/core-ui/src/modules/settings/apps/hooks/useAddAppToken.tsx (1)
  • useAddAppToken (9-25)
frontend/core-ui/src/modules/settings/apps/types.ts (1)
  • TCreateAppForm (17-17)
frontend/libs/erxes-ui/src/components/form.tsx (1)
  • Form (166-174)
frontend/libs/erxes-ui/src/components/button.tsx (1)
  • Button (43-57)
frontend/libs/erxes-ui/src/components/switch.tsx (1)
  • Switch (5-23)
frontend/libs/ui-modules/src/modules/team-members/components/SelectUsersGroup.tsx (1)
  • SelectUsersGroup (217-225)
frontend/libs/erxes-ui/src/components/date-picker.tsx (1)
  • DatePicker (19-95)
frontend/libs/erxes-ui/src/components/spinner.tsx (1)
  • Spinner (42-134)
frontend/core-ui/src/modules/settings/apps/components/AppsHeader.tsx (3)
frontend/libs/erxes-ui/src/components/button.tsx (1)
  • Button (43-57)
frontend/libs/erxes-ui/src/components/breadcrumb.tsx (1)
  • Breadcrumb (109-116)
frontend/libs/ui-modules/src/modules/header/components/PageHeader.tsx (2)
  • PageHeaderStart (34-49)
  • PageHeaderEnd (53-68)
frontend/core-ui/src/modules/settings/apps/hooks/useAddAppToken.tsx (1)
frontend/core-ui/src/modules/settings/apps/types.ts (1)
  • IApp (4-15)
frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx (3)
frontend/libs/ui-modules/src/states/currentOrganizationState.ts (1)
  • currentOrganizationState (18-18)
frontend/core-ui/src/modules/settings/constants/data.ts (1)
  • SETTINGS_PATH_DATA (111-182)
frontend/libs/erxes-ui/src/modules/navigation-menu/components/NavigationMenu.tsx (1)
  • NavigationMenuLinkItem (7-56)
frontend/core-ui/src/modules/settings/apps/components/AppsSettings.tsx (4)
frontend/core-ui/src/modules/settings/apps/hooks/useAppsTokens.tsx (1)
  • useAppsTokens (9-19)
frontend/libs/erxes-ui/src/modules/record-table/index.ts (1)
  • RecordTable (23-40)
frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx (1)
  • appsSettingsColumns (56-92)
frontend/core-ui/src/modules/settings/apps/types.ts (1)
  • IApp (4-15)
frontend/core-ui/src/modules/app/components/SettingsRoutes.tsx (2)
frontend/libs/ui-modules/src/states/currentOrganizationState.ts (1)
  • currentOrganizationState (18-18)
frontend/core-ui/src/modules/settings/apps/components/AppsSettings.tsx (1)
  • AppsSettings (6-26)
frontend/core-ui/src/modules/settings/apps/types.ts (1)
frontend/core-ui/src/modules/settings/apps/schema.ts (1)
  • CREATE_TOKEN_SCHEMA (3-27)
frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx (4)
frontend/core-ui/src/modules/settings/apps/types.ts (1)
  • IApp (4-15)
frontend/libs/erxes-ui/src/hooks/use-confirm.tsx (1)
  • useConfirm (133-181)
frontend/libs/erxes-ui/src/components/button.tsx (1)
  • Button (43-57)
frontend/libs/erxes-ui/src/modules/record-table/components/RecordTableCellInline.tsx (1)
  • RecordTableInlineCell (100-104)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: core-ui
  • GitHub Check: frontline_ui-ci
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx (1)

230-231: Confirm PositionBadge variant support and update JSX literal

  • Verify PositionBadge accepts "secondary" and that its contrast, hover, and focus states meet WCAG across themes.
  • Replace variant={'secondary'} with variant="secondary"
frontend/core-ui/src/modules/types/paths/SettingsPath.ts (1)

17-17: No changes required — AppsCatchAll usage verified
All occurrences of SettingsWorkspacePath.Apps remain in the sidebar/nav and post-create navigation (data.ts, CreateToken.tsx), and the only route definition uses SettingsWorkspacePath.AppsCatchAll in SettingsRoutes.tsx.

Optional: consider migrating this enum to a const map + union type later for improved tree-shaking and DX.

frontend/core-ui/src/modules/settings/apps/graphql/queries/index.ts (1)

1-1: LGTM.

Barrel re-export aligns with the module structure.

frontend/core-ui/src/modules/app/components/SettingsRoutes.tsx (1)

172-175: No changes needed: SettingsWorkspacePath.AppsCatchAll ("apps/*") matches both /apps and its subpaths, so links using SettingsWorkspacePath.Apps ("apps") correctly resolve.

frontend/core-ui/src/modules/settings/apps/types.ts (3)

1-2: Good: zod-backed form typing

Inferring TCreateAppForm from the schema keeps UI types in sync with validation.


17-17: LGTM: exported form type

Exporting TCreateAppForm is convenient for hooks/components using the schema.


6-14: Align IApp with actual GraphQL response shapes

  • createdAt should be string | Date
  • expireDate should be optional and allow string | Date | null (absent when noExpire = true)
  • userGroupId should be optional and allow string | null (absent when allowAllPermission = true)
 export interface IApp {
   _id: string;
   accessToken: string;
   allowAllPermission: boolean;
-  createdAt: Date;
-  expireDate: Date;
+  createdAt: string | Date;
+  expireDate?: string | Date | null;
   isEnabled: boolean;
   name: string;
   noExpire: boolean;
   refreshToken: string;
-  userGroupId: string;
+  userGroupId?: string | null;
 }

Prefer importing your GraphQL codegen–generated types if available to avoid type drift.

frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx (2)

1-1: Client-only usage check

This module uses hooks and navigator.clipboard. Ensure it’s only imported by client components; otherwise add "use client" at the top.


61-66: LGTM: name cell

Straightforward, typed, and sized column.

Comment on lines 68 to 79
id: 'expireDate',
accessorKey: 'expireDate',
header: 'Expiration',
cell: ({ cell }) => {
const { expireDate } = cell.row.original;
return (
<RecordTableInlineCell>
{formatDate(expireDate, 'yyyy-MM-dd')}
</RecordTableInlineCell>
);
},
},
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Null-safety for expiration + correct formatting

Handle noExpire/missing dates and format robustly for string | Date.

Apply:

   {
     id: 'expireDate',
     accessorKey: 'expireDate',
     header: 'Expiration',
     cell: ({ cell }) => {
-      const { expireDate } = cell.row.original;
-      return (
-        <RecordTableInlineCell>
-          {formatDate(expireDate, 'yyyy-MM-dd')}
-        </RecordTableInlineCell>
-      );
+      const { expireDate, noExpire } = cell.row.original;
+      const value =
+        noExpire
+          ? 'Never'
+          : expireDate
+            ? formatDate(
+                typeof expireDate === 'string' ? new Date(expireDate) : expireDate,
+                'yyyy-MM-dd',
+              )
+            : '-';
+      return <RecordTableInlineCell>{value}</RecordTableInlineCell>;
     },
   },
📝 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
id: 'expireDate',
accessorKey: 'expireDate',
header: 'Expiration',
cell: ({ cell }) => {
const { expireDate } = cell.row.original;
return (
<RecordTableInlineCell>
{formatDate(expireDate, 'yyyy-MM-dd')}
</RecordTableInlineCell>
);
},
},
id: 'expireDate',
accessorKey: 'expireDate',
header: 'Expiration',
cell: ({ cell }) => {
const { expireDate, noExpire } = cell.row.original;
const value =
noExpire
? 'Never'
: expireDate
? formatDate(
typeof expireDate === 'string' ? new Date(expireDate) : expireDate,
'yyyy-MM-dd',
)
: '-';
return <RecordTableInlineCell>{value}</RecordTableInlineCell>;
},
},
🤖 Prompt for AI Agents
In
frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx
around lines 68-79, the expiration cell assumes expireDate is a Date and never
null; update it to handle noExpire and missing/nullable values and to accept
string | Date: read noExpire from cell.row.original and if true return a clear
label like "No expiration"; if expireDate is null/undefined return a placeholder
(e.g. "—"); if expireDate is a string parse it to a Date (use a safe parser like
parseISO or new Date) then validate with isValid before formatting; finally pass
the validated Date to formatDate('yyyy-MM-dd') and fall back to the placeholder
on parse/validation failure.

Comment on lines +3 to +30
const ADD_TOKEN = gql`
mutation AppsAdd(
$name: String
$userGroupId: String
$expireDate: Date
$allowAllPermission: Boolean
$noExpire: Boolean
) {
appsAdd(
name: $name
userGroupId: $userGroupId
expireDate: $expireDate
allowAllPermission: $allowAllPermission
noExpire: $noExpire
) {
_id
accessToken
allowAllPermission
createdAt
expireDate
isEnabled
name
noExpire
refreshToken
userGroupId
}
}
`;
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Validate variable nullability and avoid caching sensitive tokens.

  • Confirm which inputs are required by the server. If name or userGroupId are mandatory, mark them non-null:
-    $name: String
-    $userGroupId: String
+    $name: String!
+    $userGroupId: String!
  • Ensure the hook that calls this mutation sets fetchPolicy: 'no-cache' (or immediately removes token fields from Apollo cache) since accessToken/refreshToken are sensitive.

Check the hook options:


🏁 Script executed:

#!/bin/bash
fd useAddAppToken.tsx
if [[ -n "$(fd useAddAppToken.tsx)" ]]; then
  rg -n -C2 "useMutation\(|refetchQueries|awaitRefetchQueries|fetchPolicy" $(fd useAddAppToken.tsx)
fi

# Also verify the Date scalar matches server schema (Date vs DateTime)
rg -n "scalar\s+Date(Time)?" backend | head || true

Length of output: 670


🏁 Script executed:

#!/bin/bash
# Search for the GraphQL type definition for the appsAdd mutation
rg -n "appsAdd" -C3 backend

Length of output: 1705


🏁 Script executed:

#!/bin/bash
rg -n "commonFields" -C3 backend/core-api/src/modules/apps/graphql/schemas.ts

Length of output: 495


Add no-cache fetchPolicy and align mutation types with server SDL

  • In frontend/core-ui/src/modules/settings/apps/hooks/useAddAppToken.tsx, pass fetchPolicy: 'no-cache' in the useMutation options to prevent Apollo from caching accessToken/refreshToken.
  • Don’t change name or userGroupId to non-null in the client mutation unless the backend SDL (which currently declares them as String nullable) is updated to String!.
🤖 Prompt for AI Agents
In frontend/core-ui/src/modules/settings/apps/graphql/mutations/addToken.ts
around lines 3 to 30, keep the GraphQL mutation argument types as nullable
(String) to match the backend SDL and do not change `name` or `userGroupId` to
non-null; instead, update the client hook
frontend/core-ui/src/modules/settings/apps/hooks/useAddAppToken.tsx to pass
fetchPolicy: 'no-cache' in the useMutation options so Apollo does not cache
sensitive fields like accessToken/refreshToken.

Comment on lines 7 to 15
accessToken
allowAllPermission
createdAt
expireDate
isEnabled
name
noExpire
refreshToken
userGroupId
Copy link

@coderabbitai coderabbitai bot Sep 4, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Do not return refresh tokens to the client.

Fetching refreshToken to the browser is a high‑risk secret exposure. Remove it from this query and ensure the server never returns it to UI.

   query Apps {
     apps {
       _id
       accessToken
       allowAllPermission
       createdAt
       expireDate
       isEnabled
       name
       noExpire
-      refreshToken
       userGroupId
     }
   }

If copying access tokens is required, ensure RBAC restricts this query to privileged users and consider masking tokens in the table, revealing only via explicit copy actions.


🏁 Script executed:

#!/bin/bash
rg -n "refreshToken" -g "frontend/core-ui/src/modules/settings/apps/graphql/queries/getApps.ts"
rg -n "refreshToken" -g "frontend/core-ui/src/modules/settings/apps/graphql/queries/*.ts"

Length of output: 367


Remove refreshToken from getApps GraphQL query
Delete the refreshToken field (see diff) and ensure server-side schema never returns refresh tokens to the UI.

   query Apps {
     apps {
       _id
       accessToken
       allowAllPermission
       createdAt
       expireDate
       isEnabled
       name
       noExpire
-      refreshToken
       userGroupId
     }
   }
📝 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
accessToken
allowAllPermission
createdAt
expireDate
isEnabled
name
noExpire
refreshToken
userGroupId
query Apps {
apps {
_id
accessToken
allowAllPermission
createdAt
expireDate
isEnabled
name
noExpire
userGroupId
}
}
🤖 Prompt for AI Agents
In frontend/core-ui/src/modules/settings/apps/graphql/queries/getApps.ts around
lines 7 to 15, the GraphQL query currently requests the refreshToken field;
remove refreshToken from the query selection set and update any dependent
TypeScript types/usages that assume its presence. Additionally coordinate with
the backend: ensure the server-side GraphQL schema/resolvers do not expose
refreshToken to the UI (remove it from public types or mark it as internal) so
refresh tokens are never returned to the client.

Copy link
Collaborator

Choose a reason for hiding this comment

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

shalgah

Copy link

Choose a reason for hiding this comment

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

@KhBaterdene Thank you for acknowledging the security concern. Please ensure the refreshToken field is removed from both the frontend GraphQL query and the backend schema to prevent sensitive token exposure to the client.

Comment on lines 11 to 27
.superRefine((data, ctx) => {
if (!data.allowAllPermission && !data.userGroupId) {
ctx.addIssue({
code: 'custom',
message: 'userGroupId is required when allowAllPermission is false',
path: ['userGroupId'],
});
}

if (!data.noExpire && !data.expireDate) {
ctx.addIssue({
code: 'custom',
message: 'expireDate is required when noExpire is false',
path: ['expireDate'],
});
}
});
Copy link

@coderabbitai coderabbitai bot Sep 4, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Cross-field rules: require future expireDate; prevent conflicting inputs.

  • If noExpire is false, expireDate must be present and in the future.
  • If noExpire is true, disallow providing expireDate.
  • Treat empty userGroupId as missing when allowAllPermission is false.

Apply:

   .superRefine((data, ctx) => {
-    if (!data.allowAllPermission && !data.userGroupId) {
+    const userGroupId = (data.userGroupId ?? '').trim();
+    if (!data.allowAllPermission && !userGroupId) {
       ctx.addIssue({
         code: 'custom',
         message: 'userGroupId is required when allowAllPermission is false',
         path: ['userGroupId'],
       });
     }

-    if (!data.noExpire && !data.expireDate) {
+    if (!data.noExpire && !data.expireDate) {
       ctx.addIssue({
         code: 'custom',
         message: 'expireDate is required when noExpire is false',
         path: ['expireDate'],
       });
     }
+    if (!data.noExpire && data.expireDate) {
+      const now = new Date();
+      if (data.expireDate <= now) {
+        ctx.addIssue({
+          code: 'custom',
+          message: 'expireDate must be in the future',
+          path: ['expireDate'],
+        });
+      }
+    }
+    if (data.noExpire && data.expireDate) {
+      ctx.addIssue({
+        code: 'custom',
+        message: 'Remove expireDate when noExpire is true',
+        path: ['expireDate'],
+      });
+    }
   });

Follow-up: if backend expects ISO strings for dates, convert in submit handler:

// in CreateToken.tsx onSubmit
appsAdd({
  variables: {
    ...data,
    userGroupId: data.allowAllPermission ? undefined : data.userGroupId?.trim(),
    expireDate: data.noExpire ? undefined : data.expireDate?.toISOString(),
  },
  ...
});

🏁 Script executed:

#!/bin/bash
# Show the superRefine block in schema.ts
rg -nP --type ts '\\.superRefine' -C5 frontend/core-ui/src/modules/settings/apps/schema.ts

# Inspect field definitions for expireDate, noExpire, and allowAllPermission
rg -nP --type ts 'expireDate' -C3 frontend/core-ui/src/modules/settings/apps/schema.ts
rg -nP --type ts 'noExpire' -C3 frontend/core-ui/src/modules/settings/apps/schema.ts
rg -nP --type ts 'allowAllPermission' -C3 frontend/core-ui/src/modules/settings/apps/schema.ts

Length of output: 1874


Enforce trimmed userGroupId, future-only expireDate, and disallow conflicting inputs

  • Trim userGroupId and treat empty values as missing when allowAllPermission is false.
  • Require expireDate to be present and strictly in the future if noExpire is false.
  • Disallow providing expireDate when noExpire is true.

Apply:

   .superRefine((data, ctx) => {
-    if (!data.allowAllPermission && !data.userGroupId) {
+    const userGroupId = (data.userGroupId ?? '').trim();
+    if (!data.allowAllPermission && !userGroupId) {
       ctx.addIssue({
         code: 'custom',
         message: 'userGroupId is required when allowAllPermission is false',
         path: ['userGroupId'],
       });
     }

-    if (!data.noExpire && !data.expireDate) {
+    if (!data.noExpire && !data.expireDate) {
       ctx.addIssue({
         code: 'custom',
         message: 'expireDate is required when noExpire is false',
         path: ['expireDate'],
       });
     }

+    if (!data.noExpire && data.expireDate) {
+      const now = new Date();
+      if (data.expireDate <= now) {
+        ctx.addIssue({
+          code: 'custom',
+          message: 'expireDate must be in the future',
+          path: ['expireDate'],
+        });
+      }
+    }
+
+    if (data.noExpire && data.expireDate) {
+      ctx.addIssue({
+        code: 'custom',
+        message: 'Remove expireDate when noExpire is true',
+        path: ['expireDate'],
+      });
+    }
   });

Optionally, ensure your submit handler converts values appropriately:

appsAdd({
  variables: {
    ...data,
    userGroupId: data.allowAllPermission ? undefined : data.userGroupId?.trim(),
    expireDate: data.noExpire ? undefined : data.expireDate?.toISOString(),
  },
});
🤖 Prompt for AI Agents
In frontend/core-ui/src/modules/settings/apps/schema.ts around lines 11 to 27,
update the superRefine to trim userGroupId and treat empty strings as missing
when allowAllPermission is false, require expireDate to be present and strictly
in the future when noExpire is false, and add a validation error if expireDate
is provided while noExpire is true; implement these checks by trimming
data.userGroupId before testing, comparing expireDate to now (strictly greater
than now) and adding ctx.addIssue with appropriate path and messages for each
violation. Also ensure the submit handler converts values as suggested: set
userGroupId to undefined when allowAllPermission is true or when trimmed value
is empty, and set expireDate to undefined when noExpire is true, otherwise
serialize expireDate to ISO string.

Copy link
Collaborator

Choose a reason for hiding this comment

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

check

Copy link

Choose a reason for hiding this comment

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

@KhBaterdene Could you clarify what you'd like me to check? Are you asking me to:

  1. Verify the current schema implementation against my suggested improvements?
  2. Double-check my analysis of the validation logic?
  3. Review something else specific about the schema?

Let me know what aspect you'd like me to focus on and I'll take a closer look! 🐰

  ∩───∩
  │ ・ ・ │
  │  ω  │ ♪
  │     │
  └─────┘

Comment on lines +176 to +180
{
name: 'Apps',
icon: IconApi,
path: SettingsWorkspacePath.Apps,
},
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Confirm org-type gating for “Apps” entry matches product rules.

Ensure this new nav item is shown/hidden per org type (SaaS/OS) as intended and the route target exists.


🏁 Script executed:

#!/bin/bash
# Verify gating and route existence for SettingsWorkspacePath.Apps
rg -nP -C2 '\bSettingsWorkspacePath\.Apps\b'
rg -nP -C2 'org(Type)?|organization(Type)?' frontend/core-ui/src/modules/settings
rg -nP -C2 'AppsCatchAll|apps/\*' frontend/core-ui/src

Length of output: 1990


Add org‐type gating for “Apps” nav item

  • No org-type checks found around the “Apps” entry in frontend/core-ui/src/modules/settings/constants/data.ts; wrap its inclusion so it’s shown only for the intended org types (e.g., SaaS).
  • The route target already exists (AppsCatchAll<AppsSettings /> in SettingsRoutes.tsx).
🤖 Prompt for AI Agents
In frontend/core-ui/src/modules/settings/constants/data.ts around lines 176-180,
the "Apps" nav item is always included; change it so the entry is added only for
the intended org types (e.g., SaaS). Wrap the Apps object creation/array push
behind an org-type check (use the existing org-type helper or currentOrg.type
=== 'SaaS' / equivalent utility used across the app), and ensure any needed
helper import is added at top of the file. Keep the path/icon the same and do
not alter the existing route (AppsCatchAll).

</Form.Item>
)}
/>
{!noExpire && (
Copy link
Collaborator

Choose a reason for hiding this comment

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

fix

const CopyTokenButton = ({ cell }: { cell: Cell<IApp, unknown> }) => {
const { accessToken } = cell.row.original;
const copy = () => {
navigator.clipboard.writeText(accessToken);
Copy link
Collaborator

Choose a reason for hiding this comment

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

zasah

});
};
return (
<Button variant="outline" size={'icon'} onClick={() => copy()}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

hi

Comment on lines 7 to 15
accessToken
allowAllPermission
createdAt
expireDate
isEnabled
name
noExpire
refreshToken
userGroupId
Copy link
Collaborator

Choose a reason for hiding this comment

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

shalgah

Comment on lines 11 to 19
const handleAdd = (options?: MutationFunctionOptions<IResult, any>) => {
mutate({
...options,
onCompleted: (data) => {
options?.onCompleted?.(data);
},
refetchQueries: ['Apps'],
});
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

shaardlagatai bish

Comment on lines 11 to 27
.superRefine((data, ctx) => {
if (!data.allowAllPermission && !data.userGroupId) {
ctx.addIssue({
code: 'custom',
message: 'userGroupId is required when allowAllPermission is false',
path: ['userGroupId'],
});
}

if (!data.noExpire && !data.expireDate) {
ctx.addIssue({
code: 'custom',
message: 'expireDate is required when noExpire is false',
path: ['expireDate'],
});
}
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

check

key={positionId}
positionId={positionId}
variant={'default'}
variant={'secondary'}
Copy link
Collaborator

Choose a reason for hiding this comment

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

{} shaardlagagui

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Caution

Changes requested ❌

Reviewed 27699b4 in 2 minutes and 6 seconds. Click for details.
  • Reviewed 234 lines of code in 8 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. frontend/core-ui/src/modules/settings/apps/hooks/useAddAppToken.tsx:9
  • Draft comment:
    The removal of the 'refetchQueries' option from the mutation may lead to stale UI data. Ensure that the Apollo cache is updated or data is refetched as needed.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% The comment identifies a real issue - removing refetchQueries could cause stale data. However, we don't know if this is actually a problem because: 1) The mutation response might automatically update the cache 2) There might be cache updates elsewhere 3) The Apps query might not even be used anymore. We'd need more context about the GraphQL schema and cache setup. I might be underestimating the severity of removing cache management code. Stale data bugs can be serious production issues. While cache issues are serious, making assumptions about cache requirements without full context could lead to unnecessary complexity. The PR author likely tested this change. Delete this comment. While it raises a valid concern, we don't have enough context to be confident that this is actually a problem rather than an intentional simplification.
2. frontend/core-ui/src/modules/settings/apps/schema.ts:3
  • Draft comment:
    The previous schema used superRefine to enforce that userGroupId (when allowAllPermission is false) and expireDate (when noExpire is false) are required. These conditional validations have been removed; please confirm if this is intentional.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% This is a significant change in validation behavior that could lead to invalid states being accepted. The comment correctly identifies removed business logic. However, the comment asks "please confirm if this is intentional" which violates our rules about asking for confirmation or explanation. The validation logic might have been moved elsewhere in the codebase. Without seeing the full context of the changes, we can't be certain this is actually a problem. While the validation could be elsewhere, removing validation from the schema itself is a concrete issue worth raising, as schemas are the canonical place for such validation. The comment identifies a real issue but should be rewritten to be more direct about the missing validation rather than asking for confirmation.
3. frontend/core-ui/src/modules/settings/apps/graphql/queries/getApps.ts:13
  • Draft comment:
    The 'refreshToken' field has been removed from the GET_APPS query. Confirm that no dependent code relies on this field.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to confirm that no dependent code relies on the removed field. This is a request for confirmation, which violates the rule against asking the author to confirm their intention or to double-check things.

Workflow ID: wflow_KHVn8JRLlaWUMhpb

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

</Form.Item>
)}
/>
{(allowAllPermission && null) || (
Copy link

Choose a reason for hiding this comment

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

Prefer using a clear conditional like {!allowAllPermission && ( ... )} rather than using {(allowAllPermission && null) || ( ... )}. This improves readability.

Suggested change
{(allowAllPermission && null) || (
{!allowAllPermission && (

</Form.Item>
)}
/>
{(noExpire && null) || (
Copy link

Choose a reason for hiding this comment

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

Use a simpler conditional rendering, e.g. {!noExpire && ( ... )} instead of {(noExpire && null) || ( ... )}, for better clarity.

Suggested change
{(noExpire && null) || (
{!noExpire && (

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

♻️ Duplicate comments (5)
frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx (1)

20-22: Switched to explicit isOs gating — matches routes and prior feedback.

This prevents OS-only links from appearing when type is null/unknown. Nice.

frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx (3)

5-5: date-fns import fixed — good catch resolved.
Using format from date-fns is correct and addresses the earlier import issue.


44-54: Clipboard try/catch added — resolved.
The copy flow now handles runtime errors from navigator.clipboard.writeText.


76-87: Guard invalid/missing dates and support noExpire.
format(expireDate, ...) throws on undefined/invalid Date and doesn’t reflect noExpire.

Apply:

-import { format } from 'date-fns';
+import { format, isValid, parseISO } from 'date-fns';
-    cell: ({ cell }) => {
-      const { expireDate } = cell.row.original;
-      return (
-        <RecordTableInlineCell>
-          {format(expireDate, 'yyyy-MM-dd')}
-        </RecordTableInlineCell>
-      );
-    },
+    cell: ({ cell }) => {
+      const { expireDate, noExpire } = cell.row.original;
+      const value = noExpire
+        ? 'Never'
+        : expireDate
+          ? (() => {
+              const d =
+                typeof expireDate === 'string' ? parseISO(expireDate) : expireDate;
+              return isValid(d) ? format(d, 'yyyy-MM-dd') : '-';
+            })()
+          : '-';
+      return <RecordTableInlineCell>{value}</RecordTableInlineCell>;
+    },
frontend/core-ui/src/modules/settings/apps/schema.ts (1)

3-13: Add cross-field validation: require future expireDate, gate userGroupId by allowAllPermission, and disallow conflicting inputs

Without superRefine, invalid combinations pass (e.g., noExpire=true with expireDate set). Treat empty userGroupId as missing.

Apply:

 export const CREATE_TOKEN_SCHEMA = z.object({
   name: z
     .string()
     .trim()
     .min(1, 'Name is required')
     .max(100, 'Name must be ≤ 100 chars'),
   userGroupId: z.string().trim().optional(),
   expireDate: z.coerce.date().optional(),
   allowAllPermission: z.boolean().default(false),
   noExpire: z.boolean().default(false),
-});
+}).superRefine((data, ctx) => {
+  const userGroupId = (data.userGroupId ?? '').trim();
+  if (!data.allowAllPermission && !userGroupId) {
+    ctx.addIssue({
+      code: 'custom',
+      path: ['userGroupId'],
+      message: 'User group is required unless “Allow all” is enabled',
+    });
+  }
+  if (!data.noExpire && !data.expireDate) {
+    ctx.addIssue({
+      code: 'custom',
+      path: ['expireDate'],
+      message: 'Expire date is required unless “No expire” is enabled',
+    });
+  }
+  if (!data.noExpire && data.expireDate && data.expireDate <= new Date()) {
+    ctx.addIssue({
+      code: 'custom',
+      path: ['expireDate'],
+      message: 'Expire date must be in the future',
+    });
+  }
+  if (data.noExpire && data.expireDate) {
+    ctx.addIssue({
+      code: 'custom',
+      path: ['expireDate'],
+      message: 'Remove expire date when “No expire” is enabled',
+    });
+  }
+});
🧹 Nitpick comments (6)
frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx (1)

231-236: Avoid duplicate entries in selectedPositions on onCompleted.

onCompleted appends blindly; duplicates can accumulate as badges resolve. Deduplicate when pushing.

@@
-          onCompleted={(position) => {
-            if (!position) return;
-            if (selectedPositionIds.includes(position._id)) {
-              setSelectedPositions([...selectedPositions, position]);
-            }
-          }}
+          onCompleted={(position) => {
+            if (!position) return;
+            if (selectedPositionIds.includes(position._id)) {
+              setSelectedPositions((prev) =>
+                prev.some((p) => p._id === position._id)
+                  ? prev
+                  : [...prev, position],
+              );
+            }
+          }}
@@
-          onCompleted={(position) => {
-            if (!position) return;
-            if (positionIds.includes(position._id)) {
-              setSelectedPositions([...selectedPositions, position]);
-            }
-          }}
+          onCompleted={(position) => {
+            if (!position) return;
+            if (positionIds.includes(position._id)) {
+              setSelectedPositions((prev) =>
+                prev.some((p) => p._id === position._id)
+                  ? prev
+                  : [...prev, position],
+              );
+            }
+          }}

Also applies to: 321-326

frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx (1)

61-76: OS-only filter is correct; minor cleanups for scalability and key stability.

  • Extract OS-only paths into a Set to avoid repeating comparisons.
  • Use item.path as the React key for stability.

Add once near the top (module scope):

const OS_ONLY_PATHS = new Set<string>([
  SettingsWorkspacePath.FileUpload,
  SettingsWorkspacePath.MailConfig,
]);

Apply within this block:

-          {SETTINGS_PATH_DATA.nav
-            .filter((item) => {
-              const isRestricted =
-                item.path === SettingsWorkspacePath.FileUpload ||
-                item.path === SettingsWorkspacePath.MailConfig;
-              return isOs || !isRestricted;
-            })
+          {SETTINGS_PATH_DATA.nav
+            .filter((item) => isOs || !OS_ONLY_PATHS.has(item.path))
             .map((item) => (
-              <Sidebar.MenuItem key={item.name}>
+              <Sidebar.MenuItem key={item.path}>
                 <NavigationMenuLinkItem
                   pathPrefix={AppPath.Settings}
                   path={item.path}
                   name={item.name}
                 />
               </Sidebar.MenuItem>
             ))}

Optional: Per guidelines, consider changing ../constants/data to an absolute import alias for consistency.

frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx (3)

41-60: Handle unsupported Clipboard API, empty token, and add a11y labels.
Show a clear toast when Clipboard API is unavailable; guard empty tokens; add aria-label/title.

Apply:

   const { accessToken } = cell.row.original;
   async function copy() {
-    if (typeof navigator !== 'undefined' && navigator.clipboard) {
+    if (!accessToken) {
+      toast({ title: 'No token available', variant: 'destructive' });
+      return;
+    }
+    if (typeof navigator !== 'undefined' && navigator.clipboard) {
       try {
         await navigator.clipboard.writeText(accessToken);
         toast({
           title: 'Token copied to clipboard',
         });
       } catch {
         toast({
           title: 'Failed to copy token',
           variant: 'destructive',
         });
       }
-    }
+    } else {
+      toast({
+        title: 'Clipboard API unavailable in this context',
+        variant: 'destructive',
+      });
+    }
   }
   return (
-    <Button variant="outline" size="icon" onClick={copy}>
+    <Button
+      variant="outline"
+      size="icon"
+      aria-label="Copy token"
+      title="Copy token"
+      onClick={copy}
+    >
       <IconCopy />
     </Button>
   );

69-74: Type-safe cell signature; prefer row over cell.
Use row.original directly; it’s simpler and keeps strong typing with TanStack.

-    cell: ({ cell }) => {
-      const { name } = cell.row.original;
-      return <RecordTableInlineCell>{name ?? ''}</RecordTableInlineCell>;
-    },
+    cell: ({ row }) => (
+      <RecordTableInlineCell>{row.original.name ?? ''}</RecordTableInlineCell>
+    ),

89-99: Add header and disable sorting for actions column.
Improves accessibility and avoids misleading sortable UI.

   {
     id: 'actions',
-    cell: ({ cell }) => {
+    header: 'Actions',
+    enableSorting: false,
+    cell: ({ cell }) => {
       return (
         <RecordTableInlineCell className="flex justify-center gap-2">
           <RemoveButton cell={cell} />
           <CopyTokenButton cell={cell} />
         </RecordTableInlineCell>
       );
     },
     size: 100,
   },
frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx (1)

51-59: Accessibility: add aria-label to icon-only back button

Improves screen-reader usability.

Apply:

-            <Button
+            <Button
               variant={'ghost'}
               size={'icon'}
               onClick={() => navigate(-1)}
+              aria-label="Back"
             >
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b4eb468 and 27699b4.

📒 Files selected for processing (8)
  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/graphql/queries/getApps.ts (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/hooks/useAddAppToken.tsx (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/schema.ts (1 hunks)
  • frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx (2 hunks)
  • frontend/core-ui/src/pages/settings/workspace/AppSettingsPage.tsx (1 hunks)
  • frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • frontend/core-ui/src/modules/settings/apps/graphql/queries/getApps.ts
  • frontend/core-ui/src/pages/settings/workspace/AppSettingsPage.tsx
  • frontend/core-ui/src/modules/settings/apps/hooks/useAddAppToken.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx}: Use functional and declarative programming patterns; avoid classes.
Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use maps instead.

Files:

  • frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx
  • frontend/core-ui/src/modules/settings/apps/schema.ts
  • frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx
  • frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx
  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx,js,jsx}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Avoid console logs.
Always use absolute paths when importing.
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.

**/*.{ts,tsx,js,jsx}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Use absolute path when import.

Files:

  • frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx
  • frontend/core-ui/src/modules/settings/apps/schema.ts
  • frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx
  • frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx
  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{tsx,jsx}: Favor named exports for components.
Use declarative JSX.
Use Shadcn UI, Radix, and Tailwind for components and styling.
Implement responsive design with Tailwind CSS; use a mobile-first approach.

**/*.{tsx,jsx}: Structure files: exported component, subcomponents, helpers, static content, types.
Favor named exports for components.
Use declarative JSX.
Use Shadcn UI, Radix, and Tailwind for components and styling.
Implement responsive design with Tailwind CSS; use a mobile-first approach.

Files:

  • frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx
  • frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx
  • frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx
  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx
**/*.tsx

📄 CodeRabbit inference engine (.cursorrules)

**/*.tsx: Use functional components with TypeScript interfaces.
Minimize 'useEffect', and 'setState'.
Wrap client components in Suspense with fallback.
Use dynamic loading for non-critical components.
Limit 'use client': Favor server components and Next.js SSR. Use only for Web API access in small components. Avoid for data fetching or state management.

Files:

  • frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx
  • frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx
  • frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx
  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx
**/*.{webp,tsx,jsx}

📄 CodeRabbit inference engine (.cursorrules)

Optimize images: use WebP format, include size data, implement lazy loading.

Files:

  • frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx
  • frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx
  • frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx
  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx
🧬 Code graph analysis (3)
frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx (4)
frontend/core-ui/src/modules/settings/apps/types.ts (1)
  • IApp (4-15)
frontend/libs/erxes-ui/src/hooks/use-confirm.tsx (1)
  • useConfirm (133-181)
frontend/libs/erxes-ui/src/components/button.tsx (1)
  • Button (43-57)
frontend/libs/erxes-ui/src/modules/record-table/components/RecordTableCellInline.tsx (1)
  • RecordTableInlineCell (100-104)
frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx (3)
frontend/libs/ui-modules/src/states/currentOrganizationState.ts (1)
  • currentOrganizationState (18-18)
frontend/core-ui/src/modules/settings/constants/data.ts (1)
  • SETTINGS_PATH_DATA (111-182)
frontend/libs/erxes-ui/src/modules/navigation-menu/components/NavigationMenu.tsx (1)
  • NavigationMenuLinkItem (7-56)
frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx (9)
frontend/core-ui/src/modules/settings/apps/hooks/useCreateAppForm.tsx (1)
  • useCreateAppForm (6-21)
frontend/core-ui/src/modules/settings/apps/hooks/useAddAppToken.tsx (1)
  • useAddAppToken (9-16)
frontend/core-ui/src/modules/settings/apps/types.ts (1)
  • TCreateAppForm (17-17)
frontend/libs/erxes-ui/src/components/form.tsx (1)
  • Form (166-174)
frontend/libs/erxes-ui/src/components/button.tsx (1)
  • Button (43-57)
frontend/libs/erxes-ui/src/components/switch.tsx (1)
  • Switch (5-23)
frontend/libs/ui-modules/src/modules/team-members/components/SelectUsersGroup.tsx (1)
  • SelectUsersGroup (217-225)
frontend/libs/erxes-ui/src/components/date-picker.tsx (1)
  • DatePicker (19-95)
frontend/libs/erxes-ui/src/components/spinner.tsx (1)
  • Spinner (42-134)
🪛 ESLint
frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx

[error] 90-90: Unexpected constant truthiness on the left-hand side of a || expression.

(no-constant-binary-expression)


[error] 127-127: Unexpected constant truthiness on the left-hand side of a || expression.

(no-constant-binary-expression)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: core-ui
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
frontend/libs/ui-modules/src/modules/structure/components/SelectPositions.tsx (1)

320-320: Consistency: badge variant switched to "secondary" matches usage elsewhere.

This aligns SelectPositionsBadgesView with PositionsList and the PR objective. Looks good.

frontend/core-ui/src/modules/settings/components/SettingsSidebar.tsx (2)

9-9: LGTM on state imports.

Importing currentOrganizationState keeps the sidebar in sync with org type.


15-15: Good: centralizing path literals via SettingsWorkspacePath.

Avoids brittle string comparisons elsewhere.

frontend/core-ui/src/modules/settings/apps/schema.ts (1)

4-11: Field-level rules look solid

Trimmed name with min/max and z.coerce for dates are good foundations.

Comment on lines +30 to +41
const onSubmit = (data: TCreateAppForm) => {
appsAdd({
variables: data,
onCompleted: () => {
toast({ title: 'Created a token' });
navigate(SettingsWorkspacePath.Apps);
reset();
},
onError: (error) =>
toast({ title: error.message, variant: 'destructive' }),
});
};
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Normalize payload before mutation (trim, gate fields, ISO date)

Prevents sending empty userGroupId or expireDate when toggles disable them; avoids backend reject.

Apply:

   const onSubmit = (data: TCreateAppForm) => {
-    appsAdd({
-      variables: data,
+    const variables = {
+      ...data,
+      name: data.name.trim(),
+      userGroupId: data.allowAllPermission
+        ? undefined
+        : data.userGroupId?.trim() || undefined,
+      expireDate: data.noExpire
+        ? undefined
+        : data.expireDate?.toISOString(),
+    };
+    appsAdd({
+      variables,
       onCompleted: () => {
         toast({ title: 'Created a token' });
         navigate(SettingsWorkspacePath.Apps);
         reset();
       },
       onError: (error) =>
         toast({ title: error.message, variant: 'destructive' }),
     });
   };
📝 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 onSubmit = (data: TCreateAppForm) => {
appsAdd({
variables: data,
onCompleted: () => {
toast({ title: 'Created a token' });
navigate(SettingsWorkspacePath.Apps);
reset();
},
onError: (error) =>
toast({ title: error.message, variant: 'destructive' }),
});
};
const onSubmit = (data: TCreateAppForm) => {
const variables = {
...data,
name: data.name.trim(),
userGroupId: data.allowAllPermission
? undefined
: data.userGroupId?.trim() || undefined,
expireDate: data.noExpire
? undefined
: data.expireDate?.toISOString(),
};
appsAdd({
variables,
onCompleted: () => {
toast({ title: 'Created a token' });
navigate(SettingsWorkspacePath.Apps);
reset();
},
onError: (error) =>
toast({ title: error.message, variant: 'destructive' }),
});
};
🤖 Prompt for AI Agents
In frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx around
lines 30 to 41, the mutation is sending raw form data which can include empty or
disabled fields and non-ISO dates; normalize the payload before calling appsAdd
by trimming string fields (e.g., name, description), omitting userGroupId when
the corresponding toggle is off or the value is empty, omitting expireDate when
the expiration toggle is off, and converting any expireDate to an ISO string
(e.g., expireDate.toISOString()) before passing variables; then call appsAdd
with this sanitized payload so the backend doesn't receive empty/invalid fields.

Comment on lines 15 to 36
<Button
variant="outline"
size={'icon'}
onClick={() =>
confirm({
message: `Are you sure you want to remove the app '${name}'`,
options: confirmOptions,
}).then(() =>
appsRemove({
variables: { _id },
onCompleted: () => {
toast({
title: `App '${name}' removed`,
variant: 'default',
});
},
}),
)
}
>
<IconTrash />
</Button>
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add onError handling, a11y labels, and minor nits in RemoveButton.

  • Missing error path on mutation; surface failures to users.
  • Add aria-label/title for icon-only button; fix punctuation; unify prop style.

Apply:

-    <Button
-      variant="outline"
-      size={'icon'}
-      onClick={() =>
-        confirm({
-          message: `Are you sure you want to remove the app '${name}'`,
-          options: confirmOptions,
-        }).then(() =>
-          appsRemove({
-            variables: { _id },
-            onCompleted: () => {
-              toast({
-                title: `App '${name}' removed`,
-                variant: 'default',
-              });
-            },
-          }),
-        )
-      }
-    >
+    <Button
+      variant="outline"
+      size="icon"
+      aria-label={`Remove ${name}`}
+      title="Remove app"
+      onClick={() =>
+        void confirm({
+          message: `Are you sure you want to remove the app '${name}'?`,
+          options: confirmOptions,
+        }).then(() =>
+          appsRemove({
+            variables: { _id },
+            onCompleted: () =>
+              toast({ title: `App '${name}' removed`, variant: 'default' }),
+            onError: (error) =>
+              toast({
+                title: `Failed to remove '${name}'`,
+                description: error?.message,
+                variant: 'destructive',
+              }),
+          }),
+        )
+      }
+    >
📝 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
<Button
variant="outline"
size={'icon'}
onClick={() =>
confirm({
message: `Are you sure you want to remove the app '${name}'`,
options: confirmOptions,
}).then(() =>
appsRemove({
variables: { _id },
onCompleted: () => {
toast({
title: `App '${name}' removed`,
variant: 'default',
});
},
}),
)
}
>
<IconTrash />
</Button>
<Button
variant="outline"
size="icon"
aria-label={`Remove ${name}`}
title="Remove app"
onClick={() =>
void confirm({
message: `Are you sure you want to remove the app '${name}'?`,
options: confirmOptions,
}).then(() =>
appsRemove({
variables: { _id },
onCompleted: () =>
toast({ title: `App '${name}' removed`, variant: 'default' }),
onError: (error) =>
toast({
title: `Failed to remove '${name}'`,
description: error?.message,
variant: 'destructive',
}),
}),
)
}
>
<IconTrash />
</Button>
🤖 Prompt for AI Agents
In
frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx
around lines 15 to 36, the RemoveButton lacks mutation error handling and
accessibility labels and has inconsistent prop styling; add an onError handler
to appsRemove that shows a toast with the error message/description on failure,
add aria-label and title attributes to the icon-only Button (e.g.,
aria-label={`Remove ${name}`} title={`Remove ${name}`}), fix prop style to use
consistent double-quoted strings (size="icon"), and tidy punctuation in the
confirm message as needed.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Caution

Changes requested ❌

Reviewed 7d327fd in 2 minutes and 8 seconds. Click for details.
  • Reviewed 84 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx:124
  • Draft comment:
    Similarly, simplify the conditional rendering for 'noExpire'. Use '!noExpire && (…) ' instead of adding an '|| null' fallback.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx:18
  • Draft comment:
    Good accessibility improvement: Adding 'aria-label' and 'title' to the Remove button improves screen reader support.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
3. frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx:21
  • Draft comment:
    Using 'void' before confirm() helps suppress unhandled promise warnings. Consider adding a brief comment to clarify its intent.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The void operator is a somewhat obscure JavaScript feature and its use here is non-obvious. The comment correctly identifies that it's used to suppress unhandled promise warnings. However, asking for a comment is a minor stylistic suggestion that doesn't affect functionality. The code works correctly either way. The comment might be too nitpicky - developers familiar with TypeScript/JavaScript would understand the void operator's purpose. Also, adding comments for every technical detail could lead to comment bloat. While experienced developers might understand it, the void operator is uncommon enough that a brief comment would help maintainability and serve as documentation for less experienced developers. The comment should be deleted as it's a minor stylistic suggestion about documentation rather than a significant code quality issue.

Workflow ID: wflow_mq4LJgkqe4RzFN6d

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

</Form.Item>
)}
/>
{(!allowAllPermission && (
Copy link

Choose a reason for hiding this comment

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

Consider simplifying conditional rendering. Instead of using '(!allowAllPermission && (…) ) || null', you could simply use '!allowAllPermission && (…)'.

Suggested change
{(!allowAllPermission && (
{!allowAllPermission && (

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (2)
frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx (2)

90-109: Simplify conditional render for User group field.

Use a direct condition; current (a && b) || null pattern hurts readability and can trip ESLint.

Apply:

-            {(!allowAllPermission && (
+            {!allowAllPermission && (
               <Form.Field
                 control={control}
                 name="userGroupId"
                 render={({ field }) => (
                   <Form.Item>
                     <Form.Label>User group</Form.Label>
                     <Form.Control>
                       <SelectUsersGroup.FormItem
                         value={field.value}
                         mode="single"
                         onValueChange={field.onChange}
                       />
                     </Form.Control>
                     <Form.Message />
                   </Form.Item>
                 )}
               />
-            )) ||
-              null}
+            )}

128-147: Simplify conditional render for Expire date field.

Same readability nit as above; render only when needed.

Apply:

-            {(!noExpire && (
+            {!noExpire && (
               <Form.Field
                 control={control}
                 name="expireDate"
                 render={({ field }) => (
                   <Form.Item>
                     <Form.Label>Expire date</Form.Label>
                     <Form.Control>
                       <DatePicker
                         defaultMonth={field.value || new Date()}
                         value={field.value}
                         onChange={field.onChange}
                       />
                     </Form.Control>
                     <Form.Message />
                   </Form.Item>
                 )}
               />
-            )) ||
-              null}
+            )}
🧹 Nitpick comments (3)
frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx (3)

151-154: Prefer ternary for the icon/spinner.

Clearer than using (a && b) || c.

Apply:

-            {(loading && <Spinner />) || <IconPlus />}
+            {loading ? <Spinner /> : <IconPlus />}

30-41: Normalize CreateToken onSubmit payload and verify mutation types

-  const onSubmit = (data: TCreateAppForm) => {
-    appsAdd({
-      variables: data,
+  const onSubmit = (data: TCreateAppForm) => {
+    const variables = {
+      ...data,
+      name: data.name.trim(),
+      userGroupId: data.allowAllPermission
+        ? undefined
+        : (data.userGroupId || '').trim() || undefined,
+      expireDate: data.noExpire
+        ? undefined
+        : (data.expireDate instanceof Date
+            ? data.expireDate.toISOString()
+            : data.expireDate),
+    };
+    appsAdd({
+      variables,

Prevents backend rejects for empty userGroupId or unused expireDate. Optional refactor; happy to help. Please verify that the GraphQL addToken mutation expects ISO date strings and optional userGroupId, and adjust types if needed.


51-57: Add aria-label to Back button for accessibility

Button already defaults to type="button". Update to:

- <Button variant="ghost" size="icon" onClick={() => navigate(-1)}>
+ <Button variant="ghost" size="icon" aria-label="Back" onClick={() => navigate(-1)}>
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 27699b4 and 7d327fd.

📒 Files selected for processing (2)
  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx (1 hunks)
  • frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • frontend/core-ui/src/modules/settings/apps/components/table/AppsSettingsColumns.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx}: Use functional and declarative programming patterns; avoid classes.
Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use maps instead.

Files:

  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx,js,jsx}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Avoid console logs.
Always use absolute paths when importing.
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.

**/*.{ts,tsx,js,jsx}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Use absolute path when import.

Files:

  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{tsx,jsx}: Favor named exports for components.
Use declarative JSX.
Use Shadcn UI, Radix, and Tailwind for components and styling.
Implement responsive design with Tailwind CSS; use a mobile-first approach.

**/*.{tsx,jsx}: Structure files: exported component, subcomponents, helpers, static content, types.
Favor named exports for components.
Use declarative JSX.
Use Shadcn UI, Radix, and Tailwind for components and styling.
Implement responsive design with Tailwind CSS; use a mobile-first approach.

Files:

  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx
**/*.tsx

📄 CodeRabbit inference engine (.cursorrules)

**/*.tsx: Use functional components with TypeScript interfaces.
Minimize 'useEffect', and 'setState'.
Wrap client components in Suspense with fallback.
Use dynamic loading for non-critical components.
Limit 'use client': Favor server components and Next.js SSR. Use only for Web API access in small components. Avoid for data fetching or state management.

Files:

  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx
**/*.{webp,tsx,jsx}

📄 CodeRabbit inference engine (.cursorrules)

Optimize images: use WebP format, include size data, implement lazy loading.

Files:

  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx
🧠 Learnings (1)
📚 Learning: 2025-07-21T06:10:08.710Z
Learnt from: CR
PR: erxes/erxes-next#0
File: .windsurfrules:0-0
Timestamp: 2025-07-21T06:10:08.710Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.

Applied to files:

  • frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx
🧬 Code graph analysis (1)
frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx (6)
frontend/core-ui/src/modules/settings/apps/hooks/useCreateAppForm.tsx (1)
  • useCreateAppForm (6-21)
frontend/core-ui/src/modules/settings/apps/hooks/useAddAppToken.tsx (1)
  • useAddAppToken (9-16)
frontend/core-ui/src/modules/settings/apps/types.ts (1)
  • TCreateAppForm (17-17)
frontend/libs/erxes-ui/src/components/form.tsx (1)
  • Form (166-174)
frontend/libs/ui-modules/src/modules/team-members/components/SelectUsersGroup.tsx (1)
  • SelectUsersGroup (217-225)
frontend/libs/erxes-ui/src/components/date-picker.tsx (1)
  • DatePicker (19-95)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: core-ui
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
frontend/core-ui/src/modules/settings/apps/components/CreateToken.tsx (1)

1-18: Imports look good (absolute paths, correct router import).

Using absolute aliases and importing useNavigate from react-router-dom matches our guidelines.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants