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

workflow which ensures that talawa Api app starts in docker #2759

Merged
merged 61 commits into from
Jan 1, 2025

Conversation

prayanshchh
Copy link
Contributor

@prayanshchh prayanshchh commented Dec 14, 2024

What kind of change does this PR introduce?
Changing workflow to ensure that talawa api starts in docker

Issue Number:
#2638
#2598

Fixes #

Did you add tests for your changes?
No

Snapshots/Videos:

If relevant, did you update the documentation?

Summary

Does this PR introduce a breaking change?

Other information

Have you read the contributing guide?

Summary by CodeRabbit

Release Notes

  • New Features

    • Added a new Docker health check job in the GitHub Actions workflow.
    • Introduced a script for initializing MongoDB with replica set capabilities.
    • Added a prompt for user-defined working directory during Docker setup.
  • Infrastructure

    • Refined Docker Compose configuration for MongoDB and API services, including service renaming and health checks.
  • Configuration

    • Added new environment variable for working directory support in the configuration.
    • Updated sample environment configuration with the new working directory variable.
  • Development

    • Improved test helper utilities for volunteer tracking by updating recorded hours to reflect the current date.
    • Introduced a new script for checking Docker file readiness.
    • Updated user password hashes in sample data.
    • Enhanced module resolution with new path aliases in TypeScript configuration.
    • Updated database connection logic in tests to specify the "talawa-api" database.

Copy link

coderabbitai bot commented Dec 14, 2024

Walkthrough

This pull request introduces comprehensive Docker-related enhancements across multiple configuration files. The changes focus on improving the GitHub Actions workflow to include a new Docker-Check job that verifies the Talawa API can successfully start in a Docker environment. The modifications span .github/workflows/pull-request.yml, docker-compose.dev.yaml, .env.sample, setup.ts, and tests/helpers/volunteers.ts, with a primary goal of ensuring robust Docker deployment and testing capabilities.

Changes

File Change Summary
.github/workflows/pull-request.yml Added Docker-Check job to validate Docker container startup and service health
docker-compose.dev.yaml - Renamed mongodb to mongo service
- Added container health checks
- Updated port and volume mappings
- Removed caddy service
.env.sample Added new PWD environment variable
setup.ts Added prompt for working directory (PWD) during Docker setup
tests/helpers/volunteers.ts Updated volunteer hours tracking to use current date
.github/workflows/scripts/docker_file_check.sh Introduced script to check Docker file readiness
init-mongo.sh Added script for initializing MongoDB with replica set capabilities
sample_data/users.json Updated password hash for a specific user entry
src/utilities/loadSampleData.ts Specified database name in connection calls
tsconfig.json Added module path aliases for easier imports
tests/resolvers/Query/getVolunteerRanks.spec.ts Updated expected values for volunteer hours in tests
tests/setup/checkExistingMongoDB.spec.ts Modified expected MongoDB URL in tests
tests/setup/mongoDB.spec.ts Enhanced MongoDB connection tests
tests/utilities/loadSampleData.spec.ts Updated database connection in tests

Possibly related issues

Possibly related PRs

Suggested reviewers

  • palisadoes

Poem

🐰 In Docker's realm, a rabbit hops with glee,
Checking containers, setting services free!
Workflows dance, health checks take flight,
Talawa API starts, everything's just right! 🚢
Code deployment, smooth as can be! 🌟


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 generate docstrings to generate docstrings for this PR. (Beta)
  • @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

Thanks for contributing!

Testing Your Code

Remember, your PRs won't be reviewed until these criteria are met:

  1. We don't merge PRs with poor code quality.
    1. Follow coding best practices such that CodeRabbit.ai approves your PR.
  2. We don't merge PRs with failed tests.
    1. When tests fail, click on the Details link to learn more.
    2. Write sufficient tests for your changes (CodeCov Patch Test). Your testing level must be better than the target threshold of the repository
    3. Tests may fail if you edit sensitive files. Ask to add the ignore-sensitive-files-pr label if the edits are necessary.
  3. We cannot merge PRs with conflicting files. These must be fixed.

Our policies make our code better.

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

@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 and nitpick comments (2)
.github/workflows/pull-request.yml (2)

366-372: Use environment variable for health check URL

