Code Quality Improvements for PRS Backend

This document outlines concrete, actionable recommendations to make the PRS Backend code more bug-free for succeeding sprints. These recommendations are based on analysis of the current codebase and industry best practices.

1. Implement Comprehensive Automated Testing

The most effective way to prevent bugs is through automated testing:

// Example Jest test for the NonRequisitionService
describe('NonRequisitionService', () => {
  let nonRequisitionService;
  let nonRequisitionRepositoryMock;

  beforeEach(() => {
    nonRequisitionRepositoryMock = {
      findOne: jest.fn(),
      update: jest.fn(),
      create: jest.fn(),
    };

    nonRequisitionService = new NonRequisitionService({
      nonRequisitionRepository: nonRequisitionRepositoryMock,
      // other dependencies
    });
  });

  describe('updateStatus', () => {
    it('should throw error for invalid status transitions', async () => {
      // Arrange
      nonRequisitionRepositoryMock.findOne.mockResolvedValue({
        id: 1,
        status: 'closed',
      });

      // Act & Assert
      await expect(
        nonRequisitionService.updateStatus(1, 'for_approval', 'test', {})
      ).rejects.toThrow('Invalid status transition from closed to for_approval');
    });
  });
});

Implementation Steps: 1. Set up Jest or another testing framework 2. Create unit tests for all service methods 3. Create integration tests for API endpoints 4. Set up test coverage reporting 5. Integrate tests into CI/CD pipeline

2. Standardize Error Handling

Create a consistent error handling pattern across the application:

// src/app/errors/applicationError.js
class ApplicationError extends Error {
  constructor(message, statusCode, errorCode) {
    super(message);
    this.name = this.constructor.name;
    this.statusCode = statusCode;
    this.errorCode = errorCode;
    Error.captureStackTrace(this, this.constructor);
  }
}

// src/app/errors/validationError.js
class ValidationError extends ApplicationError {
  constructor(message, details) {
    super(message, 400, 'VALIDATION_ERROR');
    this.details = details;
  }
}

// In controllers
try {
  // Business logic
} catch (error) {
  if (error instanceof ValidationError) {
    return reply.status(error.statusCode).send({
      success: false,
      errorCode: error.errorCode,
      message: error.message,
      details: error.details
    });
  }

  // Log unexpected errors
  request.log.error({ error }, 'Unexpected error');
  return reply.status(500).send({
    success: false,
    errorCode: 'INTERNAL_ERROR',
    message: 'An unexpected error occurred'
  });
}

Implementation Steps: 1. Create base error classes 2. Create specific error types for different scenarios 3. Update controllers to use standardized error handling 4. Create middleware for global error handling

3. Implement Transaction Management Middleware

Create middleware to handle database transactions consistently:

// src/app/middlewares/transactionMiddleware.js
async function transactionMiddleware(request, reply) {
  const transaction = await request.db.sequelize.transaction();
  request.transaction = transaction;

  try {
    await reply;

    if (!reply.sent) {
      await transaction.commit();
    }
  } catch (error) {
    await transaction.rollback();
    throw error;
  }
}

// In route definition
fastify.route({
  method: 'POST',
  url: '/non-requisitions',
  preHandler: [transactionMiddleware],
  handler: nonRequisitionController.create.bind(nonRequisitionController),
});

Implementation Steps: 1. Create transaction middleware 2. Update route definitions to use the middleware 3. Modify controllers to use the transaction from the request object 4. Add transaction handling for nested operations

4. Enhance Validation with Zod

Leverage Zod more extensively for validation:

// src/domain/entities/nonRequisitionEntity.js
const updateStatusSchema = z
  .object({
    id: createIdParamsSchema('Non-requisition ID'),
    status: z.enum(Object.values(NON_RS_STATUS)),
    reason: z
      .string()
      .min(1, 'Reason is required for status changes')
      .max(100, 'Reason must not exceed 100 characters')
      .optional()
      .nullable()
      .refine(
        (val) => {
          return !['rejected', 'cancelled'].includes(status) || (val && val.trim().length > 0);
        },
        {
          message: 'Reason is required for rejection or cancellation',
        }
      ),
  })
  .strict();

Implementation Steps: 1. Review and enhance existing validation schemas 2. Add validation for all input parameters 3. Create reusable validation utilities 4. Implement validation middleware for routes

5. Implement Status Transition Validation

Create a reusable status transition validator:

// src/app/utils/statusTransitionValidator.js
function validateStatusTransition(currentStatus, newStatus, validTransitions) {
  if (!validTransitions[currentStatus]?.includes(newStatus)) {
    throw new ValidationError(
      `Invalid status transition from ${currentStatus} to ${newStatus}`
    );
  }
  return true;
}

