Refactor migration schema validation into a static method, update its usage in the MongoDB connection, and add a testing enhancement specification.

Author: feedbee

SPECIFICATION.MD

@@ -332,29 +332,29 @@ _No major refactoring areas identified at this time. The codebase is well-organi
 ## Future Enhancements
 
 ### High Priority
-- [x] Translations: Add Russian language support and set the bot's default language through environment variables.
-- [x] Commands: Rename /postride to /shareride.
+- [x] Feature: (Translations) Add Russian language support and set the bot's default language through environment variables.
+- [x] Feature: (Commands) Rename /postride to /shareride.
 - [x] Documentation: Improve /start message to simplify onboarding
-- [x] Ride participation: List participants on the same line after the label, separated by commas.
-- [x] Ride participation: Add an "I am thinking" option to indicate who might join the chat.
-- [x] Ride participation: Add an "Not interested" option to gather responses from those who will not join the ride.
-- [x] Ride participation: Limit the ride message to display only the first N registered participants (configurable via MAX_PARTICIPANTS_DISPLAY), and if there are more, show a label like "and X more".
-- [x] Correct dates recognition in differfent languages
-- [ ] Creating rides with AI
+- [x] Feature: (Ride participation) List participants on the same line after the label, separated by commas.
+- [x] Feature: (Ride participation) Add an "I am thinking" option to indicate who might join the chat.
+- [x] Feature: (Ride participation) Add an "Not interested" option to gather responses from those who will not join the ride.
+- [x] Feature: (Ride participation) Limit the ride message to display only the first N registered participants (configurable via MAX_PARTICIPANTS_DISPLAY), and if there are more, show a label like "and X more".
+- [x] Bug: Correct dates recognition in differfent languages
+- [ ] Feature: Creating rides with AI
 
 ### Medium Priority
-- [ ] Rides management: Add management buttons below the ride button in private chat with the bot for ride creator
-- [ ] Notifications: Send a private notification to the ride creator when a participant joins or leaves. There is a 30-second delay to prevent spamming from frequent toggling; if the participant's status changes again within this period, cancel the pending notification.
-- [ ] Notifications: Remind the joined users about upcoming ride 24 hours and 1 hour before the ride (send a private message; give each user an option to unsubscribe from all notifications).
-- [ ] Ride sharing: Optionally enable everyone to repost the ride via an additional field during ride creation or update.
+- [ ] Feature: (Rides management) Add management buttons below the ride button in private chat with the bot for ride creator
+- [ ] Feature: (Notifications) Send a private notification to the ride creator when a participant joins or leaves. There is a 30-second delay to prevent spamming from frequent toggling; if the participant's status changes again within this period, cancel the pending notification.
+- [ ] Feature: (Notifications) Remind the joined users about upcoming ride 24 hours and 1 hour before the ride (send a private message; give each user an option to unsubscribe from all notifications).
+- [ ] Feature: (Ride sharing) Optionally enable everyone to repost the ride via an additional field during ride creation or update.
 
 ### Low Priority