The health check URL is hardcoded. Consider using the HEALTH_CHECK_URL environment variable:

-until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://talawa-api-dev:4000/health" 2>&1 || [ $timeout -eq 0 ]; do
+until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "${HEALTH_CHECK_URL}/health" 2>&1 || [ $timeout -eq 0 ]; do
🧰 Tools
🪛 actionlint (1.7.4)

372-372: could not parse as YAML: yaml: line 372: could not find expected ':'

(syntax-check)

🪛 yamllint (1.35.1)

[error] 366-366: trailing spaces

(trailing-spaces)


392-407: Fix environment variables indentation

The env: section should be at the same level as steps::

-  env:
-      HEALTH_CHECK_URL: http://localhost:4000
-      COMPOSE_PROJECT_NAME: pr-${{ github.event.pull_request.number }}
+    env:
+      HEALTH_CHECK_URL: http://localhost:4000
+      COMPOSE_PROJECT_NAME: pr-${{ github.event.pull_request.number }}
🧰 Tools
🪛 yamllint (1.35.1)

[error] 407-407: trailing spaces

(trailing-spaces)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6bd0fec and 4fa5812.

📒 Files selected for processing (1)
  • .github/workflows/pull-request.yml (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml

372-372: could not parse as YAML: yaml: line 372: could not find expected ':'

(syntax-check)

🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml

[error] 322-322: trailing spaces

(trailing-spaces)


[error] 323-323: trailing spaces

(trailing-spaces)


[error] 327-327: trailing spaces

(trailing-spaces)


[error] 366-366: trailing spaces

(trailing-spaces)


[error] 407-407: trailing spaces

(trailing-spaces)


[error] 374-374: syntax error: could not find expected ':'

(syntax)

.github/workflows/pull-request.yml Outdated Show resolved Hide resolved
.github/workflows/pull-request.yml Outdated Show resolved Hide resolved
.github/workflows/pull-request.yml Outdated Show resolved Hide resolved
.github/workflows/pull-request.yml Outdated Show resolved Hide resolved
.github/workflows/pull-request.yml Outdated Show resolved Hide resolved
@prayanshchh
Copy link
Contributor Author

making changes as suggested

…ansh-talawa-api into api_fail_docker

merge# Please enter a commit message to explain why this merge is necessary,
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: 2

🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)

311-320: Optimize cache configuration

Consider adding package-lock.json to the cache key for better cache hits.

 - name: Cache Node.js dependencies
   uses: actions/cache@v4
   with:
     path: |
       ~/.npm
       node_modules
-    key: ${{ runner.os }}-docker-check-${{ hashFiles('**/package-lock.json') }}
+    key: ${{ runner.os }}-docker-check-node-${{ hashFiles('package-lock.json') }}
     restore-keys: |
-      ${{ runner.os }}-docker-check-
+      ${{ runner.os }}-docker-check-node-

333-337: Improve error handling in container startup

Consider capturing and displaying the exit code for better debugging.

-if ! timeout 300 docker-compose -f docker-compose.dev.yaml up -d --build; then
+if ! timeout 300 docker-compose -f docker-compose.dev.yaml up -d --build; then
+  exit_code=$?
   echo "Failed to start containers"
+  echo "Exit code: $exit_code"
   docker-compose -f docker-compose.dev.yaml logs
   exit 1
 fi
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4fa5812 and 57ae541.

📒 Files selected for processing (1)
  • .github/workflows/pull-request.yml (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml

372-372: could not parse as YAML: yaml: line 372: could not find expected ':'

(syntax-check)

🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml

[error] 322-322: trailing spaces

(trailing-spaces)


[error] 323-323: trailing spaces

(trailing-spaces)


[error] 327-327: trailing spaces

(trailing-spaces)


[error] 366-366: trailing spaces

(trailing-spaces)


[error] 407-407: trailing spaces

(trailing-spaces)


[error] 374-374: syntax error: could not find expected ':'

(syntax)

🔇 Additional comments (2)
.github/workflows/pull-request.yml (2)

307-307: Fix action step configuration syntax

The Node.js setup step has incorrect syntax.

-- uses: actions/setup-node@v4
+ - uses: actions/setup-node@v4

392-407: Verify environment variable configuration

Please verify that these environment variables match the ones in docker-compose.dev.yaml.

🧰 Tools
🪛 yamllint (1.35.1)

[error] 407-407: trailing spaces

(trailing-spaces)

.github/workflows/pull-request.yml Outdated Show resolved Hide resolved
.github/workflows/pull-request.yml Outdated Show resolved Hide resolved
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: 4

🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)

