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

The file routing.php ist too long #352

Open
tobias74 opened this issue Apr 23, 2024 · 2 comments
Open

The file routing.php ist too long #352

tobias74 opened this issue Apr 23, 2024 · 2 comments
Labels
php Pull requests that update Php code

Comments

@tobias74
Copy link
Contributor

This file currently has over 6000 lines, this should probably be split into multiple files

@ThomasAFink
Copy link
Member

ThomasAFink commented Dec 6, 2024

The following services all contain a routing.php file:

  1. zmsapi: routing.php
  2. zmscitizenapi (coming soon from branch feature-MPDZBS-877-zmscitizenapi): routing.php
  3. zmsadmin: routing.php
  4. zmsstatistic: routing.php
  5. zmscalldisplay: routing.php
  6. zmsticketprinter: routing.php

Each of these modules includes a routing.php file for managing their respective routes. Give me a complete visual overview for how you would refactor it @coderabbitai.

@it-at-m it-at-m deleted a comment from coderabbitai bot Dec 6, 2024
Copy link
Contributor

coderabbitai bot commented Dec 6, 2024

Tip

For best results, initiate chat on the files or code changes.

Let me analyze the codebase structure and provide a more detailed recommendation for organizing the routing files.

@ThomasAFink Based on the analysis of your codebase, I notice that each module (zmsapi, zmsadmin, etc.) follows a similar pattern with a BaseController and specific controllers for different functionalities. Here's a comprehensive recommendation for restructuring the routing:

  1. Module-Based Structure:
graph TD
    A[eappointment/] --> B[zmsapi/]
    A --> C[zmscitizenapi/]
    A --> D[zmsadmin/]
    A --> E[zmsstatistic/]
    A --> F[zmscalldisplay/]
    A --> G[zmsticketprinter/]

    subgraph "Module Structure Example"
        B --> H[routes/]
        H --> I[api.routes.php]
        H --> J[controllers/]
        H --> K[middleware/]
        
        J --> L[appointment/]
        J --> M[user/]
        J --> N[system/]
        
        K --> O[auth.middleware.php]
        K --> P[validation.middleware.php]
    end
Loading
  1. Detailed Route Organization (per module):
graph TD
    subgraph "zmsapi Routes"
        A1[routes/] --> B1[appointment.routes.php]
        A1 --> C1[user.routes.php]
        A1 --> D1[system.routes.php]
        A1 --> E1[notification.routes.php]
    end
    
    subgraph "zmsadmin Routes"
        A2[routes/] --> B2[dashboard.routes.php]
        A2 --> C2[management.routes.php]
        A2 --> D2[settings.routes.php]
        A2 --> E2[reports.routes.php]
    end
    
    subgraph "zmsstatistic Routes"
        A3[routes/] --> B3[reports.routes.php]
        A3 --> C3[analytics.routes.php]
        A3 --> D3[export.routes.php]
        A3 --> E3[warehouse.routes.php]
    end
    
    subgraph "zmscalldisplay Routes"
        A4[routes/] --> B4[display.routes.php]
        A4 --> C4[queue.routes.php]
        A4 --> D4[info.routes.php]
    end
    
    subgraph "zmsticketprinter Routes"
        A5[routes/] --> B5[printer.routes.php]
        A5 --> C5[notification.routes.php]
        A5 --> D5[process.routes.php]
    end
Loading
  1. Implementation Example (using your existing namespace structure):
// zmsapi/routes/appointment.routes.php
<?php
namespace BO\Zmsapi\Routes;

use Slim\App;
use Slim\Routing\RouteCollectorProxy;
use BO\Zmsapi\Appointment;

class AppointmentRoutes
{
    public static function register(App $app)
    {
        $app->group('/appointments', function (RouteCollectorProxy $group) {
            $group->get('/', [Appointment\ListHandler::class, 'handle'])
                ->setName('appointment.list');
                
            $group->post('/', [Appointment\CreateHandler::class, 'handle'])
                ->setName('appointment.create');
                
            $group->group('/{id}', function (RouteCollectorProxy $subgroup) {
                $subgroup->get('/', [Appointment\GetHandler::class, 'handle'])
                    ->setName('appointment.get');
                $subgroup->put('/', [Appointment\UpdateHandler::class, 'handle'])
                    ->setName('appointment.update');
                $subgroup->delete('/', [Appointment\DeleteHandler::class, 'handle'])
                    ->setName('appointment.delete');
            });
        });
    }
}