// In service
const validTransitions = {
  'draft': ['for_approval', 'cancelled'],
  'for_approval': ['closed', 'rejected', 'cancelled'],
  'rejected': ['for_approval'],
};

validateStatusTransition(nonRequisition.status, newStatus, validTransitions);

Implementation Steps: 1. Create status transition validator utility 2. Define valid transitions for each entity type 3. Implement validation in service methods 4. Add unit tests for transition validation

6. Implement Logging Strategy

Enhance logging for better debugging:

// src/app/services/nonRequisitionService.js
async updateStatus(id, newStatus, reason, transaction) {
  this.logger.info({ id, newStatus, reason }, 'Updating non-requisition status');

  try {
    const nonRequisition = await this.nonRequisitionRepository.findOne({
      where: { id },
      transaction,
    });

    if (!nonRequisition) {
      this.logger.warn({ id }, 'Non-requisition not found');
      throw new NotFoundError('Non-requisition not found');
    }

    this.logger.debug(
      { id, currentStatus: nonRequisition.status, newStatus },
      'Validating status transition'
    );

    // Status transition validation

    // Update status
    const result = await nonRequisition.update(
      { status: newStatus, statusReason: reason },
      { transaction }
    );

    this.logger.info(
      { id, oldStatus: nonRequisition.status, newStatus },
      'Non-requisition status updated successfully'
    );

    return result;
  } catch (error) {
    this.logger.error(
      { id, newStatus, error: error.message, stack: error.stack },
      'Error updating non-requisition status'
    );
    throw error;
  }
}

Implementation Steps: 1. Configure structured logging 2. Add context-rich log entries at appropriate levels 3. Implement request ID tracking across services 4. Set up log aggregation and analysis

7. Implement Database Migrations with Validation

Ensure migrations are safe and reversible:

// src/infra/database/migrations/20250411011541-alter-non-rs-table.js
'use strict';

const { NON_RS_DISCOUNT_TYPE } = require('../../../domain/constants/nonRSConstants');

module.exports = {
  async up(queryInterface, Sequelize) {
    // Start transaction
    const transaction = await queryInterface.sequelize.transaction();

    try {
      // Add new columns
      await queryInterface.addColumn(
        'non_requisitions',
        'supplier_id',
        {
          type: Sequelize.INTEGER,
          defaultValue: 1,
          allowNull: false,
        },
        { transaction }
      );

      // Commit transaction
      await transaction.commit();
    } catch (error) {
      // Rollback transaction on error
      await transaction.rollback();
      throw error;
    }
  },

  async down(queryInterface, Sequelize) {
    // Implement down migration for rollback
    const transaction = await queryInterface.sequelize.transaction();

    try {
      await queryInterface.removeColumn('non_requisitions', 'supplier_id', { transaction });
      await transaction.commit();
    } catch (error) {
      await transaction.rollback();
      throw error;
    }
  },
};

Implementation Steps: 1. Review existing migrations for completeness 2. Implement transaction handling in all migrations 3. Ensure all migrations have proper down methods 4. Add validation and data checks in migrations

8. Implement API Versioning

Prepare for future changes with API versioning:

// src/interfaces/router/index.js
function registerRoutes(fastify, options, done) {
  // v1 API routes
  fastify.register(require('./v1'), { prefix: '/api/v1' });

  // Future v2 API routes
  // fastify.register(require('./v2'), { prefix: '/api/v2' });

  done();
}

Implementation Steps: 1. Restructure routes for versioning 2. Create version-specific controllers if needed 3. Implement version negotiation middleware 4. Document API versioning strategy

9. Implement Dependency Injection Consistently

Ensure all services use dependency injection:

// src/container.js
const container = createContainer();

container.register({
  // Repositories
  nonRequisitionRepository: asClass(NonRequisitionRepository).singleton(),

  // Services
  nonRequisitionService: asClass(NonRequisitionService).singleton(),

  // Controllers
  nonRequisitionController: asClass(NonRequisitionController).singleton(),

  // Utilities
  logger: asFunction(() => logger).singleton(),
  db: asValue(db),
  constants: asValue(constants),
});

Implementation Steps: 1. Review and standardize dependency injection 2. Ensure all dependencies are properly registered 3. Implement lifecycle management for resources 4. Add unit tests for container configuration

10. Implement Consistent Documentation

Document code with JSDoc for better understanding:

/**
 * Updates the status of a non-requisition
 * 
 * @param {number} id - The ID of the non-requisition
 * @param {string} newStatus - The new status (must be one of NON_RS_STATUS values)
 * @param {string} [reason] - The reason for the status change (required for rejection/cancellation)
 * @param {Transaction} transaction - The database transaction
 * @returns {Promise<Object>} The updated non-requisition
 * @throws {ValidationError} If the status transition is invalid
 * @throws {NotFoundError} If the non-requisition is not found
 */
