Skip to content

Comments

fix: remove duplicate values found in common.keys#23477

Closed
GeneCodeSavvy wants to merge 4 commits intocalcom:mainfrom
GeneCodeSavvy:rm-duplicates
Closed

fix: remove duplicate values found in common.keys#23477
GeneCodeSavvy wants to merge 4 commits intocalcom:mainfrom
GeneCodeSavvy:rm-duplicates

Conversation

@GeneCodeSavvy
Copy link

@GeneCodeSavvy GeneCodeSavvy commented Aug 31, 2025

What does this PR do?

Visual Demo (For contributors especially)

A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).

Video Demo (if applicable):

  • Show screen recordings of the issue or feature.
  • Demonstrate how to reproduce the issue, the behavior before and after the change.

Image Demo (if applicable):

  • Add side-by-side screenshots of the original and updated change.
image

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • [] I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox. N/A
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Are there environment variables that should be set?
  • What are the minimal test data to have?
  • What is expected (happy path) to have (input and output)?
  • Any other important info that could help to test that PR

@vercel
Copy link

vercel bot commented Aug 31, 2025

@GeneCodeSavvy is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 31, 2025

Walkthrough

The PR renames several translation i18n keys and updates components to use the new shorter keys. It deletes multiple keys from apps/web/public/static/locales/en/common.json and updates usages in AppPage.tsx, WebhookForm.tsx, permission registry (permission-registry.ts), and event-type tab navigation hooks. No runtime control flow, APIs, or data structures were changed.

Assessment against linked issues

