🧪 add unit tests for agency/server blog controller#15
Conversation
Added a Jest unit test suite covering all endpoints (`getBlogs`, `getBlogById`, `createBlog`, `updateBlog`, `deleteBlog`) in `agency/server/controllers/blogController.js`. The test isolates logic by mocking Express requests/responses and the Mongoose Blog model. Also added Jest as a devDependency in agency/server/package.json. Co-authored-by: Adityavanjre <66417028+Adityavanjre@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds Jest tests for the blog controller (CRUD, role-based filters, slug/ID lookup, and error paths), adds test scripts and devDependencies to the server package, fixes a missing React hook import and a response-header type guard in the frontend, and updates root package devDependencies and the ajv override scoping. ChangesTesting, Frontend Fixes, and Configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 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)
⚔️ Resolve merge conflicts
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14836b8c10
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Added a Jest unit test suite covering all endpoints (`getBlogs`, `getBlogById`, `createBlog`, `updateBlog`, `deleteBlog`) in `agency/server/controllers/blogController.js`. The test isolates logic by mocking Express requests/responses and the Mongoose Blog model. Also added Jest as a devDependency in agency/server/package.json. Fixed CI issue by upgrading `ajv` version from `^6.12.6` to `^8.17.1` in the root `package.json` overrides to resolve a module not found error in `@nestjs/cli` when generating Prisma client and building the application. Co-authored-by: Adityavanjre <66417028+Adityavanjre@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Added a Jest unit test suite covering all endpoints (`getBlogs`, `getBlogById`, `createBlog`, `updateBlog`, `deleteBlog`) in `agency/server/controllers/blogController.js`. The test isolates logic by mocking Express requests/responses and the Mongoose Blog model. Also added Jest as a devDependency in agency/server/package.json. Fixed CI issue by upgrading `ajv` version from `^6.12.6` to `^8.17.1` in the root `package.json` overrides to resolve a module not found error in `@nestjs/cli` when generating Prisma client and building the application. Fixed `nexus/frontend` typescript errors in `src/app/(dashboard)/manufacturing/machines/page.tsx` and `src/lib/api.ts` which were causing the build step to fail. Co-authored-by: Adityavanjre <66417028+Adityavanjre@users.noreply.github.com>
Added a Jest unit test suite covering all endpoints (`getBlogs`, `getBlogById`, `createBlog`, `updateBlog`, `deleteBlog`) in `agency/server/controllers/blogController.js`. The test isolates logic by mocking Express requests/responses and the Mongoose Blog model. Also added Jest as a devDependency in agency/server/package.json. Fixed CI issue by upgrading `ajv` version from `^6.12.6` to `^8.17.1` in the root `package.json` overrides specifically for `@nestjs/cli` to resolve a module not found error when generating Prisma client and building the application, while retaining old `ajv` mapping for ESLint to prevent rule meta mapping errors. Fixed `nexus/frontend` typescript errors in `src/app/(dashboard)/manufacturing/machines/page.tsx` and `src/lib/api.ts` which were causing the build step to fail. Co-authored-by: Adityavanjre <66417028+Adityavanjre@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
agency/server/package.json (1)
26-31: 💤 Low valueConsider whether mongodb-memory-server and supertest are needed yet.
mongodb-memory-serverandsupertestare added as dev dependencies but are not imported or used in the test file included in this PR. The current tests use Jest mocks instead of spinning up an in-memory database or making HTTP requests.If these packages are intended for future integration tests or used by other test files not shown in this PR, they're fine to keep. Otherwise, consider adding them when they're actually needed to keep dependencies lean.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agency/server/package.json` around lines 26 - 31, The package.json currently includes devDependencies "mongodb-memory-server" and "supertest" which aren't used by the tests in this PR; either remove these two packages from the "devDependencies" block to keep dependencies minimal or, if you plan to add integration tests, add a short comment in package.json or the test suite referencing their intended use (mongodb-memory-server for in-memory MongoDB in integration tests and supertest for HTTP endpoint testing) so future reviewers know they are deliberate; update the "devDependencies" object (the entries "mongodb-memory-server" and "supertest") accordingly.agency/server/controllers/__tests__/blogController.test.js (3)
194-203: ⚡ Quick winInconsistent error status: updateBlog returns 400 for database errors.
The test expects
400(client error) whenBlog.findByIdrejects (line 201), but database/lookup failures typically warrant500(server error). This is inconsistent with thedeleteBlogtest at lines 232-241, which correctly returns500for the same scenario.If the controller implementation returns
400for all errors inupdateBlog, consider aligning it withdeleteBlogto return500for database errors and reserve400for validation errors (e.g., fromsave()).♻️ Suggested correction
- it('should handle 400 error', async () => { + it('should handle 500 error', async () => { req.params.id = 'some-id'; const error = new Error('Update error'); Blog.findById.mockRejectedValue(error); await updateBlog(req, res); - expect(res.status).toHaveBeenCalledWith(400); + expect(res.status).toHaveBeenCalledWith(500); expect(res.json).toHaveBeenCalledWith({ message: error.message }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agency/server/controllers/__tests__/blogController.test.js` around lines 194 - 203, The test and controller are inconsistent: change updateBlog's error handling to mirror deleteBlog by treating database/lookup rejections (e.g., errors thrown from Blog.findById) as server errors (status 500) while reserving 400 only for validation/save errors; update the updateBlog controller to catch rejections from Blog.findById and respond with res.status(500).json({ message: err.message }) (keep validation error handling for save() as 400) and then adjust the test expectation in blogController.test.js to expect 500 for the Blog.findById rejection case.
133-144: ⚡ Quick winConsider simplifying the constructor assertion.
The assertion explicitly checks for
undefinedfields (lines 137-143), which tests implementation details rather than behavior. If the controller changes to omit undefined fields (e.g., using a spread of only defined values), this test will fail even though the behavior is equivalent.♻️ Suggested simplification
expect(Blog).toHaveBeenCalledWith({ title: 'New Blog', author: 'John Doe', category: 'Tech', - image: undefined, - excerpt: undefined, - content: undefined, - readTime: undefined, - tags: undefined, - slug: undefined, status: 'published' }); + // Or use expect.objectContaining() to check only required fields: + expect(Blog).toHaveBeenCalledWith( + expect.objectContaining({ + title: 'New Blog', + author: 'John Doe', + category: 'Tech', + status: 'published' + }) + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agency/server/controllers/__tests__/blogController.test.js` around lines 133 - 144, The test currently asserts Blog was called with an object that includes many explicit undefined fields which couples the test to implementation details; update the assertion in blogController.test.js to only assert the important properties (e.g., title, author, category, status) using a partial matcher like expect.objectContaining({ title: 'New Blog', author: 'John Doe', category: 'Tech', status: 'published' }) when calling Blog, so it no longer requires unspecified fields (leave Blog and the test around it intact, just replace the full literal with an objectContaining matcher).
57-65: ⚡ Quick winConsider using async rejection instead of synchronous throw.
Line 59 uses a synchronous
throw, while other error tests (e.g., line 106, line 197) usemockRejectedValue()for async rejection. If the controller usesawait Blog.find()..., a synchronous throw occurs before the await and requires different error handling than a promise rejection.For consistency and to better match async/await patterns, consider:
♻️ Suggested async error pattern
- it('should handle 500 error', async () => { - const error = new Error('Database error'); - Blog.find.mockImplementation(() => { throw error; }); - - await getBlogs(req, res); - - expect(res.status).toHaveBeenCalledWith(500); - expect(res.json).toHaveBeenCalledWith({ message: error.message }); - }); + it('should handle 500 error', async () => { + const error = new Error('Database error'); + const mockSort = jest.fn().mockRejectedValue(error); + Blog.find.mockReturnValue({ sort: mockSort }); + + await getBlogs(req, res); + + expect(res.status).toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith({ message: error.message }); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agency/server/controllers/__tests__/blogController.test.js` around lines 57 - 65, The test for getBlogs uses a synchronous throw from Blog.find which doesn't mimic an async rejection; change the mock to reject the promise (use Blog.find.mockRejectedValue(error) or mockImplementation returning Promise.reject(error)) so it matches the controller's await/async path and keeps consistency with other tests (see Blog.find and getBlogs references) and then assert res.status(500) and res.json({ message: error.message }) as before.package.json (1)
29-32: 💤 Low valueConsider whether mockingoose is needed for current tests.
mockingooseis added to devDependencies but the test file in this PR usesjest.mock()directly instead of mockingoose's specialized Mongoose mocking utilities. If future tests will use mockingoose's features (like.toReturn()or.reset()), keeping it is fine. Otherwise, it could be deferred until needed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` around lines 29 - 32, The package.json currently adds the devDependency "mockingoose" but the tests in this PR use jest.mock() directly; either remove "mockingoose" from devDependencies in package.json and update the lockfile (to trim unused tooling), or if you intend to use mockingoose's helpers later, leave it but add a short PR note explaining that future tests will rely on mockingoose (e.g., for .toReturn()/.reset()) so reviewers know it's intentional; reference the dependency name "mockingoose" when making the change or note.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@agency/server/controllers/__tests__/blogController.test.js`:
- Around line 194-203: The test and controller are inconsistent: change
updateBlog's error handling to mirror deleteBlog by treating database/lookup
rejections (e.g., errors thrown from Blog.findById) as server errors (status
500) while reserving 400 only for validation/save errors; update the updateBlog
controller to catch rejections from Blog.findById and respond with
res.status(500).json({ message: err.message }) (keep validation error handling
for save() as 400) and then adjust the test expectation in
blogController.test.js to expect 500 for the Blog.findById rejection case.
- Around line 133-144: The test currently asserts Blog was called with an object
that includes many explicit undefined fields which couples the test to
implementation details; update the assertion in blogController.test.js to only
assert the important properties (e.g., title, author, category, status) using a
partial matcher like expect.objectContaining({ title: 'New Blog', author: 'John
Doe', category: 'Tech', status: 'published' }) when calling Blog, so it no
longer requires unspecified fields (leave Blog and the test around it intact,
just replace the full literal with an objectContaining matcher).
- Around line 57-65: The test for getBlogs uses a synchronous throw from
Blog.find which doesn't mimic an async rejection; change the mock to reject the
promise (use Blog.find.mockRejectedValue(error) or mockImplementation returning
Promise.reject(error)) so it matches the controller's await/async path and keeps
consistency with other tests (see Blog.find and getBlogs references) and then
assert res.status(500) and res.json({ message: error.message }) as before.
In `@agency/server/package.json`:
- Around line 26-31: The package.json currently includes devDependencies
"mongodb-memory-server" and "supertest" which aren't used by the tests in this
PR; either remove these two packages from the "devDependencies" block to keep
dependencies minimal or, if you plan to add integration tests, add a short
comment in package.json or the test suite referencing their intended use
(mongodb-memory-server for in-memory MongoDB in integration tests and supertest
for HTTP endpoint testing) so future reviewers know they are deliberate; update
the "devDependencies" object (the entries "mongodb-memory-server" and
"supertest") accordingly.
In `@package.json`:
- Around line 29-32: The package.json currently adds the devDependency
"mockingoose" but the tests in this PR use jest.mock() directly; either remove
"mockingoose" from devDependencies in package.json and update the lockfile (to
trim unused tooling), or if you intend to use mockingoose's helpers later, leave
it but add a short PR note explaining that future tests will rely on mockingoose
(e.g., for .toReturn()/.reset()) so reviewers know it's intentional; reference
the dependency name "mockingoose" when making the change or note.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1b2b7dd1-791d-4b73-acca-f6504972915d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
agency/server/controllers/__tests__/blogController.test.jsagency/server/package.jsonnexus/frontend/src/app/(dashboard)/manufacturing/machines/page.tsxnexus/frontend/src/lib/api.tspackage.json
Adityavanjre
left a comment
Adityavanjre
left a comment
There was a problem hiding this comment.
Reviewed in CodeRabbit Review Stack
Added a Jest unit test suite covering all endpoints (`getBlogs`, `getBlogById`, `createBlog`, `updateBlog`, `deleteBlog`) in `agency/server/controllers/blogController.js`. The test isolates logic by mocking Express requests/responses and the Mongoose Blog model. Also added Jest as a devDependency in agency/server/package.json. Fixed CI issue by upgrading `ajv` version from `^6.12.6` to `^8.17.1` in the root `package.json` overrides specifically for `@nestjs/cli` to resolve a module not found error when generating Prisma client and building the application, while retaining old `ajv` mapping for ESLint to prevent rule meta mapping errors. Added `mpath` missing dependency in `agency/server`. Fixed `nexus/frontend` typescript errors in `src/app/(dashboard)/manufacturing/machines/page.tsx` and `src/lib/api.ts` which were causing the build step to fail. Co-authored-by: Adityavanjre <66417028+Adityavanjre@users.noreply.github.com>
1 participant
🎯 What: Missing tests for
agency/server/controllers/blogController.jswere added.📊 Coverage: Covered successful retrieval, ID/slug lookup, creation, update, deletion, error statuses (400, 404, 500) and draft status checks logic for
getBlogs,getBlogById,createBlog,updateBlog, anddeleteBlog.✨ Result: Improved test coverage for the agency blog feature, ensuring robust code reliability and preventing regressions in endpoint behavior.
PR created automatically by Jules for task 6964327158673213702 started by @Adityavanjre
Summary by CodeRabbit
Bug Fixes
Tests
Chores