-
Notifications
You must be signed in to change notification settings - Fork 13k
fix: comment step out to check babel module #38210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: 4869651 The changes in this PR will be included in the next version bump. This PR includes changesets to release 40 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Other AI code review bot(s) detectedCodeRabbit 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. WalkthroughCommented out a removal entry for the Changes
Sequence Diagram(s)(Skipped — changes are CI tweak and test addition; no new multi-component control flow to visualize.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 1 file
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name=".github/actions/meteor-build/action.yml">
<violation number="1" location=".github/actions/meteor-build/action.yml:45">
P0: Commenting out this step will break the build. The 'Build Rocket.Chat' step (line ~139) still executes `mv apps/meteor/package.json.bak apps/meteor/package.json`, which expects the backup file created by this step. Since the backup is no longer created, the build will fail with 'No such file or directory'. Either also comment out the restore line, or remove both the backup creation and restore commands entirely.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
d12fc8e to
be96b93
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38210 +/- ##
===========================================
+ Coverage 70.65% 70.69% +0.03%
===========================================
Files 3136 3142 +6
Lines 108592 108820 +228
Branches 19576 19624 +48
===========================================
+ Hits 76730 76934 +204
- Misses 29850 29884 +34
+ Partials 2012 2002 -10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/meteor/tests/end-to-end/api/incoming-integrations.ts`:
- Around line 476-511: This test relies on a prior test to set permissions; call
updatePermission('manage-incoming-integrations', ['admin']) at the start of this
spec to ensure isolation (before creating the integration via
api('integrations.create')), and keep the existing cleanup via
removeIntegration(integrationId, 'incoming'); additionally add a complementary
negative spec that posts an invalid script to the same endpoint and asserts that
res.body.integration has scriptError and not scriptCompiled to cover compilation
failure behavior.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/meteor/tests/end-to-end/api/incoming-integrations.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/tests/end-to-end/api/incoming-integrations.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/tests/end-to-end/api/incoming-integrations.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/tests/end-to-end/api/incoming-integrations.ts
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
apps/meteor/tests/end-to-end/api/incoming-integrations.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/tests/end-to-end/api/incoming-integrations.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/end-to-end/api/incoming-integrations.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/tests/end-to-end/api/incoming-integrations.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/tests/end-to-end/api/incoming-integrations.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/tests/end-to-end/api/incoming-integrations.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/tests/end-to-end/api/incoming-integrations.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
apps/meteor/tests/end-to-end/api/incoming-integrations.ts
🧬 Code graph analysis (1)
apps/meteor/tests/end-to-end/api/incoming-integrations.ts (2)
packages/core-services/src/index.ts (1)
api(55-55)apps/meteor/app/integrations/server/lib/triggerHandler.ts (1)
removeIntegration(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). (12)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.2 (2/4)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 coverage (3/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 coverage (4/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 coverage (2/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 coverage (5/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 coverage (1/5)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.2 (4/4)
- GitHub Check: 🔨 Test API (EE) / MongoDB 8.2 coverage (1/1)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.2 (3/4)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.2 (1/4)
- GitHub Check: 🔨 Test API (CE) / MongoDB 8.2 (1/1)
- GitHub Check: 🔨 Test Federation Matrix
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 3 files
4c1ac5d to
9588971
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 3 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/meteor/tests/end-to-end/api/incoming-integrations.ts">
<violation number="1" location="apps/meteor/tests/end-to-end/api/incoming-integrations.ts:480">
P2: This test intends to validate behavior with only `manage-own-incoming-integrations`, but it never clears `manage-incoming-integrations`. If the broader permission remains enabled from earlier tests, the check is ineffective. Clear the broader permission in this setup so the scenario is actually tested.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Proposed changes (including videos or screenshots)
Comment out the piece of code that deletes meteor babel modules from actions/meteor-build/action.yml because when creating a new Integration (webhook), the script compilation fails because of these missing modules deleted during CI. Create a new end-to-end test case to cover scenarios where the integration isn't saved successfully (when the scriptError extra property is returned in the response).
Issue(s)
#37800 Incoming webhook payload is empty due to script compilation failure
Steps to test or reproduce
1- Run the project using docker-compose or in a production environment (crucial, since this can't be reproduced in development mode)
2- Go to Workspace settings -> Integrations -> Create Incoming
3- Fill the required fields and click on save
Further comments
SUP-958
Summary by CodeRabbit
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.