async updateStatus(id, newStatus, reason, transaction) {
  // Implementation
}

Implementation Steps: 1. Add JSDoc comments to all methods 2. Document parameters, return values, and exceptions 3. Generate API documentation from JSDoc 4. Keep documentation in sync with code changes

11. Implement Feature Flags

Use feature flags for safer deployments:

// src/config/featureFlags.js
const featureFlags = {
  enableNewNonRequisitionFlow: process.env.ENABLE_NEW_NON_REQUISITION_FLOW === 'true',
  enableEnhancedValidation: process.env.ENABLE_ENHANCED_VALIDATION === 'true',
};

// In service
if (featureFlags.enableNewNonRequisitionFlow) {
  // New implementation
} else {
  // Old implementation
}

Implementation Steps: 1. Create feature flag configuration 2. Implement feature flag checks in code 3. Create admin interface for managing flags 4. Document feature flag usage

12. Implement Automated Code Quality Checks

Set up pre-commit hooks and CI/CD pipelines:

// package.json
{
  "scripts": {
    "lint": "eslint .",
    "test": "jest",
    "pre-commit": "lint-staged && npm test"
  },
  "lint-staged": {
    "*.js": ["eslint --fix", "prettier --write"]
  },
  "husky": {
    "hooks": {
      "pre-commit": "npm run pre-commit"
    }
  }
}

Implementation Steps: 1. Configure ESLint and Prettier 2. Set up Husky for pre-commit hooks 3. Configure lint-staged for incremental linting 4. Integrate with CI/CD pipeline

13. Implement Performance Monitoring

Add performance monitoring to identify bottlenecks:

// src/app/middlewares/performanceMiddleware.js
async function performanceMiddleware(request, reply) {
  const start = process.hrtime();

  reply.on('sent', () => {
    const end = process.hrtime(start);
    const duration = (end[0] * 1e9 + end[1]) / 1e6; // Convert to milliseconds

    request.log.info({
      url: request.url,
      method: request.method,
      statusCode: reply.statusCode,
      duration,
    }, 'Request completed');

    if (duration > 1000) {
      request.log.warn({
        url: request.url,
        method: request.method,
        duration,
      }, 'Slow request detected');
    }
  });
}

Implementation Steps: 1. Create performance monitoring middleware 2. Add performance metrics collection 3. Set up alerting for performance issues 4. Implement performance dashboards

14. Implement Database Query Optimization

Optimize database queries for better performance:

// src/infra/repositories/nonRequisitionRepository.js
async findAll(options = {}) {
  // Add index hints for better performance
  const queryOptions = {
    ...options,
    indexHints: [
      { type: 'USE', values: ['idx_non_requisitions_status'] }
    ],
  };

  // Only select needed fields
  if (!queryOptions.attributes) {
    queryOptions.attributes = [
      'id', 'nonRsNumber', 'status', 'createdAt', 'updatedAt'
    ];
  }

  return super.findAll(queryOptions);
}

Implementation Steps: 1. Review and optimize database queries 2. Add appropriate indexes 3. Implement query caching where appropriate 4. Monitor query performance

15. Implement Consistent Naming Conventions

Standardize naming across the codebase:

// Bad: Inconsistent naming
const getNonRs = async (id) => { /* ... */ };
async function get_non_requisition(id) { /* ... */ }

// Good: Consistent camelCase naming
async function getNonRequisition(id) { /* ... */ }
async function updateNonRequisitionStatus(id, status) { /* ... */ }

Implementation Steps: 1. Define naming conventions for different code elements 2. Create ESLint rules to enforce conventions 3. Gradually refactor existing code to follow conventions 4. Document naming conventions for new developers

Implementation Priority

1. High Priority (Immediate Impact)
2. Standardize Error Handling (#2)
3. Implement Transaction Management (#3)
4. Enhance Validation with Zod (#4)

5. Implement Status Transition Validation (#5)

6. Medium Priority (Short-term Improvements)

7. Implement Comprehensive Automated Testing (#1)
8. Implement Logging Strategy (#6)
9. Implement Database Query Optimization (#14)

10. Implement Consistent Naming Conventions (#15)

11. Lower Priority (Long-term Improvements)

12. Implement API Versioning (#8)
13. Implement Feature Flags (#11)
14. Implement Performance Monitoring (#13)
15. Implement Consistent Documentation (#10)

By implementing these recommendations, you can significantly reduce bugs in future sprints, improve code quality, and make the codebase more maintainable. These practices will help catch issues early, provide better debugging information, and ensure consistent behavior across the application.