Code cleanup and stabilisation #870
Open
+1,482,687
−42,218
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Run PHPCBF multiple passes to auto-fix 183+ coding standard violations - Add missing @return tags in controllers - Fix parameter documentation mismatches - Fix line length issues in docblocks - Fix array/function call formatting - Fix concatenation spacing - Fix indentation and alignment issues Remaining: 7,062 errors (down from 7,245) - Large files (ObjectService, FileService, SaveObjects) need manual fixes - Many errors in complex files that PHPCBF cannot auto-fix
ARCHITECTURE IMPROVEMENT: - Move massValidateObjects business logic (443 lines) from controller to service - Controller now properly thin: validates HTTP params, delegates to service - Service contains all business logic: batch processing, metrics, error handling PHPCS IMPROVEMENTS: - SettingsController: 123 → 83 errors (40 fixed, 33% reduction) - Fix file comment format (/** instead of /*) - Add missing @param documentation - Fix parameter documentation formatting - Remove business logic helper methods from controller NEW SERVICE METHODS: - SettingsService::massValidateObjects() - main business logic - SettingsService::createBatchJobs() - batch job creation - SettingsService::processJobsSerial() - serial batch processing - SettingsService::processJobsParallel() - parallel batch processing - SettingsService::processBatchDirectly() - single batch processing - SettingsService::formatBytes() - utility method BENEFITS: - Better separation of concerns (MVC pattern) - Easier to test business logic independently - Reusable service methods for other consumers - Improved code maintainability
…m ObjectService - Created new SearchQueryHandler with 8 search-related helper methods: * buildSearchQuery() - Convert request parameters to structured queries * applyViewsToQuery() - Apply view filters to search queries * isSolrAvailable() - Check SOLR availability * cleanQuery() - Normalize query parameters * addPaginationUrls() - Generate pagination links * logSearchTrail() - Log search analytics * isSearchTrailsEnabled() - Check trail settings - Registered SearchQueryHandler in Application.php dependency injection - Updated ObjectService to inject and use SearchQueryHandler - Reduced code duplication by delegating 4 methods to handler - PHPCS compliance: 570+ lines with only 2 minor errors Part of ObjectService refactoring to separate concerns and improve maintainability.
…s from ObjectService - Delegated remaining 3 methods to SearchQueryHandler: * buildSearchQuery() - Query building logic * applyViewsToQuery() - View filter application * cleanQuery() - Query parameter normalization - All 8 search helper methods now fully delegated - Removed 250 lines of duplicate code from ObjectService - ObjectService reduced from 5,931 to 5,687 lines (4.1% reduction) - Improved separation of concerns - No syntax errors, ready for testing SearchQueryHandler integration: COMPLETE ✅
- Created PerformanceHandler with performance optimization methods: * optimizeRequestForPerformance() - Request optimization orchestration * isSimpleRequest() - Fast-path detection * optimizeExtendQueries() - Extend query optimization * preloadCriticalEntities() - Cache warmup * extractRelatedData() - Related object extraction * getCachedEntities() - Entity caching wrapper * getFacetCount() - Facet counting * calculateTotalPages() - Pagination calculations * calculateExtendCount() - Extend parameter parsing - Registered PerformanceHandler in Application.php dependency injection - Updated ObjectService to inject PerformanceHandler - Delegated optimizeRequestForPerformance() to handler - 410+ lines of performance logic extracted - PHPCS compliant (minor formatting accepted) Next: Remove helper method dead code and complete remaining delegations
- Delegated all remaining performance methods from ObjectService: * extractRelatedData() - Related object extraction * getCachedEntities() - Entity caching wrapper * getFacetCount() - Facet counting helper * calculateTotalPages() - Pagination math * calculateExtendCount() - Extend parameter parsing - Removed dead code helper methods (now in handler): * isSimpleRequest() - Fast-path detection * optimizeExtendQueries() - Extend optimization * preloadCriticalEntities() - Cache warmup - ObjectService reduced by 190+ lines - Clean separation of performance logic - All 9 PerformanceHandler methods fully integrated PerformanceHandler: COMPLETE ✅
- Created PermissionHandler for permission management: * hasPermission() - RBAC permission checking * checkPermission() - Permission enforcement with exceptions * filterObjectsForPermissions() - Object array filtering * filterUuidsForPermissions() - UUID array filtering * getActiveOrganisationForContext() - Multi-tenancy context - Registered PermissionHandler in Application.php - Injected into ObjectService constructor - 380+ lines of permission logic extracted - PHPCS compliant (file comment minor issue accepted) Ready for method delegation in next session. PermissionHandler: CREATED ✅ (delegation pending)
…egated - Delegated all 5 permission methods from ObjectService: * hasPermission() - Core RBAC permission checking * checkPermission() - Permission enforcement with exceptions * filterObjectsForPermissions() - Filter objects by permissions * filterUuidsForPermissions() - Filter UUIDs by permissions * getActiveOrganisationForContext() - Multi-tenancy context - Removed 140+ lines of duplicate permission logic - ObjectService now delegates all RBAC to PermissionHandler - Clean separation of security concerns - All permission logic centralized PermissionHandler: COMPLETE ✅
- Created DataManipulationHandler for data transformation: * getValueFromPath() - Nested value extraction via dot notation * generateSlugFromValue() - Unique slug generation with timestamp * createSlugHelper() - URL-friendly slug creation * mapObjectProperties() - Property mapping between structures - Registered DataManipulationHandler in Application.php - Injected into ObjectService constructor - All 4 methods delegated to handler - 60+ lines of utility logic extracted - PHPCS compliant DataManipulationHandler: COMPLETE ✅ Session Progress: 4 Handlers Complete! - SearchQueryHandler ✅ - PerformanceHandler ✅ - PermissionHandler ✅ - DataManipulationHandler ✅
Replace force-push dev-to-beta with PR-creation workflow, re-enable unstable release for development branch, fix feature/php-linting refs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Enforce that PRs to main can only come from beta or hotfix branches, and PRs to beta can only come from development or hotfix branches. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Handle JSON strings from database in setRequired/getRequired and setConfiguration/getConfiguration methods. When reading JSON fields from database, they come as strings but setters expected arrays. Updated docblocks to match getFacets()/setFacets() style. Fixes configuration import errors.
Fix JSON field type handling in Schema and Register entities
Multitenancy fix & webhook POST/GET call fixes
Update setAuthorization() methods to accept and decode JSON strings when loading entities from the database, preventing type errors during entity hydration. Fixes: "Argument #1 ($authorization) must be of type ?array, string given"
- Fix bug where auto-generated UUIDs were treated as UPDATE operations - Add debug logging for UUID generation flow Fixes webhook ObjectCreatedEvent not triggering during object creation.
Normalize logger usage in openregister/lib: message prefix [ClassName] and context with file/line for all logger calls.
Fix bug where auto-generated UUIDs were treated as UPDATE operations
- Use named parameters (message:, context:) for all logger calls - Add [ClassName] prefix to every log message - Ensure 'file' => __FILE__ and 'line' => __LINE__ are first in context arrays - AbstractTool: use [ClassName] toolName: functionName format for tool logs
Enforce consistent logging conventions in openregister/lib
Standardize logger usage in openregister/lib to match project rules: - Named parameters: message: and context: - [ClassName] prefix on every message - Context order: 'file' => __FILE__, 'line' => __LINE__ first - Exception details: use exception_file/exception_line to avoid overwriting - SearchTrailMapper, AuditTrailMapper: switch to named params (message:, context:), add [ClassName], put file/line first in context (6 calls) - Chat handlers: replace [ChatService] with correct [ClassName] in ContextRetrievalHandler, ToolManagementHandler, MessageHistoryHandler, ResponseGenerationHandler, ConversationManagementHandler (36+ calls)
Normalize logger calls - finishing touches
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.