// zmsapi/src/Application.php
use BO\Zmsapi\Routes;

class Application extends \BO\Slim\Application
{
    protected function registerRoutes()
    {
        Routes\AppointmentRoutes::register($this);
        Routes\UserRoutes::register($this);
        Routes\SystemRoutes::register($this);
        // ... other route registrations
    }
}
  1. Migration Strategy:
graph LR
    A[Phase 1] --> B[Phase 2] --> C[Phase 3] --> D[Phase 4]
    
    subgraph "Phase 1: Setup"
        A1[Create routes directory]
        A2[Create route classes]
        A3[Setup route registration]
    end
    
    subgraph "Phase 2: Migration"
        B1[Move routes by domain]
        B2[Update namespaces]
        B3[Add middleware]
    end
    
    subgraph "Phase 3: Testing"
        C1[Unit tests]
        C2[Integration tests]
        C3[Load tests]
    end
    
    subgraph "Phase 4: Cleanup"
        D1[Remove old routing.php]
        D2[Update documentation]
        D3[Monitor performance]
    end
Loading
  1. Key Benefits:
    • Better organization by domain/feature
    • Easier maintenance and testing
    • Clear separation of concerns
    • Improved code reusability
    • Better type hinting and IDE support
    • Easier to implement middleware per route group

Would you like me to create a GitHub issue to track this refactoring task? We can break it down module by module, starting with the largest one (zmsapi with 6452 lines) first.

🔗 Analysis chain

Let me analyze the routing files in each module to provide a comprehensive recommendation.

Let me analyze the results and provide a detailed visual overview of the recommended structure.

@ThomasAFink Based on the analysis of all routing files across different modules, here's a comprehensive recommendation for restructuring:

  1. Overall Architecture:
graph TD
    A[eappointment/] --> B[zmsapi/]
    A --> C[zmscitizenapi/]
    A --> D[zmsadmin/]
    A --> E[zmsstatistic/]
    A --> F[zmscalldisplay/]
    A --> G[zmsticketprinter/]
    
    subgraph "Module Structure"
        B --> H[routes/]
        H --> I[api.php]
        H --> J[appointment.php]
        H --> K[notification.php]
        H --> L[system.php]
        H --> M[middleware.php]
    end
Loading
  1. Detailed Module Structure (example for each service):
graph TD
    subgraph "zmsapi"
        A1[routes/] --> B1[appointment.php]
        A1 --> C1[user.php]
        A1 --> D1[notification.php]
        A1 --> E1[system.php]
    end
    
    subgraph "zmscitizenapi"
        A2[routes/] --> B2[booking.php]
        A2 --> C2[availability.php]
        A2 --> D2[service.php]
    end
    
    subgraph "zmsadmin"
        A3[routes/] --> B3[dashboard.php]
        A3 --> C3[management.php]
        A3 --> D3[settings.php]
    end
    
    subgraph "zmsstatistic"
        A4[routes/] --> B4[reports.php]
        A4 --> C4[analytics.php]
        A4 --> D4[export.php]
    end
    
    subgraph "zmscalldisplay"
        A5[routes/] --> B5[display.php]
        A5 --> C5[queue.php]
    end
    
    subgraph "zmsticketprinter"
        A6[routes/] --> B6[printer.php]
        A6 --> C6[ticket.php]
    end
Loading
  1. Implementation Example (for each module):
// zmsapi/routes/appointment.php
<?php
namespace Routes\Api;

class AppointmentRoutes
{
    public static function register($app)
    {
        // Group related endpoints
        $app->group('/appointments', function($app) {
            $app->get('/', [
                'description' => 'List all appointments',
                'handler' => '\BO\Zmsapi\Appointment\ListHandler'
            ]);
            
            $app->post('/', [
                'description' => 'Create new appointment',
                'handler' => '\BO\Zmsapi\Appointment\CreateHandler'
            ]);
            
            // Nested group for specific appointment operations
            $app->group('/{id}', function($app) {
                $app->get('/', '\BO\Zmsapi\Appointment\GetHandler');
                $app->put('/', '\BO\Zmsapi\Appointment\UpdateHandler');
                $app->delete('/', '\BO\Zmsapi\Appointment\DeleteHandler');
            });
        });
    }
}

