Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configurable Session timeout for Community #2186

Merged
merged 35 commits into from
Oct 16, 2024

Conversation

JordanCampbell1
Copy link

@JordanCampbell1 JordanCampbell1 commented Aug 16, 2024

What kind of change does this PR introduce?

Feature: Session Timeout

Issue Number:

Fixes #2151

Did you add tests for your changes?

Yes

Snapshots/Videos:

image
image
image

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 Talawa Admin will allow configurable logout that affects the community as well as automatic logout when the user is inactive for the selected session timeout period

#2151

Have you read the contributing guide?

Yes

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Added new localization strings for "Completed" in English, French, Hindi, Spanish, and Chinese to enhance user experience.
    • Improved clarity in terminology related to action items and organization settings across multiple languages.
    • Introduced new icons for "Agenda Items Category" and "Event Stats" for better visual representation.
    • Enhanced the structure and functionality of the Event Management component with new SVG icons and streamlined tab selection.
    • Added redux-thunk for improved management of asynchronous actions in the application.
    • Introduced a new Community type and updated mutations in the GraphQL schema for enhanced functionality.
  • Bug Fixes

    • Improved tooltip functionality with the latest version of the react-tooltip dependency.
    • Refined error handling in the Forgot Password process for better user feedback.
  • Chores

    • Updated project dependencies to ensure the latest features and improvements are utilized.
    • Removed outdated linting rules and added new ones to enhance type safety in TypeScript code.

Copy link

coderabbitai bot commented Aug 16, 2024

Walkthrough

The recent changes involve updates to the .eslintrc.json configuration for TypeScript linting rules, an increment in the react-tooltip dependency version, and enhancements to localization files across multiple languages. The modifications include the addition of new translation strings and the refinement of existing ones to improve clarity and user experience. Additionally, new icon components were introduced, and the structure of the EventManagement component was updated for better functionality. These updates aim to enhance the robustness of the codebase and improve user feedback mechanisms without altering other dependencies or features significantly.

Changes

File Path Change Summary
.eslintrc.json Removed @typescript-eslint/ban-types; added @typescript-eslint/no-unsafe-function-type, @typescript-eslint/no-wrapper-object-types, and @typescript-eslint/no-empty-object-type.
package.json Updated react-tooltip from 5.27.1 to 5.28.0; added redux-thunk dependency.
public/locales/en/translation.json Added new entries and renamed existing ones for clarity in localization strings.
public/locales/fr/translation.json Added new entries and modified existing translations for clarity.
public/locales/hi/translation.json Added new entries and modified existing translations for clarity.
public/locales/sp/translation.json Added new entries and modified existing translations for clarity.
public/locales/zh/translation.json Added new entries and modified existing translations for clarity.
src/components/IconComponent/IconComponent.tsx Added new icon cases for AgendaCategoryIcon and EventStatsIcon.
src/screens/EventManagement/EventManagement.tsx Replaced icon imports with SVG components; streamlined tab selection logic.
schema.graphql Added new type Community and union type ConnectionError; updated createActionItemCategory mutation and added updateSessionTimeout mutation.
src/GraphQl/Mutations/ActionItemCategoryMutations.ts Updated CREATE_ACTION_ITEM_CATEGORY_MUTATION to require isDisabled parameter.

Assessment against linked issues