393-407: LGTM: Environment variables are well configured

The environment variables are properly set with appropriate use of secrets. Just remove trailing spaces at line 407.

-      MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}}
-             
+      MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}}

299-407: Consider enhancing the Docker health check implementation

Some suggestions to improve the implementation:

  1. Consider using Docker's built-in HEALTHCHECK instruction in Dockerfile instead of shell-based health checks
  2. Consider implementing exponential backoff for health checks instead of fixed intervals
  3. Consider adding container resource limits to prevent resource exhaustion

These changes would make the health checks more robust and maintainable.

🧰 Tools
🪛 actionlint (1.7.4)

372-372: could not parse as YAML: yaml: line 372: could not find expected ':'

(syntax-check)

🪛 yamllint (1.35.1)

[error] 322-322: trailing spaces

(trailing-spaces)


[error] 323-323: trailing spaces

(trailing-spaces)


[error] 327-327: trailing spaces

(trailing-spaces)


[error] 366-366: trailing spaces

(trailing-spaces)


[error] 374-374: syntax error: could not find expected ':'

(syntax)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 57ae541 and cfec867.

📒 Files selected for processing (1)
  • .github/workflows/pull-request.yml (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml

372-372: could not parse as YAML: yaml: line 372: could not find expected ':'

(syntax-check)

🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml

[error] 322-322: trailing spaces

(trailing-spaces)


[error] 323-323: trailing spaces

(trailing-spaces)


[error] 327-327: trailing spaces

(trailing-spaces)


[error] 366-366: trailing spaces

(trailing-spaces)


[error] 408-408: trailing spaces

(trailing-spaces)


[error] 374-374: syntax error: could not find expected ':'

(syntax)

🔇 Additional comments (2)
.github/workflows/pull-request.yml (2)

299-320: LGTM: Job configuration follows best practices

The job configuration is well-structured with proper dependency management and caching strategy.


353-364: ⚠️ Potential issue

Fix Redis health check syntax

There's a syntax error in the if condition:

-          if[ $timeout -eq 0 ]; then
+          if [ $timeout -eq 0 ]; then

Likely invalid or redundant comment.

.github/workflows/pull-request.yml Outdated Show resolved Hide resolved
.github/workflows/pull-request.yml Outdated Show resolved Hide resolved
.github/workflows/pull-request.yml Outdated Show resolved Hide resolved
.github/workflows/pull-request.yml Outdated Show resolved Hide resolved
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

🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)

321-337: Fix formatting and enhance error logging.

  1. Remove trailing spaces after run : and in the if condition
  2. Add container logs to the error message when docker-compose file is not found
-        run : | 
+        run: |
           # Ensure no containers are running
           docker-compose -f docker-compose.dev.yaml down -v || true

           # Verify docker-compose file exists
-          if [ ! -f "docker-compose.dev.yaml" ]; then 
+          if [ ! -f "docker-compose.dev.yaml" ]; then
             echo "Error: docker-compose.dev.yaml not found"
+            docker-compose -f docker-compose.dev.yaml logs
             exit 1
           fi
🧰 Tools
🪛 yamllint (1.35.1)

[error] 322-322: trailing spaces

(trailing-spaces)


[error] 327-327: trailing spaces

(trailing-spaces)


366-382: Enhance API health check implementation.

Consider adding additional health check parameters to ensure comprehensive API readiness:

  1. Add request timeout to curl command
  2. Check for specific health check response content
-          until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do
+          until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf --max-time 5 "http://localhost:4000/health" 2>&1 | grep -q '"status":"healthy"' || [ $timeout -eq 0 ]; do
🧰 Tools
🪛 yamllint (1.35.1)

[error] 366-366: trailing spaces

(trailing-spaces)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cfec867 and 1544502.