// zmsapi/bootstrap.php
foreach (glob(__DIR__ . "/routes/*.php") as $routeFile) {
    $className = 'Routes\\Api\\' . basename($routeFile, '.php') . 'Routes';
    $className::register($app);
}
  1. Migration Strategy:

    1. Create new directory structure in each module:
      module/
      ├── routes/
      │   ├── index.php       # Route registration
      │   ├── feature1.php    # Feature-specific routes
      │   ├── feature2.php
      │   └── middleware.php  # Common middleware
      ├── src/
      │   └── handlers/      # Route handlers
      └── tests/
          └── routes/        # Route tests
      
    2. Implement route registration system
    3. Move routes group by group
    4. Add tests for each route group
    5. Update documentation
    6. Phase out old routing files
  2. Common Patterns:

    • Use consistent route naming: /{resource}/{id}/{action}
    • Group related routes using $app->group()
    • Include route documentation
    • Implement middleware at group level
    • Use versioning where needed (e.g., /v1/appointments)

Would you like me to create a GitHub issue to track this refactoring task? We can break it down into smaller, manageable tasks for each module.


Let me analyze the routing files in more detail to understand their current structure and patterns.

Let me provide a more detailed and tailored recommendation based on the actual codebase structure.

@ThomasAFink Based on the detailed analysis of your routing files, I notice that the largest file (zmsapi/routing.php with 6452 lines) and other modules have different patterns and needs. Here's a refined recommendation:

  1. Module-Specific Structure:
graph TD
    subgraph "Service Modules"
        A[eappointment/] --> B[zmsapi/]
        A --> C[zmscitizenapi/]
        A --> D[zmsadmin/]
        A --> E[zmsstatistic/]
        A --> F[zmscalldisplay/]
        A --> G[zmsticketprinter/]
    end

    subgraph "Internal Structure (per module)"
        H[module/] --> I[config/]
        H --> J[routes/]
        H --> K[src/]
        
        J --> L[index.php]
        J --> M[groups/]
        
        M --> N[api.routes.php]
        M --> O[admin.routes.php]
        M --> P[public.routes.php]
        
        I --> Q[middleware.php]
        I --> R[dependencies.php]
    end
Loading
  1. Specific Route Organization (by module):
graph TD
    subgraph "zmsapi Routes"
        A1[routes/] --> B1[groups/]
        B1 --> C1[appointment.routes.php]
        B1 --> D1[calendar.routes.php]
        B1 --> E1[workstation.routes.php]
        B1 --> F1[notification.routes.php]
    end
    
    subgraph "zmsadmin Routes"
        A2[routes/] --> B2[groups/]
        B2 --> C2[pickup.routes.php]
        B2 --> D2[scope.routes.php]
        B2 --> E2[management.routes.php]
    end
    
    subgraph "zmsstatistic Routes"
        A3[routes/] --> B3[groups/]
        B3 --> C3[reports.routes.php]
        B3 --> D3[metrics.routes.php]
    end
    
    subgraph "zmscalldisplay Routes"
        A4[routes/] --> B4[groups/]
        B4 --> C4[display.routes.php]
        B4 --> D4[queue.routes.php]
    end
Loading
  1. Implementation Example:
// module/routes/groups/appointment.routes.php
<?php
namespace ZMS\Routes\Groups;

use Slim\App;
use Slim\Routing\RouteCollectorProxy;

class AppointmentRoutes
{
    public static function register(App $app)
    {
        $app->group('/appointments', function (RouteCollectorProxy $group) {
            // List appointments
            $group->get('/', [
                'handler' => '\BO\Zmsapi\Appointment\ListHandler',
                'name' => 'appointment.list'
            ]);
            
            // Single appointment operations
            $group->group('/{id}', function (RouteCollectorProxy $subgroup) {
                $subgroup->get('/', '\BO\Zmsapi\Appointment\GetHandler');
                $subgroup->put('/', '\BO\Zmsapi\Appointment\UpdateHandler');
                $subgroup->delete('/', '\BO\Zmsapi\Appointment\DeleteHandler');
            });
        });
    }
}

// module/routes/index.php
<?php
use ZMS\Routes\Groups;

return function (App $app) {
    // Register route groups
    Groups\AppointmentRoutes::register($app);
    Groups\CalendarRoutes::register($app);
    // ... other route groups
};
  1. Migration Strategy (per module):