Objective Addressed Explanation
Pre-timeout warnings ( #2151 ) No implementation of session timeout warnings was found.
Enhanced logout messaging ( #2151 ) No changes related to logout messaging were made.
User feedback for session management ( #2151 ) No relevant feedback mechanisms for session management were implemented.

Possibly related PRs

Suggested labels

no-pr-activity

Suggested reviewers

  • palisadoes

Poem

🐰 In the meadow, changes bloom,
Linting rules dispel the gloom.
Tooltips updated, translations bright,
Users rejoice in newfound light!
With each tweak, our app's a delight,
Hopping forward, futures in sight! 🌼


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Our Pull Request Approval Process

We have these basic policies to make the approval process smoother for our volunteer team.

Testing Your Code

Please make sure your code passes all tests. Our test code coverage system will fail if these conditions occur:

  1. The overall code coverage drops below the target threshold of the repository
  2. Any file in the pull request has code coverage levels below the repository threshold
  3. Merge conflicts

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.

Reviewers

Do not assign reviewers. Our Queue Monitors will review your PR and assign them.
When your PR has been assigned reviewers contact them to get your code reviewed and approved via:

  1. comments in this PR or
  2. our slack channel

Reviewing Your Code

Your reviewer(s) will have the following roles:

  1. arbitrators of future discussions with other contributors about the validity of your changes
  2. point of contact for evaluating the validity of your work
  3. person who verifies matching issues by others that should be closed.
  4. person who gives general guidance in fixing your tests

CONTRIBUTING.md

Read our CONTRIBUTING.md file. Most importantly:

  1. PRs with issues not assigned to you will be closed by the reviewer
  2. Fix the first comment in the PR so that each issue listed automatically closes

Other

  1. 🎯 Please be considerate of our volunteers' time. Contacting the person who assigned the reviewers is not advised unless they ask for your input. Do not @ the person who did the assignment otherwise.
  2. Read the CONTRIBUTING.md file make

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Congratulations on making your first PR! 🎊 If you haven't already, check out our Contributing Guidelines and PR Reporting Guidelines to ensure that you are following our guidelines for contributing and creating PR.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Outside diff range, codebase verification and nitpick comments (1)
src/components/UpdateSession/UpdateSession.tsx (1)

19-22: Consider initializing state with null instead of undefined.

Using null for initial state values can be more semantically meaningful when representing the absence of data.

- const [timeout, setTimeout] = useState<number | undefined>(30);
- const [communityTimeout, setCommunityTimeout] = useState<number | undefined>(30);
+ const [timeout, setTimeout] = useState<number | null>(30);
+ const [communityTimeout, setCommunityTimeout] = useState<number | null>(30);
Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a3c3555 and ac48132.

Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
Files selected for processing (11)
  • package.json (2 hunks)
  • src/GraphQl/Mutations/mutations.ts (1 hunks)
  • src/GraphQl/Queries/Queries.ts (1 hunks)
  • src/components/ProfileDropdown/ProfileDropdown.tsx (3 hunks)
  • src/components/UpdateSession/UpdateSession.css (1 hunks)
  • src/components/UpdateSession/UpdateSession.test.tsx (1 hunks)
  • src/components/UpdateSession/UpdateSession.tsx (1 hunks)
  • src/screens/CommunityProfile/CommunityProfile.tsx (2 hunks)
  • src/screens/LoginPage/LoginPage.tsx (5 hunks)
  • src/utils/useSession.test.tsx (1 hunks)
  • src/utils/useSession.tsx (1 hunks)
Files skipped from review due to trivial changes (1)
  • src/components/UpdateSession/UpdateSession.css
Additional context used
Biome
src/screens/CommunityProfile/CommunityProfile.tsx

[error] 269-269: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

Additional comments not posted (32)
src/utils/useSession.tsx (11)

1-8: Ensure all imports are necessary and correctly used.

The imports seem appropriate for the functionality provided. However, ensure that all imported modules are necessary and used correctly in the file.


10-14: Type definition for UseSessionReturnType is clear and appropriate.

The type definition for UseSessionReturnType accurately reflects the functions returned by the useSession hook. This enhances type safety and clarity.


16-36: Review the initialization and update of session timeout.

The session timeout is initialized with a default value and updated based on query data. This is a good practice for handling asynchronous data fetching.


39-42: resetTimers function is correctly implemented.

The function clears both session and warning timers, which is crucial for preventing memory leaks and ensuring timers are correctly reset.


44-48: endSession function effectively removes event listeners.

This function ensures that event listeners are removed, which is important for preventing memory leaks and unintended behavior.


65-79: initializeTimers function is well-structured.

The function correctly sets up warning and session timeout timers, providing user feedback through toast notifications. This is a good practice for enhancing user experience.


81-84: extendSession effectively resets and reinitializes timers.

This function ensures that user activity extends the session, which is essential for dynamic session management.


86-91: startSession sets up session management effectively.

The function correctly initializes timers and sets up event listeners, ensuring that the session is managed dynamically based on user activity.


93-95: Cleanup in useEffect is correctly implemented.

The cleanup function in useEffect ensures that the session is ended when the component is unmounted, preventing potential memory leaks.


97-98: Return statement is clear and concise.

The hook returns the necessary functions for session management, making it easy to integrate into other components.


100-100: Export statement is correctly implemented.

The default export of the useSession hook is appropriate for its intended use as a utility.

src/components/ProfileDropdown/ProfileDropdown.tsx (2)

10-10: Import of useSession is correctly added.

The import statement for useSession is necessary for integrating session management into the component.


28-28: Integration of endSession enhances logout functionality.

The addition of endSession in the logout process ensures that the session is properly terminated, which is crucial for security.

package.json (2)

23-23: Addition of @testing-library/react-hooks is beneficial.

This addition enhances the project's testing capabilities, allowing for more comprehensive testing of React hooks.


54-54: Update of react-tooltip to version ^5.28.0.

Ensure that the update is compatible with the current codebase and does not introduce breaking changes.

src/utils/useSession.test.tsx (4)

67-83: Test case approved: Session timeout setting.

The test case correctly verifies that the session timeout is set based on fetched data.


85-106: Test case approved: Error handling on query error.

The test case correctly verifies that the errorHandler is called when a query error occurs.


108-132: Test case approved: Logout and token revocation.

The test case correctly verifies the logout functionality and token revocation process.


134-162: Test case approved: Event listener removal on session end.

The test case correctly verifies that event listeners are removed when the session ends.

src/components/UpdateSession/UpdateSession.test.tsx (5)

79-111: Test case approved: Component rendering.

The test case correctly verifies the rendering of the UpdateTimeout component and its elements.


113-140: Test case approved: Session timeout update.

The test case correctly verifies the session timeout update and the success toast.


142-165: Test case approved: Query error handling.

The test case correctly verifies that the errorHandler is called on query errors.


167-218: Test case approved: Update error handling.

The test case correctly verifies that the errorHandler is called on update errors.


220-256: Test case approved: Handling null community object.

The test case correctly verifies that the component handles a null community object gracefully.

src/GraphQl/Queries/Queries.ts (1)

780-786: New GraphQL Query Added: GET_COMMUNITY_SESSION_TIMEOUT_DATA.

The addition of this query allows retrieval of session timeout data, which aligns with the PR's objective to enhance session management. Ensure that the backend resolver for getCommunityData supports the timeout field.

src/screens/CommunityProfile/CommunityProfile.tsx (2)

23-23: New Component Import: UpdateSession.

The addition of this import indicates the integration of session update functionality in the CommunityProfile component. Ensure that the UpdateSession component is properly implemented and tested.


422-422: Integration of UpdateSession Component.

The UpdateSession component is now part of the CommunityProfile component, enhancing session management capabilities. Verify that its integration does not introduce any UI or functional issues.

Verification successful

Integration of UpdateSession Component Verified

The UpdateSession component is correctly integrated into the CommunityProfile component, as evidenced by its import and usage. Ensure there are no conflicts with similarly named imports like UpdateTimeout.

  • UpdateSession is imported from ../../components/UpdateSession/UpdateSession.
  • It is used in the JSX of CommunityProfile.tsx.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the `UpdateSession` component integration in `CommunityProfile`.

# Test: Check the usage of `UpdateSession` component. Expect: Proper integration without errors.
rg --type tsx 'UpdateSession'

Length of output: 57


Script:

#!/bin/bash
# Description: Verify the `UpdateSession` component integration in `CommunityProfile`.

# Test: Check for the import and usage of `UpdateSession` component in `CommunityProfile.tsx`.
fd --type f --extension tsx --exec rg 'UpdateSession' {}

Length of output: 231

src/GraphQl/Mutations/mutations.ts (1)

649-653: New GraphQL Mutation Added: UPDATE_SESSION_TIMEOUT.

This mutation enables updating the session timeout, supporting the PR's objective of configurable session management. Ensure that the backend resolver for updateSessionTimeout is implemented and tested.

src/screens/LoginPage/LoginPage.tsx (4)

37-37: Verify the import path for useSession.

Ensure that the path 'utils/useSession' is correct and consistent with the project's file structure.

Verification successful

The import path for useSession is correct.

The file useSession.tsx exists at the expected location, confirming that the import statement is accurate.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence of `useSession` in the specified path.

# Test: Check if the file exists in the expected location.
fd 'useSession.ts' --exec echo "File exists at: {}"

Length of output: 94


333-333: LGTM! Verify session starting after login.

The invocation of startSession after successful login and navigation looks good.

Ensure that the session is correctly initialized and any necessary cleanup is performed.

Verification successful

Session Initialization Verified in Login Process

The startSession function is correctly defined and integrated into the login process. It is responsible for initializing session-related activities and is invoked appropriately after a successful login.

  • startSession is defined in src/utils/useSession.tsx.
  • It is imported and used in src/screens/LoginPage/LoginPage.tsx.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the session starting process after login.

# Test: Check for any cleanup or initialization tasks in `startSession`.
ast-grep --lang typescript --pattern $'function startSession($_) { $$$ }'

Length of output: 74


Script:

#!/bin/bash
# Search for the definition of the `startSession` function in the codebase.
ast-grep --lang typescript --pattern 'function startSession($_) { $$$ }'

# Search for any import statements related to `startSession`.
rg 'startSession' -A 2

Length of output: 1558


115-118: LGTM! Verify session starting logic.

The logic for starting a session when a user is logged in looks good.

Ensure that the startSession function is correctly implemented and handles all necessary session initialization tasks.

Verification successful

Session Initialization Logic Verified

The startSession function is implemented as part of a custom hook in src/utils/useSession.tsx. It correctly handles session initialization tasks such as resetting timers and adding event listeners. This aligns well with the session management requirements in the LoginPage component.

  • File: src/utils/useSession.tsx
  • Function: startSession

The session starting logic in the LoginPage component is correctly implemented.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation of `startSession`.

# Test: Check the implementation of `startSession` to ensure it initializes sessions correctly.
ast-grep --lang typescript --pattern $'function startSession($_) { $$$ }'

Length of output: 74


Script:

#!/bin/bash
# Description: Search for `startSession` across the codebase to identify its implementation.

# Search for `startSession` function or method definitions.
rg 'startSession' -A 5

# Check if `startSession` is imported from another module.
rg 'import.*startSession'

Length of output: 2947


136-136: LGTM! Verify destructuring of useSession.

The destructuring and invocation of startSession after a successful login look good.

Ensure that useSession returns the expected object structure.

Verification successful

Destructuring of useSession is correct. The useSession function correctly returns an object containing startSession, validating its usage in LoginPage.tsx.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the structure returned by `useSession`.

# Test: Check the return structure of `useSession` to ensure it includes `startSession`.
ast-grep --lang typescript --pattern $'const useSession = () => { return { startSession: $_, $$$ } }'

Length of output: 102


Script:

#!/bin/bash
# Description: Search for the `useSession` function definition and examine its return structure.

# Find the file containing the `useSession` function definition.
rg 'function useSession|const useSession =' -A 5 --type=typescript

Length of output: 103


Script:

#!/bin/bash
# Description: Search for the `useSession` function definition and examine its return structure.

# Find the file containing the `useSession` function definition.
rg 'function useSession|const useSession =' -A 5 --type=ts

Length of output: 488


Script:

#!/bin/bash
# Description: Examine the return structure of the `useSession` function to verify the presence of `startSession`.

# Search for the return statement in the `useSession` function.
rg 'return {' -A 10 src/utils/useSession.tsx

Length of output: 126


Script:

#!/bin/bash
# Description: Examine the return structure of the `useSession` function to verify the presence of `startSession`.

# Search for the return statement in the `useSession` function with escaped curly braces.
rg 'return \{' -A 10 src/utils/useSession.tsx

Length of output: 131

src/utils/useSession.tsx Outdated Show resolved Hide resolved
src/components/UpdateSession/UpdateSession.tsx Outdated Show resolved Hide resolved
src/components/UpdateSession/UpdateSession.tsx Outdated Show resolved Hide resolved
src/components/UpdateSession/UpdateSession.tsx Outdated Show resolved Hide resolved
@Cioppolo14
Copy link

@JordanCampbell1 Can you fix the failed test application test?

@JordanCampbell1
Copy link
Author

@JordanCampbell1 Can you fix the failed test application test?

sure I will take a look

coderabbitai[bot]
coderabbitai bot previously approved these changes Aug 17, 2024
@JordanCampbell1
Copy link
Author

finished up on the fixes, waiting for review

@JordanCampbell1
Copy link
Author

@Cioppolo14

Copy link

codecov bot commented Aug 20, 2024

Codecov Report

Attention: Patch coverage is 99.86541% with 1 line in your changes missing coverage. Please review.

Project coverage is 97.62%. Comparing base (6467ea1) to head (c5ccf4e).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
...rgActionItemCategories/OrgActionItemCategories.tsx 98.90% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2186      +/-   ##
===========================================
+ Coverage    97.54%   97.62%   +0.07%     
===========================================
  Files          242      250       +8     
  Lines         6888     7188     +300     
  Branches      2015     2069      +54     
===========================================
+ Hits          6719     7017     +298     
- Misses         157      159       +2     
  Partials        12       12              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@varshith257
Copy link
Member

@JordanCampbell1 Can you fix merge conflicts?

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between ac48132 and b1f4251.

Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
Files selected for processing (10)
  • package.json (2 hunks)
  • schema.graphql (2 hunks)
  • src/GraphQl/Queries/Queries.ts (1 hunks)
  • src/components/ProfileDropdown/ProfileDropdown.test.tsx (3 hunks)
  • src/components/ProfileDropdown/ProfileDropdown.tsx (3 hunks)
  • src/components/UpdateSession/UpdateSession.tsx (1 hunks)
  • src/screens/CommunityProfile/CommunityProfile.tsx (2 hunks)
  • src/screens/LoginPage/LoginPage.tsx (5 hunks)
  • src/utils/useSession.test.tsx (1 hunks)
  • src/utils/useSession.tsx (1 hunks)
Files skipped from review due to trivial changes (2)
  • package.json
  • src/utils/useSession.tsx
Files skipped from review as they are similar to previous changes (4)
  • src/GraphQl/Queries/Queries.ts
  • src/screens/CommunityProfile/CommunityProfile.tsx
  • src/screens/LoginPage/LoginPage.tsx
  • src/utils/useSession.test.tsx
Additional context used
Biome
src/components/UpdateSession/UpdateSession.tsx

[error] 60-60: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

(lint/suspicious/noGlobalIsNan)

Additional comments not posted (7)
src/components/ProfileDropdown/ProfileDropdown.test.tsx (1)

12-12: Mock setup for GET_COMMUNITY_SESSION_TIMEOUT_DATA is correctly integrated.

The mock response for the GET_COMMUNITY_SESSION_TIMEOUT_DATA query is well-defined, simulating a timeout value of 30 with a delay of 1000 milliseconds. This setup enhances the test coverage by incorporating a new data retrieval scenario.

Also applies to: 26-38

src/components/ProfileDropdown/ProfileDropdown.tsx (1)

10-10: Integration of endSession in the logout process is correct.

The endSession method is appropriately called after clearing local storage and before redirecting to the home page, ensuring a secure and orderly termination of the user session.

Also applies to: 25-50

src/components/UpdateSession/UpdateSession.tsx (3)

52-66: Type safety in handleOnChange is well-implemented.

The use of a type guard ensures that the event is handled safely, preventing runtime errors.

Tools
Biome

[error] 60-60: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

(lint/suspicious/noGlobalIsNan)


68-85: Error handling in handleOnSubmit is robust.

The try-catch block effectively captures and handles errors, ensuring that any issues during the mutation process are managed gracefully.


87-89: Loader rendering logic is correct.

The loader is appropriately displayed when the loading state is true, providing visual feedback during data fetching.

schema.graphql (2)

164-171: The Community type is well-defined.

The fields _id, logoUrl, name, socialMediaUrls, timeout, and websiteLink are relevant and correctly defined for representing community-related data.


781-781: The updateSessionTimeout mutation is correctly defined.

The mutation accepts a timeout parameter and returns a Boolean, facilitating the update of session timeout settings effectively.

src/components/UpdateSession/UpdateSession.tsx Outdated Show resolved Hide resolved
@JordanCampbell1
Copy link
Author

just fixed it @varshith257

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b1f4251 and ae0d10d.

Files selected for processing (1)
  • src/components/UpdateSession/UpdateSession.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • src/components/UpdateSession/UpdateSession.tsx

coderabbitai[bot]
coderabbitai bot previously approved these changes Aug 21, 2024
@palisadoes
Copy link
Contributor

@DMills27 @aashimawadhwa Please review

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between ae0d10d and 398d995.

Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
Files selected for processing (1)
  • package.json (2 hunks)
Files skipped from review due to trivial changes (1)
  • package.json

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 398d995 and 539d0cc.

Files selected for processing (2)
  • src/components/UpdateSession/UpdateSession.tsx (1 hunks)
  • src/utils/useSession.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • src/components/UpdateSession/UpdateSession.tsx
  • src/utils/useSession.tsx

coderabbitai[bot]
coderabbitai bot previously approved these changes Aug 22, 2024
coderabbitai[bot]
coderabbitai bot previously approved these changes Sep 30, 2024
@JordanCampbell1
Copy link
Author

@palisadoes made the PR for the develop branch for the admin as well

@palisadoes
Copy link
Contributor

@DMills27 @aashimawadhwa Please review

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (1)
schema.graphql (1)

Line range hint 653-657: Adding required parameter isDisabled may introduce breaking changes

The createActionItemCategory mutation now includes a new required parameter isDisabled: Boolean!. This change can break existing clients that do not provide this parameter.

To maintain backward compatibility, consider one of the following options:

  • Make isDisabled optional by changing its type to Boolean.
  • Provide a default value for isDisabled in the resolver when the parameter is not provided.
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between a21a99d and 635d36e.

📒 Files selected for processing (2)
  • schema.graphql (3 hunks)
  • src/GraphQl/Mutations/ActionItemCategoryMutations.ts (1 hunks)
🧰 Additional context used

schema.graphql Show resolved Hide resolved
schema.graphql Show resolved Hide resolved
coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 14, 2024
@DMills27
Copy link
Member

@JordanCampbell1 please fix the conflicts and the failing tests.

…awa-admin into session-timeoutand adjusted test and componenet files to new graphql parameter
@JordanCampbell1
Copy link
Author

@palisadoes @DMills27 @aashimawadhwa fixed all issues, should be ready for merge now

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

🧹 Outside diff range and nitpick comments (7)
src/components/OrgActionItemCategories/OrgActionItemCategoryMocks.ts (2)

1-101: LGTM! Consider adding JSDoc comments for better documentation.

The imports and MOCKS constant declaration look good. The mock data structure is well-organized and covers various scenarios for action item category operations.

Consider adding JSDoc comments to describe the purpose of the MOCKS constant and its structure. This would enhance code readability and maintainability. For example:

/**
 * Mock data for action item category GraphQL operations.
 * Includes mocks for querying categories and creating/updating them.
 */
export const MOCKS = [
  // ... existing code ...
];

1-182: Great job on the mock data structure! Consider adding type annotations.

Overall, this file provides a comprehensive set of mock data for testing action item category-related GraphQL operations. The separation of successful and error scenarios is well-thought-out and will be valuable for thorough testing.

To further improve the file, consider adding type annotations to the exported constants. This would enhance type safety and make the structure of the mock data more explicit. For example:

import { GraphQLError } from 'graphql';

type MockData = {
  request: {
    query: any;
    variables: Record<string, any>;
  };
  result?: {
    data: Record<string, any>;
  };
  error?: GraphQLError;
}[];

export const MOCKS: MockData = [
  // ... existing code ...
];

export const MOCKS_ERROR_QUERY: MockData = [
  // ... existing code ...
];

export const MOCKS_ERROR_MUTATIONS: MockData = [
  // ... existing code ...
];

This addition would make the code more robust and self-documenting.

src/components/OrgActionItemCategories/OrgActionItemCategories.tsx (2)

276-276: Use onSubmit instead of onSubmitCapture unless necessary

At line 276, the onSubmitCapture event handler is used in the <Form> component. Unless there's a specific reason to handle the submit event during the capturing phase, it's recommended to use onSubmit for standard form submission handling.

Apply this diff to make the change:

- onSubmitCapture={modalType === 'Create' ? handleCreate : handleEdit}
+ onSubmit={modalType === 'Create' ? handleCreate : handleEdit}

303-303: Add test coverage for the disabledStatus toggle

Line 303 is not covered by tests according to the static analysis hints. Adding a unit test for the disabledStatus toggle will improve test coverage and ensure it functions as expected.

Would you like assistance in writing the unit test for this functionality?

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 303-303: src/components/OrgActionItemCategories/OrgActionItemCategories.tsx#L303
Added line #L303 was not covered by tests

src/components/OrgActionItemCategories/OrgActionItemCategories.test.tsx (3)

313-313: Correct typo in test description

There's a typo in the test description at line 313. The word "disablity" should be "disability".

Apply this change:

-      test('toggle the disablity status of an action item category', async () => {
+      test('toggle the disability status of an action item category', async () => {

346-346: Correct typo in test description

Similarly, correct the typo in the test description at line 346.

Apply this change:

-      test('toast error on unsuccessful toggling of the disablity status', async () => {
+      test('toast error on unsuccessful toggling of the disability status', async () => {

157-157: Remove commented-out code

There's commented-out code at line 157. It's good practice to remove unused code to keep the codebase clean.

Apply this diff:

-          // userEvent.click(screen.getByTestId('disabledStatusToggle')); // Toggle the disabled status
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between a19994a and c5ccf4e.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • package.json (1 hunks)
  • src/components/OrgActionItemCategories/OrgActionItemCategories.test.tsx (1 hunks)
  • src/components/OrgActionItemCategories/OrgActionItemCategories.tsx (1 hunks)
  • src/components/OrgActionItemCategories/OrgActionItemCategoryMocks.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • package.json
🧰 Additional context used
📓 Learnings (1)
src/components/OrgActionItemCategories/OrgActionItemCategories.test.tsx (2)
Learnt from: Chaitanya1672
PR: PalisadoesFoundation/talawa-admin#2049
File: src/screens/OrganizationActionItems/ActionItemUpdateModal.tsx:112-138
Timestamp: 2024-07-03T07:40:16.065Z
Learning: The `istanbul ignore next` comments in the `ActionItemUpdateModal.tsx` file were added as part of a commit that introduced tests for the `ActionItemUpdateModal` component. Removing these comments and writing tests to cover the previously ignored lines is recommended to ensure code integrity and improve test coverage.
Learnt from: Chaitanya1672
PR: PalisadoesFoundation/talawa-admin#2049
File: src/screens/OrganizationActionItems/ActionItemUpdateModal.tsx:112-138
Timestamp: 2024-10-08T16:13:41.996Z
Learning: The `istanbul ignore next` comments in the `ActionItemUpdateModal.tsx` file were added as part of a commit that introduced tests for the `ActionItemUpdateModal` component. Removing these comments and writing tests to cover the previously ignored lines is recommended to ensure code integrity and improve test coverage.
🪛 GitHub Check: codecov/patch
src/components/OrgActionItemCategories/OrgActionItemCategories.tsx

[warning] 303-303: src/components/OrgActionItemCategories/OrgActionItemCategories.tsx#L303
Added line #L303 was not covered by tests

🔇 Additional comments (2)
src/components/OrgActionItemCategories/OrgActionItemCategoryMocks.ts (1)

103-111: LGTM! Error query mock is well-structured.

The MOCKS_ERROR_QUERY constant is correctly implemented to simulate an error scenario for the action item category list query. This is useful for testing error handling in the application.

src/components/OrgActionItemCategories/OrgActionItemCategories.test.tsx (1)

22-22: Ensure correct import based on export type

At line 22, ensure that OrgActionItemCategories is imported correctly based on whether it's a default or named export. If it's a named export, it should be imported with curly braces.

Please confirm that OrgActionItemCategories is exported as a default export. If it's a named export, adjust the import statement:

-import OrgActionItemCategories from './OrgActionItemCategories';
+import { OrgActionItemCategories } from './OrgActionItemCategories';

To verify the export, you can check the component file.

@DMills27 DMills27 merged commit 025bc1d into PalisadoesFoundation:develop Oct 16, 2024
10 of 12 checks passed
JordanCampbell1 added a commit to JordanCampbell1/talawa-admin that referenced this pull request Oct 20, 2024
…on#2186). This reverts commit 025bc1d.In addition, adjusted to include my feature of session timeout as well
tasneemkoushar pushed a commit that referenced this pull request Oct 20, 2024
#2344)

* Revert Configurable Session timeout for Community (#2186). This reverts commit 025bc1d.In addition, adjusted to include my feature of session timeout as well

* removed translation from other languages due to it being removed in the PR

* missed one deletion from zh language
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants