Sample Data and Reset Data Scripts #3285
Conversation
WalkthroughThis pull request restructures how sample data is managed. The dependency list and scripts in Changes
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (6)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsample_data/images/01JMQ3EABXAD1F3KTW1NQGF99C.jpegis excluded by!**/*.jpegsample_data/images/01JMQ3F1B55K272B25V19781D0.jpegis excluded by!**/*.jpegsample_data/images/01JMQ3F9E5C806CN6M8EV31E01.jpegis excluded by!**/*.jpegsample_data/images/01JMQ3FGBEDRB9AM6XP9C97RF8.jpegis excluded by!**/*.jpegsample_data/images/01JMQ3FPS789Z1J6BBQ01YDHVQ.jpegis excluded by!**/*.jpeg
📒 Files selected for processing (7)
package.json(1 hunks)scripts/dbManagement/addSampleData.ts(1 hunks)scripts/dbManagement/helpers.ts(1 hunks)scripts/dbManagement/resetData.ts(1 hunks)scripts/dbManagement/sample_data/post_attachments.json(1 hunks)src/envConfigSchema.ts(1 hunks)src/utilities/loadSampleData.ts(0 hunks)
💤 Files with no reviewable changes (1)
- src/utilities/loadSampleData.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (8)
scripts/dbManagement/helpers.ts (4)
53-65: Interactive confirmation may fail in non-interactive/CI environments
askUserToContinueusesreadlineand expects a TTY. If run in a script or pipeline without a terminal, it may hang or fail. You might want to provide a fallback mechanism or skip prompts in non-interactive contexts.
94-137: Potential security concern when resetting user roles
ensureAdministratorExistsforces role escalation if the user already exists but is not an administrator. Confirm that’s the desired behavior. Additionally, storing admin password in an environment variable could be risky in certain deployments.
529-532: Null dates may conflict with table constraints
parseDatereturnsnullon invalid dates, potentially conflicting with not-null fields. If the schema prohibits null, consider raising an error or using a fallback value to avoid runtime exceptions.
584-593: Single-client usage indisconnect
disconnectcallsqueryClient.end()unconditionally. If multiple scripts or processes share the same client, this may cause unexpected resource release. Ensure that each script has its own client or carefully coordinates usage.scripts/dbManagement/addSampleData.ts (1)
63-83: Correct exit code usage
If an error occurs, the script setsexitCode = 1and continues gracefully. This is good practice. Just ensure unhandled rejections do not override this logic.scripts/dbManagement/resetData.ts (2)
12-60: Guard statements after formatting failure
While you do a partial rollback ifformatDatabasefails, the subsequent steps (likeemptyMinioBucket) still occur. If that’s unintentional, you could add more checks or early returns upon severe errors.
62-87: Solid pattern for graceful shutdown
Runningmainin an IIFE and then disconnecting ensures a clean exit. The exit code is updated upon errors, which improves script reliability.scripts/dbManagement/sample_data/post_attachments.json (1)
7-8: Great improvements to image format and naming!The changes bring several benefits:
- WebP format offers better compression and quality compared to JPEG.
- Descriptive naming using DALL·E prefix clearly indicates AI-generated content.
Also applies to: 15-16, 23-24, 31-32, 39-40, 47-48
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3285 +/- ##
====================================================
- Coverage 48.41% 48.36% -0.05%
====================================================
Files 456 458 +2
Lines 34225 34516 +291
Branches 937 966 +29
====================================================
+ Hits 16570 16695 +125
- Misses 17655 17821 +166 ☔ View full report in Codecov by Sentry. |
This reverts commit 96b452c.
…/talawa-api into sample-data-script
…nnu-IITI/talawa-api into sample-data-script" This reverts commit 238dde4, reversing changes made to 4ef52c4.
|
Adding tests for the final scripts..... Will inform when PR is ready. |
|
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 I am assigned #3214 |
|
Title is different as during development our goals got changed a little bit |
|
Can this be reopened or should I raise another PR? |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
scripts/dbManagement/helpers.ts(1 hunks)test/scripts/dbManagement/addSampleData.test.ts(1 hunks)test/scripts/dbManagement/resetDB.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
scripts/dbManagement/helpers.ts (1)
Learnt from: JaiPannu-IITI
PR: PalisadoesFoundation/talawa-api#3285
File: scripts/dbManagement/helpers.ts:70-93
Timestamp: 2025-02-23T10:18:23.438Z
Learning: The database management utility functions in `scripts/dbManagement/helpers.ts` are designed to be used only within their specific scripts due to their destructive nature. Error handling is intentionally handled at the script level rather than within individual utility functions.
🪛 GitHub Actions: Pull request workflow
test/scripts/dbManagement/resetDB.test.ts
[error] 1-1: Import statements could be sorted.
[error] 1-1: Formatter would have printed the following content: Code formatting issues detected.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyse Code With CodeQL (typescript)
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (12)
scripts/dbManagement/helpers.ts (9)
1-19: Great modular design and clarity in imports.The file establishes essential utilities and demonstrates well-organized imports. Overall, this structure promotes maintainability and readability.
28-28: Potential mismatch forbucketName.Currently,
bucketNameis set toenvConfig.MINIO_ROOT_USER. This environment variable name typically implies a username credential (e.g., a user or an access key) rather than a bucket name. Consider confirming that this is indeed intended or rename the variable to avoid confusion.
70-93: Limited error reporting when formatting the database.This function only returns
falseupon error, providing minimal context. If debugging proves difficult, consider rethrowing or logging a more descriptive error to help identify issues more quickly.
141-174: Check whether the bucket exists before removing objects.If the bucket name is misconfigured or the bucket does not exist,
listObjectscan fail unexpectedly. Logging “bucket not found” or verifying existence can help diagnose configuration errors at an earlier stage.
179-209: Ensure more robust JSON parse error handling.If any file is malformed, the function provides a generic error message. You may consider catching the parsing error separately and identifying which file is invalid for easier troubleshooting.
215-222: Return diagnostic info inpingDB.Currently, the function only tries a simple query. Providing more descriptive logs or returning additional diagnostic information could simplify debugging if a database connection fails.
228-250: Log partial insert details incheckAndInsertData.Since duplicates are silently ignored via
onConflictDoNothing, you could log how many rows were inserted versus ignored. This extra transparency can assist in monitoring data updates.
258-524: Reduce duplication ininsertCollections.Repeated logic for reading JSON, parsing dates, and inserting data is present for each collection. Centralizing this in a shared helper or a dynamic mapping approach can reduce code repetition and enhance maintainability.
540-584: Refine error handling incheckDataSize.If an error occurs, the function prints it but returns
false. In critical production scenarios, consider throwing the error or providing optional partial results to help with troubleshooting.test/scripts/dbManagement/addSampleData.test.ts (2)
10-50: Comprehensive test coverage for successful pathways.This set of tests efficiently mocks critical helper functions and verifies the main flow, ensuring all steps (database ping, admin creation, sample data insertion) are executed and logged as expected. Excellent use of
vitestspies to capture logs.
77-88: Valid error path testing logic.The test ensures that when
insertCollectionsfails, the function surfaces an appropriate error message. This robust error handling check helps maintain reliability of the data import process.test/scripts/dbManagement/resetDB.test.ts (1)
6-106: Thorough test coverage for destructive reset operations.These tests successfully capture user confirmation flow, database connectivity, destructive formatting, bucket cleanup, and administrator creation. The suite comprehensively verifies both failure scenarios and success paths without extraneous complexity.
|
Please merge with the latest upstream |
|
Sure sir, |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
test/scripts/dbManagement/resetDB.test.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run tests for talawa api
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (5)
test/scripts/dbManagement/resetDB.test.ts (5)
1-4: Fix import ordering.The import statements need to be reordered to comply with the pipeline requirements.
Apply this diff to fix the import order:
-import * as helpers from "scripts/dbManagement/helpers"; -import * as resetDataModule from "scripts/dbManagement/resetData"; -// tests/dbManagement/resetData.test.ts -import { beforeEach, expect, suite, test, vi } from "vitest"; +import { beforeEach, expect, suite, test, vi } from "vitest"; +import * as helpers from "scripts/dbManagement/helpers"; +import * as resetDataModule from "scripts/dbManagement/resetData";
6-10: LGTM! Good test suite setup.The test suite follows best practices by resetting all mocks before each test using
beforeEach.
12-22: LGTM! Well-structured test case.The test follows the AAA pattern and effectively validates the cancellation scenario.
24-33: LGTM! Good error handling test.The test effectively validates the error handling for database connection failures.
89-116: LGTM! Comprehensive success scenario test.The test effectively validates all success messages when operations complete successfully.
|
@palisadoes PR is ready for review.
Test for helper.ts (remaining 28% patch) will be added once we come up with decision regarding pg-mem since these interactive tests can't be performed with others right now as they are destructive in nature. Please don't close issue, I will raise follow-up for this PR with more test coverage with pg-mem installation and integration (if agreed) along with Docker shell solution to run these scripts in one line CLI. Meanwhile this can be merged to avoid conflicts like last time as this have some folder rearrangements but is ready for use and tested. |
palisadoes
merged commit 0aa74c2
into
PalisadoesFoundation:develop-postgres
Overview
Added 2 clean scripts with fail check implementation and vigorous error handling.
Closes: #3214
1.
pnpm add:sample_dataThis script is designed to handle adding sample data to already existing or formatted database and sample post_attachments to already existing or formatted Minio bucket without fail. Duplicates are handled with ease. Already existing data is not affected.
This script is non-interactive and additive in nature (non-destructive)
Scope: Available in all environments ( Dev + Prod + Demo )
Use case : This script can be used in any cron job without worrying about state of database.
Output
2.
pnpm reset:dataThis script completely formats database and minio bucket, administrator access is re-established.
This script is interactive and destructive in nature.
Scope: Available in all environment ( Dev + Prod + Demo ) but not recommended in production unless needed on a serious note.
Use case: This script should be used manually with care when needed on a serious note.
Output
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Chores