📒 Files selected for processing (1)
  • .github/workflows/pull-request.yml (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml

384-384: could not parse as YAML: yaml: line 384: could not find expected ':'

(syntax-check)

🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml

[error] 322-322: trailing spaces

(trailing-spaces)


[error] 327-327: trailing spaces

(trailing-spaces)


[error] 366-366: trailing spaces

(trailing-spaces)


[error] 409-409: trailing spaces

(trailing-spaces)


[error] 388-388: syntax error: could not find expected ':'

(syntax)

🔇 Additional comments (2)
.github/workflows/pull-request.yml (2)

299-320: LGTM! Job configuration follows best practices.

The job configuration is well-structured with proper dependency setup, Node.js configuration, and dependency caching.


299-409: Overall implementation achieves the PR objective.

The Docker check implementation successfully verifies that the Talawa API starts correctly in a Docker environment. It includes proper health checks, timeouts, and cleanup procedures. The remaining issues are mostly syntax and formatting related, which have been addressed in previous comments.

🧰 Tools
🪛 actionlint (1.7.4)

384-384: could not parse as YAML: yaml: line 384: could not find expected ':'

(syntax-check)

🪛 yamllint (1.35.1)

[error] 322-322: trailing spaces

(trailing-spaces)


[error] 327-327: trailing spaces

(trailing-spaces)


[error] 366-366: trailing spaces

(trailing-spaces)


[error] 409-409: trailing spaces

(trailing-spaces)


[error] 388-388: syntax error: could not find expected ':'

(syntax)

.github/workflows/pull-request.yml Outdated Show resolved Hide resolved
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

🧹 Outside diff range and nitpick comments (2)
.github/workflows/pull-request.yml (2)

367-380: Fix indentation in API health check.

The indentation is inconsistent in this section, which affects readability.

-            # Wait for TALAWA API to be healthy 
-          until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do
-            echo "Waiting for API to start... ($timeout seconds remaining)"
-            sleep 1
-            ((timeout--))
-           done
+          # Wait for TALAWA API to be healthy 
+          until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf "http://localhost:4000/health" 2>&1 || [ $timeout -eq 0 ]; do
+            echo "Waiting for API to start... ($timeout seconds remaining)"
+            sleep 1
+            ((timeout--))
+          done
🧰 Tools
🪛 yamllint (1.35.1)

[error] 367-367: trailing spaces

(trailing-spaces)


394-409: Fix trailing spaces in environment variables section.

Remove trailing spaces to maintain consistent formatting.

-        MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}}
-             
+        MAIL_PASSWORD: ${{secrets.MAIL_PASSWORD}}
+
🧰 Tools
🪛 yamllint (1.35.1)

[warning] 395-395: wrong indentation: expected 6 but found 8

(indentation)


[error] 409-409: trailing spaces

(trailing-spaces)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1544502 and 0b6ef08.