-- [ ] Translations: Override the bot's default language based on the user's preference (the bot communicates with the user in their chosen language; it also announces the user's rides, created by them, in the preferred language).
-- [ ] Ride participation: Add a comand to list joined rides for the current user
-- [x] Quality: Better testing and fixes of setting nummeric ride attributes, like setting interval 22-25 and updating with a single value (29) gives 29-25
-- [ ] Add keyboard shortcut to keep the current value
-- [x] Update readme for installation via Docker
-- [ ] Resolve multiple mongoDB instances/connections
+- [x] Bug: Better testing and fixes of setting nummeric ride attributes, like setting interval 22-25 and updating with a single value (29) gives 29-25
+- [x] Documentation: Update readme for installation via Docker
+- [x] Bug: Resolve multiple mongoDB instances/connections
+- [ ] Feature: (Translations) Override the bot's default language based on the user's preference (the bot communicates with the user in their chosen language; it also announces the user's rides, created by them, in the preferred language).
+- [ ] Feature: (Ride participation) Add a comand to list joined rides for the current user
+- [ ] Feature: Add keyboard shortcut to keep the current value
 
 ---
 

src/__tests__/migrations/migration-runner.test.js

@@ -51,10 +51,10 @@ describe('MigrationRunner', () => {
     ];
 
     runner = new MigrationRunner('mongodb://test');
-    
+
     // Mock getMigrations to return our test migrations
-    jest.spyOn(runner, 'getMigrations').mockReturnValue(mockMigrations);
-    
+    jest.spyOn(MigrationRunner, 'getMigrations').mockReturnValue(mockMigrations);
+
     jest.clearAllMocks();
   });
 
@@ -70,7 +70,7 @@ describe('MigrationRunner', () => {
     });
 
     it('should get migrations (mocked)', () => {
-      const migrations = runner.getMigrations();
+      const migrations = MigrationRunner.getMigrations();
       expect(migrations).toHaveLength(2);
       expect(migrations[0].version).toBe(1);
       expect(migrations[0].name).toBe('Test Migration 1');
@@ -83,6 +83,29 @@ describe('MigrationRunner', () => {
     });
   });
 
+  describe('MigrationRunner.validateVersion (static)', () => {
+    it('should not throw when schema is up to date', () => {
+      expect(() => MigrationRunner.validateVersion(2)).not.toThrow();
+    });
+
+    it('should not throw when schema is ahead of required', () => {
+      expect(() => MigrationRunner.validateVersion(99)).not.toThrow();
+    });
+
+    it('should throw when schema is outdated', () => {
+      expect(() => MigrationRunner.validateVersion(0)).toThrow(
+        'Database schema is outdated. Current version: 0, Required version: 2. Please run migrations first (see README.md).'
+      );
+    });
+
+    it('should throw with correct version numbers in message', () => {
+      expect(() => MigrationRunner.validateVersion(1)).toThrow(
+        'Database schema is outdated. Current version: 1, Required version: 2. Please run migrations first (see README.md).'
+      );
+    });
+  });
+
+
   describe('Database operations with manual setup', () => {
     beforeEach(async () => {
       // Manually set up the connection state
@@ -92,29 +115,29 @@ describe('MigrationRunner', () => {
 
     it('should get current version from database', async () => {
       mockCollection.findOne.mockResolvedValue({ schemaVersion: 3 });
-      
+
       const version = await runner.getCurrentVersion();
-      
+
       expect(version).toBe(3);
       expect(mockCollection.findOne).toHaveBeenCalledWith({});
     });
 
     it('should return 0 for no version found', async () => {
       mockCollection.findOne.mockResolvedValue(null);
-      
+
       const version = await runner.getCurrentVersion();
-      
+
       expect(version).toBe(0);
     });
 
     it('should set version in database', async () => {
       mockCollection.replaceOne.mockResolvedValue({});
-      
+
       await runner.setVersion(5);
-      
+
       expect(mockCollection.replaceOne).toHaveBeenCalledWith(
         {},
-        expect.objectContaining({ 
+        expect.objectContaining({
           schemaVersion: 5,
           updatedAt: expect.any(Date)
         }),
@@ -192,36 +215,6 @@ describe('MigrationRunner', () => {
     });
   });
 
-  describe('Schema validation with mocked connection', () => {
-    beforeEach(() => {
-      // Mock the connect method
-      jest.spyOn(runner, 'connect').mockImplementation(async () => {
-        runner.client = mockClient;
-        runner.db = mockDb;
-      });
-    });
-
-    afterEach(() => {
-      runner.connect.mockRestore();
-    });
-
-    it('should pass validation when schema is up to date', async () => {
-      mockCollection.findOne.mockResolvedValue({ schemaVersion: 2 });
-
-      await expect(runner.validateSchemaVersion()).resolves.not.toThrow();
-      expect(runner.connect).toHaveBeenCalled();
-    });
-
-    it('should fail validation when schema is outdated', async () => {
-      mockCollection.findOne.mockResolvedValue({ schemaVersion: 0 });
-
-      await expect(runner.validateSchemaVersion()).rejects.toThrow(
-        'Database schema is outdated. Current version: 0, Required version: 2. Please run migrations first (see README.md).'
-      );
-      expect(runner.connect).toHaveBeenCalled();
-    });
-  });
-
   describe('Edge cases', () => {
     beforeEach(() => {
       // Mock the connect method
@@ -237,8 +230,8 @@ describe('MigrationRunner', () => {
 
     it('should handle empty migrations list', async () => {
       // Mock empty migrations
-      jest.spyOn(runner, 'getMigrations').mockReturnValue([]);
-      
+      jest.spyOn(MigrationRunner, 'getMigrations').mockReturnValue([]);
+
       mockCollection.findOne.mockResolvedValue({ schemaVersion: 0 });
 
       await runner.runMigrations();

src/migrations/MigrationRunner.js

@@ -54,22 +54,22 @@ export class MigrationRunner {
    */
   async runMigrations() {
     await this.connect();
-    
+
     try {
       const currentVersion = await this.getCurrentVersion();
-      const migrations = this.getMigrations();
+      const migrations = MigrationRunner.getMigrations();
       const pendingMigrations = migrations.filter(m => m.version > currentVersion);
-      
+
       console.log(`Current schema version: ${currentVersion}`);
       console.log(`Found ${pendingMigrations.length} pending migrations`);
-      
+
       for (const migration of pendingMigrations) {
         console.log(`Running migration ${migration.version}: ${migration.name}`);
         await migration.up(this.db);
         await this.setVersion(migration.version);
         console.log(`✓ Migration ${migration.version} completed`);
       }
-      
+
       if (pendingMigrations.length === 0) {
         console.log('No migrations to run');
       } else {
@@ -85,38 +85,32 @@ export class MigrationRunner {
    * @returns {number} Required schema version
    */
   getRequiredVersion() {
-    const migrations = this.getMigrations();
+    const migrations = MigrationRunner.getMigrations();
     return migrations.length > 0 ? Math.max(...migrations.map(m => m.version)) : 0;
   }
 
   /**
-   * Validate that the database schema is up to date
-   * @throws {Error} If schema version is outdated
+   * Validate that a given schema version meets the required version.
+   * Contains all comparison logic and error messages.
+   * @param {number} currentVersion - The current schema version from the database
+   * @throws {Error} If the schema version is outdated
    */
-  async validateSchemaVersion() {
-    await this.connect();
-    
-    try {
-      const currentVersion = await this.getCurrentVersion();
-      const requiredVersion = this.getRequiredVersion();
-      
-      if (currentVersion < requiredVersion) {
-        const errorMessage = `Database schema is outdated. Current version: ${currentVersion}, Required version: ${requiredVersion}. Please run migrations first (see README.md).`;
-        console.error('Schema validation failed:', errorMessage);
-        throw new Error(errorMessage);
-      }
-      
-      console.log(`Schema validation passed. Current version: ${currentVersion}`);
-    } finally {
-      await this.disconnect();
+  static validateVersion(currentVersion) {
+    const migrations = MigrationRunner.getMigrations();
+    const requiredVersion = migrations.length > 0 ? Math.max(...migrations.map(m => m.version)) : 0;
+    if (currentVersion < requiredVersion) {
+      const errorMessage = `Database schema is outdated. Current version: ${currentVersion}, Required version: ${requiredVersion}. Please run migrations first (see README.md).`;
+      console.error('Schema validation failed:', errorMessage);
+      throw new Error(errorMessage);
     }
+    console.log(`Schema validation passed. Current version: ${currentVersion}`);
   }
 
   /**
    * Get all available migrations
    * @returns {Array} Array of migration objects
    */
-  getMigrations() {
+  static getMigrations() {
     return [
       {
         version: 1,

src/storage/mongodb.js

@@ -62,12 +62,13 @@ export class MongoDBStorage extends StorageInterface {
       console.log('Connected to MongoDB');
       await Ride.createIndexes();
       console.log('Ride indexes ensured');
-      
+
       // Skip schema validation in test environment
       if (process.env.NODE_ENV !== 'test') {
-        // Validate schema version using MigrationRunner
-        const migrationRunner = new MigrationRunner(config.mongodb.uri);
-        await migrationRunner.validateSchemaVersion();
+        const db = mongoose.connection.db;
+        const metaDoc = await db.collection('meta').findOne({});
+        const currentVersion = metaDoc ? metaDoc.schemaVersion : 0;
+        MigrationRunner.validateVersion(currentVersion);
       }
     } catch (error) {
       console.error('MongoDB connection error:', error);
@@ -82,17 +83,17 @@ export class MongoDBStorage extends StorageInterface {
   }
 
   async createRide(ride) {
-    let rideData = { 
-      ...ride, 
+    let rideData = {
+      ...ride,
       category: normalizeCategory(ride.category),
       participation: { joined: [], thinking: [], skipped: [] }
     };
-    
+
     // Ensure messages array exists
     if (!rideData.messages) {
       rideData.messages = [];
     }
-    
+
     const newRide = new Ride(rideData);
     await newRide.save();
     return this.mapRideToInterface(newRide);
@@ -103,11 +104,11 @@ export class MongoDBStorage extends StorageInterface {
     if (!ride) {
       throw new Error('Ride not found');
     }
-    
+
     // Preserve the messages array if it's not being updated
     // This is critical to ensure message tracking works properly
     let updatesToApply = { ...updates };
-    
+
     // Set updatedAt to current time only if updatedBy is set
     if (updatesToApply.updatedBy) {
       updatesToApply.updatedAt = new Date();
@@ -116,7 +117,7 @@ export class MongoDBStorage extends StorageInterface {
     if (updatesToApply.category !== undefined) {
       updatesToApply.category = normalizeCategory(updatesToApply.category);
     }
-    
+
     // Apply updates
     Object.assign(ride, updatesToApply);
     await ride.save();
@@ -213,7 +214,7 @@ export class MongoDBStorage extends StorageInterface {
   mapRideToInterface(ride) {
     if (!ride) return null;
     const rideObj = ride.toObject ? ride.toObject() : ride;
-    
+
     // Create the ride object with the messages array
     const result = {
       id: rideObj._id.toString(),
@@ -258,7 +259,7 @@ export class MongoDBStorage extends StorageInterface {
         }))
       }
     };
-    
+
     return result;
   }
 }