graph LR
    A[Phase 1] --> B[Phase 2] --> C[Phase 3] --> D[Phase 4]
    
    subgraph "Phase 1: Setup"
        A1[Create directory structure]
        A2[Setup route registration]
    end
    
    subgraph "Phase 2: Migration"
        B1[Identify route groups]
        B2[Move routes group by group]
        B3[Add tests per group]
    end
    
    subgraph "Phase 3: Validation"
        C1[Test coverage]
        C2[Performance testing]
        C3[Documentation]
    end
    
    subgraph "Phase 4: Cleanup"
        D1[Remove old files]
        D2[Update dependencies]
    end
Loading
  1. Key Implementation Details:
    • Use RouteCollectorProxy for grouping
    • Maintain consistent naming: {module}/routes/groups/{feature}.routes.php
    • Each route group is a separate class with a static register method
    • Middleware can be applied at group level
    • Use route names for better URL generation
    • Keep route handlers in separate classes

Would you like me to create a GitHub issue to track this refactoring task? We can break it down module by module, starting with the largest one (zmsapi) first.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Get line counts for each routing file
echo "=== Line counts for routing files ==="
fd routing.php -x wc -l

# Sample route patterns from each module
for module in zmsapi zmsadmin zmsstatistic zmscalldisplay zmsticketprinter; do
  if [ -f "$module/routing.php" ]; then
    echo -e "\n=== Sample routes from $module/routing.php ==="
    rg "^\$app->(get|post|put|delete|patch)" "$module/routing.php" | head -n 5
  fi
done

# Check for any existing route grouping patterns
echo -e "\n=== Checking for route grouping patterns ==="
rg "group\(" --type php

Length of output: 2156


Script:

#!/bin/bash
# Get a sample of routes from each module to understand their patterns
for module in zmsapi zmsadmin zmsstatistic zmscalldisplay zmsticketprinter; do
  if [ -f "$module/routing.php" ]; then
    echo -e "\n=== Route patterns in $module/routing.php ==="
    echo "First 10 lines:"
    head -n 10 "$module/routing.php"
    echo -e "\nRoute examples:"
    rg "\\\$app->(get|post|put|delete|patch)" "$module/routing.php" | head -n 5
  fi
done

# Check for any namespace declarations and class structures
echo -e "\n=== Checking for namespace and class patterns ==="
rg "^namespace|^class" --type php

Length of output: 71249


✏️ Learnings added
Learnt from: ThomasAFink
URL: https://github.com/it-at-m/eappointment/issues/352

Timestamp: 2024-12-06T16:03:25.417Z
Learning: The routing structure in the eappointment project should follow a consistent pattern across all modules (zmsapi, zmsadmin, zmsstatistic, zmscalldisplay, zmsticketprinter) using RouteCollectorProxy for grouping. Each module should have a routes/ directory containing domain-specific route files (e.g., appointment.routes.php) that implement a static register method to configure routes using the existing namespace structure.

Learnt from: ThomasAFink
URL: https://github.com/it-at-m/eappointment/issues/352

Timestamp: 2024-12-06T16:03:25.418Z
Learning: Each service module (zmsapi, zmscitizenapi, zmsadmin, zmsstatistic, zmscalldisplay, zmsticketprinter) should have its routes organized in a routes/ directory with feature-specific PHP files, following a consistent structure for better maintainability. Routes should be grouped by domain and registered using a common bootstrap mechanism.

Learnt from: ThomasAFink
URL: https://github.com/it-at-m/eappointment/issues/352

Timestamp: 2024-12-06T16:03:25.418Z
Learning: The routing structure in the eappointment project should follow a consistent pattern across all modules (zmsapi, zmsadmin, etc.) using RouteCollectorProxy for grouping, with routes organized in separate files under routes/groups/ directory. Each route group should be implemented as a class with a static register method.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: ThomasAFink
PR: it-at-m/eappointment#0
File: :0-0
Timestamp: 2024-12-06T15:49:57.822Z
Learning: The routing files in the eappointment project, particularly zmsapi/routing.php (6452 lines) and zmsadmin/routing.php (492 lines), need to be refactored into smaller, feature-based files for better maintainability. Routes should be grouped by domain and moved to separate files under a routes/ directory structure.

@it-at-m it-at-m deleted a comment from coderabbitai bot Dec 6, 2024
@it-at-m it-at-m deleted a comment from coderabbitai bot Dec 6, 2024
@ThomasAFink ThomasAFink pinned this issue Dec 6, 2024
@ThomasAFink ThomasAFink added the php Pull requests that update Php code label Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
php Pull requests that update Php code
Projects
None yet
Development

No branches or pull requests

2 participants