Adding Api for Plugin using smaple_data in api #3294
Conversation
WalkthroughThis pull request introduces a new GraphQL entity for plugins. A Changes
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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}
There was a problem hiding this comment.
Actionable comments posted: 12
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
schema.graphql(3 hunks)src/graphql/inputs/QueryPluginInput.ts(1 hunks)src/graphql/types/Plugins/Plugins.ts(1 hunks)src/graphql/types/Plugins/pluginData.json(1 hunks)src/graphql/types/Query/index.ts(1 hunks)src/graphql/types/Query/plugin.ts(1 hunks)test/routes/graphql/gql.tada-cache.d.ts(1 hunks)test/routes/graphql/gql.tada.d.ts(3 hunks)
🔇 Additional comments (4)
src/graphql/types/Query/index.ts (1)
22-22: LGTM!The new plugin import is correctly placed and follows the established pattern.
src/graphql/inputs/QueryPluginInput.ts (1)
1-18: LGTM!The implementation is well-structured and follows best practices:
- Uses Zod for input validation
- Properly implements the builder pattern
- Includes appropriate field description
- Has correct type inference
test/routes/graphql/gql.tada.d.ts (1)
143-143: LGTM!The type definitions are correctly implemented:
- Plugin type with all required fields
- QueryPluginInput type with proper validation
- Updated Query type with new plugin-related fields
Also applies to: 153-153, 165-165
test/routes/graphql/gql.tada-cache.d.ts (1)
47-48: LGTM! Well-structured tag query.The tag query is well-defined with proper type safety and follows GraphQL best practices by requesting only the necessary fields.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3294 +/- ##
====================================================
- Coverage 49.45% 49.43% -0.02%
====================================================
Files 462 465 +3
Lines 34776 34987 +211
Branches 1038 1043 +5
====================================================
+ Hits 17199 17297 +98
- Misses 17577 17690 +113 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
test/routes/graphql/gql.tada.d.ts (1)
143-167: 🧹 Nitpick (assertive)Add documentation for the new Plugin types.
Consider adding JSDoc comments to document:
- The purpose of the Plugin type
- Field descriptions and constraints
- Usage examples for the plugin queries
+/** + * Represents a plugin in the system + * @field pluginCreatedBy - The creator of the plugin + * @field pluginDesc - Description of the plugin's functionality + * @field pluginName - Unique identifier for the plugin + * @field uninstalledOrgs - List of organizations where the plugin is not installed + */ 'Plugin': { kind: 'OBJECT'; name: 'Plugin'; fields: {
♻️ Duplicate comments (2)
schema.graphql (2)
2731-2743: 🧹 Nitpick (assertive)Ensure Consistent Non-null Constraints for Plugin Type Fields
While the new
Plugintype correctly captures the intended fields, previous review comments suggested making required fields non-null to enforce type safety at the schema level. Consider updating the fields (especiallypluginCreatedBy,pluginDesc, andpluginName) to use non-null types if they are always expected to have a value. For example:-type Plugin { - """Creator of the plugin""" - pluginCreatedBy: String - - """Description of the plugin""" - pluginDesc: String - - """Name of the plugin""" - pluginName: String - - """List of organization IDs where plugin is not installed""" - uninstalledOrgs: [String!] -} +type Plugin { + """Creator of the plugin""" + pluginCreatedBy: String! + + """Description of the plugin""" + pluginDesc: String! + + """Name of the plugin""" + pluginName: String! + + """List of organization IDs where plugin is not installed""" + uninstalledOrgs: [String!] +}This adjustment improves type safety and data clarity in your GraphQL schema.
2890-2893: 🧹 Nitpick (assertive)Add Description for the
pluginQuery FieldFor consistency and better API documentation, add a description to the
pluginquery field. This is in line with previous suggestions. For example, add a description such as:- plugin(input: QueryPluginInput!): Plugin + """Query field to fetch a specific plugin by name""" + plugin(input: QueryPluginInput!): PluginThis will help maintain consistency with the description already provided for the
pluginsquery.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
schema.graphql(3 hunks)src/graphql/types/Query/index.ts(1 hunks)test/routes/graphql/gql.tada.d.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run tests for talawa api
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (2)
src/graphql/types/Query/index.ts (1)
22-22: LGTM!The new plugin import follows the established pattern and is correctly ordered alphabetically.
test/routes/graphql/gql.tada.d.ts (1)
153-153: LGTM! Query type updates are well-structured.The new plugin-related query fields are correctly integrated into the Query type:
plugin: Returns a single Pluginplugins: Returns a list of non-null Plugin objects
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/graphql/types/Query/plugin.ts (1)
1-110: 💡 Verification agent🧩 Analysis chain
Run the formatter to fix pipeline failures.
Please address the pipeline error: "Formatter would have printed the following content: Code formatting issues detected. Please run the formatter to fix." Make sure to run the project's official formatting command or tool to pass the pipeline checks.
🏁 Script executed:
#!/bin/bash # Attempt to fix code formatting issues by running the project's formatter npm run format || yarn format || echo "Please manually run the correct formatter command for this project."Length of output: 874
Action Required: Update Formatter Configuration
It appears that running the formatter is currently failing because there isn’t a "format" script defined in the project's package.json, and when attempting to use Yarn, Corepack isn’t enabled (resulting in a version mismatch). To address the pipeline formatting errors, please do one (or both) of the following:
- Add a formatting script: Define a "format" script in package.json (e.g. using Prettier or your project’s formatting tool) so that the command (e.g.
npm run format) runs successfully.- Enable Corepack: If you are using Yarn with Corepack, run
corepack enableto activate the appropriate package manager version, then run the correct formatter command.Once configured, please re-run the formatter to ensure that all code formatting issues are resolved and the pipeline passes.
🧰 Tools
🪛 GitHub Actions: Pull request workflow
[error] 6-6: Formatter would have printed the following content: Code formatting issues detected. Please run the formatter to fix.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/graphql/types/Query/plugin.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/graphql/types/Query/plugin.ts (5)
Learnt from: MayankJha014
PR: PalisadoesFoundation/talawa-api#3294
File: src/graphql/types/Plugins/Plugins.ts:6-11
Timestamp: 2025-02-24T17:27:31.612Z
Learning: In the Plugins module of talawa-api, field names in Plugin type maintain consistency with sample data format, using prefixes like 'pluginName', 'pluginCreatedBy', and 'pluginDesc'.
Learnt from: MayankJha014
PR: PalisadoesFoundation/talawa-api#3294
File: src/graphql/types/Query/plugin.ts:7-35
Timestamp: 2025-02-24T17:13:27.833Z
Learning: The GraphQL builder in Talawa API doesn't support Relay-style cursor-based pagination methods like `connectionType` and `resolveArrayConnection`. Any pagination implementation should use offset-based pagination with skip/limit arguments instead.
Learnt from: MayankJha014
PR: PalisadoesFoundation/talawa-api#3294
File: src/graphql/types/Query/plugin.ts:7-35
Timestamp: 2025-02-24T17:13:27.833Z
Learning: The Talawa API uses Pothos with Relay plugin for GraphQL schema building. Pagination should be implemented using Relay-style connections with the `t.connection()` method, which automatically adds standard Relay connection arguments and types.
Learnt from: MayankJha014
PR: PalisadoesFoundation/talawa-api#3294
File: src/graphql/types/Plugins/Plugins.ts:25-40
Timestamp: 2025-02-24T17:28:19.020Z
Learning: In the Talawa API GraphQL schema, field descriptions follow a concise pattern - simple fields use brief, clear descriptions while only complex fields or types receive more detailed documentation when necessary.
Learnt from: MayankJha014
PR: PalisadoesFoundation/talawa-api#3294
File: src/graphql/types/Plugins/Plugins.ts:25-40
Timestamp: 2025-02-24T17:28:19.020Z
Learning: In the Talawa API codebase, GraphQL type field descriptions follow a simple and concise pattern, avoiding overly detailed descriptions to maintain consistency across types.
🪛 GitHub Actions: Pull request workflow
src/graphql/types/Query/plugin.ts
[error] 6-6: Formatter would have printed the following content: Code formatting issues detected. Please run the formatter to fix.
🔇 Additional comments (2)
src/graphql/types/Query/plugin.ts (2)
43-46: Caching approach looks good.Initializing a
pluginCacheimproves performance by removing the need to traversepluginsDataeach time. Do keep in mind that if the plugin data is dynamically updated, you’ll need an appropriate invalidation or refresh mechanism. For static data, this is a solid approach.
47-109: Robust validation and error handling for single plugin query.The checks for empty plugin names and missing plugins are handled properly. Including clear
invalid_argumentsandarguments_associated_resources_not_founderror codes improves maintainability and user feedback. Great job keeping the flow straightforward and consistent with the existing error-handling patterns.
| import { builder } from "~/src/graphql/builder"; | ||
| import { QueryPluginInput } from "~/src/graphql/inputs/QueryPluginInput"; | ||
| import { TalawaGraphQLError } from "~/src/utilities/TalawaGraphQLError"; | ||
| import { PluginRef } from "../Plugins/Plugins"; | ||
| import pluginsData from "../Plugins/pluginData.json" assert { type: "json" }; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Imports look fine, but please address formatting issues.
All imports appear valid and necessary. However, according to the pipeline failures, the codebase's formatting checks are failing on this file. Please ensure you run the project's formatter to resolve any style inconsistencies.
There was a problem hiding this comment.
can you format it and i do change in my code
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's retrieve the full content of the file
cat src/graphql/types/Query/plugin.ts
# Check if there are specific formatting tools/commands used in the project
ls -la package.json
cat package.json | grep -E "format|lint" | grep -v "ignore"
Length of output: 4501
🏁 Script executed:
#!/bin/bash
# Check for common formatter/linter config files
ls -la .eslintrc* .prettierrc* .editorconfig* tsconfig.json
# Check package.json scripts to find format commands
jq '.scripts' package.json
# Try to find formatting-related dependencies
jq '.devDependencies' package.json | grep -E 'eslint|prettier|format'
Length of output: 2107
🏁 Script executed:
#!/bin/bash
# Check for biome configuration file
ls -la biome.json
# If biome configuration exists, display it
if [ -f biome.json ]; then
cat biome.json
fi
# Try to run the project's formatter command on the file
pnpm fix_code_quality src/graphql/types/Query/plugin.ts || npm run fix_code_quality src/graphql/types/Query/plugin.ts
Length of output: 432
@MayankJha014 I ran the project's formatter command (pnpm fix_code_quality) on your file, but it reported "No fixes applied", which suggests the file is already correctly formatted according to the project's standards.
import { builder } from "~/src/graphql/builder";
import { QueryPluginInput } from "~/src/graphql/inputs/QueryPluginInput";
import { TalawaGraphQLError } from "~/src/utilities/TalawaGraphQLError";
import { PluginRef } from "../Plugins/Plugins";
import pluginsData from "../Plugins/pluginData.json" assert { type: "json" };
builder.queryField("getPlugins", (t) =>
t.field({
type: [PluginRef],
description: "Fetch all available plugins",
resolve: async (_parent, _args, ctx) => {
try {
// Check authentication
if (!ctx.currentClient.isAuthenticated) {
throw new TalawaGraphQLError({
extensions: {
code: "unauthenticated",
},
});
}
return pluginsData;
} catch (error) {
if (error instanceof TalawaGraphQLError) {
throw error;
}
throw new TalawaGraphQLError({
extensions: {
code: "unexpected",
},
});
}
},
}),
);
// Create a cache based on plugin ID
const pluginCacheById = new Map(
pluginsData.map((plugin) => [plugin.id, plugin]),
);
// Create a cache based on plugin name (case insensitive)
const pluginCacheByName = new Map(
pluginsData.map((plugin) => [plugin.pluginName.toLowerCase(), plugin]),
);
builder.queryField("plugin", (t) =>
t.field({
type: PluginRef,
args: {
input: t.arg({
type: QueryPluginInput,
required: true,
}),
},
resolve: async (_parent, { input }, ctx) => {
try {
// Check authentication
if (!ctx.currentClient.isAuthenticated) {
throw new TalawaGraphQLError({
extensions: {
code: "unauthenticated",
},
});
}
// Validate input (either ID or pluginName must be provided)
if (!input.pluginName?.trim() && !input.id?.trim()) {
throw new TalawaGraphQLError({
extensions: {
code: "invalid_arguments",
issues: [
{
argumentPath: ["input"],
message: "Either plugin ID or plugin name must be provided",
},
],
},
});
}
let plugin: (typeof pluginsData)[0] | undefined;
// Lookup by ID first if provided, otherwise by plugin name
if (input.id) {
plugin = pluginCacheById.get(input.id);
} else if (input.pluginName) {
plugin = pluginCacheByName.get(input.pluginName.toLowerCase());
}
if (!plugin) {
throw new TalawaGraphQLError({
extensions: {
code: "arguments_associated_resources_not_found",
issues: [
{
argumentPath: ["input"],
},
],
},
});
}
return plugin;
} catch (error) {
if (error instanceof TalawaGraphQLError) {
throw error;
}
throw new TalawaGraphQLError({
extensions: {
code: "unexpected",
},
});
}
},
}),
);
builder.mutationField("updatePluginStatus", (t) =>
t.field({
type: PluginRef,
description: "Toggle plugin installation status for an organization",
args: {
pluginId: t.arg.string({ required: true }),
orgId: t.arg.string({ required: true }),
},
resolve: async (_parent, { pluginId, orgId }, ctx) => {
try {
// Check authentication
if (!ctx.currentClient.isAuthenticated) {
throw new TalawaGraphQLError({
extensions: {
code: "unauthenticated",
},
});
}
// Find plugin
const plugin = pluginCacheById.get(pluginId);
if (!plugin) {
throw new TalawaGraphQLError({
extensions: {
code: "arguments_associated_resources_not_found",
issues: [
{
argumentPath: ["pluginId"],
},
],
},
});
}
// Check if the org is already in uninstalledOrgs
const isUninstalled = plugin.uninstalledOrgs.includes(orgId);
if (isUninstalled) {
// Remove from uninstalledOrgs (installing the plugin)
plugin.uninstalledOrgs = plugin.uninstalledOrgs.filter(
(org) => org !== orgId,
);
} else {
// Add to uninstalledOrgs (uninstalling the plugin)
plugin.uninstalledOrgs.push(orgId);
}
// Update cache
pluginCacheById.set(pluginId, plugin);
return plugin;
} catch (error) {
if (error instanceof TalawaGraphQLError) {
throw error;
}
throw new TalawaGraphQLError({
extensions: {
code: "unexpected",
},
});
}
},
}),
);The code looks well-structured with consistent indentation and proper spacing. If you're still experiencing pipeline failures, it might be related to:
- Line ending issues (CRLF vs LF)
- Different formatting rules in CI vs local
- Other files that might need formatting
You can try running the full project formatter with pnpm fix_code_quality without specifying files to fix all formatting issues across the codebase.
|
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
src/graphql/types/Plugins/Plugins.ts (1)
15-21: 🧹 Nitpick (assertive)Add maximum length constraints to string fields.
While minimum length validation is implemented, consider adding maximum length constraints to prevent potential issues with extremely long strings.
export const pluginSchema = z.object({ id: z.string().uuid(), - pluginName: z.string().min(1), - pluginCreatedBy: z.string().min(1), - pluginDesc: z.string().min(1), + pluginName: z.string().min(1).max(100), + pluginCreatedBy: z.string().min(1).max(50), + pluginDesc: z.string().min(1).max(500), uninstalledOrgs: z.array(z.string()), });schema.graphql (2)
2734-2749: 🧹 Nitpick (assertive)Add non-null constraints to required fields.
The Plugin type fields should be marked as non-null for required fields to ensure type safety at the GraphQL schema level.
type Plugin { """Unique identifier for the plugin""" id: String """Creator of the plugin""" - pluginCreatedBy: String + pluginCreatedBy: String! """Description of the plugin""" - pluginDesc: String + pluginDesc: String! """Name of the plugin""" - pluginName: String + pluginName: String! """List of organization IDs where the plugin is not installed""" uninstalledOrgs: [String!] }
2899-2899: 🧹 Nitpick (assertive)Add description for the plugin query field.
While the
pluginsquery has a description, thepluginquery lacks one. Add a description to maintain consistency and improve API documentation.- plugin(input: QueryPluginInput!): Plugin + """Query field to fetch a specific plugin by name or ID""" + plugin(input: QueryPluginInput!): Plugin
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
schema.graphql(4 hunks)src/graphql/inputs/QueryPluginInput.ts(1 hunks)src/graphql/types/Plugins/Plugins.ts(1 hunks)src/graphql/types/Plugins/pluginData.json(1 hunks)src/graphql/types/Query/plugin.ts(1 hunks)test/routes/graphql/gql.tada.d.ts(4 hunks)
🧰 Additional context used
🧠 Learnings (4)
src/graphql/types/Plugins/pluginData.json (1)
Learnt from: MayankJha014
PR: PalisadoesFoundation/talawa-api#3294
File: src/graphql/types/Plugins/pluginData.json:5-5
Timestamp: 2025-02-24T17:00:18.335Z
Learning: The plugin data in `src/graphql/types/Plugins/pluginData.json` is sample data taken from the develop branch and should not be modified for style/formatting improvements.
src/graphql/types/Plugins/Plugins.ts (2)
Learnt from: MayankJha014
PR: PalisadoesFoundation/talawa-api#3294
File: src/graphql/types/Plugins/Plugins.ts:6-11
Timestamp: 2025-02-24T17:27:31.612Z
Learning: In the Plugins module of talawa-api, field names in Plugin type maintain consistency with sample data format, using prefixes like 'pluginName', 'pluginCreatedBy', and 'pluginDesc'.
Learnt from: MayankJha014
PR: PalisadoesFoundation/talawa-api#3294
File: src/graphql/types/Plugins/Plugins.ts:25-40
Timestamp: 2025-02-24T17:28:19.020Z
Learning: In the Talawa API codebase, GraphQL type field descriptions follow a simple and concise pattern, avoiding overly detailed descriptions to maintain consistency across types.
src/graphql/types/Query/plugin.ts (3)
Learnt from: MayankJha014
PR: PalisadoesFoundation/talawa-api#3294
File: src/graphql/types/Plugins/Plugins.ts:6-11
Timestamp: 2025-02-24T17:27:31.612Z
Learning: In the Plugins module of talawa-api, field names in Plugin type maintain consistency with sample data format, using prefixes like 'pluginName', 'pluginCreatedBy', and 'pluginDesc'.
Learnt from: MayankJha014
PR: PalisadoesFoundation/talawa-api#3294
File: src/graphql/types/Query/plugin.ts:7-35
Timestamp: 2025-02-24T17:13:27.833Z
Learning: The GraphQL builder in Talawa API doesn't support Relay-style cursor-based pagination methods like `connectionType` and `resolveArrayConnection`. Any pagination implementation should use offset-based pagination with skip/limit arguments instead.
Learnt from: MayankJha014
PR: PalisadoesFoundation/talawa-api#3294
File: src/graphql/types/Query/plugin.ts:7-35
Timestamp: 2025-02-24T17:13:27.833Z
Learning: The Talawa API uses Pothos with Relay plugin for GraphQL schema building. Pagination should be implemented using Relay-style connections with the `t.connection()` method, which automatically adds standard Relay connection arguments and types.
test/routes/graphql/gql.tada.d.ts (2)
Learnt from: MayankJha014
PR: PalisadoesFoundation/talawa-api#3294
File: src/graphql/types/Plugins/Plugins.ts:6-11
Timestamp: 2025-02-24T17:27:31.612Z
Learning: In the Plugins module of talawa-api, field names in Plugin type maintain consistency with sample data format, using prefixes like 'pluginName', 'pluginCreatedBy', and 'pluginDesc'.
Learnt from: MayankJha014
PR: PalisadoesFoundation/talawa-api#3294
File: src/graphql/types/Plugins/Plugins.ts:25-40
Timestamp: 2025-02-24T17:28:19.020Z
Learning: In the Talawa API codebase, GraphQL type field descriptions follow a simple and concise pattern, avoiding overly detailed descriptions to maintain consistency across types.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run tests for talawa api
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (4)
src/graphql/inputs/QueryPluginInput.ts (1)
12-25: Implementation looks consistent.
The builder pattern correctly maps the Zod schema to a GraphQL input type, keepingidoptional andpluginNamerequired. No issues observed here.src/graphql/types/Plugins/pluginData.json (1)
1-31: Acknowledged - no changes needed.
Since this file is sample data taken from the develop branch and should not be modified, we will not suggest any styling or data updates.test/routes/graphql/gql.tada.d.ts (2)
153-153: Query additions look good.
The new query fieldsgetPluginsandpluginproperly reference thePlugintype. This aligns with the intended design for fetching and inspecting plugin data.
167-167: Input type is consistent with the Zod schema.
This matches the newQueryPluginInputschema defined insrc/graphql/inputs/QueryPluginInput.ts, ensuring parity between GraphQL and runtime validation logic.
| @@ -140,6 +140,7 @@ export type introspection_types = { | |||
| 'OrganizationVenuesConnectionEdge': { kind: 'OBJECT'; name: 'OrganizationVenuesConnectionEdge'; fields: { 'cursor': { name: 'cursor'; type: { kind: 'NON_NULL'; name: never; ofType: { kind: 'SCALAR'; name: 'String'; ofType: null; }; } }; 'node': { name: 'node'; type: { kind: 'OBJECT'; name: 'Venue'; ofType: null; } }; }; }; | |||
| 'PageInfo': { kind: 'OBJECT'; name: 'PageInfo'; fields: { 'endCursor': { name: 'endCursor'; type: { kind: 'SCALAR'; name: 'String'; ofType: null; } }; 'hasNextPage': { name: 'hasNextPage'; type: { kind: 'NON_NULL'; name: never; ofType: { kind: 'SCALAR'; name: 'Boolean'; ofType: null; }; } }; 'hasPreviousPage': { name: 'hasPreviousPage'; type: { kind: 'NON_NULL'; name: never; ofType: { kind: 'SCALAR'; name: 'Boolean'; ofType: null; }; } }; 'startCursor': { name: 'startCursor'; type: { kind: 'SCALAR'; name: 'String'; ofType: null; } }; }; }; | |||
| 'PhoneNumber': unknown; | |||
| 'Plugin': { kind: 'OBJECT'; name: 'Plugin'; fields: { 'id': { name: 'id'; type: { kind: 'SCALAR'; name: 'String'; ofType: null; } }; 'pluginCreatedBy': { name: 'pluginCreatedBy'; type: { kind: 'SCALAR'; name: 'String'; ofType: null; } }; 'pluginDesc': { name: 'pluginDesc'; type: { kind: 'SCALAR'; name: 'String'; ofType: null; } }; 'pluginName': { name: 'pluginName'; type: { kind: 'SCALAR'; name: 'String'; ofType: null; } }; 'uninstalledOrgs': { name: 'uninstalledOrgs'; type: { kind: 'LIST'; name: never; ofType: { kind: 'NON_NULL'; name: never; ofType: { kind: 'SCALAR'; name: 'String'; ofType: null; }; }; } }; }; }; | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider making essential fields non-null.
Fields like id, pluginName, and pluginCreatedBy could potentially be defined as NON_NULL for stronger type safety if they are always expected to be present.
| @@ -0,0 +1,45 @@ | |||
| import { z } from "zod"; | |||
| // src/graphql/types/Plugin/Plugin.ts | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Fix incorrect file path in the comment.
The comment references src/graphql/types/Plugin/Plugin.ts (singular) but the actual path uses the plural form Plugins.
-// src/graphql/types/Plugin/Plugin.ts
+// src/graphql/types/Plugins/Plugins.ts📝 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.
| // src/graphql/types/Plugin/Plugin.ts | |
| // src/graphql/types/Plugins/Plugins.ts |
| builder.mutationField("updatePluginStatus", (t) => | ||
| t.field({ | ||
| type: PluginRef, | ||
| description: "Toggle plugin installation status for an organization", | ||
| args: { | ||
| pluginId: t.arg.string({ required: true }), | ||
| orgId: t.arg.string({ required: true }), | ||
| }, | ||
| resolve: async (_parent, { pluginId, orgId }, ctx) => { | ||
| try { | ||
| // Check authentication | ||
| if (!ctx.currentClient.isAuthenticated) { | ||
| throw new TalawaGraphQLError({ | ||
| extensions: { | ||
| code: "unauthenticated", | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| // Find plugin | ||
| const plugin = pluginCacheById.get(pluginId); | ||
| if (!plugin) { | ||
| throw new TalawaGraphQLError({ | ||
| extensions: { | ||
| code: "arguments_associated_resources_not_found", | ||
| issues: [ | ||
| { | ||
| argumentPath: ["pluginId"], | ||
| }, | ||
| ], | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| // Check if the org is already in uninstalledOrgs | ||
| const isUninstalled = plugin.uninstalledOrgs.includes(orgId); | ||
|
|
||
| if (isUninstalled) { | ||
| // Remove from uninstalledOrgs (installing the plugin) | ||
| plugin.uninstalledOrgs = plugin.uninstalledOrgs.filter( | ||
| (org) => org !== orgId, | ||
| ); | ||
| } else { | ||
| // Add to uninstalledOrgs (uninstalling the plugin) | ||
| plugin.uninstalledOrgs.push(orgId); | ||
| } | ||
|
|
||
| // Update cache | ||
| pluginCacheById.set(pluginId, plugin); | ||
|
|
||
| return plugin; | ||
| } catch (error) { | ||
| if (error instanceof TalawaGraphQLError) { | ||
| throw error; | ||
| } | ||
| throw new TalawaGraphQLError({ | ||
| extensions: { | ||
| code: "unexpected", | ||
| }, | ||
| }); | ||
| } | ||
| }, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add persistence mechanism for plugin changes.
The current implementation modifies in-memory data with the updatePluginStatus mutation. These changes will be lost on server restart since they're not persisted to a database or file system.
Consider implementing a persistence mechanism to save plugin installation status changes:
- Use an existing database connection if available in the project
- Implement file-based persistence to update the JSON file
- Add a comment indicating this is demo-only functionality if persistence is out of scope
🏁 Script executed:
#!/bin/bash
# Check for database or persistence implementations in the codebase
echo "Searching for database connection configurations..."
rg -l "mongoose|prisma|typeorm|sequelize|knex" --type ts
echo -e "\nChecking if sample data is meant to be persisted or just for demo..."
grep -r "sampleData" --include="*.ts" --include="*.js" .Length of output: 654
Ensure persistent storage for plugin status changes
The mutation implemented in src/graphql/types/Query/plugin.ts (lines 118-181) currently updates the plugin's installation status solely in-memory. Our repository search did not reveal any database connection configurations (e.g., mongoose, prisma, typeorm, sequelize, knex) that could be used for persistence, aside from sample data handling in ./scripts/dbManagement/helpers.ts. This means that any changes made via the updatePluginStatus mutation will be lost upon server restart.
To address this, please consider one of the following actions:
- Integrate a persistence layer: Utilize an existing database connection or add a new one to store these changes persistently.
- Implement file-based persistence: Update a JSON file or similar storage mechanism to retain the plugin status.
- Clarify demo-only behavior: If persistence is intentionally out of scope, add a comment in the code to explicitly state that the changes are temporary.
| resolve: async (_parent, { pluginId, orgId }, ctx) => { | ||
| try { | ||
| // Check authentication | ||
| if (!ctx.currentClient.isAuthenticated) { | ||
| throw new TalawaGraphQLError({ | ||
| extensions: { | ||
| code: "unauthenticated", | ||
| }, | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add authorization checks for the updatePluginStatus mutation.
Currently, the mutation only checks if the user is authenticated but doesn't verify if they have permission to update the plugin status for a specific organization. Consider adding role-based authorization checks.
resolve: async (_parent, { pluginId, orgId }, ctx) => {
try {
// Check authentication
if (!ctx.currentClient.isAuthenticated) {
throw new TalawaGraphQLError({
extensions: {
code: "unauthenticated",
},
});
}
+
+ // Check authorization - verify the user has admin rights for the organization
+ const isAdmin = await ctx.dataSource.userRoleForOrganization(
+ ctx.currentClient.userId,
+ orgId
+ ) === 'ADMIN';
+
+ if (!isAdmin) {
+ throw new TalawaGraphQLError({
+ extensions: {
+ code: "unauthorized",
+ message: "Only organization administrators can update plugin installation status"
+ },
+ });
+ }
// Find plugin📝 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.
| resolve: async (_parent, { pluginId, orgId }, ctx) => { | |
| try { | |
| // Check authentication | |
| if (!ctx.currentClient.isAuthenticated) { | |
| throw new TalawaGraphQLError({ | |
| extensions: { | |
| code: "unauthenticated", | |
| }, | |
| }); | |
| } | |
| resolve: async (_parent, { pluginId, orgId }, ctx) => { | |
| try { | |
| // Check authentication | |
| if (!ctx.currentClient.isAuthenticated) { | |
| throw new TalawaGraphQLError({ | |
| extensions: { | |
| code: "unauthenticated", | |
| }, | |
| }); | |
| } | |
| // Check authorization - verify the user has admin rights for the organization | |
| const isAdmin = await ctx.dataSource.userRoleForOrganization( | |
| ctx.currentClient.userId, | |
| orgId | |
| ) === 'ADMIN'; | |
| if (!isAdmin) { | |
| throw new TalawaGraphQLError({ | |
| extensions: { | |
| code: "unauthorized", | |
| message: "Only organization administrators can update plugin installation status" | |
| }, | |
| }); | |
| } | |
| // Find plugin |
There was a problem hiding this comment.
in ctx dataSource is not available
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Let's first look at how the context object is used in the PR
rg "ctx\." src/graphql/types/Query/plugin.ts -A 2 -B 2
# Look for authorization patterns in mutation resolvers
rg "authorization|isAuthorized|hasPermission|isAdmin|userRole" src/graphql/types/ -A 5 -B 5
# Look at imports in the plugin.ts file to understand what's available
head -20 src/graphql/types/Query/plugin.ts
# Check how roles and permissions are typically checked in the codebase
rg "userRoleForOrganization|getUserRole|role.*organization" src/ -A 3 -B 3
Length of output: 9359
🏁 Script executed:
#!/bin/bash
# Let's look at the GraphQL context definition to understand its structure
rg -g "*.ts" "GraphQLContext|context" src/graphql --include="*.d.ts" --include="context.ts" -A 10 -B 10
# Look for examples of organization role checks in mutation resolvers
rg -g "*.ts" "organization.*role|isAdmin" src/graphql/types/Mutation -A 5 -B 5
# Let's check if there are examples of fetching organization memberships
rg -g "*.ts" "organizationMemberships" src/graphql/types/ -A 5 -B 5
# Look at how plugins and organization permissions are handled elsewhere
rg -g "*.ts" "plugin.*organization|organization.*plugin" src/ -A 5 -B 5
Length of output: 75586
@MayankJha014 You're right, the dataSource property isn't available in the context. Let me provide a correct implementation that follows the codebase's patterns for authorization checks:
// Check authorization - verify the user has admin rights for the organization
const currentUserId = ctx.currentClient.user.id;
const currentUser = await ctx.drizzleClient.query.usersTable.findFirst({
with: {
organizationMembershipsWhereMember: {
columns: {
role: true,
},
where: (fields, operators) =>
operators.eq(fields.organizationId, orgId),
},
},
where: (fields, operators) => operators.eq(fields.id, currentUserId),
});
const isGlobalAdmin = currentUser.role === "administrator";
const currentUserOrganizationMembership =
currentUser.organizationMembershipsWhereMember[0];
const isOrgAdmin = currentUserOrganizationMembership?.role === "administrator";
if (!isGlobalAdmin && !isOrgAdmin) {
throw new TalawaGraphQLError({
extensions: {
code: "unauthorized_action",
message: "Only organization administrators can update plugin installation status"
},
});
}
// Find pluginThis follows the authentication pattern used throughout the codebase, checking if the user is either a global admin or an admin of the specific organization.
| builder.queryField("getPlugins", (t) => | ||
| t.field({ | ||
| type: [PluginRef], | ||
| description: "Fetch all available plugins", | ||
| resolve: async (_parent, _args, ctx) => { | ||
| try { | ||
| // Check authentication | ||
| if (!ctx.currentClient.isAuthenticated) { | ||
| throw new TalawaGraphQLError({ | ||
| extensions: { | ||
| code: "unauthenticated", | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| return pluginsData; | ||
| } catch (error) { | ||
| if (error instanceof TalawaGraphQLError) { | ||
| throw error; | ||
| } | ||
| throw new TalawaGraphQLError({ | ||
| extensions: { | ||
| code: "unexpected", | ||
| }, | ||
| }); | ||
| } | ||
| }, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Implement pagination for the getPlugins query.
Currently, the query returns all plugins without pagination, which could be problematic as the number of plugins grows. Consider implementing pagination for better scalability.
builder.queryField("getPlugins", (t) =>
t.field({
type: [PluginRef],
description: "Fetch all available plugins",
+ args: {
+ skip: t.arg.int({
+ description: "Number of records to skip",
+ required: false,
+ defaultValue: 0
+ }),
+ limit: t.arg.int({
+ description: "Number of records to fetch",
+ required: false,
+ defaultValue: 10
+ })
+ },
resolve: async (_parent, _args, ctx) => {
try {
// Check authentication
if (!ctx.currentClient.isAuthenticated) {
throw new TalawaGraphQLError({
extensions: {
code: "unauthenticated",
},
});
}
- return pluginsData;
+ const { skip = 0, limit = 10 } = _args;
+ return pluginsData.slice(skip, skip + limit);
} catch (error) {
if (error instanceof TalawaGraphQLError) {
throw error;
}📝 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.
| builder.queryField("getPlugins", (t) => | |
| t.field({ | |
| type: [PluginRef], | |
| description: "Fetch all available plugins", | |
| resolve: async (_parent, _args, ctx) => { | |
| try { | |
| // Check authentication | |
| if (!ctx.currentClient.isAuthenticated) { | |
| throw new TalawaGraphQLError({ | |
| extensions: { | |
| code: "unauthenticated", | |
| }, | |
| }); | |
| } | |
| return pluginsData; | |
| } catch (error) { | |
| if (error instanceof TalawaGraphQLError) { | |
| throw error; | |
| } | |
| throw new TalawaGraphQLError({ | |
| extensions: { | |
| code: "unexpected", | |
| }, | |
| }); | |
| } | |
| }, | |
| }), | |
| ); | |
| builder.queryField("getPlugins", (t) => | |
| t.field({ | |
| type: [PluginRef], | |
| description: "Fetch all available plugins", | |
| args: { | |
| skip: t.arg.int({ | |
| description: "Number of records to skip", | |
| required: false, | |
| defaultValue: 0 | |
| }), | |
| limit: t.arg.int({ | |
| description: "Number of records to fetch", | |
| required: false, | |
| defaultValue: 10 | |
| }) | |
| }, | |
| resolve: async (_parent, _args, ctx) => { | |
| try { | |
| // Check authentication | |
| if (!ctx.currentClient.isAuthenticated) { | |
| throw new TalawaGraphQLError({ | |
| extensions: { | |
| code: "unauthenticated", | |
| }, | |
| }); | |
| } | |
| const { skip = 0, limit = 10 } = _args; | |
| return pluginsData.slice(skip, skip + limit); | |
| } catch (error) { | |
| if (error instanceof TalawaGraphQLError) { | |
| throw error; | |
| } | |
| throw new TalawaGraphQLError({ | |
| extensions: { | |
| code: "unexpected", | |
| }, | |
| }); | |
| } | |
| }, | |
| }), | |
| ); |
|
@MayankJha014 Please fix the first comment so that each issue listed automatically closes. The PR_GUIDELINES.md file has details. Please also fill in the template for the pull request as completely as you can. Please make sure all checks are passed. |
|
ok sir |
|
started working on it |
|
@MayankJha014 this is no longer the requirement. Closing this PR & please go through the project idea list once. you will get the intent of the project. We are trying to change the whole architecture of the plugin in talawa. This apis adheres to the old design which is not very streamlines. |
This is api for fethching Plugin from SampleData
Related to : PalisadoesFoundation/talawa-admin#3559
Summary by CodeRabbit
New Features
Documentation
Plugintype and associated queries in the GraphQL schema.Tests