session management feature for merge #2556
Conversation
WalkthroughThe changes introduce enhancements to session management, including the addition of a configurable session timeout feature for superadmin users. This is implemented through new GraphQL mutations and schema updates, allowing for session timeout values to be set and updated. Localization files have been updated to include relevant error messages, and documentation has been adjusted for clarity and formatting. Additionally, unit tests have been added to ensure the correct functionality of the new features. Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
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 ProcessWe have these basic policies to make the approval process smoother for our volunteer team. Testing Your CodePlease make sure your code passes all tests. Our test code coverage system will fail if these conditions occur:
The process helps maintain the overall reliability of the code base and is a prerequisite for getting your PR approved. Assigned reviewers regularly review the PR queue and tend to focus on PRs that are passing. 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: 17
🧹 Outside diff range and nitpick comments (21)
src/setup/superAdmin.ts (1)
Line range hint
11-14: Consider enhancing the user instruction messageThe current console.log message is informative, but we could make it more specific to improve user understanding.
Consider updating the message to be more explicit about the registration process:
- console.log( - "\nPlease make sure to register with this email before logging in.\n", - ); + console.log( + "\nIMPORTANT: After setup, please register an account with this email address before attempting to log in as a Super Admin.\n", + );This change provides clearer instructions to the user about when and how to use the email they're about to enter.
locales/en.json (1)
32-32: LGTM! Consider making the error message more specific.The addition of the new localization string for invalid timeout range is appropriate and aligns well with the PR objectives of implementing session management features. The key follows the existing naming convention, and its placement maintains the alphabetical order of the file.
To provide more clarity to users, consider making the error message slightly more specific. For example:
- "invalid.timeoutRange": "Invalid timeout range", + "invalid.timeoutRange": "Invalid session timeout range. Please enter a valid value.",This change would give users a clearer understanding of what the error relates to and prompt them to take corrective action.
locales/sp.json (1)
31-31: LGTM! Consider a minor adjustment for consistency.The new translation entry for "invalid.timeoutRange" is correctly formatted and appropriately conveys the intended meaning in Spanish. This addition aligns well with the PR objectives of implementing session timeout functionality.
For consistency with other entries in this file, consider capitalizing the first letter of the translation. Here's a suggested minor adjustment:
- "invalid.timeoutRange": "Rango de tiempo de espera no válido", + "invalid.timeoutRange": "Rango de tiempo de espera no válido",This change would make the entry consistent with other error messages in the file, which generally start with a capital letter.
src/models/Community.ts (2)
22-22: LGTM! Consider adding property documentation.The addition of the
timeoutproperty to theInterfaceCommunityinterface is consistent with the PR objectives for implementing session management.Consider adding JSDoc comments for the
timeoutproperty to improve code documentation:/** * Timeout duration in minutes for the community session. */ timeout: number;
39-41: LGTM! Consider reordering parameters in documentation.The added documentation for the
timeoutparameter is clear and consistent with the new property in the schema.Consider reordering the parameters in the documentation to match their order in the schema definition. Move the
@param timeoutline to be the last parameter in the list for consistency.src/utilities/auth.ts (1)
29-34: Approved with suggestion: Add error handling for community fetch.The implementation of configurable session timeouts aligns well with the PR objectives. The default timeout of 30 minutes provides a good fallback.
However, consider adding error handling for the community fetch to ensure robustness:
let timeout = 30; // in minutes try { const community = await Community.findOne().lean(); if (community) { timeout = community.timeout; } } catch (error) { console.error('Failed to fetch community settings:', error); // Optionally, you could log this error or handle it in a way that fits your error management strategy }This ensures that any issues with fetching community settings won't break the token creation process.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 33-34: src/utilities/auth.ts#L33-L34
Added lines #L33 - L34 were not covered by teststests/utilities/auth.spec.ts (2)
Line range hint
41-46: Approved: Asynchronous handling ofcreateAccessTokenThe addition of
awaitto thecreateAccessTokenfunction call is a good improvement. It ensures that the test case correctly waits for the token to be created before proceeding with assertions, which is crucial for reliable testing of asynchronous operations.For consistency, consider updating the test case description to reflect the asynchronous nature of the operation:
- it("should create a JWT token with the correct payload", async () => { + it("should asynchronously create a JWT token with the correct payload", async () => {This minor change would make the test description more accurate and align it with the updated implementation.
Line range hint
1-105: Suggestion: Add tests for session timeout functionality and ensure consistencyWhile the current changes improve the asynchronous handling of
createAccessToken, there are a few suggestions to better align the tests with the PR objectives:
Add tests for the new session timeout functionality mentioned in the PR objectives. This could include tests for:
- Setting and retrieving session timeout values
- Handling expired sessions
- Pre-timeout warnings (if implemented)
For consistency, consider updating the
createRefreshTokentest to be asynchronous, similar to thecreateAccessTokentest:- it("should create a JWT token with the correct payload", () => { + it("should asynchronously create a JWT token with the correct payload", async () => { - const token = createRefreshToken( + const token = await createRefreshToken(
- Consider adding tests for any new GraphQL mutations related to session management that were mentioned in the AI-generated summary.
These additions would ensure comprehensive test coverage for the new session management features and maintain consistency in the testing approach.
src/typeDefs/mutations.ts (1)
341-341: LGTM! Consider enhancing with input validation and more detailed return type.The
updateSessionTimeoutmutation is a good addition that aligns well with the PR objectives of implementing configurable session timeouts. The use of the@authdirective ensures that only authenticated users can modify this setting, which is appropriate for a security-related feature.To further improve this mutation, consider the following suggestions:
- Add input validation for the
timeoutargument, such as minimum and maximum allowed values.- Consider returning a more detailed type instead of just a Boolean. This could include the updated timeout value and any relevant metadata.
- If this mutation is intended only for superadmin users (as mentioned in the PR objectives), you might want to add a role-based access control directive, similar to
@role(requires: SUPERADMIN)used in other mutations.Example of a more detailed return type:
type SessionTimeoutUpdateResult { success: Boolean! updatedTimeout: Int message: String }Then the mutation could be:
updateSessionTimeout(timeout: Int!): SessionTimeoutUpdateResult! @auth @role(requires: SUPERADMIN)src/typeDefs/types.ts (2)
132-132: Session management implementation needs further developmentThe addition of the
timeoutfield to theCommunitytype is a good start towards implementing session management. However, to fully realize the session management feature as described in the PR objectives, consider the following suggestions:
Introduce additional types related to session management, such as:
type Session { _id: ID! user: User! community: Community! expiresAt: DateTime! } type SessionInfo { isActive: Boolean! expiresAt: DateTime! timeRemaining: Int! }Add mutations for managing sessions, for example:
type Mutation { createSession(userId: ID!, communityId: ID!): Session! extendSession(sessionId: ID!): Session! endSession(sessionId: ID!): Boolean! }Include queries for retrieving session information:
type Query { getSessionInfo(sessionId: ID!): SessionInfo! }Consider adding a user-specific timeout setting for more flexibility. This could be added to the
Usertype:type User { # ... existing fields sessionTimeout: Int }These additions would provide a more comprehensive framework for session management, allowing for the creation, extension, and termination of sessions, as well as retrieval of session information.
132-132: Summary: Session management implementation needs expansionThe addition of the
timeoutfield to theCommunitytype is a positive first step towards implementing session management. However, to fully meet the objectives outlined in the PR description, the following next steps are recommended:
- Expand the type definitions to include session-related types and operations.
- Consider the scope of timeout settings (community-wide vs. user-specific).
- Implement the backend logic for session creation, extension, and termination.
- Develop the frontend components for session management, including timeout warnings and logout functionality.
These enhancements will create a more robust and flexible session management system, aligning closely with the stated PR objectives. Please update the PR with these additional changes or create follow-up tasks to address these points.
src/constants.ts (1)
721-722: LGTM: Well-defined constants for timeout range.The
MINIMUM_TIMEOUT_MINUTESandMAXIMUM_TIMEOUT_MINUTESconstants are correctly defined and consistent with the description in theINVALID_TIMEOUT_RANGEconstant. This approach allows for easy modification of the range if needed in the future.Consider adding a brief comment above these constants to explain their purpose and relationship to the
INVALID_TIMEOUT_RANGEconstant. This would improve code readability and maintainability. For example:+// Define the valid range for session timeout in minutes export const MINIMUM_TIMEOUT_MINUTES = 15; export const MAXIMUM_TIMEOUT_MINUTES = 60;schema.graphql (2)
233-233: LGTM! Consider adding a description for thetimeoutfield.The addition of the
timeoutfield to theCommunitytype is appropriate for implementing configurable session timeouts.Consider adding a description to clarify the purpose and unit of the timeout (e.g., seconds, minutes). For example:
""" The session timeout duration in seconds. If not set, the default timeout will be used. """ timeout: Int
1188-1188: LGTM! Consider adding input validation for thetimeoutargument.The
updateSessionTimeoutmutation is correctly implemented to allow updating the session timeout value.Consider adding input validation for the
timeoutargument to ensure it's within a reasonable range. You could use a custom scalar or an input type with constraints. For example:scalar PositiveInt input UpdateSessionTimeoutInput { timeout: PositiveInt! } type Mutation { updateSessionTimeout(input: UpdateSessionTimeoutInput!): Boolean! }This change would ensure that only positive integers are accepted as timeout values.
src/types/generatedGraphQLTypes.ts (2)
1279-1279: LGTM: NewupdateSessionTimeoutmutation addedThe addition of the
updateSessionTimeoutmutation to theMutationtype is appropriate and aligns with the PR objective. The boolean return type is suitable for indicating the success or failure of the operation.Consider adding a comment to describe the purpose of this mutation and what the boolean return value represents (e.g.,
truefor successful update,falsefor failure). This would enhance code readability and maintainability.
1924-1928: LGTM: Resolver forupdateSessionTimeoutadded toMutationResolversThe addition of the
updateSessionTimeoutresolver to theMutationResolverstype is correct and consistent with the new mutation. Thetimeoutargument is appropriately typed as an integer input, and the use ofRequireFieldsensures that this argument is mandatory.Consider adding input validation for the
timeoutvalue, such as ensuring it's a positive integer and within a reasonable range (e.g., 1 minute to 24 hours). This could be done in the resolver implementation or by creating a custom scalar type for timeout durations.Example of a custom scalar (to be implemented separately):
export interface TimeoutDurationScalarConfig extends GraphQLScalarTypeConfig<ResolversTypes['TimeoutDuration'], any> { name: 'TimeoutDuration'; }Then update the resolver to use this custom scalar:
updateSessionTimeout?: Resolver<ResolversTypes['Boolean'], ParentType, ContextType, RequireFields<MutationUpdateSessionTimeoutArgs, 'timeout'>>;This would provide stronger typing and built-in validation for the timeout value.
src/resolvers/Mutation/updateSessionTimeout.ts (2)
40-40: Remove unnecessary commented-out codeThere is commented-out code on line 40 that is no longer needed. Removing such code improves readability and maintainability.
Apply this diff to remove the commented code:
- //const appuserprofile: InterfaceAppUserProfile | null = await AppUserProfile.findOne({userId: userId}).lean();
2-2: Inconsistent import paths for modelsThe
UserandAppUserProfilemodels are imported from"../../models", whereasCommunityis imported from"../../models/Community". For consistency and simplicity, consider importing all models from the same module if possible.Apply this diff to adjust the import statement:
- import { Community } from "../../models/Community"; + import { Community } from "../../models";Also applies to: 14-14
src/resolvers/Mutation/login.ts (1)
58-60: Simplify the query for retrievingAppUserProfileThe query includes
appLanguageCode: "en"andtokenVersion: 0, which may not be necessary to uniquely identify theAppUserProfileassociated with the user. Unless there's a specific reason for these additional filters, consider simplifying the query to:await AppUserProfile.findOne({ - userId: user._id, - appLanguageCode: "en", - tokenVersion: 0, + userId: user._id, }).lean();tests/resolvers/Mutation/UpdateSessionTimeout.spec.ts (2)
219-231: Verify the updated session timeout in the databaseIn the test "updates session timeout successfully," consider asserting that the
timeoutvalue in theCommunitycollection has been updated to the expected value. This will ensure that the mutation not only returnstruebut also effectively changes the data in the database.You can add the following assertions after line 228:
const result = await updateSessionTimeoutResolver?.({}, args, context); expect(result).toEqual(true); + const updatedCommunity = await Community.findOne({}); + expect(updatedCommunity.timeout).toEqual(15);
84-89: Unnecessary unmocking of"../../../src/constants"In the
afterEachhook, you callvi.doUnmock("../../../src/constants"), but there is no correspondingvi.mockcall for this module. This unmocking may be unnecessary and can be removed to streamline the test cleanup process.Apply this diff to remove the unnecessary unmocking:
afterEach(() => { vi.restoreAllMocks(); - vi.doUnmock("../../../src/constants"); vi.resetModules(); });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (20)
- CONTRIBUTING.md (2 hunks)
- README.md (1 hunks)
- locales/en.json (1 hunks)
- locales/fr.json (1 hunks)
- locales/hi.json (1 hunks)
- locales/sp.json (1 hunks)
- locales/zh.json (1 hunks)
- schema.graphql (2 hunks)
- src/constants.ts (4 hunks)
- src/models/Community.ts (3 hunks)
- src/resolvers/Mutation/index.ts (2 hunks)
- src/resolvers/Mutation/login.ts (3 hunks)
- src/resolvers/Mutation/updateSessionTimeout.ts (1 hunks)
- src/setup/superAdmin.ts (1 hunks)
- src/typeDefs/mutations.ts (1 hunks)
- src/typeDefs/types.ts (1 hunks)
- src/types/generatedGraphQLTypes.ts (5 hunks)
- src/utilities/auth.ts (2 hunks)
- tests/resolvers/Mutation/UpdateSessionTimeout.spec.ts (1 hunks)
- tests/utilities/auth.spec.ts (1 hunks)
✅ Files skipped from review due to trivial changes (2)
- CONTRIBUTING.md
- README.md
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/resolvers/Mutation/login.ts
[warning] 86-91: src/resolvers/Mutation/login.ts#L86-L91
Added lines #L86 - L91 were not covered by testssrc/utilities/auth.ts
[warning] 33-34: src/utilities/auth.ts#L33-L34
Added lines #L33 - L34 were not covered by tests
🔇 Additional comments (18)
src/setup/superAdmin.ts (1)
19-20: Excellent type annotation improvement!The explicit type annotation
(input: string): boolean | stringfor thevalidatefunction is a great improvement. It clearly communicates that the function returns either a boolean (for valid emails) or a string (for error messages), enhancing type safety and code readability.This change aligns with TypeScript best practices and will help catch potential type-related errors earlier in the development process. It also serves as inline documentation, making the code more self-explanatory for other developers.
locales/hi.json (1)
32-32: New localization entry looks good.The new entry
"invalid.timeoutRange": "अमान्य टाइमआउट रेंज"has been correctly added to the Hindi localization file. This addition aligns with the PR objectives of implementing session management features, specifically for handling invalid timeout ranges.A few observations:
- The key
invalid.timeoutRangefollows the existing naming convention in the file.- The Hindi translation "अमान्य टाइमआउट रेंज" appears to be accurate, translating to "Invalid timeout range" in English.
- The new entry is properly formatted and consistent with other entries in the file.
locales/fr.json (1)
31-31: LGTM! Consider verifying the translation.The new entry for "invalid.timeoutRange" has been added correctly and maintains the file's structure and formatting. The translation appears to convey the intended meaning of "Invalid timeout range" in French.
To ensure the highest quality of localization, consider having a native French speaker or professional translator verify the accuracy and naturalness of the translation "Plage de temps d'attente non valide".
src/models/Community.ts (2)
Line range hint
1-85: Summary: Changes align well with PR objectivesThe modifications to
src/models/Community.tseffectively implement the session timeout functionality as outlined in the PR objectives. The newtimeoutproperty has been consistently added to the interface, schema documentation, and schema definition. These changes provide a solid foundation for the session management feature, allowing for configurable timeout durations within the specified constraints.The implementation demonstrates attention to detail, including appropriate type definitions, constraints, and error messages. With the minor documentation improvements suggested, this file will be in excellent shape to support the new session management feature.
80-85: LGTM! Schema update aligns with PR objectives.The addition of the
timeoutfield to thecommunitySchemais well-implemented and consistent with the PR objectives for configurable session timeouts.To ensure this change is properly integrated, let's verify that the new
timeoutfield is being used in the session management logic:This script will help us confirm that the new
timeoutfield is being utilized in the session management implementation.src/utilities/auth.ts (4)
4-4: LGTM: New import for Community model.The addition of the
Communityimport is appropriate for the new functionality to fetch community-specific timeout settings.
43-43: LGTM: Timeout added to token payload.The addition of the
timeoutfield to the token payload is a good implementation. This allows for client-side session management based on the server-configured timeout, which aligns well with the PR objectives for improved session management.
Line range hint
1-91: Overall assessment: Good implementation with room for minor improvements.The changes to
createAccessTokenfunction successfully implement configurable session timeouts, aligning well with the PR objectives for improved session management. The main improvements are:
- Async function to fetch community-specific timeout settings.
- Inclusion of the timeout in the token payload for client-side session management.
Suggestions for further improvement:
- Add error handling for the community fetch operation.
- Increase test coverage, particularly for the new timeout logic.
These changes significantly enhance the session management capabilities of the application. Great work on implementing this feature!
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 33-34: src/utilities/auth.ts#L33-L34
Added lines #L33 - L34 were not covered by tests
25-28: LGTM: Function signature updated to async.The change to make
createAccessTokenan async function returning aPromise<string>is correct and necessary for the new asynchronous community settings fetch.Let's verify if all callers of this function have been updated to handle the new async nature:
✅ Verification successful
It seems the previous script failed because
rgdoesn't recognize thetypescriptfile type. Let's update the script to use the correct type identifiertsto ensure accurate searches.
Verified: All callers of
createAccessTokenproperly handle its asynchronous nature.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all calls to createAccessToken and check if they're properly awaited # Search for createAccessToken calls echo "Calls to createAccessToken:" rg --type typescript "createAccessToken\(" -A 2 # Search for async/await usage with createAccessToken echo "\nAsync/await usage with createAccessToken:" rg --type typescript "await\s+createAccessToken\("Length of output: 334
Script:
#!/bin/bash # Description: Find all calls to createAccessToken and check if they're properly awaited # Search for createAccessToken calls echo "Calls to createAccessToken:" rg --type ts "createAccessToken\(" -A 2 # Search for async/await usage with createAccessToken echo "\nAsync/await usage with createAccessToken:" rg --type ts "await\s+createAccessToken\("Length of output: 1489
src/resolvers/Mutation/index.ts (2)
119-119: LGTM: Import statement forupdateSessionTimeoutis correctly added.The import statement for
updateSessionTimeoutis properly placed and follows the existing pattern in the file. This addition aligns with the PR objectives of implementing session management functionality.
119-119: Summary: Session timeout mutation correctly integrated, consider broader implementation.The changes in this file correctly integrate the
updateSessionTimeoutmutation into the GraphQL API. This aligns well with the PR objectives of enhancing session management and allowing for configurable session timeouts.To ensure a comprehensive implementation of the session management feature:
- Verify the implementation of
updateSessionTimeoutin its dedicated file.- Check for corresponding changes in the GraphQL schema.
- Ensure proper error handling and validation in the mutation resolver.
- Confirm the existence of unit tests for this new functionality.
- Verify the integration with the frontend, especially in the admin portal where this configuration would likely be set.
To assist in verifying these points, you can run the following script:
#!/bin/bash # Description: Verify the broader implementation of the session management feature # Check for updateSessionTimeout implementation echo "Checking updateSessionTimeout implementation:" rg --type typescript "export const updateSessionTimeout" src/resolvers/Mutation # Check for GraphQL schema changes echo "\nChecking GraphQL schema for updateSessionTimeout:" rg --type graphql "updateSessionTimeout" src/typeDefs # Check for error handling in the mutation echo "\nChecking for error handling in updateSessionTimeout:" rg --type typescript "throw new" src/resolvers/Mutation/updateSessionTimeout.ts # Check for unit tests echo "\nChecking for unit tests:" fd --type f "updateSessionTimeout.test.ts" tests # Check for frontend integration (this might be in a separate repo, adjust path as needed) echo "\nChecking for frontend integration (adjust path if needed):" rg --type typescript "updateSessionTimeout" src/componentsThis script will help verify the broader implementation of the session management feature across different parts of the codebase.
Also applies to: 243-243
src/typeDefs/types.ts (1)
132-132: Approve addition oftimeoutfield with suggestions for improvementThe addition of the
timeoutfield to theCommunitytype is a good step towards implementing the session management feature. However, I have a few suggestions to enhance its implementation:
Consider marking the field as non-null (
timeout: Int!) if it's a required setting for all communities. This ensures that a timeout value is always present.Add a comment explaining the purpose and usage of the
timeoutfield. For example:# Configurable session timeout in seconds for this community timeout: IntCould you please clarify the scope of this timeout? Is it meant to be a community-wide setting applied to all users, or will it be overridden by user-specific settings?
To ensure this change is consistent with the rest of the codebase, please run the following command:
This will help us verify if there are any other similar timeout implementations that we should align with.
✅ Verification successful
Approve addition of
timeoutfield with recommended improvementsThe addition of the
timeoutfield to theCommunitytype aligns well with the session management feature. Based on existing implementations, I have a few suggestions to ensure consistency and clarity:
Mark
timeoutas non-null:timeout: Int!This ensures that a timeout value is always provided, aligning with its usage in mutations.
Add a descriptive comment:
# Configurable session timeout in seconds for this community timeout: Int!Clarify the scope of the timeout:
- Is this timeout intended to apply to all users within the community, or can it be overridden by user-specific settings?
These adjustments will enhance the reliability and maintainability of the session management feature.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other timeout-related fields or types rg --type typescript 'timeout.*:.*Int' src/Length of output: 80
Script:
#!/bin/bash # Search for other timeout-related fields or types in TypeScript files rg 'timeout\s*:\s*Int' --glob '*.ts' --glob '*.tsx' src/Length of output: 179
src/constants.ts (3)
46-51: LGTM: Well-defined error constant for app user profile not found.The
APP_USER_PROFILE_NOT_FOUND_ERRORconstant is correctly defined and follows the established pattern for error constants in this file. It provides all necessary information for proper error handling and localization.
67-72: LGTM: Well-defined error constant for community not found.The
COMMUNITY_NOT_FOUND_ERRORconstant is correctly defined and follows the established pattern for error constants in this file. It provides all necessary information for proper error handling and localization.
162-167: LGTM: Well-defined error constant for invalid timeout range.The
INVALID_TIMEOUT_RANGEconstant is correctly defined and follows the established pattern for error constants in this file. It provides all necessary information for proper error handling and localization.Let's verify if the timeout range mentioned in the description (15 to 60 minutes) is consistent with the implementation:
✅ Verification successful
Verified: The timeout range in
INVALID_TIMEOUT_RANGEaccurately reflects its implementation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the timeout range implementation echo "Checking for MINIMUM_TIMEOUT_MINUTES and MAXIMUM_TIMEOUT_MINUTES constants:" grep -n "MINIMUM_TIMEOUT_MINUTES\|MAXIMUM_TIMEOUT_MINUTES" src/constants.ts echo "\nChecking for usage of these constants in timeout validation:" rg "MINIMUM_TIMEOUT_MINUTES|MAXIMUM_TIMEOUT_MINUTES" --type typescriptLength of output: 575
Script:
#!/bin/bash # Verify the usage of MINIMUM_TIMEOUT_MINUTES and MAXIMUM_TIMEOUT_MINUTES in TypeScript files echo "Checking for usage of MINIMUM_TIMEOUT_MINUTES and MAXIMUM_TIMEOUT_MINUTES in .ts files:" rg "MINIMUM_TIMEOUT_MINUTES|MAXIMUM_TIMEOUT_MINUTES" -g "*.ts"Length of output: 707
src/types/generatedGraphQLTypes.ts (1)
312-312: LGTM: Newtimeoutfield added toCommunitytypeThe addition of the optional
timeoutfield to theCommunitytype is appropriate and aligns with the PR objective of implementing session timeout functionality. The use ofMaybe<Scalars['Int']['output']>allows for flexibility in setting or omitting this value.src/resolvers/Mutation/updateSessionTimeout.ts (1)
52-52: Access control check is appropriately implementedThe call to
superAdminCheck(appuserprofile);ensures that only superadmin users can perform this action, which is necessary for security.src/resolvers/Mutation/login.ts (1)
94-94: Confirm the asynchronous behavior ofcreateAccessTokenThe
createAccessTokenfunction is now awaited, suggesting that it returns a Promise. Ensure that this function is indeed asynchronous and that all internal logic supports this change.
…; branch 'develop' of https://github.com/PalisadoesFoundation/talawa-api into develop
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
README.md (2)
24-24: Remove extra space after hash in headingThere's an extra space after the hash symbol in the "Table of Contents" heading. This doesn't align with common markdown style guidelines and has been flagged by the Markdownlint tool (MD019).
To maintain consistency with other headings in the document, please remove the extra space. Here's the suggested change:
-## Table of Contents +## Table of ContentsWould you like me to make this change for you?
🧰 Tools
🪛 Markdownlint
24-24: null
Multiple spaces after hash on atx style heading(MD019, no-multiple-space-atx)
Line range hint
1-24: Consider updating README with new feature informationThe PR introduces a significant new feature for session management, but this isn't reflected in the README. Consider adding a brief mention of this new feature to keep the documentation up-to-date and inform users about this important change.
You could add a bullet point under the "Core features include:" section, like this:
Core features include: 1. Membership management 2. Groups management 3. Event registrations 4. Recurring meetings 5. Facilities registrations +6. Configurable session managementWould you like me to draft a more detailed description of the new feature for the README?
🧰 Tools
🪛 LanguageTool
[uncategorized] ~22-~22: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...university students to work on selected open source projects. They are mentored by software...(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Markdownlint
24-24: null
Multiple spaces after hash on atx style heading(MD019, no-multiple-space-atx)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (8)
- README.md (1 hunks)
- schema.graphql (2 hunks)
- src/models/Community.ts (3 hunks)
- src/resolvers/Mutation/index.ts (2 hunks)
- src/setup/superAdmin.ts (1 hunks)
- src/typeDefs/mutations.ts (1 hunks)
- src/typeDefs/types.ts (1 hunks)
- src/types/generatedGraphQLTypes.ts (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- schema.graphql
- src/models/Community.ts
- src/resolvers/Mutation/index.ts
- src/setup/superAdmin.ts
- src/typeDefs/mutations.ts
- src/typeDefs/types.ts
🧰 Additional context used
🪛 Markdownlint
README.md
24-24: null
Multiple spaces after hash on atx style heading(MD019, no-multiple-space-atx)
🔇 Additional comments (3)
src/types/generatedGraphQLTypes.ts (3)
321-321: LGTM! Important addition for session management.The new
timeoutfield in the Community type is a key component for implementing the configurable session timeout functionality. Its optional integer type allows for flexible configuration at the community level.
1287-1289: Excellent addition for session timeout management!The new
updateSessionTimeoutmutation is crucial for implementing the configurable session timeout feature. It provides a clear and type-safe way to update the timeout value, returning a boolean to indicate the operation's success. This directly addresses the PR objective of enhancing session management.
1923-1927: Well-defined resolver type for updateSessionTimeout!The addition of the
updateSessionTimeoutfield to the MutationResolvers type ensures type safety and consistency between the GraphQL schema and the TypeScript implementation. This is crucial for maintaining code quality and preventing potential runtime errors related to type mismatches.
…; branch 'develop' of https://github.com/PalisadoesFoundation/talawa-api into develop
What kind of change does this PR introduce?
Feature: Session Timeout
Issue Number:
Fixes #2432
Did you add tests for your changes?
Yes
Summary
Making the change to include a session timeout, hence creating sessions for the application, ensures that there is a layer of security when users use the application. These changes to the API allow sessions to be managed in Talawa Admin.
#2432
Have you read the contributing guide?
Yes
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Tests