fix(develop): remove plugin legacy code #3347
Conversation
WalkthroughThis update systematically removes plugin-related functionality across the codebase. It deletes mappings, fields, resolvers, GraphQL types, mutations, queries, subscriptions, and input types associated with plugins. Plugin data loading functions, plugin models, and plugin-specific constants have been removed. Additionally, sample data, documentation, and test cases referencing plugins have been updated or deleted. Overall, the changes eliminate any server-side and schema-level references to plugins, streamlining the code while disabling old plugin management features. Changes
Sequence Diagram(s)sequenceDiagram
participant Server as Server Startup
participant DB as Database
Server->>DB: Establish connection
Note right of Server: Plugin loading logic removed
DB-->>Server: Connection confirmed
Server->>Server: Proceed with startup without loading plugins
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Cioppolo14
added
the
ignore-sensitive-files-pr
|
@bandhan-majumder Please make sure all checks pass. If ones are expected to not pass, let me know. Thank you! |
|
@bandhan-majumder let me know when this is ready. |
bandhan-majumder
force-pushed
the
bandhan-majumder/i3337-remove-plugin-code
branch
from
09005b7 to
2e002cc
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3347 +/- ##
===========================================
- Coverage 97.65% 97.65% -0.01%
===========================================
Files 365 360 -5
Lines 18630 18461 -169
Branches 2699 2683 -16
===========================================
- Hits 18194 18028 -166
+ Misses 431 428 -3
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/resolvers/Mutation/createPost.spec.ts (1)
83-91: Function structure test may be brittle.While this test helps verify the createPost function exists and has the expected structure after plugin-related code removal, testing a function's implementation details through string representation is somewhat brittle. If the function implementation changes (even without changing behavior), this test might break.
Consider focusing more on behavior testing rather than implementation details. However, I understand this may be intentional to ensure core structures remain after plugin code removal.
- it("should verify createPost function structure", async () => { - expect(createPost).toBeDefined(); - expect(typeof createPost).toBe("function"); - - const functionString = createPost.toString(); - expect(functionString).toContain("async"); - expect(functionString).toContain("req"); - expect(functionString).toContain("res"); - }); + it("should verify createPost function exists and is callable", async () => { + expect(createPost).toBeDefined(); + expect(typeof createPost).toBe("function"); + + // Optionally verify function signature instead of implementation details + const mockReq = {} as InterfaceAuthenticatedRequest; + const mockRes = mockResponse(); + + // Just verify the function can be called without throwing TypeError + expect(() => createPost(mockReq, mockRes)).not.toThrow(TypeError); + });tests/libraries/logger.spec.ts (2)
91-93: Module mocking should be done at the top level.The
vi.mock()calls are hoisted to the top of the file during test execution, but they're conceptually clearer when placed with other mocks at the file level rather than inside a test.Move the mock to the top of the file with other mocks:
- vi.mock("./../../src/libraries/requestTracing", () => ({ - getTracingId: vi.fn().mockReturnValue("trace-123"), - }));And add this to the top-level mocks:
vi.mock("./../../src/libraries/requestTracing", () => ({ getTracingId: vi.fn().mockReturnValue("trace-123"), }));
95-95: Improve the assertion with a more specific expectation.The assertion that
printfMockis called twice is correct but doesn't fully validate the behavior.Consider adding more specific assertions:
- expect(printfMock).toHaveBeenCalledTimes(2); // Called for both colorized and non-colorized + expect(printfMock).toHaveBeenCalledTimes(2); + // Verify the context of each call + expect(printfMock.mock.calls[0][0]).toBeInstanceOf(Function); + expect(printfMock.mock.calls[1][0]).toBeInstanceOf(Function);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/libraries/logger.spec.ts(1 hunks)tests/resolvers/Mutation/createPost.spec.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/resolvers/Mutation/createPost.spec.ts (1)
Learnt from: iamanishx
PR: PalisadoesFoundation/talawa-api#3289
File: src/graphql/types/Mutation/deletePost.ts:109-129
Timestamp: 2025-02-24T05:59:01.605Z
Learning: PR #3289 is focused on test cases, specifically adding the `createRegularUserUsingAdmin` helper function for integration tests, and should not include changes to server code.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Testing Application (22.x)
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (2)
tests/resolvers/Mutation/createPost.spec.ts (2)
2-2: Well-structured import statement.Good practice to consolidate imports from the same module in a single line. This improves readability and maintainability.
76-81: Good validation of mongoose import.This test effectively validates that mongoose is properly imported and structured as expected, which is important for ensuring the test environment is correctly set up, especially after removing plugin-related code.
|
@meetulr @palisadoes PTAL |
What kind of change does this PR introduce?
Removes plugin related types, queries, mutations and model.
Issue Number:
Fixes #3337
Snapshots/Videos:
output1.mp4
If relevant, did you update the documentation?
Not sure.
Summary
Does this PR introduce a breaking change?
Yes. It removes the plugin related code from api related code.
Checklist
CodeRabbit AI Review
Test Coverage
Other information
Have you read the contributing guide?
Summary by CodeRabbit
Refactor
Documentation
Tests