Objective Addressed Explanation
Remove duplicate values in apps/web/public/static/locales/en/common.json by consolidating/removing redundant keys ([#23478], [CAL-6356]) Diff shows deletions from common.json but does not show the final merged file state; cannot confirm duplicates are fully eliminated.

Assessment against linked issues: Out-of-scope changes

Code Change Explanation
Bulk translation key remapping in PERMISSION_REGISTRY (packages/features/pbac/domain/types/permission-registry.ts) This is a registry key rename unrelated to removing duplicate values in common.json.
Label/description key changes in WebhookForm.getWebhookVariables (packages/features/webhooks/components/WebhookForm.tsx) Changes description/category i18n keys for webhook variables; not required by the duplicate-key cleanup objective.

Possibly related PRs


📜 Recent review details

Configuration used: Path: .coderabbit.yaml

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 f8a522a and cdc5c99.

📒 Files selected for processing (1)
  • packages/features/pbac/domain/types/permission-registry.ts (13 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/features/pbac/domain/types/permission-registry.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Tests / Unit
  • GitHub Check: Type check / check-types
  • GitHub Check: Linters / lint
  • GitHub Check: Codacy Static Code Analysis
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

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

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 31, 2025

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "Remove Duplicate values found in common.keys". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

@GeneCodeSavvy GeneCodeSavvy marked this pull request as ready for review August 31, 2025 10:28
@GeneCodeSavvy GeneCodeSavvy requested a review from a team August 31, 2025 10:28
@GeneCodeSavvy GeneCodeSavvy requested a review from a team as a code owner August 31, 2025 10:28
@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Aug 31, 2025
@graphite-app graphite-app bot requested a review from a team August 31, 2025 10:28
@dosubot dosubot bot added the i18n area: i18n, translations label Aug 31, 2025
@GeneCodeSavvy GeneCodeSavvy changed the title changes idk Remove Duplicate values found in common.keys Aug 31, 2025
Copy link
Contributor

@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: 3

🧹 Nitpick comments (3)
packages/platform/atoms/event-types/hooks/usePlatformTabsNavigations.tsx (1)

171-180: Key simplification looks good

Switching to t("advanced") matches the locale cleanup while keeping data-testid stable. Consider aligning the test id in a follow-up for consistency.

packages/features/pbac/domain/types/permission-registry.ts (2)

261-266: Copy mismatch: “members” vs “admins”

description says “Invite organization members” but descriptionI18nKey points to invite_organization_admins. Align them.

Apply one of:

  • Update fallback copy to match key:
-      description: "Invite organization members",
+      description: "Invite organization admins",
  • Or switch to a “members” i18n key if present:
-      descriptionI18nKey: "invite_organization_admins",
+      descriptionI18nKey: "invite_organization_members",

Verify the chosen key exists in common.json with the script above.


106-108: Mixed pbac_resource_ vs generic nouns — ensure pbac keys weren’t removed; consider standardizing*

Several resource.i18nKey entries still use pbac_resource* while others moved to generic nouns (teams, bookings, insights, workflows, attributes). If the PR removed these pbac_resource_* keys from common.json, lookups will fail. Either keep the keys in JSON or migrate these to generic nouns for consistency.

You can reuse the earlier script; it will flag any missing pbac_resource_* keys so you can switch to generic alternatives (e.g., roles, event_types, organization, routing_forms).

Also applies to: 118-121, 150-152, 236-238, 419-421

📜 Review details

Configuration used: Path: .coderabbit.yaml

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 5ebd335 and 63e091a.

📒 Files selected for processing (7)
  • apps/web/components/apps/AppPage.tsx (1 hunks)
  • apps/web/public/static/locales/en/common.json (0 hunks)
  • packages/features/eventtypes/components/tabs/advanced/CustomEventTypeModal.tsx (1 hunks)
  • packages/features/pbac/domain/types/permission-registry.ts (11 hunks)
  • packages/features/webhooks/components/WebhookForm.tsx (2 hunks)
  • packages/platform/atoms/event-types/hooks/usePlatformTabsNavigations.tsx (1 hunks)
  • packages/platform/atoms/event-types/hooks/useTabsNavigations.tsx (1 hunks)
💤 Files with no reviewable changes (1)
  • apps/web/public/static/locales/en/common.json
🧰 Additional context used
📓 Path-based instructions (4)
**/*.tsx

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Always use t() for text localization in frontend code; direct text embedding should trigger a warning

Files:

  • packages/features/eventtypes/components/tabs/advanced/CustomEventTypeModal.tsx
  • packages/platform/atoms/event-types/hooks/useTabsNavigations.tsx
  • packages/features/webhooks/components/WebhookForm.tsx
  • packages/platform/atoms/event-types/hooks/usePlatformTabsNavigations.tsx
  • apps/web/components/apps/AppPage.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • packages/features/eventtypes/components/tabs/advanced/CustomEventTypeModal.tsx
  • packages/platform/atoms/event-types/hooks/useTabsNavigations.tsx
  • packages/features/webhooks/components/WebhookForm.tsx
  • packages/platform/atoms/event-types/hooks/usePlatformTabsNavigations.tsx
  • apps/web/components/apps/AppPage.tsx
  • packages/features/pbac/domain/types/permission-registry.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • packages/features/eventtypes/components/tabs/advanced/CustomEventTypeModal.tsx
  • packages/platform/atoms/event-types/hooks/useTabsNavigations.tsx
  • packages/features/webhooks/components/WebhookForm.tsx
  • packages/platform/atoms/event-types/hooks/usePlatformTabsNavigations.tsx
  • apps/web/components/apps/AppPage.tsx
  • packages/features/pbac/domain/types/permission-registry.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/features/pbac/domain/types/permission-registry.ts
🧠 Learnings (1)
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.

Applied to files:

  • packages/features/eventtypes/components/tabs/advanced/CustomEventTypeModal.tsx
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (5)
packages/features/eventtypes/components/tabs/advanced/CustomEventTypeModal.tsx (1)

134-135: Verify copy: “webhook_title” in an event-type context

Using webhook_* text here may read oddly next to {Event type title}. If the goal is deduping keys, confirm the string for webhook_title makes sense in this UI context (e.g., “The event’s title”). Otherwise consider a more neutral key like “title” for reuse across features.

apps/web/components/apps/AppPage.tsx (1)

368-372: Missing “free” key in non-English locales
The canonical “free” key is only defined in en/common.json (line 3551); add translations for “free” to all other locale files to prevent fallback to English.

⛔ Skipped due to learnings
Learnt from: bandhan-majumder
PR: calcom/cal.com#23192
File: packages/features/insights/components/booking/LeastCompletedBookings.tsx:31-31
Timestamp: 2025-08-21T05:55:35.187Z
Learning: Cal.com uses an automated i18n system with lingo.dev that automatically propagates new translation keys from en/common.json to all other locale files and creates PRs with proper translations. The system includes a check-missing-translations.ts script that adds English placeholders for missing keys, and a GitHub workflow that triggers lingo.dev automation to translate and create PRs.
packages/platform/atoms/event-types/hooks/useTabsNavigations.tsx (1)

212-217: Consistent move to t("advanced")

Matches the platform hook change; no behavioral impact expected.

packages/features/webhooks/components/WebhookForm.tsx (2)

127-132: Possible mismatch: cancellationReason vs “webhook_cancellation_host”

Variable is “cancellationReason” (free-text reason), but the label key implies “who canceled.” Verify intent and copy. If it’s the reason, prefer a key that communicates “reason”; if it’s the actor, consider renaming the variable in UI/docs to match.


194-201: Teams category key change looks fine

Using t("teams") is consistent with the key consolidation. Please confirm the key exists across locales.

description: "All actions on all resources",
category: "system",
i18nKey: "pbac_resource_all",
i18nKey: "event_types_page_title",
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Wrong i18n key for “All” action

Using event_types_page_title for CrudAction.All is semantically incorrect and will render an unrelated label. Map to a generic key (e.g., all) or keep a dedicated pbac_action_all key.

Apply this diff:

-      i18nKey: "event_types_page_title",
+      i18nKey: "all",

Run the verification script below (next comment) to confirm the all key exists as a scalar string in common.json.


🏁 Script executed:

#!/bin/bash
set -e

# Locate all common.json files in the repository
files=$(find . -type f -name common.json)

if [ -z "$files" ]; then
  echo "No common.json files found."
  exit 1
fi

echo "Found common.json at:"
echo "$files"

# Verify that the key "all" exists as a top-level string key
for f in $files; do
  echo "Checking $f for key \"all\"..."
  if grep -P '"all"\s*:\s*"[^"]+"' "$f" >/dev/null; then
    echo "✔️ Key \"all\" exists in $f"
  else
    echo "❌ Key \"all\" NOT found in $f"
    exit 1
  fi
done

Length of output: 16796


Missing “all” key in translations – add or use a dedicated key
The proposed change to map CrudAction.All to "all" will break Indonesian locale (apps/web/public/static/locales/id/common.json), where "all" is absent. Add the "all": "All" entry (and translations) to that file (and any others missing it) or introduce a new pbac_action_all key across all common.json files before applying this diff.

🤖 Prompt for AI Agents
In packages/features/pbac/domain/types/permission-registry.ts around line 112,
the change mapping CrudAction.All to the string "all" will break locales missing
that key (notably apps/web/public/static/locales/id/common.json); add an "all":
"All" entry (and translated values) to that Indonesian common.json and any other
locale files missing it, or alternatively introduce a new key like
"pbac_action_all" in every common.json and update the mapping to use that key
consistently across all locales before applying this diff so translations remain
complete.

Comment on lines 200 to 205
description: "Update settings",
category: "team",
i18nKey: "pbac_action_update",
i18nKey: "edit",
descriptionI18nKey: "pbac_desc_update_team_settings",
dependsOn: ["team.read"],
},
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Sanity-check dependsOn strings resolve to defined permissions

Guard against typos in dependsOn (e.g., organization.listMembers). Verify each dependsOn maps to an actually defined [Resource].[Action] in PERMISSION_REGISTRY.

Also applies to: 294-300, 335-341, 436-441, 442-448


🏁 Script executed:

#!/bin/bash
set -euo pipefail
TS="packages/features/pbac/domain/types/permission-registry.ts"

node - <<'NODE'
const fs = require('fs');
const src = fs.readFileSync(process.env.TS, 'utf8');

// Parse enums into maps
function parseEnum(name){
  const m = src.match(new RegExp(`export enum ${name} \\{([\\s\\S]*?)\\}`, 'm'));
  const body = m ? m[1] : '';
  const map = {};
  for (const line of body.split('\n')) {
    const mm = line.match(/^\s*(\w+)\s*=\s*"(.*)"/);
    if (mm) map[mm[1]] = mm[2];
  }
  return map;
}
const Resource = parseEnum('Resource');
const CrudAction = parseEnum('CrudAction');
const CustomAction = parseEnum('CustomAction');

// Build set of defined permission strings from PERMISSION_REGISTRY
const regMatch = src.match(/PERMISSION_REGISTRY:\s*PermissionRegistry\s*=\s*\{([\s\S]*)\};/m);
if (!regMatch) { console.error('Registry not found'); process.exit(2); }
const reg = regMatch[1];

const defined = new Set();
const resourceBlocks = [...reg.matchAll(/\[Resource\.(\w+)\]\s*:\s*\{([\s\S]*?)\}\s*,/g)];
for (const [, resKey, block] of resourceBlocks) {
  const res = Resource[resKey];
  if (!res) continue;
  for (const m of block.matchAll(/\[(CrudAction|CustomAction)\.(\w+)\]\s*:/g)) {
    const kind = m[1], actionKey = m[2];
    const act = (kind === 'CrudAction' ? CrudAction[actionKey] : CustomAction[actionKey]);
    if (act) defined.add(`${res}.${act}`);
  }
}

// Collect dependsOn strings
const depends = [...reg.matchAll(/dependsOn:\s*\[([^\]]*)\]/g)]
  .flatMap(([, list]) => [...list.matchAll(/"([^"]+)"/g)].map(m => m[1]));

const unknown = depends.filter(d => !defined.has(d));
if (unknown.length) {
  console.log('Unknown dependsOn entries (do not match any defined permission):');
  for (const d of new Set(unknown)) console.log(' -', d);
  process.exit(1);
}
console.log('All dependsOn entries resolve to defined permissions.');
NODE

Length of output: 818


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# Directly read the file path instead of relying on an unset env var
node << 'NODE'
const fs = require('fs');
const src = fs.readFileSync(
  'packages/features/pbac/domain/types/permission-registry.ts',
  'utf8'
);

// Parse enums into maps
function parseEnum(name) {
  const m = src.match(new RegExp(\`export enum \${name} \\{([\\s\\S]*?)\\}\`, 'm'));
  const body = m ? m[1] : '';
  const map = {};
  for (const line of body.split('\n')) {
    const mm = line.match(/^\s*(\w+)\s*=\s*"(.*)"/);
    if (mm) map[mm[1]] = mm[2];
  }
  return map;
}
const Resource = parseEnum('Resource');
const CrudAction = parseEnum('CrudAction');
const CustomAction = parseEnum('CustomAction');

// Build set of defined permission strings from PERMISSION_REGISTRY
const regMatch = src.match(
  /PERMISSION_REGISTRY:\s*PermissionRegistry\s*=\s*\{([\s\S]*?)\};/m
);
if (!regMatch) {
  console.error('Registry not found');
  process.exit(2);
}
const reg = regMatch[1];

const defined = new Set();
const resourceBlocks = [
  ...reg.matchAll(/\[Resource\.(\w+)\]\s*:\s*\{([\s\S]*?)\}\s*,/g)
];
for (const [, resKey, block] of resourceBlocks) {
  const res = Resource[resKey];
  if (!res) continue;
  for (const m of block.matchAll(/\[(CrudAction|CustomAction)\.(\w+)\]\s*:/g)) {
    const kind = m[1],
      actionKey = m[2];
    const act =
      kind === 'CrudAction'
        ? CrudAction[actionKey]
        : CustomAction[actionKey];
    if (act) defined.add(\`\${res}.\${act}\`);
  }
}

// Collect dependsOn strings
const depends = [...reg.matchAll(/dependsOn:\s*\[([^\]]*)\]/g)]
  .flatMap(([, list]) =>
    [...list.matchAll(/"([^"]+)"/g)].map((m) => m[1])
  );

const unknown = depends.filter((d) => !defined.has(d));
if (unknown.length) {
  console.log(
    'Unknown dependsOn entries (do not match any defined permission):'
  );
  for (const d of new Set(unknown)) console.log(' -', d);
  process.exit(1);
}
console.log('All dependsOn entries resolve to defined permissions.');
NODE

Length of output: 711


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

node << 'EOF'
const fs = require('fs');
const src = fs.readFileSync('packages/features/pbac/domain/types/permission-registry.ts', 'utf8');

// Parse enums
const Resource = {}, CrudAction = {}, CustomAction = {};
let mode = null;
src.split('\n').forEach(line => {
  if (line.startsWith('export enum Resource')) { mode = 'Resource'; return; }
  if (line.startsWith('export enum CrudAction')) { mode = 'CrudAction'; return; }
  if (line.startsWith('export enum CustomAction')) { mode = 'CustomAction'; return; }
  if (mode && line.trim() === '}') { mode = null; return; }
  if (mode) {
    const m = line.match(/^\s*(\w+)\s*=\s*"(.*)"/);
    if (m) {
      const [, key, val] = m;
      if (mode === 'Resource') Resource[key] = val;
      if (mode === 'CrudAction') CrudAction[key] = val;
      if (mode === 'CustomAction') CustomAction[key] = val;
    }
  }
});

// Build defined permission set
const defined = new Set();
let inRegistry = false, currentRes = null;
src.split('\n').forEach(line => {
  if (!inRegistry && line.includes('PERMISSION_REGISTRY')) {
    inRegistry = true;
    return;
  }
  if (inRegistry) {
    const start = line.match(/\[Resource\.(\w+)\]\s*:\s*{/);
    if (start) {
      currentRes = Resource[start[1]];
      return;
    }
    if (currentRes && /^\s*}\s*,\s*$/.test(line)) {
      currentRes = null;
      return;
    }
    if (currentRes) {
      const m = line.match(/\[(CrudAction|CustomAction)\.(\w+)\]/);
      if (m) {
        const [, kind, actionKey] = m;
        const act = kind === 'CrudAction' ? CrudAction[actionKey] : CustomAction[actionKey];
        defined.add(`${currentRes}.${act}`);
      }
    }
  }
});

// Extract dependsOn entries
const deps = [];
const regex = /dependsOn\s*:\s*\[([^\]]*)\]/g;
let match;
while ((match = regex.exec(src)) !== null) {
  const list = match[1];
  list.split(',').map(item => item.trim().replace(/^"|"$/g, '')).forEach(s => s && deps.push(s));
}

// Report unknown entries
const unknown = deps.filter(d => !defined.has(d));
if (unknown.length) {
  console.log('Unknown dependsOn entries:');
  for (const u of [...new Set(unknown)]) console.log(' -', u);
  process.exit(1);
}
console.log('All dependsOn entries resolve to defined permissions.');
EOF

Length of output: 231


Undefined dependsOn entries in PERMISSION_REGISTRY
The following dependsOn values have no matching Resource.Action in packages/features/pbac/domain/types/permission-registry.ts:

  • role.read
  • eventType.read
  • team.read
  • organization.read
  • organization.listMembers
  • booking.read
  • workflow.read
  • organization.attributes.read
  • routingForm.read

Ensure each dependsOn string is either corrected or the corresponding permission is added to PERMISSION_REGISTRY.

@GeneCodeSavvy GeneCodeSavvy changed the title Remove Duplicate values found in common.keys fix: remove duplicate values found in common.keys Aug 31, 2025
@github-actions github-actions bot added the 🐛 bug Something isn't working label Aug 31, 2025
Comment on lines -3384 to -3389
"pbac_resource_all": "All Resources",
"pbac_resource_event_type": "Event Types",
"pbac_resource_team": "Teams",
"pbac_resource_organization": "Organization",
"pbac_resource_booking": "Bookings",
"pbac_resource_insights": "Insights",
Copy link
Contributor

Choose a reason for hiding this comment

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

These keys were removed due to duplication, but I think they are still not fully removed, or their alternatives are not being used in the code. Please verify all the removed keys. cc @kart1ka for quick review

Copy link
Contributor

Choose a reason for hiding this comment

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

"pbac_resource_all" is currently being used, so keep it

Copy link
Contributor

@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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/features/pbac/domain/types/permission-registry.ts (1)

124-147: Missing "create" key in Croatian locale
apps/web/public/static/locales/hr/common.json lacks the "create" entry for the generic action i18n keys. Add "create" (and verify that all other shared keys—edit, delete, invite, remove, etc.—are present as strings) in this file.

♻️ Duplicate comments (1)
packages/features/pbac/domain/types/permission-registry.ts (1)

39-53: Optional: centralize action i18n keys to avoid typos

Extract a small const and reuse across the registry.

+export const ActionI18nKeys = {
+  create: "create",
+  read: "pbac_action_read",
+  edit: "edit",
+  delete: "delete",
+  invite: "invite",
+  remove: "remove",
+} as const;
🧹 Nitpick comments (3)
packages/features/pbac/domain/types/permission-registry.ts (3)

150-152: Use a generic resource key for Event Types

event_types_page_title is page-specific. Prefer a generic noun key for the resource label to keep consistency with teams/bookings/etc.

-      i18nKey: "event_types_page_title",
+      i18nKey: "event_types",

Please confirm "event_types" exists (and is a string) across locales before changing.


262-268: Description key targets “admins” but the action is “Invite organization members”

Align the description key with the action scope to avoid misleading copy.

-      descriptionI18nKey: "invite_organization_admins",
+      descriptionI18nKey: "pbac_desc_invite_organization_members",

Verify that pbac_desc_invite_organization_members exists in en/common.json (and translations) or add it; lingo.dev can propagate.


419-421: Align RoutingForm resource label with generic nouns

Other resources use generic keys (teams, bookings, workflows, attributes). Consider the same here.

-      i18nKey: "pbac_resource_routing_form",
+      i18nKey: "routing_form",

Confirm routing_form exists in all locales or add it and let translation automation propagate.

📜 Review details

Configuration used: Path: .coderabbit.yaml

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 51f1d42 and f8a522a.

📒 Files selected for processing (1)
  • packages/features/pbac/domain/types/permission-registry.ts (13 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/features/pbac/domain/types/permission-registry.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • packages/features/pbac/domain/types/permission-registry.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • packages/features/pbac/domain/types/permission-registry.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: bandhan-majumder
PR: calcom/cal.com#23192
File: packages/features/insights/components/booking/LeastCompletedBookings.tsx:31-31
Timestamp: 2025-08-21T05:55:35.187Z
Learning: Cal.com uses an automated i18n system with lingo.dev that automatically propagates new translation keys from en/common.json to all other locale files and creates PRs with proper translations. The system includes a check-missing-translations.ts script that adds English placeholders for missing keys, and a GitHub workflow that triggers lingo.dev automation to translate and create PRs.
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Tests / Unit
  • GitHub Check: Linters / lint
  • GitHub Check: Type check / check-types

Comment on lines 126 to 127
dependsOn: ["role.read"],
},
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Sanity-check dependsOn strings resolve to defined permissions

Guard against typos by verifying each dependsOn matches a defined [Resource].[Action] in this registry.

Also applies to: 139-140, 171-172, 178-179, 191-192, 204-205, 211-212, 218-219, 225-226, 259-260, 267-268, 275-276, 291-292, 299-300, 318-319, 326-327, 333-334, 340-341, 363-364, 376-377, 383-384, 401-402, 408-409, 415-416, 427-428, 440-441, 447-448


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail
TS='packages/features/pbac/domain/types/permission-registry.ts'
node <<'NODE'
const fs=require('fs');const src=fs.readFileSync(process.env.TS||'packages/features/pbac/domain/types/permission-registry.ts','utf8');
function parseEnum(name){const m=src.match(new RegExp(`export enum ${name} \\{([\\s\\S]*?)\\}`,'m'));const body=m?m[1]:'';const map={};for(const ln of body.split('\n')){const mm=ln.match(/^\s*(\w+)\s*=\s*"(.*)"/);if(mm)map[mm[1]]=mm[2];}return map;}
const Resource=parseEnum('Resource'),CrudAction=parseEnum('CrudAction'),CustomAction=parseEnum('CustomAction');
const reg=(src.match(/PERMISSION_REGISTRY:[\s\S]*?=\s*\{([\s\S]*?)\};/m)||[])[1]||'';
const defined=new Set();
for (const m of reg.matchAll(/\[Resource\.(\w+)\]\s*:\s*\{([\s\S]*?)\}\s*,/g)){
  const res=Resource[m[1]], block=m[2];
  for (const a of block.matchAll(/\[(CrudAction|CustomAction)\.(\w+)\]\s*:/g)){
    const act = a[1]==='CrudAction'?CrudAction[a[2]]:CustomAction[a[2]];
    if (res&&act) defined.add(`${res}.${act}`);
  }
}
const deps=[...reg.matchAll(/dependsOn:\s*\[([^\]]*)\]/g)]
  .flatMap(([,list])=>[...list.matchAll(/"([^"]+)"/g)].map(m=>m[1]));
const unknown=[...new Set(deps.filter(d=>!defined.has(d)))];
if(unknown.length){console.log('Unknown dependsOn:', ...unknown);process.exit(1);}else{console.log('All dependsOn entries resolve.');}
NODE

Length of output: 260


Resolve undefined dependsOn permissions: The following dependsOn entries have no matching Resource.Action in permission-registry.ts: role.read, eventType.read, team.read, organization.read, organization.listMembers, booking.read, workflow.read, organization.attributes.read, routingForm.read. Add or correct these permission definitions so each dependsOn value matches an existing enum entry.

🤖 Prompt for AI Agents
In packages/features/pbac/domain/types/permission-registry.ts around lines
126-127, several dependsOn entries reference non-existent Resource.Action values
(role.read, eventType.read, team.read, organization.read,
organization.listMembers, booking.read, workflow.read,
organization.attributes.read, routingForm.read); update the file so each
dependsOn string matches a real enum member: either add the missing
Resource.Action enum entries with the exact names used in dependsOn or change
the dependsOn values to the correct existing enum identifiers used elsewhere in
this registry; after editing, run tsc/CI to ensure no unresolved enum references
remain.

Copy link
Contributor

@volnei volnei left a comment

Choose a reason for hiding this comment

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

@GeneCodeSavvy @anikdhabal
I'm not sure we should go this route. While these values are duplicated, we need to make sure the labels remain independent.

Too generic labels can lead errors and in the future, if we change a value, it might lose its meaning when used elsewhere.

@emrysal @keithwillcode thoughts?

@keithwillcode
Copy link
Contributor

Agree with @volnei. Context is king

@Devanshusharma2005
Copy link
Contributor

Closing due to inactivity and also the approach needs to be addressed once again.

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

Labels

🐛 bug Something isn't working community Created by Linear-GitHub Sync i18n area: i18n, translations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Duplicate values found in common.json keys

5 participants