📒 Files selected for processing (1)
  • .github/workflows/pull-request.yml (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml

322-322: shellcheck reported issue in this script: SC1069:error:38:3: You need a space before the [

(shellcheck)

🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml

[error] 367-367: trailing spaces

(trailing-spaces)


[warning] 395-395: wrong indentation: expected 6 but found 8

(indentation)


[error] 409-409: trailing spaces

(trailing-spaces)

🔇 Additional comments (5)
.github/workflows/pull-request.yml (5)

299-320: LGTM! Job configuration follows best practices.

The job configuration is well-structured with proper dependency management, Node.js setup, and dependency caching.


321-338: LGTM! Docker startup implementation is robust.

The implementation includes proper error handling, timeouts, and cleanup of existing containers.

🧰 Tools
🪛 actionlint (1.7.4)

322-322: shellcheck reported issue in this script: SC1069:error:38:3: You need a space before the [

(shellcheck)


343-343: Fix MongoDB health check syntax.

There's a syntax error in the until condition that needs to be fixed.

-until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongo --eval "db.runCommand('ping').ok">/dev/null 2>&1 || [ $timeout -eq 0]; do
+until docker-compose -f docker-compose.dev.yaml exec -T mongodb mongo --eval "db.runCommand('ping').ok" >/dev/null 2>&1 || [ $timeout -eq 0 ]; do

360-365: Fix Redis health check syntax and indentation.

There are syntax errors in the Redis health check implementation.

-if[ $timeout -eq 0 ]; then
+if [ $timeout -eq 0 ]; then

384-393: LGTM! Cleanup implementation follows best practices.

The cleanup function correctly preserves the exit code and uses trap for proper cleanup on script exit.

@Cioppolo14
Copy link
Contributor

There isn’t an issue assigned to you for this PR. Please follow the guidelines in our PR_GUIDELINES.md file. We have the procedures in place so that everyone has a fair chance of contributing. I will be closing this pull request. Please follow the procedures and resubmit when ready.

@Cioppolo14 Cioppolo14 closed this Dec 14, 2024
@prayanshchh
Copy link
Contributor Author

@Cioppolo14 I accidently tagged the wrong issue, but I am assigned to this issue: #2638..please re-open this

@palisadoes palisadoes reopened this Dec 15, 2024
@palisadoes
Copy link
Contributor

Please make sure all tests pass and coderabbit.ai approves the PR

@prayanshchh
Copy link
Contributor Author

Please make sure all tests pass and coderabbit.ai approves the PR

yes sir, will get this ready 5-6 days

@bandhan-majumder
Copy link

bandhan-majumder commented Dec 17, 2024

Hi @prayanshchh . Have you found any solution for GraphQLError: Syntax Error: Unexpected <EOF>. ?? I came across this Type-Checker failure today and noticed all the previous PRs are failing this check too.

@prayanshchh
Copy link
Contributor Author

Hi @prayanshchh . Have you found any solution for GraphQLError: Syntax Error: Unexpected <EOF>. ?? I came across this Type-Checker failure today and noticed all the previous PRs are failing this check too.

is that so, then ig this should be a seperate issue then, can u give ur PR link

@bandhan-majumder
Copy link

Sure. I have opened an issue regarding this since a few others also reported the same in slack.

@VanshikaSabharwal
Copy link

If you need any assistance feel free to reach out to me @prayanshchh @bandhan-majumder .

@palisadoes
Copy link
Contributor

Please fix:

  1. Any failing tests (Click on the details link for more information)
  2. Any conflicting files

Make sure CodeRabbit.ai approves your changes

coderabbitai[bot]
coderabbitai bot previously approved these changes Dec 31, 2024
@prayanshchh
Copy link
Contributor Author

should I remove eslint-disable statements from setup.ts, it would lead to lot of errors, this should be a seperate issue according to me
@palisadoes

Copy link
Contributor

@palisadoes palisadoes left a comment

Choose a reason for hiding this comment

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

  1. Please fix the lint check errors in this PR
  2. The aim of the year is to fix historically based practice
  3. We cannot merge with errors

@palisadoes
Copy link
Contributor

Please make code rabbit approve your PR

@prayanshchh
Copy link
Contributor Author

prayanshchh commented Dec 31, 2024

Please make code rabbit approve your PR

code rabbit already approves the PR, will make lint fixes and then get back to this

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

🧹 Nitpick comments (1)
setup.ts (1)

1017-1024: Add validation for PWD input.

Consider adding validation to ensure the provided PWD path exists and is accessible.

 const { pwdVariable } = await inquirer.prompt({
   type: "input",
   name: "pwdVariable",
   message:
     "Please enter the value for PWD (working directory for Docker setup):",
   default: ".",
+  validate: (input: string): boolean | string => {
+    try {
+      const fullPath = path.resolve(input);
+      if (!fs.existsSync(fullPath)) {
+        return "Directory does not exist. Please provide a valid path.";
+      }
+      fs.accessSync(fullPath, fs.constants.R_OK | fs.constants.W_OK);
+      return true;
+    } catch {
+      return "Directory is not accessible. Please check permissions.";
+    }
+  },
 });
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 78fab42 and 89f67dd.

📒 Files selected for processing (2)
  • setup.ts (2 hunks)
  • tsconfig.json (1 hunks)
🔇 Additional comments (4)
tsconfig.json (1)

11-16: LGTM! Path aliases improve code organization.

The addition of path aliases and noUncheckedSideEffectImports enhances code maintainability and type safety.

setup.ts (3)

9-9: LGTM! Imports updated to use new path aliases.

The imports have been properly updated to use the newly configured path aliases from tsconfig.json.

Also applies to: 14-33


1017-1024: LGTM! PWD configuration added for Docker setup.

The PWD configuration is essential for Docker working directory setup. The default value "." is sensible for most use cases.


1017-1046: Verify Docker container startup sequence.

The PWD configuration is part of Docker setup. Let's verify the container startup sequence works as expected.

✅ Verification successful

Let me gather more information about the Docker configuration and startup sequence.


Let me check one more aspect of the Docker configuration to ensure proper service dependencies.


Docker container startup sequence is properly configured

The verification of the Docker setup and PWD configuration shows that:

  • The PWD variable is correctly configured in both .env.sample and used in the Docker compose file for the Caddy service
  • The container startup sequence is well-defined with proper health checks and dependencies:
    • talawa-api-dev depends on mongo, redis-stack-server, and minio services
    • MongoDB has a health check configured
    • All services are connected through the talawa-network bridge network
    • Volume mounts and environment variables are properly configured

The PWD configuration in the setup script aligns with its usage in the Docker compose file for the Caddy service volume mounts.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify Docker container startup sequence and configuration

# Check if docker-compose.dev.yaml exists and contains required services
if [ ! -f "docker-compose.dev.yaml" ]; then
  echo "Error: docker-compose.dev.yaml not found"
  exit 1
fi

# Check if docker-compose.dev.yaml contains required services and PWD usage
rg -A 5 "working_dir|PWD" docker-compose.dev.yaml

# Check if .env.sample contains PWD variable
rg "^PWD=" .env.sample

Length of output: 215


Script:

#!/bin/bash
# Check Docker service definitions and dependencies
rg -A 10 "services:" docker-compose.dev.yaml

# Check health checks and container dependencies
rg "healthcheck|depends_on" docker-compose.dev.yaml

# Check if there are any Docker-related scripts or configurations
fd -e yml -e yaml -e dockerfile -e Dockerfile

Length of output: 478


Script:

#!/bin/bash
# Check complete service dependencies and environment configurations
cat docker-compose.dev.yaml

# Check if there are any Docker-related environment variables in the sample
rg "DOCKER|COMPOSE" .env.sample

# Look for any Docker-related scripts
fd setup docker

Length of output: 2201

coderabbitai[bot]
coderabbitai bot previously approved these changes Dec 31, 2024
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

🧹 Nitpick comments (1)
.github/workflows/pull-request.yml (1)

341-345: Consider pinning Docker Compose version.

For better reproducibility and to avoid potential breaking changes, consider pinning the Docker Compose version:

 - name: Install Docker Compose
   run: |
     sudo apt-get update
-    sudo apt-get install -y docker-compose
+    sudo apt-get install -y docker-compose=1.29.2*
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 89f67dd and 4cfccd0.

📒 Files selected for processing (1)
  • .github/workflows/pull-request.yml (2 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml

[error] 393-393: trailing spaces

(trailing-spaces)


[error] 398-398: trailing spaces

(trailing-spaces)


[error] 474-474: trailing spaces

(trailing-spaces)

🔇 Additional comments (5)
.github/workflows/pull-request.yml (5)

317-321: LGTM!

The job configuration follows GitHub Actions best practices with proper naming, dependencies, and runner specification.


415-424: LGTM!

The cleanup function is well-implemented with proper exit code preservation and error handling.


425-439: LGTM!

The environment variables are well-configured with secure token generation using GitHub-specific values for uniqueness.


372-374: ⚠️ Potential issue

Fix MongoDB service name inconsistency.

The service name used in the logs command doesn't match the one used in the health check:

 echo "Fetching MongoDB logs..."
-docker-compose -f docker-compose.dev.yaml logs mongodb
+docker-compose -f docker-compose.dev.yaml logs mongo

Likely invalid or redundant comment.


398-404: 🛠️ Refactor suggestion

Improve API health check reliability.

The current OPTIONS request might not accurately verify the GraphQL endpoint's functionality. Consider using a simple query instead:

-until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -v -X OPTIONS "http://talawa-api-dev:4000/graphql" 2>&1 || [ $timeout -eq 0 ]; do
+until docker-compose -f docker-compose.dev.yaml exec -T talawa-api-dev curl -sf -X POST "http://talawa-api-dev:4000/graphql" \
+  -H "Content-Type: application/json" \
+  -d '{"query":"{ __typename }"}' 2>&1 || [ $timeout -eq 0 ]; do

Likely invalid or redundant comment.

🧰 Tools
🪛 yamllint (1.35.1)

[error] 398-398: trailing spaces

(trailing-spaces)

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

🧹 Nitpick comments (3)
vite.config.mts (1)

48-48: Remove unnecessary whitespace or trailing line changes for cleaner diffs.
Although not critical, reducing extraneous changes helps keep diffs minimal.

-      
+
tests/utilities/loadSampleData.spec.ts (1)

18-18: Use environment-based DB naming for improved configurability.

Instead of specifying "talawa-api" as a hardcoded string, consider referencing a dedicated environment variable or configuration file for the database name to allow more flexible test setups and avoid accidental conflicts when using different environments (development, staging, production, etc.). For example:

- await connect("talawa-api");
+ await connect(process.env.TALAWA_DB_NAME || "talawa-api");
tests/setup/checkExistingMongoDB.spec.ts (1)

12-13: Consider using structured logging or removing unnecessary logs.

Using console.log can clutter test outputs and potentially leak sensitive information. If logging is truly necessary for debugging, you might consider using a more robust logging framework or wrapping logs in conditionals.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4cfccd0 and 6841b72.

📒 Files selected for processing (6)
  • src/utilities/loadSampleData.ts (3 hunks)
  • tests/resolvers/Query/getVolunteerRanks.spec.ts (2 hunks)
  • tests/setup/checkExistingMongoDB.spec.ts (2 hunks)
  • tests/setup/mongoDB.spec.ts (1 hunks)
  • tests/utilities/loadSampleData.spec.ts (1 hunks)
  • vite.config.mts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/utilities/loadSampleData.ts
🔇 Additional comments (6)
vite.config.mts (2)

2-2: Use node:path with caution if older Node.js versions are supported.
If the project's Node.js version is <16.0, node:path might not work. Verify that your .nvmrc or documentation clarifies the minimum Node.js version.


5-11: Great addition of path aliases for quick imports.
These aliases streamline imports and improve code readability.

tests/setup/checkExistingMongoDB.spec.ts (1)

65-66: Verify dependencies and environment configurations.

The new connection string with replica set and direct connection parameters is a good step for Docker-based setups. However, verify that all references to MONGO_DB_URL elsewhere (e.g., in your Docker or CI config) consistently match these parameters to avoid connection issues.

tests/setup/mongoDB.spec.ts (1)

120-121: Change appears consistent with your Docker configuration.

This updated connection string aligns with the replica set configuration introduced elsewhere. Good job keeping the test environment consistent with the actual runtime environment.

tests/resolvers/Query/getVolunteerRanks.spec.ts (2)

80-80: Confirm the updated expectation for monthly timeframe.

The expected hours changed to 2, which differs from previously reported hours (e.g., 6 or 8). If this new value aligns with recent logic changes in createVolunteerAndActions or getVolunteerRanks, then it’s correct. Otherwise, please verify that the test data and function logic properly reflect real-world use of the monthly timeframe.


97-97: Double-check alignment of yearly hours log.

Similarly, we've updated the expected value to 2 for the yearly timeframe. Ensure that the test setup and underlying business logic (in createVolunteerAndActions or the resolver) truly reduce the hours to 2 for a yearly duration, as that might be unintended if you expected a higher accumulation over a full year.

@prayanshchh
Copy link
Contributor Author

@palisadoes PTAL, the patch looks good

@palisadoes palisadoes merged commit b64f105 into PalisadoesFoundation:develop Jan 1, 2025
14 of 15 checks passed
@gautam-divyanshu
Copy link
Member

@prayanshchh you have changed the superadmin password and why is that?
and what is the new password?

image

@prayanshchh
Copy link
Contributor Author

@prayanshchh you have changed the superadmin password and why is that? and what is the new password?

image

sorry it was for testing purposes, will undo this immediately, shall I create new PR, or continue here @gautam-divyanshu

@gautam-divyanshu
Copy link
Member

@prayanshchh create a new pr and issue.

@prayanshchh
Copy link
Contributor Author

yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore-sensitive-files-pr The contributor has a legitimate reason for editiing protected files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants